Skip to content

feat(storage): execute declared index fast-forward plans on open#3238

Merged
Sinity merged 2 commits into
masterfrom
feature/storage/index-fast-forward-executor
Jul 21, 2026
Merged

feat(storage): execute declared index fast-forward plans on open#3238
Sinity merged 2 commits into
masterfrom
feature/storage/index-fast-forward-executor

Conversation

@Sinity

@Sinity Sinity commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

Wires an executor into the index-tier open path so a declared clone-safe schema-version gap no longer forces a full archive rebuild. Before this change, IndexDeltaDeclaration/IndexFastForwardPlan (polylogue/storage/sqlite/lifecycle.py) had no consumer: initialize_archive_database always raised the rebuild-required RuntimeError for any PRAGMA user_version gap, benign or not.

Problem

Live incident 2026-07-21: a freshly promoted v42 index.db could not be opened by v43 code, even though #3235 shipped a declared clone-safe fast-forward (IndexDeltaDeclaration v43, FastForwardOperation name=v43-messages-fts-identity, kind=REBUILD_FTS). grep confirmed nothing outside lifecycle.py consumed IndexFastForwardPlan/eligible_for_sql_fast_forward — the declaration registry existed but no open-path executor applied it, so the declared-benign v43 delta still forced the ~11h full rebuild it exists to avoid.

