Skip to content

fix(pipeline): preserve ADRs during incremental reindex#992

Open
M-Marbouh wants to merge 1 commit into
DeusData:mainfrom
M-Marbouh:fix/incremental-adr-preservation
Open

fix(pipeline): preserve ADRs during incremental reindex#992
M-Marbouh wants to merge 1 commit into
DeusData:mainfrom
M-Marbouh:fix/incremental-adr-preservation

Conversation

@M-Marbouh

Copy link
Copy Markdown

What does this PR do?

Preserves manage_adr content when index_repository takes the incremental reindex path.

This is a follow-up to #516. The full reindex path already captures ADR content from project_summaries.summary before replacing the SQLite DB and restores it afterward. The incremental path also replaces the DB in dump_and_persist(), but it only repersisted graph data, file hashes, coverage rows, and FTS state. Any ADR stored through manage_adr was therefore silently dropped on the next incremental rebuild.

The failure shape is:

  1. Index a project.
  2. Store ADR content with manage_adr mode update.
  3. Read it back with mode get; content is present.
  4. Change an existing source file so the next index_repository routes through incremental reindex.
  5. Read the ADR again; before this fix it could return successfully with empty content because project_summaries was recreated without the previous summary.

This PR mirrors the full-reindex preservation behavior in src/pipeline/pipeline_incremental.c:

  • before unlinking/rebuilding the DB, read any existing ADR content for the project;
  • after cbm_gbuf_dump_to_sqlite() recreates the DB and it is reopened, write that ADR content back with cbm_store_adr_store();
  • log a restore failure instead of silently losing the ADR.

It also adds pipeline_adr_survives_incremental_reindex, which indexes a tiny repo, stores an ADR, changes one existing file to force the incremental route, and asserts that the ADR content is still present afterward.

This PR is intentionally scoped to ADR loss during incremental reindex. It does not attempt to address any separate stale-WAL/listed-but-unqueryable project lookup behavior.

Verification

  • git fetch origin main and rebase onto current origin/main (6b57db2) succeeded without conflicts.
  • make -j2 -f Makefile.cbm cbm succeeded.
  • Direct MCP repro against the rebuilt binary passed:
    • manage_adr update stored content;
    • immediate manage_adr get returned the content;
    • a second index_repository after changing one file reported "adr_present":true;
    • post-incremental manage_adr get returned the original content.
  • make -j2 -f Makefile.cbm build/c/test-runner succeeded.
  • ASAN_OPTIONS=detect_leaks=0 build/c/test-runner pipeline reached and passed the new pipeline_adr_survives_incremental_reindex test, including the pipeline.route path=incremental branch.
  • The selected pipeline suite still exits non-zero with an unrelated existing failure: pipeline_backpressure_futile_nap_disengages at tests/test_pipeline.c:6801 (220 passed, 1 failed).

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

@M-Marbouh M-Marbouh requested a review from DeusData as a code owner July 9, 2026 23:42
Signed-off-by: Mustapha <m88e54ik@gmail.com>
@M-Marbouh M-Marbouh force-pushed the fix/incremental-adr-preservation branch from d0d5b24 to 3852990 Compare July 10, 2026 11:33
@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 10, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 10, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for the ADR persistence fix. I have triaged this into 0.9.1-rc as a normal-priority UX/behavior bug fix. Review focus will be ADR state across incremental DB replacement and making sure we do not restore stale summary data by accident.

@M-Marbouh

Copy link
Copy Markdown
Author

Thanks.
That review focus makes sense. The intended invariant here is the same as the full-reindex path from #516: capture the ADR for the exact project key from the existing DB immediately before the DB replacement, then restore it to the same project key immediately after cbm_gbuf_dump_to_sqlite() recreates the DB.
I kept the PR scoped to preserving project_summaries.summary across the incremental replacement path only. If you want an additional guard/test around stale summary restoration, I’m happy to add it.

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

Labels

bug Something isn't working priority/normal Standard review queue; useful PR with ordinary maintainer urgency. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants