Skip to content

fix(edit): kill the auto-format edit-rejection round-trip#57

Merged
agjs merged 4 commits into
mainfrom
feat/edit-autoformat-resilience
Jun 28, 2026
Merged

fix(edit): kill the auto-format edit-rejection round-trip#57
agjs merged 4 commits into
mainfrom
feat/edit-autoformat-resilience

Conversation

@agjs

@agjs agjs commented Jun 28, 2026

Copy link
Copy Markdown
Owner

From the local DeepSeek model's own UX feedback: its #1 friction is that the write-guard auto-formats a file right after create/edit (eslint --fix + prettier — the fast feedback it otherwise loves), so the model's next edit oldString no longer matches disk → not-found → a wasted turn spent re-reading the file. Confirmed by the user ("the edit tool fails because the pointer is no longer correct"). Borrowed-ideas #3 (+#2).

Two fixes

  1. Widen the fuzzy matcher. fuzzyLineReplace already folds quotes/whitespace/punctuation/Unicode; it now also tolerates a line-trailing comma or semicolon — the classic prettier drift (trailing commas in multiline literals/args; semicolon normalization). More near-miss edits just land. The unique-window guard still blocks any ambiguous match.
  2. Inline the current content on a not-found rejection. The harness already has the post-format bytes, so instead of telling the model to go read the file, the rejection now includes its current content (numbered, ≤400 lines) — the model fixes the stale anchor in the same turn. Falls back to advising read for huge files.

Also refreshed the not-found message to name the likely cause (auto-reformat: reordered imports, normalized quotes/commas).

Tests

  • Genuine multi-line fuzzy cases that miss on exact and only pass via the new comma/semicolon tolerance.
  • A doEdit test asserting a not-found rejection carries the file's current content and drops the "go read it" instruction.
  • bun run validate green (1826 tests).

Follow-ups (not here)

  • Create-side ACI echo (Feature/guardrail packs local uplift #2): return post-format content after create so anchors stay fresh preventively.
  • BAML-style lenient tool-call parsing to upgrade salvageToolCalls (#3b).
  • Gate false-positives the model flagged: test-sibling-required on pure-renderer modules; package-exact-deps noise on throwaway projects.

The local model's #1 reported friction: the write-guard reformats a file right
after create/edit (eslint --fix + prettier), so the model's next edit oldString no
longer matches disk → not-found → a wasted turn re-reading the file.

Two fixes:
- fuzzyLineReplace now also tolerates a line-trailing comma/semicolon (prettier
  adds trailing commas in multiline literals/args and normalizes semicolons), so
  more near-miss edits land silently. Unique-window guard still blocks ambiguity.
- A not-found edit rejection now INLINES the file's current content (numbered,
  <=400 lines) and drops the 'go read it' instruction, so the model repairs the
  stale anchor in the same turn. Falls back to advising read for huge files.

Tests: genuine multi-line fuzzy cases (trailing comma, semicolon) + a doEdit
rejection-carries-content test. From the DeepSeek harness feedback; borrowed-ideas
#3 (+#2).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses the 'edit-on-autoformat' friction by enhancing the fuzzy line replacement logic to tolerate trailing commas and semicolons, and by inlining the current file content (up to 400 lines) directly into 'not-found' edit rejection messages so the model can repair stale anchors in the same turn. Corresponding unit tests have been added to verify these behaviors. Feedback suggests wrapping the file-reading logic in currentFileView with a try/catch block to gracefully handle unexpected I/O errors and prevent tool execution crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/core/src/loop/tools/file-ops.ts
agjs added 2 commits June 28, 2026 21:50
The edit already failed when we build its rejection; if reading the file to inline
its content throws (race, permissions, lock), return null and fall back to advising
a read — never crash doEdit.
Drives the actual formatFile (eslint --fix + prettier), not a hand-built string:
prettier adds a trailing comma/semicolon, and the model's pre-format anchor still
lands via the widened fuzzy matcher instead of rejecting. Institutionalizes the
manual verification of the edit-on-autoformat fix.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces edit friction caused by the write-guard’s post-write auto-formatting (eslint --fix + prettier) by improving stale-anchor recovery: edits are more likely to match after formatting, and failures include enough context to retry without spending an extra turn on read.

Changes:

  • Widened fuzzyLineReplace normalization to tolerate line-trailing commas/semicolons (common prettier drift).
  • Enhanced edit not-found rejections to inline the file’s current content (up to 400 lines) so the model can repair anchors in the same turn.
  • Added unit + e2e coverage to validate both the widened matcher and the new rejection message behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/core/tests/files-edit.test.ts Adds focused tests for comma/semicolon-tolerant fuzzy matching and for inlined-content not-found rejections via doEdit.
packages/core/tests/edit-autoformat.e2e.test.ts New e2e test that drives the real formatter (formatFile) and verifies a pre-format anchor still applies post-format.
packages/core/src/loop/tools/file-ops.ts Inlines current file content on edit:not-found to avoid a forced read round-trip; refines not-found help text.
packages/core/src/files/edit.ts Extends fuzzy line normalization to ignore trailing ,/; for formatter drift tolerance.
docs/borrowed-ideas.md Updates the “Borrowed ideas” status note to reflect the new stale-anchor mitigations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/loop/tools/file-ops.ts Outdated
It returns read's numbered body WITHOUT the hashline header — by design (it
repairs a str_replace edit, which anchors on verbatim text, not a line hash).
@agjs agjs merged commit af8d84d into main Jun 28, 2026
8 checks passed
@agjs agjs deleted the feat/edit-autoformat-resilience branch June 28, 2026 20:30
agjs added a commit that referenced this pull request Jun 28, 2026
…ean write (#60)

* feat(write-guard): preventive ACI echo of post-format content on a clean write

Roadmap #2 (SWE-agent ACI), preventive half. On a CLEAN write whose content the
strip/auto-format pass reshaped, the write-guard now echoes the post-format file
(numbered, <=80 lines) so the model's next edit anchors on disk reality instead of
the now-stale text it wrote — preventing the stale-anchor not-found that #57 only
recovers from. No echo when nothing diverged (token-smart) or for large files (a
short re-read note instead). Pure reformatEcho() extracted + unit-tested.

* fix(write-guard): trim trailing newline before numbering the echo (Gemini review)

A …\n-terminated file produced a phantom empty last line in the numbered echo,
which the model could anchor on. Drop the standard trailing newline before split.
Test asserts exactly N numbered lines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants