Skip to content

fix(pipeline): make the #334 plausibility gate fire (capture committed_nodes before dump)#456

Open
nguiaSoren wants to merge 2 commits into
DeusData:mainfrom
nguiaSoren:fix/334-committed-count-capture
Open

fix(pipeline): make the #334 plausibility gate fire (capture committed_nodes before dump)#456
nguiaSoren wants to merge 2 commits into
DeusData:mainfrom
nguiaSoren:fix/334-committed-count-capture

Conversation

@nguiaSoren

Copy link
Copy Markdown

Summary

Builds on #433 (the status:degraded plausibility gate by @yangsec888) and fixes a capture-ordering bug that left the gate inert — it never tripped, so a catastrophically partial index would still report status: "indexed", the exact silent-success #334 set out to stop. @yangsec888 reviewed the finding and deferred to this fix (#433 (comment)).

Closes #334. Supersedes #433 (its gate commit is included here with authorship preserved).

The bug

dump_and_persist_hashes read the committed node count at pipeline.c after cbm_gbuf_dump_to_sqlite(), which runs release_and_remap_vectors()release_gbuf_indexes() and frees node_by_qn (graph_buffer.c:348). cbm_gbuf_node_count() therefore returned 0. With committed_nodes = 0 ≤ CBM_DUMP_VERIFY_MIN_FLOOR, cbm_dump_verify_is_degraded() short-circuits to false.

Edges survive the free (edge_by_key is freed, but edges.count is not), which is why a healthy full index reported a plausible expected_edges while expected_nodes came back 0:

nodes: 13351 | expected_nodes: 0 | expected_edges: 50438

Separately, the incremental path (pipeline_incremental.c) never set the committed count at all, so the gate was also disabled on incremental/reparse reindexes — the scenario #334 actually reproduces (repeated index_repository on an already-present index).

The fix

  1. pipeline.c — capture committed_nodes/committed_edges before the dump.
  2. pipeline_incremental.c + pipeline_internal.h — record committed counts on the incremental path via a new cbm_pipeline_set_committed_counts() (the incremental unit cannot see the opaque cbm_pipeline struct).
  3. tests/test_pipeline.cpipeline_committed_counts_match_persisted: drives the real pipeline and asserts committed_nodes > 0 and == persisted. The existing dump_verify tests feed the predicate synthetic values, so they cover the pure function but not the pipeline wiring; this test fails on the pre-fix code and passes after.

Verification

  • Built clean on macOS arm64 (scripts/build.sh).
  • After the fix, a healthy full index reports expected_nodes == nodes on both the full and incremental paths (was 0 before).
  • Full suite green: scripts/test.sh → 5,620 passing, including the new test.
  • The new test fails on the current code (committed_nodes == 0) and passes after, so it genuinely guards the regression.

All commits are DCO signed-off.

yangsec888 and others added 2 commits June 12, 2026 13:52
Compare persisted SQLite node counts to in-memory dump counts after
index_repository completes so partial WAL/durability loss surfaces as
status:"degraded" instead of silent indexed.

Signed-off-by: Sam Li <yangsec888@gmail.com>
…eusData#334)

The DeusData#334 plausibility gate compares committed (extracted) node count
against persisted rows, but committed_nodes was read AFTER
cbm_gbuf_dump_to_sqlite(), which calls release_gbuf_indexes() and frees
node_by_qn. cbm_gbuf_node_count() therefore returned 0, committed_nodes
fell at/below CBM_DUMP_VERIFY_MIN_FLOOR, and the gate never fired (a
healthy index reported expected_nodes:0 while expected_edges was correct,
since edges.count survives the dump).

- pipeline.c: capture committed node/edge counts before the dump.
- pipeline_incremental.c: set committed counts on the incremental path too
  (via new cbm_pipeline_set_committed_counts), so the gate covers
  incremental/reparse reindexes — the scenario DeusData#334 actually reproduces.
- test_pipeline.c: drive the real pipeline and assert committed_nodes > 0
  and == persisted (the existing dump_verify tests fed synthetic counts and
  could not catch the capture-ordering bug).

Signed-off-by: soren <nguiasoren@gmail.com>
@nguiaSoren

Copy link
Copy Markdown
Author

Friendly ping on this — CI is green across all platforms (tests, lint, security, DCO) and @yangsec888 has signed off on the approach. Happy to rebase or adjust anything if it'd help it land. No rush, and thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Silent index corruption after rapid kill/restart cycles

2 participants