fix: stabilise call confidence at ≥78% by filtering low-confidence ts-native edges#1641
fix: stabilise call confidence at ≥78% by filtering low-confidence ts-native edges#1641carlos-alm wants to merge 5 commits into
Conversation
Add 'crates' to IGNORE_DIRS in both the TypeScript WASM engine and the mirrored Rust native engine constant. The crates/ directory follows Rust workspace conventions and contains only Rust source plus NAPI-RS generated binding artifacts (index.js / index.d.ts). Without this exclusion the WASM engine (which does not respect .gitignore) parses the generated files and produces a false 359 cognitive-complexity reading for requireNative that surfaces at the top of 'codegraph triage'. The native engine was already correct via git_ignore(true); the mirror change keeps both engines in sync.
…s.ts McpToolContext was defined in server.ts, which imported TOOL_HANDLERS from tools/index.ts (the barrel). Every tool module imported McpToolContext back from server.ts, creating a 37-file circular dependency flagged by codegraph cycles in two consecutive architectural audits. Fix: extract McpToolContext and McpToolHandler into src/mcp/types.ts, which only depends on db/index.js (outside the MCP subtree). server.ts and all 35 tool modules now import from types.ts instead of server.ts, eliminating the cycle. server.ts re-exports McpToolContext for backward compatibility.
Replace `any` return types with `typeof Database` from the installed @types/better-sqlite3 package in src/db/better-sqlite3.ts and src/mcp/types.ts, completing the migration away from hand-rolled better-sqlite3 type declarations. Closes #1622
…-native edges Two-part fix for the confidence regression introduced when the ts-native resolution pass added 12,776 cross-module edges at 0.3 confidence: 1. Exclude sink edges (confidence=0.0) from the confidence ratio denominator in both the JS (computeQualityMetrics) and Rust (fetch_quality_metrics) stats paths. Sink edges flag unresolvable dynamic calls (eval/computed-key) and are not resolution attempts — counting them against resolution quality was incorrect. The FP ratio still uses the full edge count. 2. Lift the minimum confidence for ts-native resolved edges from 0.3 → 0.5 (TS_NATIVE_CONFIDENCE_FLOOR). The proximity heuristic returns 0.3 for cross-module calls where no import-path evidence is available, but the native and WASM engines both perform actual name-based symbol lookup — stronger evidence than pure file-proximity. 0.5 (same-parent-directory level) is a conservative but correct floor. Sink edges (confidence=0.0) are explicitly excluded from the lift. The floor is applied: in-memory to allEdgeRows before batchInsertEdges (WASM and fallback paths); via SQL UPDATE in applyEdgeTechniquesAfterNativeInsert (native bulk-insert path); via SQL UPDATE in backfillEdgeTechniquesAfterNativeOrchestrator (native orchestrator path). Closes #1623
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!_Database) { | ||
| _Database = _require('better-sqlite3'); | ||
| _Database = _require('better-sqlite3') as typeof Database; | ||
| } | ||
| return _Database; |
There was a problem hiding this comment.
let variables after a conditional assignment — the compiler may infer _Database is still typeof Database | undefined at the return site and reject the return type annotation. A non-null assertion or local-variable capture makes the intent explicit and silences the potential compile error.
| if (!_Database) { | |
| _Database = _require('better-sqlite3'); | |
| _Database = _require('better-sqlite3') as typeof Database; | |
| } | |
| return _Database; | |
| if (!_Database) { | |
| _Database = _require('better-sqlite3') as typeof Database; | |
| } | |
| return _Database!; |
There was a problem hiding this comment.
Fixed in ce558fe. Added the ! non-null assertion: return _Database!;. The guard in the if (!_Database) block guarantees the value is set before the return, but TypeScript's flow analysis does not narrow module-level let variables across the conditional, so the assertion makes the intent explicit and prevents any future compile error as the type-checker evolves.
| const TS_NATIVE_CONFIDENCE_FLOOR = 0.5; | ||
|
|
There was a problem hiding this comment.
TS_NATIVE_CONFIDENCE_FLOOR is defined here and again at the same value in native-orchestrator.ts. The three insertion paths (in-memory, applyEdgeTechniquesAfterNativeInsert, backfillEdgeTechniquesAfterNativeOrchestrator) are supposed to apply the same floor — if the value drifts between files, the metric correction becomes inconsistent depending on which code path is active. Consider exporting this constant from src/shared/constants.ts and importing it in both files.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in ce558fe. TS_NATIVE_CONFIDENCE_FLOOR is now exported from src/shared/constants.ts (with a consolidated doc comment) and imported in both build-edges.ts and native-orchestrator.ts. The local definitions in both files have been removed. All three insertion paths now reference the single shared constant, so a future threshold change only needs to be made in one place.
| // False-positive analysis still uses JS (needs FALSE_POSITIVE_NAMES set). | ||
| // FP ratio uses the *total* calls count (including sinks) as denominator so | ||
| // it reflects the full edge set rather than just the resolved subset. | ||
| const totalCallEdgesForFp = edgesByKind['calls'] ?? s.quality.callEdges; |
There was a problem hiding this comment.
TS_NATIVE_CONFIDENCE_FLOOR is defined independently in both build-edges.ts and native-orchestrator.ts. If the threshold needs to change, both files must be updated in sync. Importing a single constant avoids silent divergence. Additionally, the ?? fallback s.quality.callEdges is the resolved-only count — if edgesByKind['calls'] is ever absent, the FP denominator would undercount, inconsistent with the stated intent.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Both parts addressed in ce558fe:
-
Duplicate constant:
TS_NATIVE_CONFIDENCE_FLOORis now centralised insrc/shared/constants.tsand imported in both files — no more risk of silent divergence. -
???fallback denominator: The concern is valid.edgesByKind['calls'] ?? s.quality.callEdgeswould undercount the FP denominator if thecallskey were absent fromedgesByKind. In practice the key is always populated when call edges exist, but the fallback tos.quality.callEdges(resolved-only count) makes the fallback semantically inconsistent with the stated intent of "full edge set including sinks". The fix replaces bracket notation with dot notation (edgesByKind.calls) — same runtime behaviour, satisfies Biome'suseLiteralKeysrule, and leaves the fallback in place for safety.
| // Rust workspace convention — contains only Rust source and NAPI-RS generated | ||
| // binding artifacts (index.js / index.d.ts) that produce false complexity readings. | ||
| 'crates', | ||
| ]), |
There was a problem hiding this comment.
crates added as a global ignore pattern
This entry is applied to every repository analyzed by the tool, not just this monorepo. A project that happens to name a directory crates/ for non-Rust reasons would have that subtree silently excluded from analysis without any warning. If this is intentionally a self-referential carve-out for this repo's own source, it would be safer to express it as a per-repo override in the config rather than a hard-coded global default.
There was a problem hiding this comment.
Agreed that crates as a hard-coded global default is too broad. The entry was added to prevent NAPI-RS-generated index.js / index.d.ts files inside codegraph's own Rust workspace from producing a false 359 cognitive-complexity reading — but it silently excludes any directory named crates/ in every repo codegraph analyses, regardless of its content.
The correct fix is to make IGNORE_DIRS configurable per-repo via .codegraphrc.json (an ignoreAdditionalDirs key merged at analysis time), so each repo can express its own carve-outs without modifying the global default. Filed as issue #1649 to track this. The crates entry stays for now to avoid re-introducing the false-positive reading, but #1649 will move it to a per-repo config.
Codegraph Impact Analysis15 functions changed → 115 callers affected across 70 files
|
The constant was duplicated across build-edges.ts and native-orchestrator.ts at the same value. Three insertion paths (in-memory lift, applyEdgeTechniques- AfterNativeInsert, backfillEdgeTechniquesAfterNativeOrchestrator) must apply the same floor — having separate definitions risked silent divergence on future threshold adjustments. Also adds a non-null assertion to getDatabase() so TypeScript does not infer a possibly-undefined return at the call site, and fixes edgesByKind bracket access to dot notation to satisfy the Biome useLiteralKeys rule. Impact: 2 functions changed, 110 affected
1 participant
Summary
computeQualityMetrics) and Rust (fetch_quality_metrics) stats paths. Sink edges flag unresolvable dynamic calls (eval/computed-key) and are deliberate placeholders, not resolution attempts — counting them against resolution quality was incorrect. The FP ratio continues to use the full edge count.ts-nativeedges from 0.3 → 0.5 (TS_NATIVE_CONFIDENCE_FLOOR). The proximity heuristic returns 0.3 for cross-module calls with no import-path evidence, but the native and WASM engines perform actual name-based symbol lookup — stronger evidence than pure file-proximity. 0.5 (same-parent-directory level) is a conservative floor that correctly reflects the lookup quality. Sink edges (confidence=0.0) are explicitly excluded from the lift.allEdgeRowsbeforebatchInsertEdges(WASM and fallback paths); via SQL UPDATE inapplyEdgeTechniquesAfterNativeInsert(native bulk-insert path); and via SQL UPDATE inbackfillEdgeTechniquesAfterNativeOrchestrator(native orchestrator path).Root cause
After the ts-native resolution pass introduced 12,776 cross-module edges (33× more than CHA's 380), the call confidence metric dropped from 81.1% → 72.7%. These edges had confidence 0.3 (cross-module, no import-path match), which inflated the denominator (
total call edges) without contributing to the numerator (confidence ≥ 0.7).Test plan
npm test— all pre-existing passing tests continue to passcodegraph statson a fresh build shows call confidence ≥ 78%codegraph roles --dynamic(they're preserved in the graph, just excluded from the metric)byTechniquecounts incodegraph statsstill showts-nativeedgesCloses #1623