Repository navigation
fix(architecture): stop same-file duplicate names from multiplying edge weights - #39
Merged
Merged
Conversation
…ge weights Two same-named symbols in one file each received the shared per-file callee list, so every edge was counted once per symbol: with only A.run calling alpha, c/dual.js -> a/alpha.js read 2. Match codegraph's per-definition result by the enumerated id first; count any shared answer once per file. Document that --limit applies per definition.
…ition once codegraph groups same-file overloads into one definition whose id is the first member's; the rest only appear in roots, so they fell to the shared answer and were counted on top of the head's. Match on roots too and count a matched definition once. Document the fork/npm weight difference for same-file duplicate names, and narrow the --diff --limit note.
inth3shadows
added a commit
that referenced
this pull request
Sep 26, 2026
Follow-up (a) from #39's review. `probeFileEdges`' once-per-key counting was only covered by the CLI fixture test, which the codegraph-free CI jobs skip. It now takes an injectable `run` (default `runCodegraph`, same pattern as `probeSymbolCallees`) and is exported for tests. New fake-run tests: - shared answer, two same-file duplicates → 1 edge - id-matched same-file definitions → each counted, only for what it calls - overload group matched via `roots` → 1 edge - unresolved union across two files → one edge per file, both names reported - no codegraph answer → skipped No behavior change. Local: `node test/run.js` 175 passed, 0 failed (fork codegraph); new tests also pass with codegraph off PATH.
inth3shadows
added a commit
that referenced
this pull request
Sep 27, 2026
Follow-up (b) from #39's review. N same-named symbols ran N identical `callees` probes, plus identical `node -f` probes on npm when a name repeated within a file. `probeFileEdges` now wraps `run` in `memoizeRepeatedNames`: for a name that occurs more than once, the answer is cached under the full args, so `callees` is asked once per name and `node -f` once per name+file. Names that occur once go straight through, so memory holds only answers that are read again. No output change: every symbol reads the same answer it would have fetched. New test: three `run`s across two files plus one `solo` symbol → 4 codegraph calls (was 6), with the same edges and unresolved names. Local: `node test/run.js` 176 passed, 0 failed (fork codegraph).
inth3shadows
added a commit
that referenced
this pull request
Sep 27, 2026
…42) Follow-up (c) from #39's review. `--architecture` ignored truncation, so file edges cut by `--limit` disappeared with no warning. - `probeSymbolCallees` returns `truncated`, read by `readTruncation` from the slice it actually answered with: the matched definition's own `truncated`, else the response's, else the hit-the-limit heuristic (npm). The `node -f` trail is never flagged: it has no `--limit`, and `parseNodeCalls` already refuses a trail cut at 12. - `probeFileEdges` collects those names; `architectureTruncationWarning` prints one aggregated stderr line, like `diffTruncationWarning`. - README and TECHNICAL.md document it (and TECHNICAL.md no longer calls `probeFileEdges` unexported). Real run on this repo at the default `--limit 50`: warns for the `callgraph.js` file node, which has 57 callees (`total: 57, truncated: true`). So `docs/architecture.svg` has been missing some of that file's edges. This PR does not regenerate it. Local: `node test/run.js` 180 passed, 0 failed (fork codegraph).
inth3shadows
added a commit
that referenced
this pull request
Sep 27, 2026
…ht (#43) Follow-up (d) from #39's review. codegraph lists every symbol a file imports among the file node's callees (`imports` edges). So a function `main.ts` imports and calls only inside its own `run()` weighed 2 on `main.ts → alpha.ts`: 1 for the call, 1 for the import. - The fork's per-definition slices carry `edges[]` with a kind per edge. New `importOnlyIds` returns callees whose every edge is `imports`, and `probeSymbolCallees` drops them on the id-matched and same-named-file routes. - A real top-level call from the file (`gamma();`, or one inside an anonymous callback) is a `calls` edge to the same target, so it is kept. - npm codegraph (through 1.6.0) has the same +1 but returns no `edges`, so nothing changes there. TECHNICAL.md Known Limitations documents the fork/npm weight difference. Measured on a TS fixture: master drew weight 5, this draws 4 (the 4 real calls). This repo's CommonJS `docs/architecture.svg` is unchanged: `--check` passes locally. Local: `node test/run.js` 183 passed, 0 failed (fork codegraph).
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.
Two same-named symbols in one file (e.g.
A.runandB.run) each got the shared per-file callee list, so each edge was counted once per symbol. Measured: with onlyA.runcallingalpha,c/dual.js -> a/alpha.jsreadlabel=2.probeSymbolCalleesnow matchesdefinitions[].definition.idagainst the enumerated symbol'sidfirst, which gives each symbol only its own callees.node -f, or the union) is shared, soprobeFileEdgescounts it once per name and file.--limitapplies per definition when codegraph reports per-definition results.Tradeoff on npm codegraph (no ids): if both same-file methods call the same function, the edge now counts 1 instead of 2. The weight can come out low, but no edge goes missing.
Tests: 168/0 against the fork build and 168/0 against npm codegraph 1.6.0 (
codegraph-npm-public). The fixture test now checks that both edges fromc/dual.jsare labeled 1, and a new unit test covers the id match.Review follow-up: codegraph groups same-file overloads into one definition, with the first overload's
idand every overload's id inroots. Symbols now also match throughroots, and a matched definition counts once: a TS overload fixture now renders 3/2 (it was 4/3).TECHNICAL.mdKnown Limitations now says the fork and npm can draw different weights for same-file duplicate names, which can make--checkfail when the committing and checking builds differ. Tests: 170/0 on the fork and on npm 1.6.0.