Repository navigation
feat(codegraph-adapter): in-process import reads (#574 S2) - #598
Ancienttwo wants to merge 3 commits into
Conversation
Preserve the S2 worker changes for cross-machine continuation. Draft PR; acceptance and integration remain pending.
|
repo-harness acceptance claim |
Ancienttwo
left a comment
Ancienttwo
left a comment
There was a problem hiding this comment.
repo-harness acceptance review
Round: 1
Subject: 2de1f72
Tier: medium
Verdict: FAIL
Reviewer: pi (gpt-6-astra)
Model relation: different-family
FAIL: moving-index population reads do not consistently enforce revision/truncation and retained-node limits, and the shared capability test edits exceed the seven-case boundary exception. Confirmed green PR checks on the exact subject. Reviewed all seven assertion-change items: CLI fixture/argv replacements follow the SDK migration; the four refactor frozen expectations are unchanged; constraints uses the existing dev launcher, which preloads Darwin SQLite then invokes the same main.ts with forwarded argv, output and exit status, without changing assertions; deps adds real source-staleness coverage. Default cap is 64000; import-population has no CLI status/query read or fallback. Remaining projection-handshake CLI reads belong to the explicitly deferred S3 scope. Two focused injected-session reproductions ran in disposable HOME directories; no passing test or typecheck was rerun. Worktree unchanged.
NITs
None.
| imports, | ||
| count: reportedCount ?? imports.length, | ||
| truncated: reportedCount !== null && imports.length !== reportedCount, | ||
| imports: limitExceeded ? imports.slice(0, limit) : imports, |
There was a problem hiding this comment.
P1 GK598-1
Moving-index population integrity has two sibling failures. (1) capabilityImportGraphs uses only imports.length !== count for truncation; neither it nor the new withCodeGraphReadSession callback in selector-index.ts:236-240 calls unchangedSinceOpen. A session is not a SQLite transaction. In a focused reproduction importing the actual implementation, getStats returned one import and getNodesByKind advanced the revision while returning one import: revisionChecks=0, importPopulation.truncated=false and every graph.truncated=false. Repository imports do check the revision, so the two consumers now disagree with the PR/spec promise that a revision change means truncation. The debt consumer's separate before/after readiness sessions do not compare revisions across the import read. (2) The retained prefix is sliced only when the earlier count exceeds the limit. If the count is at/below the limit and the population grows before getNodesByKind, the whole new population escapes the cap. A second focused reproduction with count=2, limit=2 and four distinct returned imports produced four pairs/edges, importNodeLimit=2 and importNodeLimitExceeded=false. This shared helper affects both repositoryImportPairs and capabilityImportGraphs; the new unbounded SDK read makes the overshoot unbounded. Both reproductions used disposable HOME, the actual import-population module and an injected session; only product-version manifest loading was stubbed. Missing test branches are same-count revision movement and a writer starting during capability reads (below/equal/above cap), count growth across the cap in both consumers, and growth/shrink while the population remains above the cap.
Fix: Check unchangedSinceOpen before completing capability import reads and propagate movement to importPopulation and every graph (or fail the read); preserve the repository reader's final check after freshness. Always bound the returned sorted node prefix to limit independently of the initial count, keep count/revision inconsistency marked truncated, and make the limit-exceeded evidence account for observed growth. Extend the existing import cases for the sibling branches listed above.
There was a problem hiding this comment.
fixed 7fece6f (merged into head 3f52d61). The sorted prefix is always sliced to the limit; importNodeLimitExceeded uses max(count, returned); a count/read mismatch stays truncated; capabilityImportGraphs checks unchangedSinceOpen() and marks importPopulation and every graph truncated. New table-driven cases cover both consumers (same-count revision movement, a writer starting during the read below/at/above the cap, growth across the cap, growth/shrink above the cap).
There was a problem hiding this comment.
独立 reviewer 增量复核（非 acceptor / acceptance record；不替代 round 2）：
复核 base d63dfada79d63fea54bc6e3da046f4a62a316078、head 3f52d61df8eebbfec416099d25986341f50866d2。相对原评审 head 2de1f7238bac3e49847e0b0ff49ac3b13d5e0929，GK598-1 的两个具体缺口在静态调用链上已补齐：
- 共享 population reader L216–223 现在无条件
slice(0, limit)，超限证据取Math.max(count, imports.length) > limit，并保留数量不一致的 truncated。原例 count=2、limit=2、返回4个节点，现在最多保留2个节点、limitExceeded=true、truncated=true（代码推导，非本轮执行复现）。 - capability 路径 L245–274 在数量一致时调用
unchangedSinceOpen()，将 revision/活跃 writer 的拒绝传播到 population 及每个 graph；数量已不一致时短路仍保留 truncated=true。loadCapabilityCodeGraphProjectionInputs继续经此入口，architecture-debt 使用返回的 importPopulation。repository 路径 freshness 后的最终 revision check 未被移除。
回归测试增量通过 loadCapabilityCodeGraphProjectionInputs / repositoryImportPairs 公共入口覆盖同数量 revision 移动、读中 writer 在 cap 下/等于/以上启动、跨 cap 增长及 cap 以上增减；原 frozen assertions 保留。此处核对的是测试代码，不把它表述为我已运行通过。
验证限制：本轮只读静态复核，未重跑测试、未执行真实并发 SQLite writer 场景；当前新 head CI 尚在运行。原 finding 与历史状态保留，正式 round-2 结论及是否关闭线程由既有席位处理。
| try { | ||
| const evidence = (index: CodeGraphSelectorIndex) => loadCapabilityCodeGraphProjectionInputs(root, model, { | ||
| binary, | ||
| provider, |
There was a problem hiding this comment.
P2 GK598-2
The acceptance packet and joint boundary answer #574 (comment) grant S2 only seven import-graph cases in this S4-shared file. The diff changes the seven-case block (equal count, short read, below-count limit, invalid limit, missing index, no footprint/entrypoints, read failure), plus 'measures import edges and exact node-v2 selector evidence in one index pass', 'selector proof is stable under unrelated fanout and fails closed for missing or ambiguous identities', the common seedWorkspace/fakeCli setup also used by snapshot tests, and evidenceFor in the selector-only built-in suite. The latter affects all six sibling cases: rejecting a built-in without a sink import; accepting it with a sink import; ignoring a gitignored candidate; a non-built-in call; references-only edges; and duplicate sink declarations. Thus the PR body's description of one extra builtin selector case understates the helper's reach. These are mostly necessary setup/signature adaptations and the selector semantic assertions remain, but listing them in the PR body does not expand the recorded one-writer exception. The four refactor-baseline import cases stay within their separate exception. This is a scope-contract gap, not a request for duplicate test assertions.
Fix: Record an explicit expanded joint boundary decision naming the additional shared setup, combined import/selector case, selector fanout case and six built-in-suite cases, with S4 coordination and the existing merge-only/merge-tree requirements, before accepting this scope; otherwise confine the diff to the granted region. Do not restore the retired CLI path or weaken assertions to satisfy the boundary.
There was a problem hiding this comment.
Coordinator answer: the remedy this finding asks for is now recorded. The joint decision #574 (comment) extends the S2 boundary in this file. It covers the shared seedWorkspace/fakeCli setup, the combined import/selector case, the selector-fanout case, and evidenceFor with its six built-in cases. S4 stays out of these regions, and the conflict resolves in S4's merge-only sync. No code change is expected for GK598-2. The thread stays open until round 2 confirms it.
Seat: orchestrator t366
|
repo-harness acceptance record
Model relation: different-family Findings:
|
…n read (#574 S2, GK598-1)
…pulation # Conflicts: # packages/local-runtime/codegraph-adapter/test/refactor-baseline.test.ts
|
Merge-only sync with main d63dfad (S5), no rebase. Seat: worker-important t38s |
|
repo-harness acceptance claim |
Ancienttwo
left a comment
Ancienttwo
left a comment
There was a problem hiding this comment.
repo-harness acceptance review
Round: 2
Subject: 3f52d61
Tier: medium
Verdict: PASS
Reviewer: pi (gpt-6-astra)
Model relation: different-family
PASS after reviewing fix delta 2de1f72..7fece6f, the sibling cases of GK598-1, the expanded scope decision, and merge-only subject 3f52d61. GK598-1: confirmed fixed. capabilityImportGraphs now checks unchangedSinceOpen after reading imports and propagates movement to importPopulation and every capability graph; the selector-index callback reaches this checked function. Count mismatch already marks truncation, so short-circuiting the revision check in that case cannot report completeness. repositoryImportPairs retains its final revision/writer-lock check after freshness. The shared population helper always slices the sorted nodes to limit and sets limitExceeded from Math.max(reported count, observed length), independently of truncation. The 11 added table rows cover same-count revision movement; writers starting during capability reads below, at and above the cap; count growth across the cap in both consumers (repository rows cover both below and equal initial counts); and growth/shrink while above the cap in both consumers. Capability tests assert both graphs and importPopulation, and repository tests pin the sorted prefix. Existing normal-limit, missing/stale-index, short-read and revision assertions remain. GK598-2: settled by #574 (comment); the shared-file diff stays within the expanded setup, combined selector/import, selector-fanout, built-in-suite and import-test regions. The four refactor-baseline import cases remain within their exception. Reviewed both merge parents, the conflicted-file delta and git show --remerge-diff: the sole textual conflict was the header comment. Both S2 importIndex and S5 importDumpReadProvider remain, with all fixture cases, expectFrozen equality and digest assertions preserved. Writer's merge-tree record: #598 (comment). Confirmed exact PR head and 35/35 green checks (run 43552803764). The PR records local real-index SQLite failures on both old and new content; current cross-platform CI is green, including real-index coverage. No tests or typecheck rerun, no additional defect found, and worktree unchanged.
NITs
None.
|
repo-harness acceptance record
Model relation: different-family Findings:
|
Ancienttwo
left a comment
Ancienttwo
left a comment
There was a problem hiding this comment.
repo-harness acceptance review
Round: 2
Subject: 3f52d61
Tier: medium
Verdict: PASS
Reviewer: pi (gpt-6-astra)
Model relation: different-family
PASS after reviewing fix delta 2de1f72..7fece6f, the sibling cases of GK598-1, the expanded scope decision, and merge-only subject 3f52d61. GK598-1: confirmed fixed. capabilityImportGraphs now checks unchangedSinceOpen after reading imports and propagates movement to importPopulation and every capability graph; the selector-index callback reaches this checked function. Count mismatch already marks truncation, so short-circuiting the revision check in that case cannot report completeness. repositoryImportPairs retains its final revision/writer-lock check after freshness. The shared population helper always slices the sorted nodes to limit and sets limitExceeded from Math.max(reported count, observed length), independently of truncation. The 11 added table rows cover same-count revision movement; writers starting during capability reads below, at and above the cap; count growth across the cap in both consumers (repository rows cover both below and equal initial counts); and growth/shrink while above the cap in both consumers. Capability tests assert both graphs and importPopulation, and repository tests pin the sorted prefix. Existing normal-limit, missing/stale-index, short-read and revision assertions remain. GK598-2: settled by #574 (comment); the shared-file diff stays within the expanded setup, combined selector/import, selector-fanout, built-in-suite and import-test regions. The four refactor-baseline import cases remain within their exception. Reviewed both merge parents, the conflicted-file delta and git show --remerge-diff: the sole textual conflict was the header comment. Both S2 importIndex and S5 importDumpReadProvider remain, with all fixture cases, expectFrozen equality and digest assertions preserved. Writer's merge-tree record: #598 (comment). Confirmed exact PR head and 35/35 green checks (run 43552803764). The PR records local real-index SQLite failures on both old and new content; current cross-platform CI is green, including real-index coverage. No tests or typecheck rerun, no additional defect found, and worktree unchanged. Supplemental round-2 clarification requested by orchestrator t366 after the initial publication: GK598-1 is confirmed fixed for the debt-consumer sibling too. readArchitectureDebtCodeFacts (architecture-debt-code-facts.ts:28) calls loadCapabilityCodeGraphProjectionInputs, whose in-session callback calls capabilityImportGraphs; that function now checks unchangedSinceOpen and returns importPopulation.truncated=true for revision movement or a writer starting during the import read. architecture-debt-code-facts.ts passes that population to projectionArchitectureDebt, which forwards population.truncated to measureCapabilityDependencies. certifyCodeFacts then returns coverage=partial with code-facts-truncated even if both independent outer readiness inspections returned ready; dependencyDebtFromMeasurement preserves that coverage and reason. Thus the unchanged runtime-daemon code already propagates the new in-session protection, and no runtime-daemon edit is required to resolve this sibling. This is protection of the import-read interval, not a claim that the outer readiness sessions compare their revisions or that selector-read work deferred to S3 has been completed. Inspected the full propagation chain; no tests rerun and verdict remains PASS. This publication supplements the earlier round-2 review under the same subject, reviewer and acceptance claim.
NITs
None.
|
repo-harness acceptance record
Model relation: different-family Findings:
|
Summary
将 repositoryImportPairs / capabilityImportGraphs 的 import 读取迁到 ADR-0058 的 in-process read-only session，删除此路径的 CLI status/query 读取；将默认 import-node cap 提升到 64,000。Refs #574、#585。Tier: Medium。
Head:
3f52d61df8eebbfec416099d25986341f50866d2（GK598-1 修复7fece6f1+ merge-only 同步 maind63dfada）。Base:maind63dfada。Draft，等 round 2 review；不关闭父 issue，不合并。Behavior and boundaries
codeGraphImportPopulation，规则一致（GK598-1）：limit，与先读到的 count 无关，所以 count 之后增长的 population 越不过 cap。importNodeLimitExceeded取 count 与实际返回节点数的较大者，反映读取中观察到的增长。capabilityImportGraphs读完后检查unchangedSinceOpen()（revision 变化或 writer lock 出现），变化时importPopulation和每个 graph 都标 truncated。repositoryImportPairs保留 freshness 之后的最终检查。importPopulation.truncated拿到读取中的 index 移动。refactor scan 的版本证明改成实际 loaded bundle version + packaged runtime digest。Merge-only sync with S5
git merge origin/main（不 rebase）。git merge-tree --write-tree 7fece6f1 origin/main→d7bbb8c8ef4f47b2579738a773d8e227ff905d9b，唯一冲突在packages/local-runtime/codegraph-adapter/test/refactor-baseline.test.ts的文件头注释（stage 123043a88、oursccbd9e8e、theirs7db2d7b4）。解决方式：注释改为两个入口都经 injected read-only provider（S2 import graphs、S5 buildContext）；S2 的importIndex与 S5 的importDumpReadProvider、import 行和 fixture 用例全部保留，无 assertion 改动。#585 measurement
原 S2 pane 测量：64k synthetic nodes，Node 24，每个 cap 三次。50k cap 574–580 ms / 335–338 MiB；64k cap 604–638 ms / 338–340 MiB；增量 +6.4% wall / +1.2% RSS。旧 CLI 1.6–2.2 s + 约 600 MiB child。headroom 约 10x repo-harness（6,420 import nodes）。本轮未重跑 benchmark。
Verification (head
3f52d61d)本机 Bun 1.4.4：
bun run typecheck→ exit 0。bun test packages/local-runtime/codegraph-adapter/test/→ 114 pass / 18 fail。18 个失败全部是真实索引用例（index-open-failed，SQLite read-only open 在本机报unable to open database file），包括 main 上 S5 的真实索引用例；未改动的2de1f723上同样失败，与本 diff 无关。所有 injected-provider 用例通过。-t "truncated on|during the read keeps a capped"→ 11 pass / 0 fail。bun test packages/local-runtime/runtime-daemon/test/architecture-debt-code-facts.test.ts packages/local-runtime/codegraph-adapter/test/read-session.test.ts→ 27 pass / 0 fail。3f52d61d：35/35 checks pass，run 43552803764（macOS / Ubuntu / Windows，Node 22/24/25）。真实索引用例在 CI 上通过。Changed assertions and retained coverage
countShortBy；新增一个表驱动用例覆盖 count 从 cap 下方/等于 cap 增长越过 cap、cap 上方增长、cap 上方收缩，断言 truncated、limitExceeded、count 和截到 cap 的排序前缀。measures import edges and exact node-v2 selector evidence in one index pass、selector proof is stable under unrelated fanout and fails closed for missing or ambiguous identities、evidenceFor 及其六个 built-in 用例（只移除失效 binary option）、七个 import 用例。GK598-1：importIndexfixture 加returned、revisionMoves、writerStartsDuringRead；新增一个两 capability 的表驱动用例：同 count revision 移动、读取中 writer 启动（cap 下方/等于/上方）、count 增长越过 cap、cap 上方增长、cap 上方收缩，断言importPopulation和每个 graph 都 truncated、limitExceeded、pairs 数不超过 cap。expectFrozen不变。合并 S5 后只改文件头注释。没有削弱任何 assertion，也没有恢复退役的 CLI 读取路径。
Test budget exception
git diff --numstat origin/main...HEAD（packages/**/test/**对packages/**/src/** + scripts/**）：新增 test 376 行，product 126 行，ratio 2.98:1；新增 test file 0；docs 5 行另计。超过 2:1：多个已有 CLI stub fixtures 迁到 read-only provider 且保留原语义 assertions，加上真实索引、no-spawn、deterministic prefix、stale deps 覆盖，以及 GK598-1 要求的两个 consumer 的移动 index 分支（两个表驱动用例，共 11 个 case）。全部扩展已有测试文件；单个既有 test 无法同时覆盖 adapter、projection、debt 和 CLI 消费边界。
Release note (0.15.0)
Import reads 改用 in-process read-only SDK；默认 cap 从 50,000 调至 64,000，超限前缀改为稳定字段排序，读取中 index 增长、收缩或移动时 coverage/truncation 都显式报告。
Risk / Rollback
S3 未完成；与 S4 共享的 capability-projection-inputs 冲突在 S4 的 merge-only 同步里解决。Rollback: revert 本 PR；不涉及 ledger/schema/data migration。