fix: rebuild index when mode adds capabilities - #1263
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes incremental routing decisions across index mode transitions by persisting the effective index capability level in Project metadata (properties.index_mode), forcing a full rebuild when the requested mode requires capabilities not present in the stored index, and preserving stronger stored capabilities during downgrades (while still honoring the requested mode for discovery/exclusions). It also ensures persistent artifacts are refreshed after reindexing and adds regression coverage for mode upgrades/downgrades and malformed/legacy metadata handling.
Changes:
- Persist effective capability mode in the Project node (
properties.index_mode) and use exact JSON string comparisons when reading it. - Route mode upgrades to full rebuilds; keep stored effective mode for incremental changed-file re-extraction on downgrades.
- Refresh existing artifacts after reindexing (FAST quality unless persistence is explicitly requested) and add end-to-end tests for these transitions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pipeline.c | Adds regression tests covering upgrade/downgrade routing, capability preservation, malformed metadata, and artifact refresh behavior. |
| src/store/store.h | Clarifies that index_mode in coverage metadata is the requested discovery mode; effective capability mode is stored on the Project node. |
| src/pipeline/pipeline.c | Persists Project index_mode, reads stored mode safely via yyjson, routes upgrades to rebuilds, and adjusts post-publish artifact export behavior. |
| src/pipeline/pipeline_internal.h | Exposes cbm_pipeline_mode_name for consistent internal serialization and updates incremental API contract docs/signature. |
| src/pipeline/pipeline_incremental.c | Accepts an effective_mode for changed-file re-extraction, preserves macro extraction capability on downgrades, and propagates persistence failures when required. |
| src/mcp/mcp.c | Updates MCP tool schema description to document artifact refresh behavior when an artifact already exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
17cf892 to
902262e
Compare
|
Thank you for the contribution and for covering both index mode upgrades and downgrades, including malformed metadata. This is now triaged as a high-priority graph correctness bug for |
902262e to
52866c1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/pipeline/pipeline.c:1527
- Assigning the stored mode to
p->modehere also changes discovery, becausecbm_pipeline_run_stagedlater passesp->modetocbm_discover_ex2. Afull → fastdowngrade therefore still scans full-only directories/files instead of using the requested discovery scope, contradicting the stated requested/effective-mode separation and defeating the fast-mode workaround for large repositories. Keep the requested discovery mode separate and use the stored mode only for effective extraction/post-passes and persisted capability metadata.
p->mode = stored_mode;
src/pipeline/pipeline_incremental.c:2426
- This exports the artifact during the inner staged run, but the successful return is then marked incremental and
export_after_publishexports it again (BEST quality for explicit persistence, FAST for an existing artifact). Thus every exact no-op that needs an artifact performs compression/VACUUM twice, with the first export occurring before final publication. Let the existing post-publication path perform the single export.
return cbm_pipeline_refresh_artifact(p, db_path);
5545555 to
cb0dfa6
Compare
3959fdf to
3a4ad3a
Compare
Keep caller-requested discovery scope while rebuilding changed weaker-mode requests at the stronger stored coverage. Export persistent artifacts only after the replacement database generation is published. Signed-off-by: astandrik <astandrik@yandex-team.ru>
3a4ad3a to
d024f41
Compare
|
Merged (415a64a) — thank you for what was the technically strongest external PR in this batch. The export-exactly-once-after-rename fault-injection tests are the reason this could land as one unit: they prove the publish-boundary design under cancel/retry/delete, which is precisely the evidence a ~400-line rework of the pipeline's most critical routing needs. The requested/effective mode split completes #1273's downgrade half cleanly. We chose your centralized export over our own per-route refresh design — #1163 is being closed in its favor with credit. |
Summary
Fixes #1273
Related: #563
index_coverage_metaWhat this PR completes
Current main already stores the canonical mode in
index_coverage_metaand forces a rebuild when a stronger request needs capabilities missing from the stored generation. This PR does not reimplement that upgrade route. It completes the remaining downgrade half of #1273 and fixes artifact publication ordering in the same pipeline path.Previously, preserving stronger stored coverage promoted the pipeline's single mutable mode before discovery. A
full -> fastrequest therefore widened back to full discovery/reporting. Conversely, comparing only the requested subset could leave changed or deleted full-only inputs stale.The pipeline now keeps
requested_modeand effectivemodeseparate. It reports exclusions from requested discovery, but when stored coverage is stronger it compares the stored generation with a fresh manifest built at that effective scope. An exact match remains an incremental no-op. Any stored-only change or deletion forces a complete effective-scope rebuild, after which the requested exclusion/ignored report is restored.Artifact export is centralized at the outer publication boundary. No route exports from the staging database; explicit persistence and refresh of an existing artifact happen only after the final database rename.
Mode contract
Cancellation or a staging/publication failure before the final rename preserves the previous database and artifact. An explicitly requested artifact-export failure is returned after the database has been published; automatic refresh of an existing artifact remains best-effort.
The public MCP API, SQLite schema, and artifact format are unchanged.
index_coverage_metaremains the sole mode representation. This PR intentionally does not add duplicate Project-node metadata, special normalization of externally modified/corrupt mode values, or generic SQLite read-error hardening; those are separate artifact-integrity/recovery concerns rather than normal #1273 mode transitions.Regression coverage
fast -> moderaterun forces a full rebuild, createsSIMILAR_TO, recordsindex_mode=moderate, and refreshes the artifact; a laterfastrequest retains those capabilitiesfull -> fastreportstools/as excluded while retaining FULL stored coverageVerification
Exact published head:
3a4ad3a2on base43531195.343 passed(pipeline,pipeline_semantic_manifest_repro,artifact, and upstreammem)42 passed32 passedin a disposable focused fixture)fast -> moderate -> fastcapability contract:1 passedgit diff --check, diff-only clang-format, static security audit, cppcheck, and CI lint: passedExact-head GitHub CI is still running; clean-runner sanitizer, platform, and analyzer jobs remain authoritative.