Fix false-success CSV reporting on cancel, stale plan, and mid-decide cancellation - #107
Merged
Merged
Conversation
ConversionOrchestrator's decide pass marks eligible entries "would convert" before the user is ever asked. Cancelling, hitting a stale plan, or failing to plan returned without correcting that, so the same entries later exported to CSV as Converted even though nothing was written. Mark them NotAttempted at each no-write return, the same idiom already used for an interrupted run. Also corrects SAFETY.md/CONVERSION-WORKFLOW.md, which said an already-matching file is not decoded (it is now strictly validated before being reported Unchanged), and strengthens WhatIf_NeverModifiesAnything, which was silently refused before reaching the dry-run branch it meant to test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous fix marked entries NotAttempted at the confirmation loop's no-write return points, but cancellation could also escape earlier: from RefreshSourceSnapshots, from either whatIf decide-pass RunPass call, or from the explicit ThrowIfCancellationRequested check, none of which were guarded. Any of those left the decide pass's "would convert" marks on entries with no reconciliation, reproducing the same false-success CSV export the prior fix closed. Run() now wraps the whole decide/confirm/write sequence (extracted to DecideConfirmAndRun) in one cancellation handler that marks every entry NotAttempted before rethrowing. The write pass's own cancellation handling is untouched: it returns an Interrupted result rather than throwing, so it never reaches the new outer catch. Also covers the ApplyChosenSource-fails-to-Cancelled branch with a test using a mixed batch, since the existing test for it used a fixture that was never marked Converted in the first place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 task
pull Bot
pushed a commit
to jnnycn007/EncodingChecker
that referenced
this pull request
Sep 13, 2026
CLAUDE.md and AGENTS.md carry the same project-specific instructions (safety rules, shared-detector boundary, build/test and file conventions) for each agent's own convention, differing only in which global preferences file they say they supplement. Also logs BL-33 in the defect ledger: the false-success CSV reporting defect fixed across PR amrali-eg#107, distinct from BL-31 which covered only the write-pass-interrupted case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConversionOrchestrator's decide pass marks eligible entries "would convert" before the user is ever asked. Cancelling the confirmation, hitting a stale plan, failing to plan, or theChooseSourceEncodingretry not matching any file all returned without correcting that, so those same entries could later export to CSV asConvertedeven though nothing was written. Fixed by marking themNotAttemptedat each no-write return, the same idiom already used for an interrupted run.Run()fromRefreshSourceSnapshots, eitherwhatIfdecide-pass call, or the explicitThrowIfCancellationRequestedcheck was completely unguarded.Run()now wraps the whole decide/confirm/write sequence in one cancellation handler that reconciles entries before rethrowing; the write pass's own cancellation handling (which returnsInterruptedrather than throwing) is untouched.docs/SAFETY.md/docs/CONVERSION-WORKFLOW.md, which said an already-matching file is not decoded — it is now strictly validated in full before being reportedUnchanged.WhatIf_NeverModifiesAnything, which was silently refused before ever reaching the dry-run branch it meant to test.Test plan
dotnet build sources/EncodingChecker.sln --configuration Releasedotnet test sources/EncodingChecker.sln --configuration Release --no-build— 819/826 pass; the 7 failures are pre-existingGuiSmokeExecutionTestsfailures on this machine (pwsh.exenot on PATH), unrelated to this change.🤖 Generated with Claude Code