Skip to content

fix(apply-diff): report all failed diff blocks and diagnose mistyped start_line markers - #15

Merged
simurg79 merged 1 commit into
mainfrom
fix/apply-diff-failure-reporting
Aug 2, 2026
Merged

simurg79 merged 1 commit into
mainfrom
fix/apply-diff-failure-reporting

Conversation

@simurg79

@simurg79 simurg79 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Problem

When apply_diff was called with multiple diff blocks and more than one block failed to apply, only the last failure was surfaced back to the model. Every earlier failure was silently overwritten. The model therefore received an error message describing a single block, with no indication that other blocks had also failed, and would typically retry blindly — often resending the blocks that had already applied successfully, or "fixing" the one block it could see while the real breakage lay elsewhere.

A closely related failure mode made this worse: a common way to break a diff block is to mistype the :start_line: marker (wrong spelling, missing colons, stray indentation). That produces a generic "search content not found" error that gives the model no signal about the actual cause, so it retries with the same malformed marker.

Defects fixed

1. Only the last failure reached the model.
All failed blocks are now reported together. Each is labeled with a --- Diff block N of M --- header so the model can map a failure back to the specific block it sent, with bounded per-block detail (see caps below).

2. Mistyped start-line markers produced an unhelpful generic error.
When a failed block's first line looks like a mistyped start-line marker, the error now includes a LIKELY CAUSE hint naming that as the probable problem, instead of only reporting that the search content was not found.

3. Partial-success messaging was ambiguous.
The partial-success path now reads Applied X of Y diff blocks and explicitly instructs the model to resend only the failed blocks. Previously the wording invited resending the entire diff, which risked re-applying already-applied blocks.

Bounded output

Reporting every failure risks flooding the context window, so the output is capped:

  • a maximum number of individually reported failed blocks, with a summary line accounting for any remainder;
  • a per-block cap on the amount of failure detail emitted;
  • truncation of overlong content within a block's detail.

This keeps the worst case bounded regardless of how many blocks were sent or how large each one is.

Validation

  • 90/90 tests pass, including two new suites:
    • src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts
    • src/core/diff/strategies/__tests__/multi-search-replace-marker-hint.spec.ts
  • check-types clean across all packages.
  • ESLint clean (--max-warnings=0).
  • Prettier clean.

Known limitation

searchBlocks is counted with a regex over occurrences of the literal <<<<<<< SEARCH. If that literal appears inside a block's content (for example, a diff that is itself editing diff-handling code or documentation about the diff format), the count will be too high. This value is advisory only — it is used for the N of M labels and the Applied X of Y summary, and does not drive matching or application logic, so an overcount degrades only the wording of the message and cannot cause an incorrect edit.

…start_line markers

Previously only the last failure reached the model, so multi-block apply_diff errors were effectively invisible and the model retried blind.

- Report every failed block with '--- Diff block N of M ---' labels and bounded per-block detail instead of surfacing only the final failure.

- Emit a 'LIKELY CAUSE' hint when a failed block's first line looks like a mistyped start-line marker.

- Partial success now reads 'Applied X of Y diff blocks' and instructs resending only the failed blocks.
@simurg79
simurg79 merged commit 0f58420 into main Aug 2, 2026
9 checks passed
simurg79 pushed a commit to simurg79/Zoo-Code that referenced this pull request Oct 17, 2026
Port simurg79/Roo-Code#15.

Upstream-commit: d9d96e47006786acbb05d6ef0ee7a927f91f8f12

Retain Zoo diagnostics and telemetry; adapt regression tests to typed mocks and verify repeated-failure reporting. Exclude upstream changeset.
simurg79 pushed a commit to simurg79/Zoo-Code that referenced this pull request Oct 19, 2026
Port simurg79/Roo-Code#15.

Upstream-commit: d9d96e47006786acbb05d6ef0ee7a927f91f8f12

Retain Zoo diagnostics and telemetry; adapt regression tests to typed mocks and verify repeated-failure reporting. Exclude upstream changeset.
yarcowang pushed a commit to yarcowang/Zoo-Code that referenced this pull request Oct 23, 2026
…oo-Code-Org#2053)

