Conversation
…edges Add Verilog/SystemVerilog (.v .vh .sv .svh) as a new indexed language, using the vendored tree-sitter-systemverilog grammar (ABI 15, ships a prebuilt wasm -- no build needed). One grammar/extractor covers both V and SV. Extracts modules/interfaces/packages (-> class/interface), functions/tasks, parameters (-> constant), typedefs, and package imports. The high-value edge is module instantiation: module_instantiation -> instantiates, giving the HDL design hierarchy (top module -> leaf module, the HDL equivalent of a call path). Because SystemVerilog nests declared names deep inside *_ansi_header / *_body_declaration wrappers, all structural extraction is done in a custom visitNode hook; only package imports use the generic importTypes + extractImport path. Ports and internal signals are intentionally not extracted (they would explode the node count without aiding structural queries). Also make codegraph_trace / callers / callees follow instantiates edges (previously calls/references/imports only) so the module hierarchy is queryable by the flagship tools -- without this the edges existed but no tool walked them. instantiates is a precise tree-sitter edge, so it does not widen traces the way fuzzy matches would; getImpactRadius already traversed all edge kinds. In OO languages this also surfaces new X() construction dependencies. Wiring: src/types.ts (LANGUAGES), src/extraction/grammars.ts (wasm map, EXTENSION_MAP, vendored-path branch, display name), new src/extraction/languages/verilog.ts, languages/index.ts (EXTRACTORS), vendored src/extraction/wasm/tree-sitter-systemverilog.wasm. Tests: 5 new Verilog cases in extraction.test.ts (detection + module/function/ instantiation/typedef/import/call); graph/resolution/context suites 61/61 green (no regression from the traversal change). Docs: README (languages bullet + table row), CHANGELOG (Unreleased), corpus.json (picorv32/ibex/cva6), coverage-playbook section 6 row, and the implementation plan at docs/plans/2026-05-25-verilog-support.md.
|
related issue: #262 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class Verilog/SystemVerilog support to CodeGraph, including extraction of module hierarchy edges and updating traversal/tools so that HDL “flow” can be traced through instantiations.
Changes:
- Added
verilogas a supported language with file extension mapping and a vendored tree-sitter grammar. - Implemented a custom SystemVerilog extractor that emits containers, functions/tasks, params/typedefs, imports, call edges, and
instantiatesedges. - Updated trace/callers/callees traversals to include
instantiatesedges; added tests + docs/CHANGELOG updates.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Registers verilog in the language list. |
| src/mcp/tools.ts | Makes codegraph_trace consider instantiates edges. |
| src/graph/traversal.ts | Includes instantiates edges in recursive traversal for callers/callees. |
| src/extraction/languages/verilog.ts | New Verilog/SystemVerilog extractor (custom visitNode). |
| src/extraction/languages/index.ts | Wires verilogExtractor into the extractor registry. |
| src/extraction/grammars.ts | Adds verilog wasm + extensions + display name + vendored wasm-path selection. |
| tests/extraction.test.ts | Adds language detection + extractor behavior tests for verilog. |
| README.md | Documents Verilog/SystemVerilog as supported. |
| CHANGELOG.md | Notes new language support + traversal behavior change. |
| docs/plans/2026-05-25-verilog-support.md | Historical plan/outcome writeup for the feature. |
| docs/design/dynamic-dispatch-coverage-playbook.md | Adds Verilog/SV entry and validation notes. |
| .claude/skills/agent-eval/corpus.json | Adds Verilog evaluation corpus entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| structTypes: [], | ||
| enumTypes: [], | ||
| typeAliasTypes: [], | ||
| importTypes: ['package_import_declaration'], |
| extractImport: (node, source) => { | ||
| // package_import_declaration → package_import_item → simple_identifier (pkg) | ||
| const item = firstChildOfType(node, ['package_import_item']) ?? node; | ||
| const id = firstSimpleIdentifier(item); | ||
| if (!id) return null; | ||
| return { | ||
| moduleName: getNodeText(id, source), | ||
| signature: getNodeText(node, source).trim(), | ||
| }; | ||
| }, |
| function handleInstantiation(node: SyntaxNode, ctx: ExtractorContext): boolean { | ||
| const typeNode = getChildByField(node, 'instance_type') ?? firstSimpleIdentifier(node); | ||
| if (typeNode && ctx.nodeStack.length > 0) { | ||
| const fromId = ctx.nodeStack[ctx.nodeStack.length - 1]; | ||
| const moduleName = getNodeText(typeNode, ctx.source).trim(); | ||
| if (fromId && moduleName) { | ||
| ctx.addUnresolvedReference({ | ||
| fromNodeId: fromId, | ||
| referenceName: moduleName, | ||
| referenceKind: 'instantiates', | ||
| line: node.startPosition.row + 1, | ||
| column: node.startPosition.column, | ||
| }); | ||
| } | ||
| } |
| it('should extract package typedefs, functions, imports and call edges', () => { | ||
| const code = ` |
| module worker (input logic clk); | ||
| import math_pkg::*; | ||
| function automatic int caller(int y); | ||
| return add(y); | ||
| endfunction | ||
| endmodule | ||
| `; |
c89f4fb to
a8821f8
Compare
…ied instance types Address PR review feedback: - importTypes now targets package_import_item rather than the whole package_import_declaration, so "import a::*, b::*;" indexes every package instead of silently dropping all but the first -- one import node per item. - handleInstantiation normalizes qualified instance types to the trailing segment (factored into a shared trailingSegment helper, also used by handleCall), so a "pkg::mod" instance still resolves to an unqualified "mod" declaration -- consistent with how call references are matched. - Add a multi-package-import extraction test: both packages are indexed and call edges still resolve when several imports share one statement.
| const edgeKinds: Edge['kind'][] = ['calls', 'instantiates']; | ||
| const MAX_HOPS = 7; |
| function handleInstantiation(node: SyntaxNode, ctx: ExtractorContext): boolean { | ||
| const typeNode = getChildByField(node, 'instance_type') ?? firstSimpleIdentifier(node); | ||
| if (typeNode && ctx.nodeStack.length > 0) { | ||
| const fromId = ctx.nodeStack[ctx.nodeStack.length - 1]; | ||
| // Normalize qualified types (`pkg::mod`) to the trailing segment, matching | ||
| // handleCall, so a `pkg::mod` instance still resolves to a `mod` declaration. | ||
| const moduleName = trailingSegment(getNodeText(typeNode, ctx.source).trim()); | ||
| if (fromId && moduleName) { | ||
| ctx.addUnresolvedReference({ | ||
| fromNodeId: fromId, | ||
| referenceName: moduleName, | ||
| referenceKind: 'instantiates', | ||
| line: node.startPosition.row + 1, | ||
| column: node.startPosition.column, | ||
| }); | ||
| } | ||
| } | ||
| // Port-connection expressions rarely hold user calls worth tracking; skip them. | ||
| return true; | ||
| } |
…trace hop guard, instantiation calls
Address GitHub Copilot review on the Verilog PR:
- Multi-package imports: importTypes targets package_import_item rather than the
whole package_import_declaration, so "import a::*, b::*;" indexes every package
instead of dropping all but the first -- one import node per item.
- Qualified-name consistency: factor a shared trailingSegment helper; both call
references and module-instantiation types are normalized to the trailing
segment, so a "pkg::mod" instance resolves to an unqualified "mod" declaration.
- Trace hop guard, per edge kind: cap the wander-prone CALL hops (dense call
graphs produce spurious long shortest paths) but do not penalize module-
instantiation depth (tree-like SoC hierarchies are legitimate), with a generous
total-length sanity bound. Deep HDL hierarchies (>7 levels) no longer falsely
report "no direct path".
- Instantiation port/param calls: handleInstantiation now walks its children, so
function calls inside parameter overrides (".W(f(x))") and port connections
(".a(helper(sig))") emit calls references instead of being silently dropped.
Tests: +3 Verilog cases (multi-package import; instantiation port/param calls;
plus the earlier instantiation case). graph/resolution/context suites stay
61/61 green.
| const edgeKinds: Edge['kind'][] = ['calls', 'instantiates']; | ||
| const MAX_CALL_HOPS = 7; | ||
| const MAX_TOTAL_HOPS = 20; | ||
| const callHops = (p: Array<{ node: Node; edge: Edge | null }>) => | ||
| p.filter((h) => h.edge?.kind === 'calls').length; |
| if (callHops(p) <= MAX_CALL_HOPS && p.length - 1 <= MAX_TOTAL_HOPS) { path = p; break; } | ||
| if (!overCap || p.length < overCap.length) overCap = p; |
| // 'instantiates' surfaces the module-instantiation hierarchy (HDLs) and | ||
| // `new X()` dependencies — "what instantiates this". | ||
| const incomingEdges = this.queries.getIncomingEdges(nodeId, ['calls', 'references', 'imports', 'instantiates']); |
| case 'tf_call': | ||
| return handleCall(node, ctx); |
|
Rebased this onto current Rebase: https://github.com/danusha2345/codegraph/tree/eval/402-verilog — two commits. The first is your three squashed, under your authorship: extractor, vendored grammar, design note and tests unchanged; the wiring re-applied to today's On real designs (wasm arm; there is no kernel walker for this grammar):
423 / 179
Without it, the same queries render blast radius and source but no Flow section, which for an HDL is the question being asked. One thing to know, not a defect: the reader has Suites on the rebase: |
|
Three more commits on the same branch, all yours to take or leave:
Also ran it on a real SystemVerilog corpus, pulp-platform/axi: 177 modules and packages, 7 interfaces, 277 instantiations (185 → module, 92 → interface, twelve spot-checked against source, all right), 2 of 93 files with local parse errors ( |
Порт extractor из colbymchenry#402 и eval/402-verilog; HDL flow ограничен Verilog. Проверены Dual UART, eMMC reader и pulp-platform/axi. Co-authored-by: FHYQ-Dong <FHYQ-Dong@users.noreply.github.com>
|
Follow-up: the rebased port of this extractor, plus instance/port-binding resolution, signal access queries, build profiles and slang-backed semantics, is now open against current |
|
Hi @danusha2345 , thanks a lot for picking this up, and for the rebase and the real-design validation. I'm sorry, I no longer have the bandwidth to keep #402 moving, so I'm very happy for you to take it over in #1875. I'll close #402 in favor of this one. I compared the extractors, and one thing from my local (unpushed) work isn't in #1875 yet: localparam dependency edges. Right now handleParam only creates the constant node. In my version, each identifier used in the value expression also got a references edge from the parameter: module m;
parameter N = 8;
localparam K = (N + 1) * 2; // K --references--> N
localparam M = K << 1; // M --references--> K
endmoduleWith these edges, impact on N follows the whole localparam chain (N → K → M). That's a common question in HDL ("what changes if I change DATA_W?"). The implementation was small: walk the param_assignment subtree, skip the first simple_identifier (that's the declared name itself), cap at ~32 identifiers per expression, and emit a references ref from the constant for each one. Feel free to adopt it or drop it, whatever fits your resolver design best. Thanks again for carrying this forward! |
…puted from `handleParam` only created the constant node, so `localparam K = (N + 1) * 2;` had no edge to `N` and impact on a width parameter stopped at the parameter itself. The value expression now goes through the same scoped signal walk the processes use: identifiers become `hdl:signal:` references FROM the parameter, callee names and package qualifiers are left out, and the resolver binds them in the parameter's lexical scope only, so a same-named parameter of another module is never the target. Idea and shape by FHYQ-Dong (comment on colbymchenry#402). Measured on two real designs, nodes unchanged: a Tang Nano 9k card reader +16 edges, pulp-platform AXI +222 edges; every sampled edge stays inside its module. Co-authored-by: FHYQ-Dong <FHYQ-Dong@users.noreply.github.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thank you — for the original extractor and for the comparison. Adopted: #1875 now has the localparam dependency edges (8f9d7dd, you are co-authored on the commit). One difference from your sketch: instead of a separate identifier walk with a cap, the value expression goes through the same scoped signal walk the processes use. That already leaves out callee names ( Measured on two real designs, node count unchanged: a Tang Nano 9k card reader +16 edges, pulp-platform AXI +222; sampled edges like |
What
Adds Verilog / SystemVerilog (
.v.vh.sv.svh) as a new indexed language, plus a small traversal change so the flagship tools can walk the HDL design hierarchy.Extraction
tree-sitter-systemverilog@0.3.1wasm (ABI 15, ships prebuilt — no Docker/emscripten build). One grammar/extractor covers both Verilog and SV.src/extraction/languages/verilog.ts(customvisitNodehook — SV nests names deep in*_ansi_header/*_body_declaration, so generic field-based name extraction can't reach them). Extracts:class/interface(containers)function; parameters / localparams →constanttype_alias; package imports →importmodule_instantiation→instantiatesedge — the highest-value HDL relationship (the design hierarchy is the HDL equivalent of a call path)Traversal (cross-cutting)
codegraph_trace/codegraph_callers/codegraph_calleesnow followinstantiatesedges (previouslycalls/references/importsonly). Without this the instantiation edges existed but no flagship tool walked them.instantiatesis a precise tree-sitter edge, so it doesn't widen traces the way fuzzy matches would;getImpactRadiusalready traversed all edge kinds. In OO languages this also surfacesnew X()construction dependencies.Wiring
src/types.ts—LANGUAGESsrc/extraction/grammars.ts— wasm map,EXTENSION_MAP, vendored-path branch, display namesrc/extraction/languages/verilog.ts(new) +languages/index.ts(EXTRACTORS)src/extraction/wasm/tree-sitter-systemverilog.wasm(vendored; shipped bycopy-assets; plain git, ~0.77 MB gzipped — consistent with existing wasms, no LFS)src/graph/traversal.ts,src/mcp/tools.ts—instantiatestraversalTesting
5 new Verilog cases in
__tests__/extraction.test.ts(detection + module/function/task/typedef/import + theinstantiatesreference) — green.graph/resolution/contextsuites: 61/61 green — no regression from the traversal change.Deterministic extraction probes on 3 real repos:
.v)picorv32_axi → picorv32_pcpi_mul= 3 hops.sv)ibex_top → ibex_decoder= 4 hops.sv)cva6 → load_store_unit= 3 hopsBenchmark —⚠️ provisional, NOT the canonical methodology
The agent A/B below was run on DeepSeek
deepseek-v4-pro, n=1 per arm — it is not the project's standard benchmark methodology (Claude Opus 4.7 × median of 4 runs/arm), so these numbers do not qualify for the README benchmark table ordocs/benchmarks/codegraph-ab-matrix.md. Anyone publishing those must re-run with the standard methodology. Reproduce with:npm run build && ./scripts/local-install.shthenscripts/add-lang/bench.sh verilog <name> <url> "<question>" headless.Directional results (with codegraph → without), all with-arms correct with 0 Read / 0 Grep:
Full data + caveats:
docs/design/dynamic-dispatch-coverage-playbook.md§6.Docs
README (languages bullet + table row), CHANGELOG (
[Unreleased]), corpus.json (picorv32/ibex/cva6), coverage-playbook §6 row, and the implementation plan atdocs/plans/2026-05-25-verilog-support.md.Notes
.v/.svfiles are present.