Skip to content

perf(kuzu): batch relationship UNWIND writes; fix the two flaky-test issues; finish the #1527 parameter audit - #1648

Merged
Shashankss1205 merged 5 commits into
mainfrom
perf/kuzu-batch-relationships
Aug 18, 2026
Merged

Shashankss1205 merged 5 commits into
mainfrom
perf/kuzu-batch-relationships

Conversation

@Shashankss1205

Copy link
Copy Markdown
Collaborator

Four commits, each with its own regression tests. Full unit suite 1376 passed / 7 skipped (~32s); full integration suite 79 passed (51 original — now ~4:09 instead of ~5:06 — plus 28 newly-unskipped hermetic CLI tests).

perf(kuzu): scope the per-row fallback to node-MERGE shapes (#1605)

The forced per-row fallback existed to dodge a Kùzu planner bug. I reproduced that bug minimally on 0.11.3 and it is real — but only for node-MERGE inside UNWIND: when a merged node's key repeats non-adjacently across the batch ([(A,P),(B,X),(C,P)]), the later row binds to the most-recently-created node instead of re-matching its key (C gets X). This exact corruption was visible in an A/B of the old always-batched path: MethodInjector (which inherits type) picked up another row's parent.

Relationship-only MERGEs (MATCH … MATCH … MERGE (a)-[r]->(b)) do not exhibit the bug — verified with interleaved repeated endpoint keys and duplicate pairs — so they now batch. Measured: a sample-project index halves (10.7s → 5.5s) and the integration suite drops 20%. All 21 goldens (exact endpoint comparison) pass through the batched path. Node-MERGE shapes keep the per-row fallback, now with a comment stating the reproduced mechanism. Inheritance batches are additionally deduped writer-side.

fix(indexer): retry a failed per-file write once (#1612)

The LadybugDB parity flake (intermittently ~51 CONTAINS short, load-correlated) matches the pipeline's per-file swallow point: one transient write failure silently costs a whole file's edges. Writes are MERGE-idempotent, so a single retry converts the blip into success; persistent failures still land in write_failures and the error log. Combined with the batching change (which removes most of the per-row write pressure the flake correlated with), this addresses both the mechanism and the trigger.

test(cgcignore): hermetic + relocated (#1422)

The TC-12+ cases shared one fixed /tmp/cgc_test/_home database across every test and run, wiped it mid-suite, and were gated on a live Neo4j — permanently skipped on most machines, intermittently failing with a different name each full-suite run where they did run. Now: fresh HOME per test, explicit embedded backend (no server, no gate, no wipe), and the subprocess-driven cases moved to tests/integration where that cost belongs. All 28 run everywhere, deterministically.

fix(cpp): lambda parameters (#1527, final case)

_find_lambda_assignments clobbered its correctly-walked parameter list with a nonexistent field and then filtered the wrong node types — every C++ lambda got args: []. Now routed through the shared param helper (same declarator→parameters field chain as function_definition), golden regenerated with the three new lambda Parameter nodes. With Ruby (#1581), the PHP key fix, PHP promoted/variadic (#1643) and JS destructured/rest (#1591) all previously landed, every case in the #1527 audit is closed.

Fixes #1605, fixes #1612, fixes #1422, fixes #1527

…fallback to node-MERGE shapes (#1605)

Kuzu 0.11.3's MERGE pipeline mis-binds when a merged NODE's key repeats
non-adjacently in one UNWIND batch ([(A,P),(B,X),(C,P)] binds C to X) —
reproduced minimally and pinned in tests. Relationship-only MERGEs
(MATCH..MATCH..MERGE rel) do not exhibit the bug, verified with interleaved
duplicate endpoint keys and duplicate pairs, so they now run batched: the
full integration suite drops from ~5:06 to ~4:09 (-20%) and a sample-project
index halves. Inheritance batches are also deduped writer-side so a repeated
(child, parent) record cannot double an INHERITS edge on embedded backends.
…ng it (#1612)

A transient write failure under runner load silently cost the graph a whole
file's edges (LadybugDB intermittently 51 CONTAINS short in the parity run).
Writes are MERGE-idempotent, so one retry converts the blip into success;
persistent failures still land in write_failures and the error log. Together
with the #1605 batching change (which removes most of the per-row write
pressure the flake correlated with), this addresses the observed drop.
…egration suite (#1422)

The TC-12+ cases shared one fixed /tmp/cgc_test/_home database across every
test and run, wiped it mid-suite, and were gated on a live Neo4j — so they
were permanently skipped on most machines and intermittently failed with a
different name each full-suite run wherever they did run. Each test now gets
a fresh HOME and an explicit embedded backend (no server, no gate, no wipe),
and the subprocess-driven cases live in tests/integration where that cost
belongs. tc01-tc11 (pure PathSpec logic) stay in the unit suite.
…1527)

The inline version first clobbered its correctly-walked parameter_list with
a nonexistent 'parameters' field on lambda_expression, then filtered for
bare identifiers over parameter_declaration nodes — every lambda assignment
got args: []. lambda_expression carries the same declarator→parameters field
chain as function_definition, so the shared extractor handles it, rendering
args in the project's established 'type name' form. Last remaining case of
the #1527 parameter audit (Ruby #1581, PHP key + #1643, JS #1591 all landed
previously).
@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
code-graph-context-pack Ready Ready Preview Aug 18, 2026 8:17pm

@github-project-automation github-project-automation Bot moved this to Backlog tasks in CGC Progress Board Aug 18, 2026
@Shashankss1205
Shashankss1205 merged commit 66f5a8e into main Aug 18, 2026
1 of 2 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog tasks to Done in CGC Progress Board Aug 18, 2026
@Shashankss1205
Shashankss1205 deleted the perf/kuzu-batch-relationships branch August 18, 2026 20:18
@github-actions

Copy link
Copy Markdown
Contributor

Hi! 👋 Join our CodeGraphContext Discord channel to collaborate: https://discord.gg/dR4QY32uYQ

@github-actions

Copy link
Copy Markdown
Contributor

🔍 PR Code Graph Analysis

perf(kuzu): batch relationship UNWIND writes; fix the two flaky-test issues; finish the #1527 parameter audit (#1648)

📊 Interactive Visualization

View the blast radius graph: PR Reviewer Dashboard

📦 Artifacts

The graph JSON has been uploaded as a build artifact: pr-code-graph-1648


Generated by CodeGraphContext using FalkorDB Lite

mollyjester added a commit to mollyjester/CodeGraphContext that referenced this pull request Aug 19, 2026
write_inheritance_links iterates a 12x12 (child_label, parent_label)
grid and passes the FULL resolved batch to every surviving pair. CodeGraphContext#1617
made the grid sparse by probing each label table once and skipping pairs
whose table is empty -- but every pair that survives the probe still
receives the whole batch, and the number of survivors is a property of
the repo's label inventory, not of the edges being written.

1. Resolution now attaches the exact child/parent node label to each
   row. build_inheritance_and_csharp_files builds a path -> {name ->
   set(labels)} index over the 12 inheritance-eligible construct kinds
   and pins a row's child_label / parent_label only when that (path,
   name) maps to exactly one kind. The writer then runs one query per
   (child_label, parent_label) pair that actually occurs.

2. Rows whose (path, name) is ambiguous carry no label and fall back to
   the original enumeration. This is not hypothetical: as
   write_binds_links' docstring already records, Kotlin does not qualify
   `name` by enclosing scope, so a top-level interface and an unrelated
   nested class can legitimately share name+path. CodeGraphContext#1617's
   populated_labels set is retained and applied to exactly those
   fallback paths. It is deliberately NOT applied to the grouped pairs:
   those pairs come from the rows themselves, so they are never empty,
   and probing them would be pure overhead while a stale or
   lazily-created label table would let the probe silently drop a real
   INHERITS edge.

3. batch_size = 500 was declared and never used; the batch is now
   chunked. This is a separate robustness fix -- it keeps a five-figure
   UNWIND payload out of a single driver round-trip -- and it is not
   what produces the numbers below. On a repo with few populated labels
   it slightly *raises* the raw query count while cutting total work.

Relationship to CodeGraphContext#1617: complementary, not overlapping. CodeGraphContext#1617's saving is
a function of how many of the 12 label tables are empty; this change's
saving is a function of how many (child, parent) pairs the data actually
contains. On a repo where all 12 tables are non-empty, CodeGraphContext#1617 removes no
pairs at all and the full fan-out survives.

Measured, Neo4j, the pass in isolation on a 27,530-file
Python/JavaScript repo (756k nodes, 20,719 resolved rows), same store
and page cache on both sides, INHERITS deleted between runs so each
starts identical:

                      upstream/main      this change
  resolution              1.05 s            1.77 s
  write_inheritance_links 7433.5 s          620.4 s     12.0x
  batch-rows processed    207,340           21,487       9.7x
  queries                 32                80          0.4x

Query count goes UP because batch_size chunking splits the one large
pair into ~21 round-trips; the work per round-trip is what falls.
Upstream's 32 queries are 12 probes + 16 internal + 4 external, and
solving 16*I + 4*E = 207,340 against I + E = 20,719 gives I = 10,372, E
= 10,347 -- i.e. the internal batch really is passed 16 times and the
external batch 4 times, where this change passes each once. Resolution
costs 0.72 s more, for the path -> {name -> set(labels)} index over all
27,530 files.

These figures were measured against 810ea8a, before CodeGraphContext#1648 added row
deduplication to this function. Dedupe shrinks the batch identically on
both sides, so the batch-pass structure and the ratios above are
unaffected; only the absolute row counts would be smaller today.

Qualification: that database was provisioned by upstream's own schema,
which has no composite (name, path) index, so every pair query scans
rather than seeks and the absolute times are dominated by that scan
cost. The quantity this change controls is the number of batch passes,
and that is unaffected by indexing. On Kuzu the same structure shows up
without any index question, because the Kuzu adapter unrolls `UNWIND
$batch` into one query per row to sidestep a relationship planner bug:
with all 12 label tables populated, the added test's 3-row input issues
468 session.run calls before this change and 18 after.

The batch-pass count is structural: with P of the 12 labels populated
the old code passes the internal batch P^2 times and the external batch
P times. P was 4 here; at P = 12, CodeGraphContext#1617 skips nothing and the factor is
78x.

Output is unchanged: the produced INHERITS edge set is identical. On the
corpus above both sides produced the same 20,963 INHERITS edges with an
identical per-(child_label, parent_label) breakdown across all seven
pairs that occur. Also verified by (a) the 21-project parser golden
suite (tests/fixtures/goldens), whose node/edge exports are unchanged,
(b) upstream's three existing write_inheritance_links tests, which feed
label-less rows and so exercise the fallback enumeration unmodified --
including CodeGraphContext#1617's probe-failure invariant, and (c) the added test, whose
exact-edge-set assertion passes identically before and after the change
while the query count collapses. On that corpus the ambiguity guard
fired on 74 of 20,719 rows (0.36%), which took the enumeration fallback.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shashankss1205 pushed a commit that referenced this pull request Aug 19, 2026
…1652)

write_inheritance_links iterates a 12x12 (child_label, parent_label)
grid and passes the FULL resolved batch to every surviving pair. #1617
made the grid sparse by probing each label table once and skipping pairs
whose table is empty -- but every pair that survives the probe still
receives the whole batch, and the number of survivors is a property of
the repo's label inventory, not of the edges being written.

1. Resolution now attaches the exact child/parent node label to each
   row. build_inheritance_and_csharp_files builds a path -> {name ->
   set(labels)} index over the 12 inheritance-eligible construct kinds
   and pins a row's child_label / parent_label only when that (path,
   name) maps to exactly one kind. The writer then runs one query per
   (child_label, parent_label) pair that actually occurs.

2. Rows whose (path, name) is ambiguous carry no label and fall back to
   the original enumeration. This is not hypothetical: as
   write_binds_links' docstring already records, Kotlin does not qualify
   `name` by enclosing scope, so a top-level interface and an unrelated
   nested class can legitimately share name+path. #1617's
   populated_labels set is retained and applied to exactly those
   fallback paths. It is deliberately NOT applied to the grouped pairs:
   those pairs come from the rows themselves, so they are never empty,
   and probing them would be pure overhead while a stale or
   lazily-created label table would let the probe silently drop a real
   INHERITS edge.

3. batch_size = 500 was declared and never used; the batch is now
   chunked. This is a separate robustness fix -- it keeps a five-figure
   UNWIND payload out of a single driver round-trip -- and it is not
   what produces the numbers below. On a repo with few populated labels
   it slightly *raises* the raw query count while cutting total work.

Relationship to #1617: complementary, not overlapping. #1617's saving is
a function of how many of the 12 label tables are empty; this change's
saving is a function of how many (child, parent) pairs the data actually
contains. On a repo where all 12 tables are non-empty, #1617 removes no
pairs at all and the full fan-out survives.

Measured, Neo4j, the pass in isolation on a 27,530-file
Python/JavaScript repo (756k nodes, 20,719 resolved rows), same store
and page cache on both sides, INHERITS deleted between runs so each
starts identical:

                      upstream/main      this change
  resolution              1.05 s            1.77 s
  write_inheritance_links 7433.5 s          620.4 s     12.0x
  batch-rows processed    207,340           21,487       9.7x
  queries                 32                80          0.4x

Query count goes UP because batch_size chunking splits the one large
pair into ~21 round-trips; the work per round-trip is what falls.
Upstream's 32 queries are 12 probes + 16 internal + 4 external, and
solving 16*I + 4*E = 207,340 against I + E = 20,719 gives I = 10,372, E
= 10,347 -- i.e. the internal batch really is passed 16 times and the
external batch 4 times, where this change passes each once. Resolution
costs 0.72 s more, for the path -> {name -> set(labels)} index over all
27,530 files.

These figures were measured against 810ea8a, before #1648 added row
deduplication to this function. Dedupe shrinks the batch identically on
both sides, so the batch-pass structure and the ratios above are
unaffected; only the absolute row counts would be smaller today.

Qualification: that database was provisioned by upstream's own schema,
which has no composite (name, path) index, so every pair query scans
rather than seeks and the absolute times are dominated by that scan
cost. The quantity this change controls is the number of batch passes,
and that is unaffected by indexing. On Kuzu the same structure shows up
without any index question, because the Kuzu adapter unrolls `UNWIND
$batch` into one query per row to sidestep a relationship planner bug:
with all 12 label tables populated, the added test's 3-row input issues
468 session.run calls before this change and 18 after.

The batch-pass count is structural: with P of the 12 labels populated
the old code passes the internal batch P^2 times and the external batch
P times. P was 4 here; at P = 12, #1617 skips nothing and the factor is
78x.

Output is unchanged: the produced INHERITS edge set is identical. On the
corpus above both sides produced the same 20,963 INHERITS edges with an
identical per-(child_label, parent_label) breakdown across all seven
pairs that occur. Also verified by (a) the 21-project parser golden
suite (tests/fixtures/goldens), whose node/edge exports are unchanged,
(b) upstream's three existing write_inheritance_links tests, which feed
label-less rows and so exercise the fallback enumeration unmodified --
including #1617's probe-failure invariant, and (c) the added test, whose
exact-edge-set assertion passes identically before and after the change
while the query count collapses. On that corpus the ambiguity guard
fired on 74 of 20,719 rows (0.36%), which took the enumeration fallback.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment