Repository navigation
fix(architecture): stop the CLI probes fabricating and dropping edges - #46
Merged
Merged
Conversation
Two defects found by comparing --architecture against codegraph's index: - Import nodes were probed by name. An import statement's node has no callee edges (0 of 3,659 on codegraph's repo), but its text as a name (terse.capture, ../src/mcp/tools) resolves to another symbol and borrows its callees. probeFileEdges now skips them. - #43's import-only filter trusted per-definition edges[], which lists one edge per callee, so an imports entry could hide an instantiates or references edge and drop a real call. The filter is removed; an import-only callee overcounts its file edge by 1 instead.
inth3shadows
added a commit
that referenced
this pull request
Sep 27, 2026
…obe per symbol (#47) Roadmap item: make `--architecture` (and `--forbid`) cheap enough to run on every commit. **Why:** the scan spent nearly all its time starting one `codegraph callees` process per symbol: 48.7s on this repo, where codeshot's own work is ~0.4s. **What:** on Node 22.5+, `readIndexRows` reads `<path>/.codegraph/codegraph.db` once via `node:sqlite` (read-only, no new dependency). It pulls every edge of codegraph's own callee kinds (`calls/references/imports/instantiates/navigates`, from its `getCallees`). `fileEdgesFromIndexRows` then derives each definition's answer using codegraph's own overload grouping (name + file + qualified name). - Enumeration stays on the CLI, so the symbol set and `--max-symbols` are unchanged. - Falls back to the CLI probes on Node 18/20, when there's no index at that path, on any read failure, or with `CODESHOT_INDEX_READ=cli`. - The read sees every edge, so it drops import-only callees exactly (the CLI's one-edge-per-callee JSON can't; see #46). - `--limit` caps nothing on this path. On Node 22.5+, fork and npm codegraph now draw the same weights. **Proof by equality with the incumbent:** with the import-only rule switched off, the direct read renders **byte-identical** diagrams to the CLI probes on: this repo (1 edge), the duplicate-name fixture (7), the TS fixture (1), terse at 300 symbols (31), and codegraph's own repo at 300 symbols (55). The fork CI test also asserts both paths draw the same file edges, with index weights ≤ CLI weights. **Speed:** this repo 48.7s → 1.3s; codegraph's repo, 500 symbols: 2.8s. Local: `node test/run.js` 191 passed, 0 failed on the fork and 191/0 on npm 1.6.0. Docs: README, USAGE (speed + troubleshooting), TECHNICAL.md. Follow-up not in this PR: `--diff` roots can also be import nodes, which have the same name-probe problem as #46.
inth3shadows
added a commit
that referenced
this pull request
Sep 27, 2026
Follow-up from #46/#47. `matchRootSymbols` excluded `"kind":"file"` nodes but not `"kind":"import"` ones. So a changed file's import statements became bold `--diff` roots, and probing an import's text as a name (`../src/mcp/tools`) can resolve to another symbol and draw its callers/callees: the same fabrication #46 fixed for `--architecture`. Import nodes are now excluded too. Reproduced on a TS repo with a changed `test/t.ts`: before, two roots (`runTest` and `"../src/mcp/tools"`); after, only `runTest`. The regression test was written first and failed on the old code. Local: `node test/run.js` 192 passed, 0 failed (fork codegraph). TECHNICAL.md updated.
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
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.
Two defects in
--architecture's per-symbol CLI probes. They were found by comparing the probes against codegraph's index directly (groundwork for the direct index read, next PR).terse.capture,../src/mcp/tools) resolves to another symbol and borrows its callees, which fabricates file edges.probeFileEdgesnow skips import nodes; this also removes thousands of wasted probes on TS repos.edges[], but that list holds ONE edge per callee (whichever codegraph found first). So animportsentry can hide aninstantiates/referencesedge to the same target (codegraph's repo: a test that imports and instantiatesToolHandlerlost the edge). The filter is removed from the CLI path: an import-only callee now overcounts its file edge by 1 instead of a real call going missing. TECHNICAL.md Known Limitations is rewritten to match.Regression tests: both shapes from the real failures (
probeSymbolCalleeskeeps an imported callee whose one listed edge isimports;probeFileEdgesnever probes an import node).Local:
node test/run.js188 passed, 0 failed (fork codegraph);--checkondocs/architecture.svgis up to date.