From ee460de97b0f9e50367eaae078df4d3c244d1f2e Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 6 May 2026 15:40:36 +0800 Subject: [PATCH] docs(attribution): align cleanup-branch comments with noteCommitWithoutClearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three doc/test-fixture stale-after-refactor cleanups (Copilot 4MDx / 4MEI / 4MEa): - shell.ts:1944 (around the stagedInfo === null branch): the comment still claimed the finally block "falls back to a full clear", but 1ece87438 switched analysis-failure cleanup to noteCommitWithoutClearing(). Update the comment so the reasoning matches what the code actually does (and so a future reader doesn't reintroduce the wholesale clear thinking it's already there). - shell.ts: getCommittedFileInfo docstring carried the same stale "full clear" claim for the `null` return value. Update to describe the noteCommitWithoutClearing() fallback and the smaller-evil trade-off for the just-committed file. - chatRecordingService.test.ts: baseSnapshot fixture for the recordAttributionSnapshot tests still carried `baselines: {}`, even though that field was removed from AttributionSnapshot in 296fb55ae's dead-code purge. Structural typing let it compile, but the fixture didn't reflect the production shape — drop it. --- .../src/services/chatRecordingService.test.ts | 1 - packages/core/src/tools/shell.ts | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 0459e93df..b866252bf 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -435,7 +435,6 @@ describe('ChatRecordingService', () => { version: 1, surface: 'cli', fileStates: {}, - baselines: {}, promptCount: 0, promptCountAtLastCommit: 0, }; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 2afb2d8d7..fd87c6a36 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -1943,8 +1943,12 @@ export class ShellToolInvocation extends BaseToolInvocation< // null = analysis failed (shallow clone, --amend without reflog, // partial diff failure, etc.). Leave `committedAbsolutePaths` - // null so the finally block falls back to a full clear and we - // don't leak stale per-file attributions into the next commit. + // null so the finally block calls `noteCommitWithoutClearing()` + // — snapshotting the prompt counter while leaving per-file + // attributions intact. (Earlier revisions of this code did a + // wholesale clear here, but that erased pending unstaged AI + // edits for files outside the just-failed commit; the + // smaller-evil trade-off is documented in the finally block.) // Skip the note write entirely — emitting a structurally valid // but factually wrong all-zero note is worse than no note. if (stagedInfo === null) { @@ -2109,8 +2113,12 @@ export class ShellToolInvocation extends BaseToolInvocation< * - `null` when analysis itself failed (shallow clone with no parent * object, --amend with no reflog, partial diff failure, exception). * The caller treats this as "could not determine the committed - * set" and falls back to a full clear so stale per-file state - * doesn't leak into a subsequent commit. + * set" and falls back to `noteCommitWithoutClearing()` — snapshots + * the prompt counter but leaves per-file attribution intact, so + * pending AI edits for files NOT in the just-committed set don't + * get wiped along with the analysis failure. (The just-committed + * file's stale entry may re-attribute on a later commit; that's + * the smaller evil compared to wholesale loss.) */ private async getCommittedFileInfo( cwd: string,