perf(kuzu): batch relationship UNWIND writes; fix the two flaky-test issues; finish the #1527 parameter audit - #1648
Merged
Conversation
…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).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Hi! 👋 Join our CodeGraphContext Discord channel to collaborate: https://discord.gg/dR4QY32uYQ |
Contributor
🔍 PR Code Graph Analysisperf(kuzu): batch relationship UNWIND writes; fix the two flaky-test issues; finish the #1527 parameter audit (#1648) 📊 Interactive VisualizationView the blast radius graph: PR Reviewer Dashboard 📦 ArtifactsThe graph JSON has been uploaded as a build artifact: 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 inheritstype) 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_failuresand 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/_homedatabase 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 totests/integrationwhere that cost belongs. All 28 run everywhere, deterministically.fix(cpp): lambda parameters (#1527, final case)
_find_lambda_assignmentsclobbered its correctly-walked parameter list with a nonexistent field and then filtered the wrong node types — every C++ lambda gotargs: []. Now routed through the shared param helper (samedeclarator→parametersfield chain asfunction_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