Skip to content

fix(native): write dynamic_kind for sink edges in native orchestrator path#1698

Merged
carlos-alm merged 3 commits into
mainfrom
release/3.15.0
Jun 22, 2026
Merged

fix(native): write dynamic_kind for sink edges in native orchestrator path#1698
carlos-alm merged 3 commits into
mainfrom
release/3.15.0

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • Root cause: insert_call_edge_rows in pipeline.rs mapped ComputedEdge → EdgeRow but silently dropped dynamic_kind. Native sink edges (eval, computed-key, unresolved-dynamic) were inserted with dynamic_kind IS NULL. The benchmark filter added in 39b9a00 (OR e.dynamic_kind IS NULL) then included these rows as false positives, breaking precision for dynamic-javascript, dynamic-typescript, and dynamic-scala.
  • Rust fix: Add dynamic_kind: Option<String> to EdgeRow; update do_insert_edges to a 6-column INSERT (CHUNK 199→165 to stay under SQLite's 999 param limit); wire e.dynamic_kind.clone() through insert_call_edge_rows so the field reaches the DB.
  • Benchmark fix: Add AND tgt.kind != 'file' to extractResolvedEdges — the correct semantic filter since sink edges are the only calls edges that ever target the file node, removing the dependency on dynamic_kind being set.

Test plan

  • npm test passes locally (all 206 tests green, including the 4 previously failing dynamic-* precision gates)
  • dynamic-javascript precision 100%, recall ≥ 75%
  • dynamic-typescript precision 100%, recall ≥ 75%
  • dynamic-scala precision 100%, recall 100%

… path

`insert_call_edge_rows` in pipeline.rs mapped ComputedEdge→EdgeRow but
silently dropped dynamic_kind, so native sink edges landed in the DB with
dynamic_kind IS NULL.  The benchmark filter added in 39b9a00
(`OR e.dynamic_kind IS NULL`) then matched those NULL-kind rows and counted
them as false positives, breaking precision for dynamic-javascript,
dynamic-typescript, and dynamic-scala.

Two-layer fix:

1. edges.rs: add dynamic_kind: Option<String> to EdgeRow and update
   do_insert_edges to a 6-column INSERT (CHUNK 199→165 to stay under
   SQLite's 999 bind-parameter limit).  None binds as SQL NULL for all
   non-sink edges; Some(dk) writes the kind string for sink edges.

2. pipeline.rs: pass e.dynamic_kind.clone() in insert_call_edge_rows so
   ComputedEdge.dynamic_kind reaches the DB insert.

3. resolution-benchmark.test.ts: add AND tgt.kind != 'file' to
   extractResolvedEdges — the correct semantic filter since sink edges are
   the only calls that ever target the file node, and no legitimate
   resolution does.  This filter is correct regardless of dynamic_kind being
   set, removing the dependency on the back-fill path.
@greptile-apps

greptile-apps Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a silent field-drop bug in the native orchestrator path where dynamic_kind was never written to the edges table, causing sink edges to appear as false positives in the benchmark precision filter. The fix is two-pronged: a Rust-side schema change that propagates dynamic_kind through EdgeRow and do_insert_edges, and a benchmark SQL change that uses tgt.kind != 'file' as the authoritative semantic guard against sink edges.

  • Rust fix (edges.rs, pipeline.rs): EdgeRow gains a 6th field dynamic_kind: Option<String>; do_insert_edges is updated to a 6-column INSERT with CHUNK reduced from 199 → 165 (165×6=990, safely under the 999 param limit); insert_call_edge_rows clones e.dynamic_kind into the EdgeRow so the value reaches SQLite.
  • Benchmark fix (resolution-benchmark.test.ts): extractResolvedEdges adds AND tgt.kind != 'file' — the correct semantic definition of a sink edge — while retaining the compound confidence/dynamic_kind clause as belt-and-suspenders for databases built before this fix; an inline comment documents both guards.

Confidence Score: 5/5

Safe to merge — the change is a targeted one-field fix in the Rust native orchestrator path with no behavioural changes to the JS/WASM path, and the benchmark filter gains a semantically-correct authoritative guard.

The root-cause field drop is definitively fixed in insert_call_edge_rows; the CHUNK math is verified (165×6=990≤999); as_deref() is the correct way to bind Option<String> as a nullable SQLite text parameter; the JS path was already handling dynamic_kind via a post-insert UPDATE and is untouched; and the benchmark SQL change is semantically correct and well-commented.

No files require special attention — all three touched code locations (edges.rs, pipeline.rs, resolution-benchmark.test.ts) are clean and consistent with each other.

Important Files Changed

Filename Overview
crates/codegraph-core/src/db/repository/edges.rs Adds dynamic_kind: Option<String> to EdgeRow, updates INSERT to 6 columns, and reduces CHUNK from 199 to 165 (165×6=990 ≤ 999 param limit). Binding uses as_deref() correctly for Option→Option<&str>.
crates/codegraph-core/src/domain/graph/builder/pipeline.rs One-line fix: adds dynamic_kind: e.dynamic_kind.clone() to the ComputedEdge → EdgeRow mapping in insert_call_edge_rows, closing the field-drop bug on the native orchestrator path.
tests/benchmarks/resolution/resolution-benchmark.test.ts Adds AND tgt.kind != 'file' as the authoritative semantic guard against sink edges in extractResolvedEdges; retains the compound confidence/dynamic_kind clause as a belt-and-suspenders guard for pre-fix DBs; comment explains the rationale clearly.
CHANGELOG.md Adds v3.15.0 release entry documenting the full dynamic-call feature set and associated bug fixes.
README.md Adds codegraph roles --dynamic example and ignoreAdditionalDirs config snippet; minor roadmap phase renumbering.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Extractor as Rust Extractor
    participant Builder as build_edges (Rust)
    participant Pipeline as pipeline.rs
    participant EdgeRepo as edges.rs (do_insert_edges)
    participant DB as SQLite edges table

    Extractor->>Builder: "ComputedEdge { dynamic_kind: Some(eval) }"
    Builder->>Pipeline: "insert_call_edge_rows(&edges)"
    Pipeline->>Pipeline: map ComputedEdge to EdgeRow
    note over Pipeline: was: dynamic_kind dropped
    note over Pipeline: now: dynamic_kind: e.dynamic_kind.clone()
    Pipeline->>EdgeRepo: "do_insert_edges(conn, &edge_rows)"
    EdgeRepo->>DB: INSERT INTO edges (source_id, target_id, kind, confidence, dynamic, dynamic_kind)
    note over EdgeRepo,DB: 165 rows x 6 params = 990 under 999 limit
Loading
%%{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"}}}%%
sequenceDiagram
    participant Extractor as Rust Extractor
    participant Builder as build_edges (Rust)
    participant Pipeline as pipeline.rs
    participant EdgeRepo as edges.rs (do_insert_edges)
    participant DB as SQLite edges table

    Extractor->>Builder: "ComputedEdge { dynamic_kind: Some(eval) }"
    Builder->>Pipeline: "insert_call_edge_rows(&edges)"
    Pipeline->>Pipeline: map ComputedEdge to EdgeRow
    note over Pipeline: was: dynamic_kind dropped
    note over Pipeline: now: dynamic_kind: e.dynamic_kind.clone()
    Pipeline->>EdgeRepo: "do_insert_edges(conn, &edge_rows)"
    EdgeRepo->>DB: INSERT INTO edges (source_id, target_id, kind, confidence, dynamic, dynamic_kind)
    note over EdgeRepo,DB: 165 rows x 6 params = 990 under 999 limit
Loading

Reviews (3): Last reviewed commit: "fix(benchmark): add comment explaining c..." | Re-trigger Greptile

Comment on lines +265 to 266
AND tgt.kind != 'file'
AND (e.confidence IS NULL OR e.confidence > 0 OR e.dynamic_kind IS NULL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant filter after Rust fix

With dynamic_kind now correctly written to the DB for sink edges, the new AND tgt.kind != 'file' guard already excludes all sink edges semantically. The residual third clause OR e.dynamic_kind IS NULL was the original root-cause condition and is now effectively dead for correctly-built databases — any edge with confidence = 0 and tgt.kind != 'file' that also has dynamic_kind IS NULL would be a DB anomaly rather than a sink edge.

Both filters together are harmless and the belt-and-suspenders approach is reasonable as defence-in-depth, but a brief comment clarifying why the compound condition is kept (e.g., for databases built before the Rust fix) would prevent future confusion.

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!

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a 4-line comment above the two guards explaining: (1) tgt.kind != 'file' is the authoritative semantic guard for sink edges (the only calls edges that target file nodes), and (2) the compound confidence/dynamic_kind clause is kept as belt-and-suspenders for databases built before the Rust fix in #1698 where dynamic_kind was NULL for sink edges.

…xtractResolvedEdges (#1698)

The tgt.kind != 'file' guard is the authoritative semantic exclusion for
sink edges; the confidence/dynamic_kind clause is kept as belt-and-suspenders
for databases built before the native fix where dynamic_kind was NULL.
@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 9217d18 into main Jun 22, 2026
33 checks passed
@carlos-alm carlos-alm deleted the release/3.15.0 branch June 22, 2026 08:03
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant