feat(mcp/developer): accept -1 for insert_line number #4112
Conversation
jamadeo
left a comment
jamadeo
left a comment
There was a problem hiding this comment.
I'm not sure I follow
there is no easy way to append to the end of the file
because the tool description does say "the number after which to insert", but if Gemini is trying to do this, we should accommodate. I wonder though, is the Gemini model misinterpreting the index, thinking it's the insert-before?
| ambiguous. The entire original string will be replaced with `new_str`. | ||
| To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning) | ||
| To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end) |
There was a problem hiding this comment.
The "correct" way to insert at the end is for the model to pass len(lines). We should still accommodate Gemini but I'd leave this out of the tool description, since the intention is to always use a nonnegative index.
There was a problem hiding this comment.
The "correct" way to insert at the end is for the model to pass len(lines).
This requires that the model knows and keeps track of the exact number of lines in the file, which I think doesn't come "for free", e.g. if the model previously only needed to see the start of the file or some rg results, or if the model modified the file since the last time it looked at it.
In any case, I think appending to a file should be easy for the model to do, but I guess it doesn't have to be using the insert command.
There was a problem hiding this comment.
That's a good point and I'm all for making this easier on the model.
@baxen to your knowledge, might this impact the performance of the editor with claude? I know this tool schema closely resembles https://docs.anthropic.com/en/docs/agents-and-tools/tool-use/text-editor-tool#insert . This is a subtle instruction change but would be good to merge with high confidence in no regression.
I noticed that Gemini 2.5 Pro kept trying to use `insert` with `insert_line` set to -1. This makes sense, because: - there is no easy way to append to the end of the file - we use -1 to mean "end of file" in other contexts (`view_range`). So, let's make -1 do the expected thing! Signed-off-by: sings-to-bees-on-wednesdays <[email protected]>
understood-the-assignment
force-pushed
the
negative_insert_line
branch
from
095c14b to
f02f84b
Compare
| ambiguous. The entire original string will be replaced with `new_str`. | ||
| To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning) | ||
| To use the insert command, you must specify both `insert_line` (the line number after which to insert, 0 for beginning, -1 for end) |
There was a problem hiding this comment.
That's a good point and I'm all for making this easier on the model.
@baxen to your knowledge, might this impact the performance of the editor with claude? I know this tool schema closely resembles https://docs.anthropic.com/en/docs/agents-and-tools/tool-use/text-editor-tool#insert . This is a subtle instruction change but would be good to merge with high confidence in no regression.
|
agreed - I also noted gpt-oss:20b would do similar things, nice catch |
|
Sounds good, let's make it so then |
Signed-off-by: sings-to-bees-on-wednesdays <[email protected]> Signed-off-by: Jack Wright <[email protected]>
* main: docs: add figma tutorial (#4231) Add Nix flake for reproducible builds (#4213) Enhanced onboarding page visual design (#4156) feat: adds mtls to all providers (#2794) (#2799) Don't show a confirm dialog for quitting (#4225) Fix: Missing smart_approve in CLI /mode help text and error message (#4132) Clean up langfuse docs and scripts (#4220) feat: add remark-breaks plugin to preserve single newlines in markdown (#4217) feat(mcp/developer): accept -1 for insert_line number (#4112) Remove dead code and old settings migration (#4180) removed tests from lint-staged (#4203) docs: openrouter and ollama easy desktop setup (#4195) Custom providers update (#4099) docs: goose_terminal env var (#4205) Desktop alerts when suspicious unicode characters found in Recipe (#4080) chore: remove the google drive built-in extension (#4187) Move out app init (#4185)
3 participants
I noticed that Gemini 2.5 Pro kept trying to use
insertwithinsert_lineset to -1. This makes sense, because:view_range).So, let's make -1 do the expected thing!