Solution

  • polylogue/storage/sqlite/archive_tiers/index_fast_forward_executor.py (new): apply_index_fast_forward(conn, plan) applies a plan's declarations one at a time, each inside its own BEGIN IMMEDIATE/commit transaction, bumping PRAGMA user_version in the same transaction. A crash mid-chain rolls back to the previous version; the next open rebuilds the plan from whatever version actually landed and re-applies idempotently.
  • Dispatch is generic over FastForwardOperationKind, not per-version:
    • REBUILD_FTS — drops the declared triggers, calls ensure_fts_index_sync, then literal DELETE FROM messages_fts / DELETE FROM messages_fts_identity + insert_all_message_*_rows_sql() for whichever surfaces the declaration names (narrow identity-only refresh for v43, not a full content rescan).
    • REPLACE_TABLE — copy-forwards columns shared between the existing and canonical shape onto a temp table, then renames it in (handles the declared v33/v36/v38/v41 constraint-widening/column-removal deltas without a raw reparse).
    • REPLACE_VIEW / CREATE_INDEX / DROP_TABLE — straightforward canonical-DDL re-run / drop.
  • polylogue/storage/sqlite/lifecycle.py: added resolve_canonical_index_objects (executes INDEX_DDL against a scratch :memory: connection to resolve each declared object's exact canonical CREATE statement) — lives here rather than in the executor so it stays a read-only helper, not itself a mutation the writer-module inventory has to account for.
  • polylogue/storage/sqlite/archive_tiers/bootstrap.py: initialize_archive_database now calls index_fast_forward_plan(current_version, expected_version) before raising the rebuild-required error for ArchiveTier.INDEX; applies the plan and returns when one is eligible, falls through to the existing error otherwise (e.g. a SEMANTIC_REPARSE declaration is in the span, as with v39/v42).
  • polylogue/storage/fts/fts_lifecycle.py: split rebuild_fts_index_sync into reusable rebuild_messages_fts_content_sync/rebuild_messages_fts_identity_sync helpers (used internally; the executor inlines the literal DELETE statements itself so devtools verify layering's writer-module inventory can see and account for the index-tier mutation).
  • docs/plans/layering.yaml: registered the new module as an index-tier writer (Writer module: index docstring marker + entrypoints: [apply_index_fast_forward]).
  • docs/internals.md: documented the open-path executor in the Schema Versioning Model section.
  • docs/plans/topology-target.yaml / docs/topology-status.md: regenerated for the new module.

Verification

  • devtools test tests/unit/storage/test_index_fast_forward_executor.py — 5 passed. New tests build a v43-schema index.db, downgrade it to the exact v42-shaped trigger bodies (no messages_fts_identity, pre-v43 trigger bodies), then open it through initialize_archive_database and assert: no RuntimeError, PRAGMA user_version == 43, the identity ledger is populated for every indexable block, message_identity_mismatch_sql() returns 0, a second open is idempotent, a SEMANTIC_REPARSE declaration still raises the rebuild-required error, and dispatch is exercised generically (not v43-hardcoded) via a synthetic DROP_TABLE declaration.
  • devtools test tests/unit/storage/test_index_fast_forward_lifecycle.py tests/unit/storage/test_archive_tiers_ddl.py tests/unit/storage/test_archive_tier_init.py — 46 passed, 4 pre-existing failures in test_index_fast_forward_lifecycle.py reproduced identically on unmodified master via git stash (unrelated to this change; xdist-order-dependent schema_policy/monkeypatch leakage, not caused here).
  • /realm/project/polylogue/.venv/bin/python -m mypy --strict on all four touched modules — Success: no issues found.
  • /realm/project/polylogue/.venv/bin/python -m devtools verify layeringNo layering violations found.
  • devtools verify --quickexit_code: 0 (format, lint, mypy, render-all-check, verify-layering, verify-closure-matrix, schema-roundtrip, manifests, ci-workflows, doc-commands, docs-coverage, test-infra-currency, test-clock-hygiene, pytest-timeout-overrides, degrade-loudly all green).
  • devtools test tests/unit/storage/test_fts_identity_ledger.py tests/unit/storage/test_fts_bloat_invariants.py tests/unit/storage/test_fts_repair_sql.py tests/unit/storage/test_dangling_fts_derived_surfaces.py tests/unit/daemon/test_fts_global_repair.py — 1 pre-existing failure (test_dangling_fts_derived_surfaces.py::test_reset_and_repair_fts_rows_recovers_excess_message_rows, no such table: main.derived_refresh_guard) reproduced identically on unmodified master.

AC matrix (Ref polylogue-t3gk)

AC Status
Wire an executor into the index-tier open path Satisfied — bootstrap.py calls apply_index_fast_forward
Contiguous chain of eligible declarations applied one at a time, user_version bumped + committed per declaration Satisfied
Operation kinds dispatch generically via FastForwardOperationKind, not v43-only Satisfied — registry-style if/elif on kind; exercised by a synthetic non-v43 declaration in tests
Fall back to rebuild-required error when a gap version lacks an eligible declared plan Satisfied — SEMANTIC_REPARSE test
Single-writer-safe, no new locking layer Satisfied — runs on the connection bootstrap already opened; BEGIN IMMEDIATE only within that connection's own transaction
Tests: v42-shaped fixture, ledger populated, user_version==43, zero identity mismatch Satisfied
Tests: SEMANTIC_REPARSE declaration still refuses Satisfied
docs/internals.md updated Satisfied

Residual risk

REPLACE_TABLE/REPLACE_VIEW/CREATE_INDEX dispatch (v33/v34/v36/v38/v41 declarations) is implemented and unit-exercised only indirectly (through the generic-dispatch synthetic test); it is not exercised end-to-end against a real historical fixture in this PR because those spans are not reachable in practice today — index_fast_forward_plan requires a contiguous eligible chain, and the v39 and v42 SEMANTIC_REPARSE declarations block any span crossing them, so the only currently-reachable production gap is v42→v43. If an operator ever needs v32-era fast-forward again, add a fixture-based test for that span before relying on it.

Sinity and others added 2 commits July 21, 2026 14:37
Wire an executor into the index-tier open path so a declared clone-safe
schema-version gap no longer forces a full archive rebuild.

Ref polylogue-t3gk

Co-Authored-By: Claude <noreply@anthropic.com>
Move canonical-DDL resolution into lifecycle.py and use literal, in-module
mutation SQL for the FTS rebuild path so the new fast-forward executor
passes devtools verify layering as a properly inventoried index-tier writer.

Ref polylogue-t3gk

Co-Authored-By: Claude <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Sinity, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dcc72f2d-d5d4-4d16-8f1d-be6d417c7769

📥 Commits

Reviewing files that changed from the base of the PR and between 46f6c13 and ed21880.

📒 Files selected for processing (9)
  • docs/internals.md
  • docs/plans/layering.yaml
  • docs/plans/topology-target.yaml
  • docs/topology-status.md
  • polylogue/storage/fts/fts_lifecycle.py
  • polylogue/storage/sqlite/archive_tiers/bootstrap.py
  • polylogue/storage/sqlite/archive_tiers/index_fast_forward_executor.py
  • polylogue/storage/sqlite/lifecycle.py
  • tests/unit/storage/test_index_fast_forward_executor.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/storage/index-fast-forward-executor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Sinity

Sinity commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Coordinator stand-in review (CodeRabbit rate-limited): read the full 778-line diff.

  • Executor design is right: generic dispatch over FastForwardOperationKind (never version-branched), canonical DDL resolved from the live INDEX_DDL via a scratch connection (same source of truth as the offline clone actuator), per-declaration BEGIN IMMEDIATE + user_version bump in the same transaction → crash-resume rebuilds the plan from on-disk version and every op is idempotent. REPLACE_TABLE copy-forward over shared columns matches the declared v33/v36/v38/v41 delta semantics.
  • The live-incident test is the exact reproduction: v42-shaped fixture (pre-v43 trigger bodies, no ledger) opened through initialize_archive_database — the same runtime path polylogued and every CLI entry uses — asserting fast-forward + populated ledger + zero mismatch + idempotent reopen. SEMANTIC_REPARSE refusal pinned at both the plan and executor layers.
  • Residual risk accepted: REPLACE_TABLE/REPLACE_VIEW/CREATE_INDEX arms are synthetic-tested only — currently unreachable in production (SEMANTIC_REPARSE at v39/v42 blocks those spans), correctly flagged in the PR body.
  • 5 new tests green, mypy strict clean, quick-gate green; pre-existing failures stash-verified on unmodified master.

Merging on green quick-gate. Operational note: first open of the live v42 archive by post-merge code will execute the v43 fast-forward (ledger population over 4.77M blocks) — coordinator will sequence that after the in-flight Hermes reprocess releases the writer.

@Sinity
Sinity merged commit 4bb84e2 into master Jul 21, 2026
2 of 3 checks passed
@Sinity
Sinity deleted the feature/storage/index-fast-forward-executor branch July 21, 2026 12:55
Sinity added a commit that referenced this pull request Jul 21, 2026
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.

1 participant