fix(roles): honour exported=1 flag for exported interfaces with no cross-file edges - #1599
fix(roles): honour exported=1 flag for exported interfaces with no cross-file edges#1599carlos-alm merged 3 commits into
Conversation
Sorry, something went wrong.
… engine parity The WhatsApp/tree-sitter-erlang npm devDependency was removed in #1478 (GHSA-rphw-c8qj-jv84 malware advisory). This left the WASM engine without an Erlang grammar, causing parity-compare to report 39 node diffs and 66 edge diffs on every run (native has built-in Erlang support; WASM gets 0). The WASM file that was present before removal has been validated clean: - correct magic bytes (0061736d) - exactly one tree_sitter_erlang export - no disallowed WASM imports Add a .gitignore negation rule (!grammars/tree-sitter-erlang.wasm) so this specific file is tracked in the repo without relying on the removed devDep. Update the build-wasm.ts comment to reflect the new committed-WASM approach and document how to rebuild it from a safe source if the grammar is updated. Closes #1582
… cross-file edges The role classifier inferred "is exported" purely from cross-file calls/imports-type edges and barrel reexports. This missed symbols that are declared with the `export` keyword but are only used as type annotations within the same file — no edge is produced for same-file type usage, so the symbol's fan-in stayed 0 and isExported=false, yielding a spurious dead-unresolved classification. Fix: after the existing cross-file-edge and reexport-barrel checks, query `WHERE exported = 1` and add those IDs to exportedIds. Applied to all four classification paths: classifyNodeRolesFull and classifyNodeRolesIncremental in TypeScript, and do_classify_full and do_classify_incremental in the Rust native engine. docs check acknowledged Closes #1583
Greptile SummaryThis PR fixes a false dead-unresolved classification for exported interfaces (and other exported symbols) that have no cross-file edges because they are only used as same-file type annotations. The fix adds a new step 3c to all four classification paths (TypeScript full/incremental and Rust native full/incremental) that queries nodes WHERE exported = 1 and adds those IDs to exportedIds, giving them isExported=true, fanIn=0 → entry.
Confidence Score: 5/5Safe to merge — the change is a targeted, additive fix that only promotes correctly-exported symbols from dead-unresolved to entry, applied consistently across all four classification paths in both the TypeScript and Rust engines. The fix is a single well-scoped SQL query added at the same position in all four classify paths, the existing kind-exclusion filter is reused verbatim, the incremental variant correctly narrows the query to affected files, and the new unit tests cover the full path, the negative boundary, and the incremental path. No existing query is modified. grammars/tree-sitter-erlang.wasm — committed binary with undocumented source provenance; worth ensuring the build team has recorded the origin commit and hash out-of-band. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[classifyNodeRoles] --> B{changedFiles provided?}
B -- No --> C[classifyNodeRolesFull]
B -- Yes --> D[classifyNodeRolesIncremental]
C --> E[Step 3a: cross-file calls/imports-type edges\n→ exportedIds]
C --> F[Step 3b: reexports from prod-reachable barrels\n→ exportedIds]
C --> G["Step 3c NEW: WHERE exported=1\n→ exportedIds"]
E & F & G --> H[Step 4: prodFanInMap\nexcludes test callers]
H --> I[classifyRoles\nfanIn + isExported → role]
D --> J[Step 3a: cross-file edges scoped\nto allAffectedFiles → exportedIds]
D --> K[Step 3b: reexports scoped\nto allAffectedFiles → exportedIds]
D --> L["Step 3c NEW: WHERE exported=1\nAND file IN allAffectedFiles\n→ exportedIds"]
J & K & L --> M[Step 4: prodFanInMap\nscoped to allAffectedFiles]
M --> I
I --> N{isExported AND fanIn=0?}
N -- Yes --> O[entry]
N -- No --> P[core / utility / adapter / leaf / dead-*]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[classifyNodeRoles] --> B{changedFiles provided?}
B -- No --> C[classifyNodeRolesFull]
B -- Yes --> D[classifyNodeRolesIncremental]
C --> E[Step 3a: cross-file calls/imports-type edges\n→ exportedIds]
C --> F[Step 3b: reexports from prod-reachable barrels\n→ exportedIds]
C --> G["Step 3c NEW: WHERE exported=1\n→ exportedIds"]
E & F & G --> H[Step 4: prodFanInMap\nexcludes test callers]
H --> I[classifyRoles\nfanIn + isExported → role]
D --> J[Step 3a: cross-file edges scoped\nto allAffectedFiles → exportedIds]
D --> K[Step 3b: reexports scoped\nto allAffectedFiles → exportedIds]
D --> L["Step 3c NEW: WHERE exported=1\nAND file IN allAffectedFiles\n→ exportedIds"]
J & K & L --> M[Step 4: prodFanInMap\nscoped to allAffectedFiles]
M --> I
I --> N{isExported AND fanIn=0?}
N -- Yes --> O[entry]
N -- No --> P[core / utility / adapter / leaf / dead-*]
Reviews (3): Last reviewed commit: "test: add incremental-path coverage for ..." | Re-trigger Greptile |
Sorry, something went wrong.
| it('does not classify exported interface as dead when used only as same-file type annotation (#1583)', () => { | ||
| // Simulate: exported interface whose only usage is as a parameter type in the same file. | ||
| // No cross-file imports-type edge exists because same-file type annotations don't produce edges. | ||
| // The extractor marks the interface as exported=1. The classifier must honour that flag. | ||
| db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run( | ||
| 'MyOpts', | ||
| 'interface', | ||
| 'src/helpers.ts', | ||
| 10, | ||
| 1, | ||
| ); | ||
|
|
||
| classifyNodeRoles(db); | ||
| const role = db.prepare("SELECT role FROM nodes WHERE name = 'MyOpts'").get(); | ||
| // Should be entry (exported, fan-in 0), not dead-unresolved | ||
| expect(role.role).toBe('entry'); | ||
| }); | ||
|
|
||
| it('classifies non-exported interface with no callers as dead-unresolved (#1583 boundary)', () => { | ||
| // An interface without export keyword and without cross-file references is genuinely dead. | ||
| db.prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)').run( | ||
| 'InternalOpts', | ||
| 'interface', | ||
| 'src/helpers.ts', | ||
| 20, | ||
| 0, | ||
| ); | ||
|
|
||
| classifyNodeRoles(db); | ||
| const role = db.prepare("SELECT role FROM nodes WHERE name = 'InternalOpts'").get(); | ||
| expect(role.role).toBe('dead-unresolved'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
New unit tests only cover the full-classify path
Both new tests call classifyNodeRoles, which delegates to classifyNodeRolesFull. The step-3c change was also applied to classifyNodeRolesIncremental (and its Rust counterpart), but there is no unit test that constructs an incremental scenario — an exported interface with exported=1 in an affected file and verifies it emerges as entry rather than dead-unresolved after an incremental pass. Since the incremental path has its own scoping logic (AND file IN (${placeholders})), a regression there would go undetected by the new tests.
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!
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — added a unit test that exercises classifyNodeRolesIncremental directly (by passing changedFiles=['src/helpers.ts'] to classifyNodeRoles) with an exported=1 interface node and no cross-file edges, confirming it is promoted to entry rather than dead-unresolved.
Sorry, something went wrong.
Codegraph Impact Analysis4 functions changed → 1 callers affected across 1 files
|
Sorry, something went wrong.
|
Addressed Greptile P2: added unit test covering classifyNodeRolesIncremental for exported interfaces with no cross-file edges. |
Sorry, something went wrong.
Sorry, something went wrong.
Summary
Test plan
Closes #1583