fix: consider code with existing errors in code suggestions
What does this merge request do and why?
For some complex/large code files, e.g.: spec/models/commit_spec.rb
or app/models/project.rb:
- Codestral seems to misinterpret some of the code blocks in the prefix/content_above_cursor and suggests closing content when it's unnecessary
- The
fix_end_block_errors
, which parses the code using treesitter, also seem to "think" that the original code before the suggestion is inserted already has errors
The combination of the above factors mean that Codestral unnecessarily adds closing content, and the fix_end_block_errors
cannot resolve it.
This MR adds a new fix_end_block_errors_codestral
post-processor that, instead of checking whether the full code with the suggestion has errors, it checks whether there is an increase in the number of errors after the suggestion is inserted to the code.
This does not change fix_end_block_errors
for the following reasons:
-
fix_end_block_errors
is used by the active Code Completion model (Code Gecko). We do not want any change for Codestral to disrupt Code Gecko behavior. Notably,fix_end_block_errors_codestral
will be slower because this parses the code twice. - We are expecting more problems of this variety (see #655 (closed)), so having a specific
fix_end_block_errors_codestral
means we can make future post-processing changes specific to codestral
Example/Illustration of change:
If we have this original code:
def hello(
[cursor here]
end
With this suggestion:
puts 'hello'
end
fix_end_block_errors
WILL NOT trim out the extra end
in the suggestion because the original code has parse errors (the un-closed (
).
fix_end_block_errors_codestral
SHOULD trim out the extra end
because it is not counting the the errors from the original code.
How to set up and validate locally
This problem has been observed in Ruby, so we will test with a Ruby example.
-
Create a simple Ruby spec file:
# frozen_string_literal: true require 'spec_helper' RSpec.describe "Person" do using RSpec::Parameterized::TableSyntax describe "#fullname" do context 'with different name variations' do where(:person, :expected_full_name) do build(:person, first_name: "Pam", last_name: nil) | "Pam" build(:person, first_name: nil, last_name: "Artiaga") | "Artiaga" build(:person, first_name: "Pam", last_name: "Artiaga") | "Pam Artiaga" end it 'returns the correct full name' do end end end end
-
On your IDE, open up another large Ruby file. This is to provide more "confusion" to the model since currently opened files are sent as additional context. For example, you can use the (
spec/models/commit_spec.rb
) in the GitLab Rails monolith. -
Type around inside the
it 'returns the correct full name'
of the spec.
Merge request checklist
-
Tests added for new functionality. If not, please raise an issue to follow up. -
Documentation added/updated, if needed.
Closes #655 (closed)