Skip to content

feat: fewer round trips for fixes: optional equivalenceKey, dryRun with diffs, multi-document writes - #53

Merged
Arthurvdv merged 6 commits into
mainfrom
feat/43-fix-ergonomics
Oct 29, 2026
Merged

Arthurvdv merged 6 commits into
mainfrom
feat/43-fix-ergonomics

Conversation

@Arthurvdv

Copy link
Copy Markdown
Member

Summary

Fewer round trips for the fix tools, and fixes that edit several files are written completely:

  • equivalenceKey is optional on apply_fix. Without it, a rule with one distinct fix is applied directly; a rule with several returns the error Ambiguous with candidates, exactly as apply_fix_all does. Both tools choose the fix through one shared CodeFixRunner.ChooseFix, and the result reports the equivalenceKey that was applied.
  • dryRun on apply_fix, and a unified diff per changed file on both writers. Every apply_fix / apply_fix_all result carries changes: [{ filePath, diff, truncated }], on a dry run and after a real write, so an agent can show what changed without re-reading files. The diff is hunks only (3 context lines, no ---/+++ header), built by a hand-written line differ (UnifiedDiff: Myers O(ND) on the trimmed middle, edit-distance cap, 200 KB per-file cap cut at a line boundary). No new package.
  • Every document a fix changes is written. CodeFixRunner.CollectChangesAsync diffs the code action's solution against the original (Solution.GetChanges) and is now the only place changed documents are discovered, for both tools. Side edits to other files are written too; the iterative fix-all fallback threads the whole Solution so they survive. apply_fix writes a multi-file fix all-or-nothing (GuardedFileWriter.WriteAllIfUnchangedAsync(allOrNothing: true)); apply_fix_all does the same whenever its change set includes a side edit to a file without an occurrence. A fix that adds, removes or renames files, or edits a file outside the project, is NotFound with the new reason UnsupportedFix and nothing is written.

Closes #43.

Commits

  1. a33ae8d feat: apply_fix without equivalenceKey applies the single fix or returns Ambiguous
  2. 835dd9b feat: write every document a fix changes
  3. 061d0f6 feat: dryRun on apply_fix and unified diffs in both writers' results
  4. d4554d9 fix: address review round 1 (all-or-nothing fix-all side edits, diff cap/marker edge cases, rename = UnsupportedFix, docs, tests)
  5. a853b41 fix: address review round 2 (unfixed positions after a refused batch, get_fixes wording, tests)
  6. 18a1c64 fix: address review round 3 (one unfixed-diagnostics rule for every unwritten change, docs)

Contract changes

  • apply_fix: equivalenceKey leaves required; new parameter dryRun (default false); result is { applied, dryRun, diagnosticId, fixTitle, equivalenceKey, filePath, changes } (previously { applied, filePath, fixTitle, diagnosticId }). New error outcome Ambiguous (no key, several fixes).
  • apply_fix_all: result gains changes, which describes exactly the files in filesChanged; filesChanged is now ordered by path.
  • NotFound gains the reason UnsupportedFix.
  • tools/list contract tests updated accordingly; titles, hints and the pinned sentences in ToolDescriptions are unchanged.

Verification

  • dotnet build --configuration Release --no-incremental: 0 warnings. dotnet test --configuration Release: all tests green (378 on main, 413 on this branch).
  • New tests: UnifiedDiffTests (hunk headers and counts, merge distance, CRLF and mixed terminators, no-newline marker, cap fallback, truncation), MultiDocumentFixTests (two-document fix written by both tools, add-document fix refused, stale second file writes nothing, invalid UTF-8 second file, failed second commit rolled back, stale side-edit file in fix-all, multi-file dry run), apply_fix without a key (single fix applies, AC0012 is Ambiguous), round trip of every advertised key.
  • Three Sonnet review rounds (effort high) before this PR, each followed by a fix commit; round 3 found nothing blocking. The round-1 reviewer fuzzed UnifiedDiff with about 260k random inputs (an applier re-derived the new text from the hunks) and 40 large inputs through the edit-distance cap: no incorrect hunk.

Known limitations (deliberate, from the review rounds)

  • Diffs are returned on real writes too and are capped per file only; a project-wide apply_fix_all over many files returns a large result. Per the issue; an aggregate cap was considered and not taken.
  • In the iterative fix-all fallback (a provider without a FixAllProvider), a cross-file side edit into a file whose own diagnostics are still pending can shift their spans; the remaining-diagnostics recheck reports what was missed. No current ALCops provider edits a second document.
  • The apply_fix_all all-or-nothing trigger is "a changed file without an occurrence of the rule". A cross-file fix whose second file also has an occurrence keeps the documented per-file stale handling.
  • apply_fix applies the first action per equivalence key and its first ApplyChangesOperation; two different actions that both have a null key collapse into one candidate (pre-existing, documented).
  • A fix that only changes line endings is written but shows an empty diff; a truncated diff keeps its original hunk headers.
  • CollectChangesAsync does not see additional documents or project-level changes; a case-only rename is not detected (the Workspaces API currently cannot produce a rename at all).

