Skip to content

refactor(codegraph-adapter): remove snapshots (#574 S4) - #599

Draft
Ancienttwo wants to merge 3 commits into
mainfrom
feat/574-s4-delete-snapshot
Draft

Ancienttwo wants to merge 3 commits into
mainfrom
feat/574-s4-delete-snapshot

Conversation

@Ancienttwo

@Ancienttwo Ancienttwo commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

Summary

删除 CodeGraph snapshot copy / child reader 路径,让 ref impact 和 flows check --no-sync 通过 ADR-0058 read-only session 读取已就绪的 index,并删除三个已退役的 snapshot failure reasons。Refs #574。Tier: High。

Draft,等待 review。Head 735b9ddeff45413901040ab1050e8ec463e72214,base 6efd32ad1e2448e7f4e1dd9b69d6dcd23c5c9284。按 JD-1 本 PR 最后合并,合并前由 coordinator 安排 merge-only sync。

Changes

  • 删除 snapshot.ts、snapshot-query.ts 及 index.ts 的 snapshot re-exports。
  • 新增 ready-index-read.ts:一个 read-only session 内检查 readiness 再读取,revision 在读取期间变化时按 not-ready 失败;ref-impact-facts 和 flow-selector-check 改走这里。
  • failure.ts 删除 code-facts-snapshot-changed、code-facts-snapshot-limit-exceeded、code-facts-snapshot-layout-unsupported,contracts 与 CLI failure-envelope fixture 同步。
  • CLI flows help(main.ts 的 case "flows" 块)改为描述 ADR-0058 read-only session,不再提 disposable private snapshots 和 unsupported-layout / over-limit snapshots。
  • Node integration 分别 bundle adapter entry 和 ready-index-read;更新 ref-impact runbook。
  • 配置了 clean filter 的仓库现在能拿到 evidence(snapshot 过去直接拒绝)。

S2 的 packagedCodeGraphRuntimeDigest export 与 S2 名下的 capability-projection-inputs.test.ts 区域(import-graph cases、seedWorkspace/fakeCli、combined import/selector、selector-fanout、evidenceFor 六个 built-in cases)本 PR 未改;冲突在本 PR 的 merge-only sync 中处理。

Decisions resolved

见 joint decision:

  1. JD-2:test/real-index.ts 保留私有 codeGraphNodeRuntime()(+15/-4),不为测试保留 production export。
  2. JD-3:flows help 在本 PR 修正(commit 735b9dd),只改 case "flows" 区域。
  3. JD-4:以 ADR-0058 Decision 3 为准。test/git-spy.ts 的 CodeGraph read allow-list 固定为 ls-files、rev-parse、check-ignore、config --get core.excludesFile;ref impact 自己的 ls-tree / cat-file / log 只在 ref-impact 测试里以调用参数传入,不进入该 allow-list。flows 测试不传额外 verb。ref-impact 测试标题原写 "no config",已改成 "no ... config beyond core.excludesFile" 与断言一致;无断言改动。

Deleted assertions → retained coverage

Deleted test (old file) Behavior Retained coverage
private snapshots refuse the literal filter driver %s (capability-projection-inputs) 读取不得执行 clean filter ref-impact a configured clean filter still gets evidence…:marker 不出现 + git spy 无 filter 相关命令;flow-selector-check no-sync … ready git spy
private snapshots allow inert filter configuration and reject effective filters (capability-projection-inputs) 读取不改 .git/index、不跑 filter 同上;read-session a fresh index is ready; the read changes neither the database nor .git/index, and runs no git status or diff
private snapshot reads real facts without changing original SQLite sidecars (live WAL) (capability-projection-inputs) 读取不改数据库与 WAL,live holder 不受影响 Node integration readyIndexReads: closed-and-live-wal-preserved;flow-selector-check no-sync preserves original checkout and index for *(ADR-0058 Decision 4 允许的 shm/空 WAL 除外)
private snapshot reads Git objects through a read-only alternate… (capability-projection-inputs) snapshot 期间不写 Git objects;code-facts-snapshot-changed 行为随 snapshot 删除:读取不再复制 Git;ref impact 的 blob 读取由 real CLI binds CodeGraph ranges to Git blobs without touching the source index 覆盖;读取期间 index 变化由 read-session G4 a revision that moves between reads… 覆盖
private snapshot runs no repository or configured hook… (capability-projection-inputs) 读取不触发 hook 两处 git spy 断言只允许 Decision 3 命令(这些命令不触发 hook);hooksPath pin 按 ADR-0058 Decision 3 不再需要
private snapshot limit stops copying… (capability-projection-inputs) code-facts-snapshot-limit-exceeded 行为随 snapshot 删除(不再复制),reason 从 contracts 删除,见 release note
private source mirror preserves provider includeIgnored, include and extension overrides (capability-projection-inputs) mirror 与 provider 配置一致 行为随 mirror 删除:读取直接使用仓库自身 index,不存在 mirror;freshness 由 read-session freshness() is CodeGraph's change test 与 an edit, a new source and a deletion are counted 覆盖
private snapshot readiness mapping (C2): index-open-failed through the snapshot (read-session) 打不开的 index 报 index-open-failed flow-selector-check 新增 unopenable 状态;read-session a read-only open that throws is index-open-failed、a linked .codegraph is index-open-failed
flow-selector-check foreign 期望 code-facts-snapshot-layout-unsupported linked/foreign .codegraph 失败关闭 同一测试改为期望 index-open-failed
failure-envelope fixtures 中的 snapshotLimit 字段(contracts、CLI) envelope 字段集 同一测试保留,字段随 reason 删除

Verification (head 735b9dd)

本机运行(load average 约 96,后降到 27):

  • bun run typecheck:clean。
  • node scripts/codegraph-selector-node-integration.mjs:status: verified。
  • node scripts/package-boundary-audit.mjs:passed。
  • bun test packages/local-runtime/runtime-daemon/test/flow-selector-check.test.ts packages/surfaces/cli/test/ref-impact.test.ts packages/local-runtime/codegraph-adapter packages/contracts/test/failure-envelope.test.ts packages/surfaces/cli/test/failure-envelope.test.ts:第一次 159 pass / 4 fail,第二次失败的是另一组测试;全部是超时(spawnSync ETIMEDOUT、测试超时 5s/60s 后杀掉子进程),没有 assertion failure,归类为本机负载问题。以本 head 的 CI 结果为准。

Test changes and budget

git diff --numstat 6efd32ad...HEAD:tests 127 added / 224 deleted,product 116 added / 595 deleted(src + scripts),ratio 1.09:1。docs 22 added / 29 deleted 另计。新增 test 文件 1:packages/local-runtime/codegraph-adapter/test/git-spy.ts。

Release note (0.15.0)

删除 code-facts-snapshot-changed、code-facts-snapshot-limit-exceeded、code-facts-snapshot-layout-unsupported 三个 failure reasons,属于已批准的 contracts breaking change;code-facts-read-not-ready 保留。

Risk / Rollback

不再提供 snapshot isolation:读取直接打开仓库自身 index(只读),依赖 ADR-0058 Decision 3/4 的 git 命令与 sidecar 约束,由上面的 git spy 和 sidecar 断言覆盖。Rollback: revert 本 PR;无 ledger/schema/data migration。

… and ref impact read the ready index in-process (#574 S4)

Refs #574
…removal (#574 S4)

flows check --no-sync imports withReadyCodeGraphIndex from its module, as
model-store-yaml imports ref-impact-facts; the Node integration bundles that
module beside the adapter entry.

Refs #574
@Ancienttwo

Copy link
Copy Markdown
Owner Author

repo-harness acceptance claim
Role: acceptor
Round: 1
Subject: 735b9dd
Tier: high
Agent: pi (gpt-6-astra), read-only

@Ancienttwo Ancienttwo left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

repo-harness acceptance review
Round: 1
Subject: 735b9dd
Tier: high
Verdict: FAIL
Reviewer: pi (gpt-6-astra)
Model relation: different-family

FAIL: readiness and fact queries use two read-only sessions instead of the required single session, and the deleted provider-configuration test loses coverage of still-supported source enumeration/freshness behavior. Confirmed subject and green CI with gh pr checks 599. Reviewed all ten deletion-table rows, ADR-0058, joint decisions and the scoped diff. Snapshot files/exports and three failure reasons are retired; code-facts-read-not-ready remains; PR release note and CLI envelopes agree; JD-4 keeps ref-impact verbs separate; S2-owned regions are untouched. One targeted disposable-HOME probe confirmed two opens and two closes; no passing suite or typecheck was rerun.

NITs

None.

case "open": {
// Readiness opens its own session inside this one. An unchanged revision after the read
// (design G4) proves that both sessions read one index state.
const facts = inspectRepositoryCodeFacts(root);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

P2 GK599-1

withReadyCodeGraphIndex opens an outer session, then inspectRepositoryCodeFacts(root) opens and closes a second session in repository-code-facts.ts before read(opened.session.graph) queries the first handle. This contradicts the acceptance requirement that readiness and the read run in one read-only session, and the one-handle claim in the updated runbook. A targeted probe using the existing fakeReadProvider under a disposable HOME returned opensAtCallback=2, closesAtCallback=1, and final open=2/close=2. Thus cleanup succeeds on the probed success path, but readiness is not evaluated on the queried handle. The cited G4 test at read-session.test.ts:118 exercises withCodeGraphReadSession/inspectRepositoryCodeFacts/assertRepositoryCodeFactsReady, never this new helper; it does not pin the helper's post-query revision rejection or lifetime branches. Both flows --no-sync and ref impact share this helper.

Fix: Evaluate compatibility and freshness on opened.session and reuse the existing readiness mapping without opening another handle. Add focused coverage of this helper showing one open/close for a successful read, no query on failed readiness, code-facts-read-not-ready when revision changes during the query or a writer lock appears, and closure on readiness rejection and callback failure. Keep the shared fix for both callers.

@@ -185,195 +184,6 @@ function builtinSelectorModel(sinkName: string): NativeModel {
// shared by the import-graph and exact-selector assertions: spawning it per assertion made the
// neighbouring adapter suite time out under parallel test execution.
describe("capability documentation projection inputs from the code index", () => {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

P2 GK599-2

The removed 'private source mirror preserves provider includeIgnored, include and extension overrides' case covered more than mirroring: it built facts from an opted-in ignored nested repository and an included custom-extension source, asserted both names were readable, then added external-projects/embedded/new.ts and asserted the readiness gate refused the stale index. These source-enumeration/freshness behaviors remain supported and are explicitly promised by docs/runbooks/ref-impact.md:39-40. The deletion table calls this retired with the mirror and cites read-session.test.ts:141 and :268; the first uses fakeReadProvider and the second uses ordinary src/*.ts files with no provider configuration. Neither pins includeIgnored/include/extensions behavior, and the retained test/script search has no replacement configuration fixture. This drops coverage of behavior that still exists, contrary to the review acceptance contract.

Fix: Port the still-applicable configuration assertions to the ready-index read path using a real provider fixture: ready reads must expose the opted-in ignored and custom-extension declarations, and adding a source under the configured ignored population must make the read fail as code-facts-read-not-ready. Retire only mirror-specific and old filter-refusal assertions, update the deletion-to-retained coverage table, and keep S2-owned regions untouched.

@Ancienttwo

Copy link
Copy Markdown
Owner Author

repo-harness acceptance record
Claim: #599 (comment)

Round Role Subject Verdict
1 pi (gpt-6-astra), read-only 735b9ddeff45413901040ab1050e8ec463e72214 FAIL: 2 P2

Model relation: different-family

Findings:

  • P2 GK599-1 at packages/local-runtime/codegraph-adapter/src/ready-index-read.ts:30: withReadyCodeGraphIndex opens an outer session, then inspectRepositoryCodeFacts(root) opens and closes a second session in repository-code-facts.ts before read(opened.session.graph) queries the first handle. This contradicts the acceptance requirement that readiness and the read run in one read-only session, and the one-handle claim in the updated runbook. A targeted probe using the existing fakeReadProvider under a disposable HOME returned opensAtCallback=2, closesAtCallback=1, and final open=2/close=2. Thus cleanup succeeds on the probed success path, but readiness is not evaluated on the queried handle. The cited G4 test at read-session.test.ts:118 exercises withCodeGraphReadSession/inspectRepositoryCodeFacts/assertRepositoryCodeFactsReady, never this new helper; it does not pin the helper's post-query revision rejection or lifetime branches. Both flows --no-sync and ref impact share this helper. -> Evaluate compatibility and freshness on opened.session and reuse the existing readiness mapping without opening another handle. Add focused coverage of this helper showing one open/close for a successful read, no query on failed readiness, code-facts-read-not-ready when revision changes during the query or a writer lock appears, and closure on readiness rejection and callback failure. Keep the shared fix for both callers.
  • P2 GK599-2 at packages/local-runtime/codegraph-adapter/test/capability-projection-inputs.test.ts:186: The removed 'private source mirror preserves provider includeIgnored, include and extension overrides' case covered more than mirroring: it built facts from an opted-in ignored nested repository and an included custom-extension source, asserted both names were readable, then added external-projects/embedded/new.ts and asserted the readiness gate refused the stale index. These source-enumeration/freshness behaviors remain supported and are explicitly promised by docs/runbooks/ref-impact.md:39-40. The deletion table calls this retired with the mirror and cites read-session.test.ts:141 and :268; the first uses fakeReadProvider and the second uses ordinary src/*.ts files with no provider configuration. Neither pins includeIgnored/include/extensions behavior, and the retained test/script search has no replacement configuration fixture. This drops coverage of behavior that still exists, contrary to the review acceptance contract. -> Port the still-applicable configuration assertions to the ready-index read path using a real provider fixture: ready reads must expose the opted-in ignored and custom-extension declarations, and adding a source under the configured ignored population must make the read fail as code-facts-read-not-ready. Retire only mirror-specific and old filter-refusal assertions, update the deletion-to-retained coverage table, and keep S2-owned regions untouched.

@Ancienttwo

Copy link
Copy Markdown
Owner Author

repo-harness acceptance claim
Role: acceptor
Round: 1
Subject: 735b9dd
Tier: high
Agent: claude (claude-opus), read-only

@Ancienttwo Ancienttwo left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

repo-harness acceptance review
Round: 1
Subject: 735b9dd
Tier: high
Verdict: FAIL
Reviewer: claude (claude-opus)
Model relation: same-family

The snapshot path is fully retired: no re-export, no fallback, no stale help, and the three reasons are removed consistently from contracts, both envelope fixtures and the PR-body 0.15.0 note, with code-facts-read-not-ready kept. JD-2, JD-3 and JD-4 are met. ready-index-read runs readiness and the read inside one read-only session, and the session closes in a finally. Two families block: (1) the new ref-impact runbook says core.fsmonitor is not executed, but ls-files and check-ignore (both Decision 3 commands) do run a configured fsmonitor hook, which I proved in a disposable HOME; (2) two coverage-table rows point at tests that do not pin the claimed behavior. withReadyCodeGraphIndex's moved-revision and build-in-progress branches are untested, and freshness over provider include, includeIgnored and extension settings lost its only test. CI on the subject is green.

NITs

  • P3-retryable-split: ready-index-read.ts:30-34: when the inner inspectRepositoryCodeFacts detects a live build or a moved revision, it returns not-ready with index-incomplete. That is rethrown as code-facts-read-not-ready with the policy default retryable:false, because inspectRepositoryCodeFacts drops the tier's retryable flag. The same condition caught by the outer session (indexMoving) is retryable:true. Consider using the tier result's retryable.
  • P3-runbook-reasons: docs/runbooks/ref-impact.md:43-45 lists the no-sync reasons as index-missing, index-open-failed or code-facts-read-not-ready. withReadyCodeGraphIndex also returns code-facts-unavailable when the provider cannot load.
  • P3-duplicate-git-spy: The PR adds a shared test/git-spy.ts, but read-session.test.ts:292 keeps its own private gitSpy with the same mechanism. Consolidate in a later cleanup (S6).

The read runs only the git commands ADR-0058 allows: `ls-files`, `rev-parse`,
`check-ignore` and `config --get core.excludesFile`, plus the `rev-parse`,
`ls-tree`, `cat-file` and `log` reads of the immutable refs. It never runs `git
status` or `git diff`, so configured clean/process filters, `core.fsmonitor` and

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

P2 F1-fsmonitor-claim

The runbook says the allowed git commands mean configured clean/process filters, core.fsmonitor and index hooks are not executed. That is false for fsmonitor. Probe in a disposable HOME (git 2.50.1, GIT_CONFIG_NOSYSTEM=1) with core.fsmonitor set to a marker script: git ls-files --cached --others --exclude-standard -z (the exact listGitVisibleFiles call, git-adapter/src/index.ts:63) ran the hook once, and git check-ignore ran it again. rev-parse did not. Git's index read (tweak_fsmonitor/add_fsmonitor) calls the hook when the index has no FSMN extension. Because ls-files never writes the index, this happens on every read. Sibling: the PR-body coverage row for the deleted 'runs no repository or configured hook' test says the Decision 3 commands trigger no hooks. That holds for post-index-change and hooksPath, but not for the fsmonitor hook. ADR-0058 Consequences accepts that fsmonitor repositories now get facts. It does not claim the hook stays unexecuted.

Fix: Correct the runbook sentence. Filters, status/diff and index-writing hooks do not run, but ls-files and check-ignore invoke a configured core.fsmonitor hook as ordinary git reads do. Also correct the coverage-table hook row. Suppressing fsmonitor with -c core.fsmonitor=false would be a product change outside ADR-0058 Decision 3. If wanted, take that to the orchestrator.

}
if (facts.state === "not-ready") throw new FailureError("AC_CODE_FACTS_UNAVAILABLE", "code-facts-read-not-ready", { issues: facts.issues });
const result = read(opened.session.graph);
if (!opened.session.unchangedSinceOpen()) throw indexMoving();

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

P2 F2-coverage-table-gaps

Two rows in the deleted-assertion table name retained tests that do not pin the behavior. (a) The 'reads Git objects / code-facts-snapshot-changed' row cites read-session G4 for an index that moves during the read. G4 (read-session.test.ts:118) exercises session.unchangedSinceOpen() and inspectRepositoryCodeFacts/assertRepositoryCodeFactsReady. It never calls withReadyCodeGraphIndex. The only callers are the Node integration's ready and stale cases. So the post-read check at ready-index-read.ts:37, the build-in-progress branch at :25 (both mapping to code-facts-read-not-ready with retryable:true) and the non-FailureError -> code-facts-unavailable mapping have no test. Deleting line 37 fails nothing. withReadyCodeGraphIndex accepts no provider, so the fakeReadProvider revisionMoves fixture cannot reach it. (b) The 'private source mirror preserves provider includeIgnored, include and extension overrides' row is marked retired and covered by read-session freshness tests. The deleted test also asserted that a new source inside an includeIgnored directory (configured in codegraph.json) makes the read fail. That freshness behavior still exists (scanSourceFiles -> provider scanDirectory, read-session.ts:105/283). No remaining test sets codegraph.json include, includeIgnored or extensions (grep over adapter, daemon, CLI tests and the integration script finds none). 'an edit, a new source and a deletion are counted' uses only src/ under default settings.

Fix: Pass an optional { provider } through withReadyCodeGraphIndex to withCodeGraphReadSession and inspectRepositoryCodeFacts, as both already accept. Then add one read-session case that asserts revisionMoves gives code-facts-read-not-ready, retryable:true, and that open equals close. Extend the real-index freshness case, or the flows no-sync stale case, with a codegraph.json includeIgnored directory, and assert that a new file there makes the read not-ready.

@Ancienttwo

Copy link
Copy Markdown
Owner Author

repo-harness acceptance record
Claim: #599 (comment)

Round Role Subject Verdict
1 claude (claude-opus), read-only 735b9ddeff45413901040ab1050e8ec463e72214 FAIL: 2 P2, 3 P3

Model relation: same-family

Findings:

  • P2 F1-fsmonitor-claim at docs/runbooks/ref-impact.md:49: The runbook says the allowed git commands mean configured clean/process filters, core.fsmonitor and index hooks are not executed. That is false for fsmonitor. Probe in a disposable HOME (git 2.50.1, GIT_CONFIG_NOSYSTEM=1) with core.fsmonitor set to a marker script: git ls-files --cached --others --exclude-standard -z (the exact listGitVisibleFiles call, git-adapter/src/index.ts:63) ran the hook once, and git check-ignore ran it again. rev-parse did not. Git's index read (tweak_fsmonitor/add_fsmonitor) calls the hook when the index has no FSMN extension. Because ls-files never writes the index, this happens on every read. Sibling: the PR-body coverage row for the deleted 'runs no repository or configured hook' test says the Decision 3 commands trigger no hooks. That holds for post-index-change and hooksPath, but not for the fsmonitor hook. ADR-0058 Consequences accepts that fsmonitor repositories now get facts. It does not claim the hook stays unexecuted. -> Correct the runbook sentence. Filters, status/diff and index-writing hooks do not run, but ls-files and check-ignore invoke a configured core.fsmonitor hook as ordinary git reads do. Also correct the coverage-table hook row. Suppressing fsmonitor with -c core.fsmonitor=false would be a product change outside ADR-0058 Decision 3. If wanted, take that to the orchestrator.
  • P2 F2-coverage-table-gaps at packages/local-runtime/codegraph-adapter/src/ready-index-read.ts:37: Two rows in the deleted-assertion table name retained tests that do not pin the behavior. (a) The 'reads Git objects / code-facts-snapshot-changed' row cites read-session G4 for an index that moves during the read. G4 (read-session.test.ts:118) exercises session.unchangedSinceOpen() and inspectRepositoryCodeFacts/assertRepositoryCodeFactsReady. It never calls withReadyCodeGraphIndex. The only callers are the Node integration's ready and stale cases. So the post-read check at ready-index-read.ts:37, the build-in-progress branch at :25 (both mapping to code-facts-read-not-ready with retryable:true) and the non-FailureError -> code-facts-unavailable mapping have no test. Deleting line 37 fails nothing. withReadyCodeGraphIndex accepts no provider, so the fakeReadProvider revisionMoves fixture cannot reach it. (b) The 'private source mirror preserves provider includeIgnored, include and extension overrides' row is marked retired and covered by read-session freshness tests. The deleted test also asserted that a new source inside an includeIgnored directory (configured in codegraph.json) makes the read fail. That freshness behavior still exists (scanSourceFiles -> provider scanDirectory, read-session.ts:105/283). No remaining test sets codegraph.json include, includeIgnored or extensions (grep over adapter, daemon, CLI tests and the integration script finds none). 'an edit, a new source and a deletion are counted' uses only src/ under default settings. -> Pass an optional { provider } through withReadyCodeGraphIndex to withCodeGraphReadSession and inspectRepositoryCodeFacts, as both already accept. Then add one read-session case that asserts revisionMoves gives code-facts-read-not-ready, retryable:true, and that open equals close. Extend the real-index freshness case, or the flows no-sync stale case, with a codegraph.json includeIgnored directory, and assert that a new file there makes the read not-ready.
  • P3 P3-retryable-split: ready-index-read.ts:30-34: when the inner inspectRepositoryCodeFacts detects a live build or a moved revision, it returns not-ready with index-incomplete. That is rethrown as code-facts-read-not-ready with the policy default retryable:false, because inspectRepositoryCodeFacts drops the tier's retryable flag. The same condition caught by the outer session (indexMoving) is retryable:true. Consider using the tier result's retryable.
  • P3 P3-runbook-reasons: docs/runbooks/ref-impact.md:43-45 lists the no-sync reasons as index-missing, index-open-failed or code-facts-read-not-ready. withReadyCodeGraphIndex also returns code-facts-unavailable when the provider cannot load.
  • P3 P3-duplicate-git-spy: The PR adds a shared test/git-spy.ts, but read-session.test.ts:292 keeps its own private gitSpy with the same mechanism. Consolidate in a later cleanup (S6).

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