fix: rebuild crashes with "cannot start a transaction within a transaction"#181
Open
realkotob wants to merge 1 commit intotirth8205:mainfrom
Open
fix: rebuild crashes with "cannot start a transaction within a transaction"#181realkotob wants to merge 1 commit intotirth8205:mainfrom
realkotob wants to merge 1 commit intotirth8205:mainfrom
Conversation
…ction"
Running `code-review-graph build` on a large repo (~9k files) after an
earlier interrupted build would crash almost immediately with:
sqlite3.OperationalError: cannot start a transaction within a
transaction
I hit this on the Godot source tree. The first build ran to completion
on the parse phase, then I Ctrl-C'd it while community detection was
stuck (separate perf issue). On the retry, the rebuild blew up on the
very first file.
Why it happens:
`full_build` starts by purging any files that used to exist in the DB
but no longer exist on disk. The purge loop calls
`store.remove_file_data()` for each stale path, which issues two
`DELETE` statements and returns without committing. Under Python's
default (legacy) sqlite3 isolation mode, those DELETEs silently open an
implicit transaction. The next `store_file_nodes_edges()` call then runs
an explicit `BEGIN IMMEDIATE` — and SQLite rejects it because one is
already open.
The reason nobody had tripped this before is that the purge loop is
usually a no-op: if every file still exists, `existing_files -
current_abs` is empty. But my interrupted build had left the DB with
thousands of file rows whose absolute paths no longer matched the set
the rebuild was computing, so every single one hit the purge path and
the connection was left dirty before parsing even started.
Fix is two parts:
1. `full_build` now calls `store.commit()` once after the purge loop,
so the parse phase starts with a clean connection.
2. `store_file_nodes_edges` checks `conn.in_transaction` and flushes
any dangling implicit transaction before its `BEGIN IMMEDIATE`.
Defensive: if any future code path leaves DML uncommitted, the
atomic per-file replace still works instead of crashing the build.
Adds a regression test that primes an implicit transaction via
`remove_file_data` and then asserts `store_file_nodes_edges` still
succeeds.
ghiemer
added a commit
to ghiemer/code-review-graph
that referenced
this pull request
Apr 10, 2026
…205#135, tirth8205#181) Python's sqlite3 with default isolation_level="" silently opens a DEFERRED transaction on the first DML statement. When remove_file_data() is called for stale/deleted files, the DELETE statements open an implicit transaction that is never committed. The subsequent store_file_nodes_edges() then executes BEGIN IMMEDIATE inside that already-open transaction, causing: sqlite3.OperationalError: cannot start a transaction within a transaction This affects both full_build (stale-file purge loop) and incremental_update (deleted-file handling). Fix: - graph.py: Add in_transaction guard before BEGIN IMMEDIATE in store_file_nodes_edges() to flush any pending implicit transaction - graph.py: Add public rollback() method to GraphStore - incremental.py: Commit after stale-file purge loop in full_build() - incremental.py: Commit after deleted-file removal in incremental_update() Tests: - test_store_after_remove_no_transaction_error (single remove → store) - test_store_after_multiple_removes_no_transaction_error (bulk removes → store) Closes tirth8205#135, tirth8205#181, relates to tirth8205#162, tirth8205#193, tirth8205#94 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
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.
The problem
Running
code-review-graph buildon a large repo (~9k files) after an earlier interrupted build crashes almost immediately with:I hit this on a big source tree on my Macbook M1 Pro. The first build ran through the parse phase to completion, then I Ctrl-C'd it while community detection was stuck (separate perf issue — the file-based fallback is quadratic on large graphs). On the retry, the rebuild blew up on the very first file:
Why it happens
full_buildstarts by purging any files that used to exist in the DB but no longer exist on disk:remove_file_dataissues twoDELETEstatements and returns without committing. Under Python's default (legacy)sqlite3isolation mode, thoseDELETEs silently open an implicit transaction. The very nextstore_file_nodes_edges()call then runs an explicitBEGIN IMMEDIATE— and SQLite rejects it because one is already open.The reason this went undetected is that the purge loop is usually a no-op: if every file still exists on disk,
existing_files - current_absis empty and the buggy path never runs. My interrupted build had left the DB with thousands of file rows whose absolute paths no longer matched the set the rebuild was computing, so every single one was treated as stale and the connection was left dirty before parsing even started.The fix
Two parts:
incremental.py—full_buildnow callsstore.commit()once after draining the purge loop, so the parse phase starts with a clean connection.graph.py—store_file_nodes_edgeschecksconn.in_transactionand flushes any dangling implicit transaction before itsBEGIN IMMEDIATE. Defensive: if any future code path leaves DML uncommitted, the atomic per-file replace still works instead of crashing the build.Test plan
test_store_file_nodes_edges_after_dangling_dmlprimes an implicit transaction viaremove_file_dataand assertsstore_file_nodes_edgesstill succeeds. Fails onmain, passes with the fix.uv run pytest tests/test_graph.py tests/test_incremental.py— 45 passeduv run pytest tests/test_tools.py tests/test_integration_v2.py— 66 passed