fix: CALLS_SERVICE edge writes use a real transaction instead of BEGIN/COMMIT split across fresh connections#36
Open
pradeepmouli wants to merge 2 commits into
Conversation
…GIN/COMMIT split across fresh connections detect_dynamic_urls's CALLS_SERVICE edge writer issued BEGIN TRANSACTION/CREATE/COMMIT as three separate raw_query calls, each getting a brand-new Kuzu Connection, so the transaction never actually spanned the writes and COMMIT reliably failed with "No active transaction for COMMIT" whenever any edge was written (silently swallowed as a non-fatal warning). Add GraphBackend::write_calls_service_edges, which owns its own Connection/transaction internally (same convention as upsert_files_bulk/derive_tested_by_edges/resolve_calls), implement it for KuzuBackend, and update both dynamic_urls.rs call sites to use it instead of hand-rolling transaction-control strings via raw_query. Pre-commit hook's full suite run hit the pre-catalogued write_lock_perf::test_contended_lock_throughput environmental timing flake (unrelated to this change, and nothing else failed); bypassing hooks for this commit per that documented exception.
pradeepmouli
requested review from
johnintuit,
murari316 and
sandeep-mewara
as code owners
July 25, 2026 14:24
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
1 participant
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.
detect_dynamic_urls's CALLS_SERVICE edge writer issuedBEGIN TRANSACTION/CREATE/COMMITas three separateraw_querycalls — butGraphBackend::raw_query(Kùzu) opens a freshConnectionon every call, so the transaction never actually spanned the writes.COMMITreliably failed with "No active transaction for COMMIT" whenever any edge was written (silently caught as a non-fatal warning, so this went unnoticed — confirmed by reproducing it against this repo's owncrates/tree during investigation).Moved the write onto a new
GraphBackend::write_calls_service_edgestrait method that owns its own transaction/connection internally, matching every other multi-step write already in the trait (upsert_files_bulk,derive_tested_by_edges,resolve_calls). Implemented for real in bothKuzuBackend(one held connection, correct BEGIN/loop/COMMIT with rollback on error) andNeo4jBackend(single UNWIND-parameterized query, auto-committed) — a no-op default would have silently dropped these edges for whichever backend didn't get a real implementation, so both ship together here.One incidental behavior change worth calling out: the old code silently swallowed every per-row CREATE failure (
let _ = ...); the new code surfaces per-row errors and rolls back the batch on failure, making the write atomic per pass. The caller (crates/infigraph-mcp/src/tools/index.rs) already treats a returnedErras a non-fatal warning, so this is a pure improvement with no regression risk.Also fixes the incidental per-edge connection-construction overhead this pattern caused (one connection for the whole batch instead of N+2).
Test plan
cargo test -p infigraph-core --test calls_service_edges— new regression test reproduces the original multi-edge failure mode and asserts it's fixedcargo test -p infigraph-core --features neo4j --test neo4j_backend— 17/17 passing against a live Neo4j, including 2 new tests for the Neo4j implementationinfigraph-core/infigraph-cli/infigraph-mcp) — clean modulo pre-existing, unrelated flakes confirmed via base-commit comparison--features remotebuild + clippy — clean (pre-existing clippy findings elsewhere in the file confirmed byte-identical on the base commit, unrelated to this diff)cargo fmt --all -- --check— cleancrates/(13 dynamic URLs detected, 6 matched to routes) — no "no active transaction" warning, and the graph was queried directly to confirm all 6CALLS_SERVICEedges were actually committed