🤖 Generated with Claude Code

Arthurvdv and others added 6 commits October 28, 2026 16:03
…rns Ambiguous

equivalenceKey is now optional on apply_fix. Without it, a rule that
offers one distinct fix is applied directly; a rule with several is the
error Ambiguous with candidates, worded like apply_fix_all. apply_fix and
apply_fix_all choose the fix through one shared CodeFixRunner.ChooseFix,
and FixApplyResult gains a FixApplyStatus mirroring FixAllStatus. The
first action per key is the one applied; an action that changes nothing
is NoFixForDiagnostic.

Refs #43

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CodeFixRunner.CollectChangesAsync diffs the fix's solution against the
original with Solution.GetChanges and is now the only place changed
documents are discovered, for apply_fix and apply_fix_all alike. Side
edits to other files are written too; the iterative fix-all fallback
threads the whole Solution so they survive. A fix that adds or removes a
file or project is NotFound with the new reason UnsupportedFix and
nothing is written.

apply_fix writes a multi-file fix through
WriteAllIfUnchangedAsync(allOrNothing: true): a stale file is Stale and
nothing is written, another phase-1 conflict is Faulted with its kind,
and a failed commit is rolled back and reported as Faulted/WriteFailed.
The tools forward to an internal RunAsync that takes analyzer resolution
as a delegate, the test seam for test-only fix providers.

Refs #43

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
apply_fix gains dryRun (after analyzers, as on apply_fix_all) and now
returns { applied, dryRun, diagnosticId, fixTitle, equivalenceKey,
filePath, changes }, where equivalenceKey is the key that was applied.
apply_fix_all adds changes next to filesChanged and describes exactly
those files: every computed change on a dry run, the written files on a
real run, none after a rolled-back batch.

Each change is { filePath, diff, truncated }. The diff comes from the new
UnifiedDiff, a hand-written line-based differ: hunks only, 3 lines of
context, GNU-style counts and no-newline markers, Myers O(ND) on the
trimmed middle with an edit-distance cap of 2,000, and a 200 KB per-file
cap cut at a line boundary.

Refs #43

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s, diff edge cases, docs, tests)

- apply_fix_all: a fix-all whose changes include side edits to files without
  an occurrence is written all or none; on a phase-1 conflict nothing is
  written and every change's diagnostics are reported as unfixed.
- UnifiedDiff: stop building hunks once past the 200 KB (200 x 1024
  characters) cap, and never cut between a line and its
  "\ No newline at end of file" marker.
- CollectChangesAsync: a document rename is UnsupportedFix; the
  UnsupportedFix wording covers added, removed or renamed files and edits to
  files outside the project.
- Docs: line-ending-only changes, truncated hunk headers, rolled-back batch
  wording, where the RunAsync catch lives.
- Tests: multi-file apply_fix dry run, invalid UTF-8 second file, failed
  second commit; stale side-edit file in apply_fix_all; diff truncation.
  No rename test: the BC Workspaces API has no Solution.WithDocumentFilePath
  or WithDocumentName.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d batch, get_fixes wording, tests)

- apply_fix_all: after an all-or-nothing refusal, unfixedDiagnostics keeps
  Unfixed entries only for untouched files (post-fix coordinates of changed
  files do not exist on disk) plus every change's original diagnostics.
- UnifiedDiff.Cut never ends on a bare hunk header.
- get_fixes description, README and AGENTS.md: a listed fix can still be
  refused as NotFound/UnsupportedFix.
- Tests: refusal merge, all-or-nothing batch with a commit failure, Cut
  dropping a bare hunk header.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…itten change, docs)

- apply_fix_all: one MergeUnfixed rule for every real run. A changed file that
  was not written (skipped as a conflict, refused all-or-nothing, or rolled
  back) reports its original diagnostics and drops its post-fix residuals,
  which only exist in text that never reached the disk; a dry run keeps the
  post-fix residuals because they describe what would remain.
- Docs: an empty diff with truncated: true means the first changed line alone
  exceeds the cap; the apply_fix description says filePath is the file of the
  diagnostic and that changes, not filePath, lists what changed.

Refs #43

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Arthurvdv
Arthurvdv merged commit c86ca80 into main Oct 29, 2026
14 checks passed
@Arthurvdv
Arthurvdv deleted the feat/43-fix-ergonomics branch October 29, 2026 10:39
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.

Fewer round trips for fixes: optional equivalenceKey, dryRun with diff, multi-document changes written

1 participant