From 75bff99ca5435a38f5e70ed4b6a5f40d3dc1ec4c Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Thu, 26 Sep 2024 15:29:58 +0000
Subject: [PATCH] Fixes VR diff creation bug

---
 .../create_from_vulnerability_data_service.rb |  6 ++-
 ...te_from_vulnerability_data_service_spec.rb | 42 +++++++++++++++++--
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb
index cc58ac2299b9a..63e25295529ae 100644
--- a/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb
+++ b/ee/app/services/merge_requests/create_from_vulnerability_data_service.rb
@@ -119,10 +119,12 @@ def create_llm_diff_from_blocks
       fixed_code = finding_presenter.source_code.dup
 
       changes.each do |old_code, new_code|
-        fixed_code.gsub!(old_code.lstrip, new_code.lstrip)
+        fixed_code.gsub!(old_code.strip, new_code.strip)
       end
 
-      create_llm_diff_from_given_code(fixed_code)
+      # Diffy expects fixed_code to include a trailing newline
+      # since we're stripping the newlines above, we add it here.
+      create_llm_diff_from_given_code("#{fixed_code}\n")
     end
 
     def create_llm_diff_from_code_block
diff --git a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb
index f25128cd7acf8..19b8f66ba0b61 100644
--- a/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb
+++ b/ee/spec/services/merge_requests/create_from_vulnerability_data_service_spec.rb
@@ -458,7 +458,7 @@
             diff --git a/src/main.c b/src/main.c
             --- a/src/main.c
             +++ b/src/main.c
-            @@ -4,5 +4,5 @@
+            @@ -4,5 +4,6 @@
              {
                char buf[8];
             -  memcpy(&buf, "123456789");
@@ -471,7 +471,7 @@
             file: 'vulnerabilities/remediation',
             formats: :patch,
             locals: hash_including(
-              diff: diff
+              diff: "#{diff}+\n"
             )
           ).and_call_original
           expect(service).to receive(:render_template).with(
@@ -493,7 +493,7 @@
               diff --git a/src/main.c b/src/main.c
               --- a/src/main.c
               +++ b/src/main.c
-              @@ -4,5 +4,5 @@
+              @@ -4,5 +4,6 @@
                {
                  char buf[8];
               -  memcpy(&buf, "123456789");
@@ -507,7 +507,7 @@
               file: 'vulnerabilities/remediation',
               formats: :patch,
               locals: hash_including(
-                diff: diff
+                diff: "#{diff}+\n"
               )
             ).and_call_original
             expect(service).to receive(:render_template).with(
@@ -540,6 +540,40 @@
             end
           end
         end
+
+        context 'when extra newlines are returned from the LLM' do
+          let(:llm_patch) { "<old_code>\n  memcpy(&buf, \"123456789\");\n\n</old_code><new_code>\n  memcpy(&buf, \"1\");\n</new_code>" }
+
+          it 'successfully creates a functional diff and patch from the LLM patch' do
+            diff = <<~HEREDOC
+              diff --git a/src/main.c b/src/main.c
+              --- a/src/main.c
+              +++ b/src/main.c
+              @@ -4,5 +4,6 @@
+               {
+                 char buf[8];
+              -  memcpy(&buf, "123456789");
+              +  memcpy(&buf, "1");
+                 printf("hello, world!");
+               }
+            HEREDOC
+
+            expect(service).to receive(:render_template).with(
+              file: 'vulnerabilities/remediation',
+              formats: :patch,
+              locals: hash_including(
+                diff: "#{diff}+\n"
+              )
+            ).and_call_original
+            expect(service).to receive(:render_template).with(
+              file: 'vulnerabilities/merge_request_description',
+              formats: :md,
+              locals: instance_of(Hash)
+            ).and_call_original
+
+            expect(result[:status]).to eq(:success)
+          end
+        end
       end
 
       context 'when suggestion_merge_request is present in params' do
-- 
GitLab