Skip to content

fix(runtime): preserve Edit diffs in large files - #3652

Merged
Astro-Han merged 2 commits into
mainfrom
fix/edit-span-diff
Aug 24, 2026
Merged

fix(runtime): preserve Edit diffs in large files#3652
Astro-Han merged 2 commits into
mainfrom
fix/edit-span-diff

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve unified diff previews when Edit makes a small localized replacement in a large file. The runtime now uses the match span already produced by Edit, so diff cost depends on the changed window instead of the full file. Oversized replacements remain bounded and fall back to the existing summary without blocking the edit.

Also removes the unused UI diff-line classifier and barrel export; DiffCodePreview is the remaining rendering authority.

The branch temporarily carries the one-line canonical storage decoder repair already present in #3633 because current main does not build without it. That prerequisite disappears from this diff when #3633 lands.

Verification

  • Runtime build passed.
  • Runtime and UI typechecks passed.
  • Biome format check passed across 1604 files.
  • 101 focused runtime tests passed, including red-to-green coverage for large-file Edit diffs and oversized replacement fallback.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex diagnosed the diff cutoff, implemented the localized bounded diff path, removed the proven-dead classifier, and added focused tests.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@Astro-Han
Astro-Han changed the base branch from main to fix/steering-durable-admission August 23, 2026 21:16
@Astro-Han
Astro-Han force-pushed the fix/edit-span-diff branch 2 times, most recently from eae2546 to a600527 Compare August 23, 2026 21:18
@Astro-Han
Astro-Han changed the base branch from fix/steering-durable-admission to main August 23, 2026 21:18
@yunaremaia

Copy link
Copy Markdown
Contributor

Reviewed the diff on 2fe6c14.

The problem statement is clear: createUnifiedDiff produced a full-file diff for small localized edits in large files, which was either suppressed entirely or returned an unwieldy wall of context. The fix introduces createEditUnifiedDiff and narrows the diff to the localized hunks — the right abstraction at the right layer.

What I checked:

  • filesystem-executor.ts now captures originalContent inside the transform closure of the readModifyWrite path, and threads it into createEditUnifiedDiff. This is the correct place: the closure is the only point where both pre-edit and post-edit content are available in scope, and workspace.readModifyWrite does not expose them after the fact (the result only carries previous / finalContent, which are post-traversal). The pattern avoids leaking capture by the scope of the closure itself — clean.

  • filesystem-worker/operations.ts mirrors the same change on the worker path, where the file is read from a FileHandle rather than a readModifyWrite contract. Same direction, same fix. DRY is preserved by sharing createEditUnifiedDiff.

  • The test matrix covers three cases: (1) localized change in 900-line file returns file_diff with the targeted hunk — happy path; (2) same case on the worker path with readFile confirmation; (3) oversized replacement (900-line expansion) still omits the diff to avoid token blow-up. This last case is the one I'd expect to regress later if someone does not read the diff-limit guard — the assertion on diff === undefined + readFile confirming the replacement landed is a good pin.

  • One thing to check: the computeEditedSource return includes edited.content (post-edit) and edited.oldStart/edited.oldEnd/edited.newStart/edited.newEnd (hunk boundaries). If createEditUnifiedDiff reads those hunks for context and those values are 0-indexed vs 1-indexed, the diff could be off-by-one in edge cases. The diff itself is not in this PR's changed lines (packages/runtime/src/unified-diff.ts), so I cannot verify the boundary arithmetic here — but the test diff matching (/-const v500 = 500;/ and /\+const v500 = -1;/) confirms it is correct for at least this case. Worth a pinning assertion on the actual line numbers in the diff header if the codebase does not have one elsewhere.

  • The diff-limit guard (MAX_DIFF_CHARS or equivalent) is the right backstop — if the replacement is large enough that the localized diff still exceeds the budget, the old behavior (suppress the diff) is preserved. That avoids a token-explosion regression when the model writes back a large file.

Clean, focused change. LGTM.

@Astro-Han
Astro-Han marked this pull request as ready for review August 24, 2026 02:13
Generate Edit previews from the known replacement span so small changes in large files retain their unified diff without running a full-file quadratic comparison. Keep both calculation and output bounded for oversized replacements.

Generated-by: Codex
DiffCodePreview owns diff row classification, leaving the former tool-result helper and barrel export without consumers.

Generated-by: Codex

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GO on exact head 208590409572a0bfd086108e01ca51727ad8f235; 0×P0–P3.

The Edit path now uses its already-authoritative match span to bound diff work to the changed window while retaining 3 lines of context. Window line/byte caps and a final output-byte cap make oversized replacements fall back to the existing structured Edit summary without blocking the write. A 5,000-case randomized differential application covered first/middle/last positions plus multiline insert/delete shapes; every emitted patch reconstructed the exact new content. Focused Runtime evidence passed 82/82, and reverting the three new call sites to the old full-file generator made exactly the two large-file regressions fail. Runtime/UI builds and changed-file Biome passed. diffLineKind has no remaining repository consumer, so removing its implementation and barrel export leaves DiffCodePreview as the single classifier.

Fresh gate: PR OPEN, non-draft, MERGEABLE; exact-head test and windows_sandbox_w0_protocol are completed/success; zero stale or current APPROVED reviews existed before this action; no live P0–P2 or hold. Published by the Kabi review lane under @WAWQAQ standing authorization. No merge performed.

@Astro-Han
Astro-Han merged commit 8e7fa68 into main Aug 24, 2026
2 checks passed
@Astro-Han
Astro-Han deleted the fix/edit-span-diff branch August 24, 2026 02:37
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.

3 participants