* fix(apply-diff): port bounded failure reporting from upstream PR15

Port simurg79/Roo-Code#15.

Upstream-commit: d9d96e47006786acbb05d6ef0ee7a927f91f8f12

Retain Zoo diagnostics and telemetry; adapt regression tests to typed mocks and verify repeated-failure reporting. Exclude upstream changeset.

* fix(apply-diff): name target file in multi-block failure output

* test(apply-diff): kill surviving mutants in failure reporting and marker hint

* test(apply-diff): pass toolCallId required by ToolCallbacks on main

* fix(apply-diff): count line-level markers and retain main guidance

* fix(apply-diff): avoid claiming failed blocks are absent; assert full replacement content

* fix(apply-diff): report partial-apply totals from the parsed block count

* test(apply-diff): assert repaired first-block content

* fix(apply_diff): limit file excerpts in multi-failure and partial-apply reports

Only the first failed block keeps Search/Best Match/Original Content sections (main parity); later blocks and the partial-apply result keep diagnostics only.

* fix(apply_diff): report only non-sensitive diagnostics on diff failures

Stop echoing search, best-match and original file content from failed SEARCH blocks and never forward DiffResult details. Failures now carry numeric scores, line locations and constant hints for single, multi-block, partial-success and fallback paths.

* fix(apply_diff): stop echoing malformed marker text; align e2e error fixture

The marker hint no longer interpolates the first search line. The e2e mock fixture matched on search text that the failure output no longer contains, so the error smoke test timed out.

* fix(apply_diff): guard marker-hint regex length, label failures by ordinal, cover zero-applied clamp

* style(apply_diff): split marker-hint guard to satisfy prettier

* test(apply-diff): make overlong marker fixture exercise the length guard

* test: pin 64/65-char boundary of marker hint length guard

---------

Co-authored-by: Bertan Ari <bertanari@microsoft.com>
edelauna pushed a commit to myk1yt/Zoo-Code that referenced this pull request Oct 28, 2026
…oo-Code-Org#2053)

* fix(apply-diff): port bounded failure reporting from upstream PR15

Port simurg79/Roo-Code#15.

Upstream-commit: d9d96e47006786acbb05d6ef0ee7a927f91f8f12

Retain Zoo diagnostics and telemetry; adapt regression tests to typed mocks and verify repeated-failure reporting. Exclude upstream changeset.

* fix(apply-diff): name target file in multi-block failure output

* test(apply-diff): kill surviving mutants in failure reporting and marker hint

* test(apply-diff): pass toolCallId required by ToolCallbacks on main

* fix(apply-diff): count line-level markers and retain main guidance

* fix(apply-diff): avoid claiming failed blocks are absent; assert full replacement content

* fix(apply-diff): report partial-apply totals from the parsed block count

* test(apply-diff): assert repaired first-block content

* fix(apply_diff): limit file excerpts in multi-failure and partial-apply reports

Only the first failed block keeps Search/Best Match/Original Content sections (main parity); later blocks and the partial-apply result keep diagnostics only.

* fix(apply_diff): report only non-sensitive diagnostics on diff failures

Stop echoing search, best-match and original file content from failed SEARCH blocks and never forward DiffResult details. Failures now carry numeric scores, line locations and constant hints for single, multi-block, partial-success and fallback paths.

* fix(apply_diff): stop echoing malformed marker text; align e2e error fixture

The marker hint no longer interpolates the first search line. The e2e mock fixture matched on search text that the failure output no longer contains, so the error smoke test timed out.

* fix(apply_diff): guard marker-hint regex length, label failures by ordinal, cover zero-applied clamp

* style(apply_diff): split marker-hint guard to satisfy prettier

* test(apply-diff): make overlong marker fixture exercise the length guard

* test: pin 64/65-char boundary of marker hint length guard

---------

Co-authored-by: Bertan Ari <bertanari@microsoft.com>
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.

1 participant