fix(storage): harden durable change-train admission - #3875
Conversation
|
@codex review |
|
|
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR changes raw-failure lifecycle sampling to use grouped SQL aggregates and global recency ranking. It adds durable bootstrap markers, archive identity continuity checks, live schema validation, startup evidence caching, forward-version receipts, CLI output, and regression coverage. ChangesRaw failure lifecycle sampling
Durable forward-version evidence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DurableChangeTrain
participant SQLiteLiveTier
participant ArchiveIdentity
participant MaintenanceCLI
DurableChangeTrain->>SQLiteLiveTier: capture live version and schema evidence
DurableChangeTrain->>ArchiveIdentity: validate archive identity continuity
SQLiteLiveTier-->>DurableChangeTrain: return integrity and schema inventory
DurableChangeTrain-->>MaintenanceCLI: return DurableForwardVersionReceipt
MaintenanceCLI-->>MaintenanceCLI: serialize receipt or render admission message
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26a82734ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return None | ||
| historical = _historical_schema_evidence(train) | ||
| expected_identity = train.apply_evidence.post.archive_identity_digest | ||
| if actual.archive_identity_digest != expected_identity: |
There was a problem hiding this comment.
When a rebuildable index.db is promoted between two durable schema trains, the historical train stores the old authority_identity_digest, while the live evidence contains the new index generation's inode. ArchiveIdentity.authority_identity_digest includes active_generation and every non-audit tier, so this comparison rejects the same source/user archive during startup even though only a rebuildable tier changed and the latest train can otherwise pass. Bind this check to an identity that is invariant across index/embeddings/ops replacement rather than the whole file-set digest.
AGENTS.md reference: AGENTS.md:L114-L122
Useful? React with 👍 / 👎.
| f"{train.tier.value} durable tier immutable archive identity differs from historical train " | ||
| f"v{train.target_version} after later train advancement" | ||
| ) | ||
| if actual.quick_check != ("ok",): |
There was a problem hiding this comment.
For a later-version durable database with index/table inconsistencies, this now accepts actual.quick_check == ("ok",) where the previous code ran PRAGMA integrity_check. SQLite's quick check deliberately omits checks such as verifying that index entries agree with table contents and validating UNIQUE constraints, so a corrupt durable tier can receive forward admission. Keep the full integrity check on this safety-critical admission path, even if quick-check evidence is also retained.
AGENTS.md reference: AGENTS.md:L114-L122
Useful? React with 👍 / 👎.
| GROUP BY origin, validation_status, artifact_kind, support_status | ||
| """ | ||
| ).fetchall() | ||
| sample_rows = conn.execute( |
There was a problem hiding this comment.
When the daemon commits a raw failure or lifecycle-artifact update between these two SELECTs, SQLite can give summary_rows and sample_rows different read snapshots because this read-only connection never begins an explicit transaction. The returned evidence can then contain a sample whose lifecycle has a zero count, or omit rows counted in the summary; previously both classifications and samples came from the same failed_rows result. Start a read transaction around all lifecycle queries or derive the bounded sample and aggregates from one statement.
AGENTS.md reference: AGENTS.md:L161-L166
Useful? React with 👍 / 👎.
| ) | ||
| _verify_released_train_live_tier(archive_root, live, train) | ||
| return DurableChangeTrainExecution(train=train, manifest_path=manifest_path, migration_result=None) | ||
| forward_version_receipt = _verify_released_train_live_tier( |
There was a problem hiding this comment.
No valid call to execute_durable_change_train can reach this receipt assignment: the function selects only the sidecar at current_version + 1, while a released train at that slot requires the live database to have already reached the slot and is rejected by startup reconciliation when it has not. Once the database is at the runtime target, there is no next sidecar and the function returns at the earlier no-sidecar branch with forward_version_receipt=None. Thus the newly exposed execution field never carries the historical/current receipt; collect receipts while reconciling historical manifests or explicitly load the released current/historical train instead.
AGENTS.md reference: AGENTS.md:L477-L482
Useful? React with 👍 / 👎.
| ORDER BY r.acquired_at_ms DESC, r.raw_id DESC | ||
| """ | ||
| failed_rows = conn.execute(failure_query).fetchall() | ||
| summary_rows = conn.execute( |
There was a problem hiding this comment.
With a large source.db, sample_limit=1 still performs work proportional to the full failure population: this aggregate must resolve the correlated artifact queries for every failed raw, and the subsequent sample query must scan and sort all matching raw_sessions before applying LIMIT because there is no index supporting the failure predicate plus acquired_at_ms ordering. It therefore adds a second full evaluation while the durable-train runtime probe retains archive ownership; the regression only traces that the SQL text contains LIMIT and does not demonstrate bounded work. Add a samples-only probe path or an indexed/set-based plan whose scanned rows and artifact lookups are actually bounded.
AGENTS.md reference: AGENTS.md:L477-L482
Useful? React with 👍 / 👎.
Problem: released historical train manifests admitted a later durable tier using only SQLite integrity, so a healthy database from another archive could pass. Lifecycle probes also materialized every failed raw before applying their sample limit. What changed: require immutable archive identity and the train's frozen historical fresh-schema proof for forward admission, record the historical and current targets in the execution receipt, and reuse one captured evidence object per tier while reconciling historical manifests. Aggregate lifecycle counts in SQLite and limit detailed sample selection in SQL. Compatibility/migration: current-target continuity remains exact; later versions are admitted only for the same archive with retained historical schema evidence. Ref polylogue-dcrmm Co-Authored-By: Codex <noreply@openai.com>
26a8273 to
04299d5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04299d56c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if integrity != ("ok",): | ||
| _verify_persisted_live_tier_continuity(conn, train, actual=actual) | ||
| return None | ||
| historical = _historical_schema_evidence(train) |
There was a problem hiding this comment.
When a same-archive later migration drops or changes an object from the historical schema while leaving SQLite structurally healthy, this path still admits it: _historical_schema_evidence() only validates fields within the stored train, while neither actual.schema_inventory_sha256 nor the live schema objects are compared with that proof. Because quick_check remains ok and the archive identity remains unchanged, a higher user_version can therefore pass without the historical schema being represented; compare the live inventory against the historical object set or require a verified chain of later train proofs.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Reissue the exact carrier head after rebasing onto current master.
Problem: forward admission validated a released tier's version, quick check, and aggregate schema digest, but did not prove that historical objects still existed with their original definitions after later train advancement. What changed: reconstruct the released train's canonical historical inventory from the package DDL, require every historical object and definition to remain present in the live tier, and add a regression that rejects a dropped table. Compatibility/migration: this strengthens admission and startup validation for durable tiers. It does not alter migration SQL or mutate live archives. Ref polylogue-6k0na Co-Authored-By: Claude <noreply@anthropic.com>
Problem: strict mypy rejects direct access to the migration runner's runtime DDL mapping because it is not part of that module's explicit export surface. What changed: read the existing runtime mapping through the module namespace and cast it to the established ArchiveTier-to-SQL type. Compatibility/migration: typing-only follow-up to the durable train admission check. Runtime behavior is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Refresh the non-draft PR trigger after publishing the verified historical-schema admission repair. No product behavior changes.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5524b84c80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fresh.executescript(archive_ddl[train.tier]) | ||
| fresh.execute(f"PRAGMA user_version = {train.target_version}") | ||
| _migration_runner._prepare_fresh_connection_for_target(fresh, train.tier, train.target_version) |
There was a problem hiding this comment.
When a later valid durable migration adds a column or copy-forwards an existing table, this reconstruction starts from the newest canonical DDL, while _prepare_fresh_connection_for_target() only removes objects owned by later riders and cannot restore an earlier definition of an existing object. Its inventory therefore differs from the released historical proof, causing startup reconciliation to reject an archive that advanced through an allowed additive numbered migration. Persist the historical object definitions or replay canonical migrations to the historical slot instead of projecting backward from current DDL.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Problem: durable forward admission was coupled to rebuildable generation identity, skipped SQLite integrity checks, and did not expose a forward receipt when the live tier had already reached its target. Raw-failure lifecycle summaries and samples also came from separate read snapshots. What changed: bind durable evidence to source/user identities, run full integrity checks, compare the live schema with the exact canonical version, return the newest historical forward receipt, and derive lifecycle counts and bounded samples from one read transaction and one query. Tests cover schema drift, receipt reachability, lifecycle snapshot consistency, and sample bounds. Compatibility/migration: durable admission is stricter for schema drift and integrity failures. Existing valid additive migrations remain admitted against their canonical live version. Ref polylogue-dcrmm Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59c21ed470
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| durable_id = ArchiveIdentity.resolve(live_path.parent).durable_id | ||
| archive_identity_digest = hashlib.sha256(durable_id.encode("utf-8")).hexdigest() |
There was a problem hiding this comment.
When upgrading an archive whose released train manifest was written by the previous build, the persisted archive_identity_digest contains authority_identity_digest, but every new capture now contains sha256(durable_id). Because the manifest format remains polylogue.durable-change-train.v1 and continuity compares these fields directly, startup reconciliation rejects an unchanged durable archive before it can run another migration. Version the evidence format or explicitly admit/transition the legacy digest before enforcing the new value.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Problem: Existing durable train manifests store the pre-split full-archive identity digest, while new captures use the durable-tier identity. Startup continuity would reject an unchanged archive before another migration.\n\nWhat changed: Accept a legacy digest only when it matches the current legacy archive identity and the fresh evidence matches the durable identity. Apply the compatibility check to rollback, source continuity, and forward-admission paths, with a regression test.\n\nCompatibility/migration: New evidence remains durable-tier bound. Legacy manifests are admitted only during the narrow transition case and still require matching schema, content, and integrity evidence.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4353c45b7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Manifests written before the durable-tier identity split contain the | ||
| # old full-archive digest. Admit that legacy evidence only when the | ||
| # current archive still has the same legacy identity and the newly | ||
| # captured evidence proves the durable identity is unchanged. | ||
| return expected_digest == legacy_digest and actual_digest == durable_digest |
There was a problem hiding this comment.
When upgrading an archive whose v1 train was captured before the digest change, replacing or promoting index.db causes the old expected_digest to differ from the newly recomputed legacy_digest, even though the durable source/user files and tier content are unchanged. Startup reconciliation therefore still rejects valid legacy manifests after a routine rebuildable-tier replacement. Fresh evidence in this revision is that the new compatibility helper explicitly requires equality with the current full-archive digest; transition legacy evidence using durable-tier continuity without requiring rebuildable identities to remain fixed.
AGENTS.md reference: AGENTS.md:L114-L122
Useful? React with 👍 / 👎.
| SELECT raw_id, origin, validation_status, artifact_kind, support_status, sample_rank | ||
| FROM ranked |
There was a problem hiding this comment.
When status or verification renders more than one failure sample, this outer query does not guarantee that rows are returned in sample_rank order: ordering inside ROW_NUMBER() determines rank values but does not order the final result set. The selected rows are still the newest failures, but their presentation can vary by query plan rather than retaining the previous newest-first contract; add an outer ORDER BY sample_rank.
Useful? React with 👍 / 👎.
| integrity_check = tuple(str(row[0]) for row in conn.execute("PRAGMA integrity_check")) | ||
| if integrity_check != ("ok",): |
There was a problem hiding this comment.
On daemon startup with multiple released historical trains below the live version, _reconcile_durable_change_train_startup_locked invokes this branch once per manifest. The new cache reuses actual_evidence, but every invocation still runs a fresh PRAGMA integrity_check, so a large source.db is scanned once for each historical train and startup cost grows with both archive size and migration count. Capture the full integrity result once per tier and reuse it while archive ownership prevents intervening writes.
AGENTS.md reference: AGENTS.md:L181-L183
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/storage/raw_failure_lifecycle.py`:
- Around line 172-189: Update the lifecycle query around the failure_rows
iteration in polylogue/storage/raw_failure_lifecycle.py: perform lifecycle
aggregation in SQL and apply the sample_limit bound in SQL before rows reach
Python, while preserving the existing summary and sample result shapes. Update
tests/unit/daemon/test_raw_failure_sample.py lines 605-608 to assert the traced
statement contains both SQL aggregation and the SQL sample bound, not merely
ROW_NUMBER().
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 1485-1513: Update _reconcile_durable_change_train_startup_locked
and _verify_released_train_live_tier to cache each tier’s integrity result and
live schema inventory in live_evidence_by_tier alongside the existing evidence
cache, reusing them for every released manifest in that tier. Remove the
redundant quick_check or integrity_check assertion, retaining only one integrity
validation, and reuse the cached inventory instead of repeatedly calling
capture_durable_schema_inventory and _canonical_schema_inventory.
- Around line 1437-1446: Harden _canonical_schema_inventory by coercing
target_version with int() before interpolating it into PRAGMA user_version.
Replace the vars(_migration_runner)["ARCHIVE_DDL_BY_TIER"] lookup with explicit
handling that raises a clear error when tier has no DDL, while preserving the
existing archive DDL execution for supported tiers.
- Line 863: Update _assert_durable_database_continuity in migration_runner.py to
accept archive_root instead of relying on the connection passed from the exited
with block. In the source continuity refresh path, pass the already in-scope
archive_root when invoking the method, while preserving the existing
continuity-check behavior.
In `@tests/unit/daemon/test_raw_failure_sample.py`:
- Around line 605-608: Extend the assertions in the test around
lifecycle_queries to inspect the single traced lifecycle SQL statement and
verify it performs SQL-side aggregation and applies a sample-selection bound of
3, such as sample_rank <= 3 or LIMIT 3, while preserving the existing count and
sample-length assertions.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1119-1129: Update the test covering the third
execute_durable_change_train call to assert the ownership-release results after
the existing receipt assertions. Verify released equals [True, True], preserving
the current contract that only the expected executions invoke
release_archive_ownership.
- Around line 1191-1207: Add negative coverage to
test_continuity_admits_legacy_full_archive_identity_digest: assert that
replacing archive_identity_digest with an arbitrary digest raises, and assert
that the legacy digest also raises when _assert_durable_database_continuity is
called without connection. Keep the existing connection-backed legacy acceptance
assertion unchanged, proving the fallback is explicitly opt-in.
- Around line 1131-1149: Add coverage in the test block around
_verify_released_train_live_tier for the forward-path runtime-target upper
bound: invoke it with current_target_version=2 while the live tier is v3 and
assert the “is newer than current target” rejection. Also add a forward-path
case that exercises the legacy-digest fallback at the relevant manifest/proof
setup, while preserving the existing successful v2-to-v3 continuity assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4abe7e00-526f-47c3-8ae7-ead8b164926b
📒 Files selected for processing (5)
polylogue/storage/raw_failure_lifecycle.pypolylogue/storage/sqlite/durable_change_train.pypolylogue/storage/sqlite/migration_runner.pytests/unit/daemon/test_raw_failure_sample.pytests/unit/storage/test_durable_change_train.py
Problem: Lifecycle summaries scanned every failed row into Python and capped samples after materialization, making the read-only status path scale with an unbounded in-memory result. What changed: Group failure lifecycle evidence in SQL and apply the sample limit in SQL inside the existing read transaction. The focused test now checks both the aggregate and bounded sample statements. Compatibility/migration: Snapshot fields and lifecycle classification remain unchanged.
Problem: Startup reconciliation repeated full integrity scans and schema inventory construction for every released historical train in the same durable tier. What changed: Reuse per-tier integrity and schema inventories during startup reconciliation, and validate canonical inventory target versions and DDL registration before constructing them. Compatibility/migration: Direct forward-admission callers retain the same checks; only repeated startup observations are cached.
Problem: The canonical schema helper reached through a private module attribute that strict mypy rejects. What changed: Import the archive-tier DDL registry through its exported package boundary. Compatibility/migration: No schema or runtime behavior changes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65eab44c81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isinstance(target_version, bool): | ||
| raise DurableChangeTrainError("canonical schema inventory target version must be an integer") | ||
| try: | ||
| archive_ddl = ARCHIVE_DDL_BY_TIER[tier] |
There was a problem hiding this comment.
When test_maintenance_route_replays_historical_sidecars_before_current_target patches migration_runner.ARCHIVE_DDL_BY_TIER and bootstrap.ARCHIVE_DDL_BY_TIER with its three-table fixture schema, this module-level binding still points to the original production mapping. The third execution therefore reconstructs the full production source schema and raises the canonical-live-version error before the new receipt assertions, making the changed storage test fail deterministically. Dereference the patchable registry at call time or update the fixture to patch this binding too.
Useful? React with 👍 / 👎.
| if train.tier not in live_integrity_by_tier: | ||
| live_integrity_by_tier[train.tier] = tuple( | ||
| str(row[0]) for row in live.execute("PRAGMA integrity_check") | ||
| ) |
There was a problem hiding this comment.
When startup has only the released train for the live version—the normal post-migration state—this eagerly executes PRAGMA integrity_check and constructs both schema inventories even though _verify_released_train_live_tier takes its equal-version branch and never reads any of those cached arguments. Because bootstrap calls this reconciliation on startup, a large durable source.db now receives an unnecessary full integrity scan on every daemon start; populate these forward-admission caches only when actual.user_version > train.target_version.
Useful? React with 👍 / 👎.
Problem: equal-version durable train reconciliation performed integrity and schema scans that are only required when a live tier has advanced, and schema inventory lookup bypassed runtime registry overrides.\n\nWhat changed: defer forward-admission evidence scans until the live version exceeds the released target, and resolve the canonical DDL registry from migration_runner at call time.\n\nVerification: devtools test tests/unit/storage/test_durable_change_train.py tests/unit/daemon/test_raw_failure_sample.py (79 passed).
Refresh the non-draft PR validation boundary after the durable train review fixes. The commit has no source change; it exists to run checks against the current scope carrier.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0ed05d875
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| train=None, | ||
| manifest_path=None, | ||
| migration_result=legacy_result, | ||
| forward_version_receipt=forward_version_receipt, |
There was a problem hiding this comment.
When an operator reruns ops maintenance migrate-tier at the current target with a released historical train, this field receives the new receipt, but polylogue/cli/commands/maintenance/_migrate_tier.py:123-150 neither includes it in the JSON payload nor renders it in plain output; because migration_result is None, the command instead reports only “No pending durable migration.” A repo-wide search shows no production consumer of forward_version_receipt, so the admission evidence remains inaccessible outside the storage return value despite the unit test proving only that assignment.
AGENTS.md reference: AGENTS.md:L480-L482
Useful? React with 👍 / 👎.
| with _open_existing_tier(tier_path) as live: | ||
| forward_version_receipt = _forward_version_receipt_for_current_tier( |
There was a problem hiding this comment.
When execute_durable_change_train is invoked at the current target after at least one historical train was released, the reconciliation call at line 1642 has already captured the live database evidence, run PRAGMA integrity_check, and constructed both schema inventories. This helper call then invokes _verify_released_train_live_tier again without any of those cached values, repeating the content dump, full integrity scan, and schema work before returning the no-op receipt. Although the startup loop now caches evidence across manifests, this subsequent execution step discards that cache, so a large source.db is still scanned twice on every no-op maintenance invocation.
Useful? React with 👍 / 👎.
Problem: no-op durable maintenance produced a forward-version receipt that was invisible on the CLI, and the same live evidence was rescanned after startup reconciliation.\n\nWhat changed: expose the typed receipt in JSON and plain maintenance output, and pass startup-captured evidence into the no-op admission path. Add CLI and scan-reuse regression coverage.\n\nVerification: devtools test tests/unit/storage/test_durable_change_train.py tests/unit/daemon/test_raw_failure_sample.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_executes_and_persists_a_future_change_train tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt (81 passed).\n\nResidual: two existing rebuild-index CLI tests remain red independently of this change: test_rebuild_index_byte_budget_defers_then_reaches_terminal_ready_candidate and test_rebuild_index_deadline_defers_postflight_until_resume.
Keep the durable train cache regression test on migration_runner's declared public owner so strict mypy can resolve the symbol.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86213e4d6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if train.state is DurableChangeTrainState.RELEASED and train.target_version < current_version: | ||
| historical.append(train) |
There was a problem hiding this comment.
When the live tier is at vN but its vN or an intermediate released manifest is missing, this accepts the newest available older train solely because its target is below current_version; the subsequent checks prove only current canonical schema, SQLite integrity, and inode identity, not that each intervening migration ran under its train and backup authority. Deleting a later manifest after an unreceipted migration—or manually recreating the canonical objects and bumping user_version—therefore lets daemon startup admit unauthenticated durable contents. Fresh evidence in this revision is that _forward_version_receipt_for_current_tier explicitly selects any older released manifest without checking for a complete pre/post manifest chain; require released evidence for every intervening slot before issuing the receipt.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
| ROW_NUMBER() OVER ( | ||
| PARTITION BY raw_id, origin, source_path, source_index | ||
| ORDER BY last_observed_at_ms DESC, artifact_id DESC | ||
| ) AS artifact_rank | ||
| FROM raw_artifacts |
There was a problem hiding this comment.
On a large source.db, even a tiny sample_limit materializes and window-sorts the entire raw_artifacts table here, and the identical CTE is executed again for the summary and sample statements. The current DDL already has the unique source-identity index at archive_tiers/source.py:578-582, so the sample query can first select its bounded failed raws and then perform indexed artifact lookups instead. Fresh evidence after the earlier sampling comment is that the replacement now puts a global ROW_NUMBER() scan in SQL text that is evaluated twice, so the new LIMIT still does not bound sample-probe work.
Useful? React with 👍 / 👎.
Problem: forward durable-train admission could skip an intervening unreleased schema train, and raw-failure sampling ranked the complete artifact table before applying its sample limit.\n\nWhat changed: require released evidence for every target version between the historical receipt and the live schema, and perform raw-failure sampling before correlated latest-artifact lookups. The focused tests cover both fail-closed admission and SQL shape.\n\nRef #3875\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: source continuity refresh reused a connection after its read context ended, and legacy identity compatibility lacked negative regression coverage.\n\nWhat changed: allow continuity checks to use the already-scoped archive root, pass it through source refresh validation, and pin foreign and connection-free legacy identities plus the forward target bound.\n\nRef #3875\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57790366ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| current, | ||
| retained_current, | ||
| label="source continuity retained refresh", | ||
| connection=connection, | ||
| ) |
There was a problem hiding this comment.
When refreshing a released source train whose stored evidence still uses the legacy full-archive identity digest, _assert_durable_database_continuity() takes its compatibility branch and calls _connection_main_path(connection), but this connection was closed when the with sqlite3.connect(...) block ended at line 834. SQLite therefore raises ProgrammingError: Cannot operate on a closed database instead of admitting the legacy evidence or producing the intended continuity error. Keep the connection open through these checks or pass the already-known archive root without dereferencing a closed connection.
Useful? React with 👍 / 👎.
| _verify_released_train_live_tier( | ||
| archive_root, | ||
| live, | ||
| train, | ||
| actual_evidence=actual, | ||
| integrity_check=live_integrity_by_tier.get(train.tier), | ||
| live_inventory=live_inventory_by_tier.get(train.tier), | ||
| canonical_inventory=canonical_inventory_by_tier.get(train.tier), | ||
| ) |
There was a problem hiding this comment.
When the live tier is at vN and an intervening or current released manifest is missing, daemon startup still verifies each manifest that happens to exist independently and admits an older train against the canonical live schema. Fresh evidence in this revision is that the contiguous missing_targets check was added only to _forward_version_receipt_for_current_tier(), which is called by the maintenance no-op path, while this startup path continues to call _verify_released_train_live_tier() directly; deleting the vN manifest therefore blocks maintenance but not startup. Apply the same complete-chain check before startup admits forward-version evidence.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
What changed: retain the historical manifest path needed by the later receipt assertions and pin startup rejection to the missing current target version.\n\nVerification: tests/unit/storage/test_durable_change_train.py passed 44 tests.\n\nRef polylogue-dcrmm\n\nCo-Authored-By: Claude <noreply@anthropic.com>
What changed: assert that startup refusal of an incomplete released-train chain does not release or otherwise mutate the durable-train ownership result. Verification: tests/unit/storage/test_durable_change_train.py passed 44 tests. Ref polylogue-dcrmm Co-Authored-By: Claude <noreply@anthropic.com>
Problem: forward admission accepted a chain that skipped the adoption floor, startup validated released trains before recovering later committed trains, and legacy source continuity refreshes could fail manifest validation after the durable identity split. What changed: normalize retained legacy post-apply identity evidence during an authenticated continuity refresh, require every released train after the adoption floor, and recover all non-released lifecycle states before checking released manifests. Focused tests cover each ordering and compatibility boundary. Verification: tests/unit/storage/test_durable_change_train.py passed 46 tests; devtools verify --quick passed all 24 steps. Ref polylogue-dcrmm Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the adoption-floor chain validator retained a historical target parameter that no longer influenced validation after the chain rule was corrected.\n\nWhat changed: remove the unused parameter and its now-unneeded local selection, then update the focused regression call site.\n\nVerification: devtools test tests/unit/storage/test_durable_change_train.py (46 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/unit/daemon/test_raw_failure_sample.py (1)
573-613: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the no-artifacts sample query.
initialize_archive_databasecreatesraw_artifacts, so this test exercises only thehas_artifactsbranch. The other branch inread_raw_failure_lifecyclebuilds a differentsample_sqlwithout thesampledCTE and a differentsummary_sqlwith literalNULLprojections. That branch has no regression coverage for its column ordering or itsLIMIT ?parameter binding. A column-order mistake there would surface as wrong sample fields at runtime.Add a case that drops
raw_artifactsbefore the read and asserts the sample fields and counts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/daemon/test_raw_failure_sample.py` around lines 573 - 613, Extend test_lifecycle_samples_are_sql_bounded_for_large_file_backed_failure_sets, or add a focused companion case, that removes raw_artifacts after initialize_archive_database and before read_raw_failure_lifecycle. Assert the no-artifacts branch returns the expected parse-failure/unexplained counts and sample_limit, and validate each sample’s fields to catch column-order errors; retain SQL tracing assertions sufficient to confirm the sample query binds LIMIT ? correctly.polylogue/storage/sqlite/durable_change_train.py (1)
1957-1977: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA recovered train that reaches a non-applied state is omitted from the returned tuple.
The
BACKUP_AUTHORIZEDbranch persists the recovered train at line 1970 but does not callrecord_reconciled. Recording depends on theAPPLIED/PROVENblock at lines 1972-1977 or on the released pass at line 2020.If
reconcile_interrupted_durable_change_trainreturns a state that is none ofAPPLIED,PROVEN, orRELEASED, the manifest is transitioned on disk but never appears in the returned tuple. Callers use that tuple to report which manifests were reconciled, so the report understates the work performed.Record the manifest as soon as the transition is persisted.
🐛 Proposed fix
train = _persist_train_transition(manifest_path, recovered, expected_revision=train.revision) + record_reconciled(manifest_path) if train.state in {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@polylogue/storage/sqlite/durable_change_train.py` around lines 1957 - 1977, Update the BACKUP_AUTHORIZED recovery flow after persisting the recovered train with _persist_train_transition to immediately call record_reconciled(manifest_path). Keep the existing APPLIED/PROVEN handling unchanged, while ensuring recovered trains in other non-released states are included in the returned reconciliation tuple.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/cli/commands/maintenance/_migrate_tier.py`:
- Around line 141-153: In the payload-building logic around the
forward_version_receipt field, bind execution.forward_version_receipt to a local
receipt once, then use receipt for the null check and all serialized fields.
Match the existing receipt-local pattern used by the plain-output branch while
preserving the current None behavior.
In `@polylogue/storage/raw_failure_lifecycle.py`:
- Around line 178-188: Update the sampled CTE ordering in the failure sampling
query to prioritize unexplained failures before recency, ensuring a bounded
sample includes evidence whenever blocking is true. Preserve the existing
acquired_at_ms and raw_id descending ordering as tie-breakers for rows with the
same explanation status.
- Around line 143-160: Add a composite index covering raw_id, origin,
source_path, source_index, and last_observed_at_ms for the latest-artifact
lookup used by latest_artifact_join, and ensure it is created through the
existing schema/index initialization path.
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 1730-1731: The startup reconciliation and reused forward-version
evidence path must require a caller-held archive lease. Document this
precondition in the docstring for execute_durable_change_train and
reconcile_durable_change_train_startup, or ensure ownership is reacquired around
the receipt construction at the forward-version evidence path; preserve the
existing reentrant production flow.
- Around line 999-1011: Update the apply_evidence rewrite in the train update
flow to require the explicit legacy-to-tier identity-migration condition
established by the earlier guard, rather than checking only for differing
archive_identity_digest values. Also correct the apply_evidence docstring near
its definition to state that this field may be rewritten for that legacy upgrade
case.
- Around line 1586-1597: The _forward_version_receipt_for_current_tier flow must
validate released-train continuity before returning None when forward admission
begins above the adoption floor, even if historical is empty. Move or add
_require_released_train_chain with current_version=current_version before the
historical early return, preserving the existing historical receipt logic
afterward.
- Around line 1988-1993: Hoist the _released_train_manifests_by_target call out
of the per-train validation loop and cache its result per tier alongside the
existing live_*_by_tier caches. Reuse the cached tier map when calling
_require_released_train_chain, while preserving the current validation behavior
for each train.
In `@tests/unit/cli/test_archive_maintenance_cli.py`:
- Around line 1592-1598: Update the plain-output assertions for the migrate-tier
invocation to also require the rendered target-version fragment from the
command’s migration message, including the expected target version. Keep the
existing admission-message assertion unchanged and anchor the new check to the
output from cli_runner.invoke.
In `@tests/unit/daemon/test_raw_failure_sample.py`:
- Around line 606-612: Update the assertions around the sample query in the raw
failure snapshot test to verify that the SQL statement includes a LIMIT clause,
preserving the existing query-shape checks and ensuring the sample bound is
enforced by SQL rather than Python truncation.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1192-1196: Update the comment above the schema_inventories
assertion to explicitly mention the nested inventory capture performed by
capture_durable_database_evidence via capture_durable_schema_inventory, so it
accounts for all three inventories while preserving the existing explanation of
the reused inventories.
- Around line 474-477: Update the test assertions around refreshed and released
apply evidence to compare refreshed.apply_evidence.post.archive_identity_digest
against the expected current tier identity digest, rather than only asserting it
differs from released.apply_evidence.post.archive_identity_digest; use the
existing symbol or fixture that represents that digest.
---
Outside diff comments:
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 1957-1977: Update the BACKUP_AUTHORIZED recovery flow after
persisting the recovered train with _persist_train_transition to immediately
call record_reconciled(manifest_path). Keep the existing APPLIED/PROVEN handling
unchanged, while ensuring recovered trains in other non-released states are
included in the returned reconciliation tuple.
In `@tests/unit/daemon/test_raw_failure_sample.py`:
- Around line 573-613: Extend
test_lifecycle_samples_are_sql_bounded_for_large_file_backed_failure_sets, or
add a focused companion case, that removes raw_artifacts after
initialize_archive_database and before read_raw_failure_lifecycle. Assert the
no-artifacts branch returns the expected parse-failure/unexplained counts and
sample_limit, and validate each sample’s fields to catch column-order errors;
retain SQL tracing assertions sufficient to confirm the sample query binds LIMIT
? correctly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1ed15a76-eec5-4a23-a30f-6602c63e6001
📒 Files selected for processing (7)
polylogue/cli/commands/maintenance/_migrate_tier.pypolylogue/storage/raw_failure_lifecycle.pypolylogue/storage/sqlite/durable_change_train.pypolylogue/storage/sqlite/migration_runner.pytests/unit/cli/test_archive_maintenance_cli.pytests/unit/daemon/test_raw_failure_sample.pytests/unit/storage/test_durable_change_train.py
| latest_artifact_join = """ | ||
| LEFT JOIN raw_artifacts AS a | ||
| ON a.raw_id = f.raw_id | ||
| AND a.origin = f.origin | ||
| AND a.source_path = f.source_path | ||
| AND a.source_index = f.source_index | ||
| AND NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM raw_artifacts AS newer | ||
| WHERE newer.raw_id = a.raw_id | ||
| AND newer.origin = a.origin | ||
| AND newer.source_path = a.source_path | ||
| AND newer.source_index = a.source_index | ||
| AND (newer.last_observed_at_ms > a.last_observed_at_ms | ||
| OR (newer.last_observed_at_ms = a.last_observed_at_ms | ||
| AND newer.artifact_id > a.artifact_id)) | ||
| ) | ||
| """ |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect raw_artifacts DDL and indexes in the archive schema.
set -euo pipefail
fd -e sql -e py . polylogue/storage --exec rg -n -C4 'raw_artifacts' {} \
| rg -n -C4 'CREATE (TABLE|UNIQUE )?INDEX|CREATE TABLE|PRIMARY KEY|UNIQUE'Repository: Sinity/polylogue
Length of output: 8626
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- target file outline/section ---\n'
wc -l polylogue/storage/raw_failure_lifecycle.py
sed -n '1,220p' polylogue/storage/raw_failure_lifecycle.py
printf '\n--- raw_artifacts DDL/index occurrences ---\n'
rg -n -C6 'CREATE TABLE raw_artifacts|CREATE INDEX idx_raw_artifacts_raw_id|CREATE UNIQUE INDEX idx_raw_artifacts_source_identity|idx_raw_artifacts' polylogue/storageRepository: Sinity/polylogue
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target file section ---'
wc -l polylogue/storage/raw_failure_lifecycle.py
sed -n '1,220p' polylogue/storage/raw_failure_lifecycle.py
printf '%s\n' ''
printf '%s\n' '--- raw_artifacts DDL/index occurrences ---'
rg -n -C6 'CREATE TABLE raw_artifacts|CREATE INDEX idx_raw_artifacts_raw_id|CREATE UNIQUE INDEX idx_raw_artifacts_source_identity|idx_raw_artifacts' polylogue/storageRepository: Sinity/polylogue
Length of output: 18383
🌐 Web query:
SQLite left join ON column NOT EXISTS correlated subquery index raw_id origin last_observed_at_ms artifact_id performance
💡 Result:
In SQLite, optimizing a query involving a LEFT JOIN or a NOT EXISTS correlated subquery relies heavily on indexing the columns used in the join or correlation condition [1][2][3][4]. Performance considerations: 1. Indexing: For a LEFT JOIN, the column on the right-hand table must be indexed to allow the query planner to efficiently search for matches [3]. Similarly, for a correlated NOT EXISTS subquery, the columns in the subquery's WHERE clause that link to the outer query must be indexed to allow the subquery to perform a search rather than a full table scan [2][4]. 2. LEFT JOIN vs. NOT EXISTS: While often functionally equivalent (especially when checking for non-existence via IS NULL), they are handled differently by the query planner [1][2]. A LEFT JOIN forces the database to construct a join result, which may be less efficient than a NOT EXISTS clause if the query can be short-circuited [1]. A NOT EXISTS clause, when properly indexed, allows the database to stop searching the subquery as soon as a single match is found [1]. 3. Correlated Subquery Overhead: In older or specific configurations, correlated subqueries were sometimes routed to a slower in-memory interpreter, leading to significant performance degradation [5]. However, in modern, standard SQLite, a correlated subquery that references an indexed column in the inner table should be executed efficiently using SEARCH operations [2][4]. Ensure your column affinities match, as mismatched types can sometimes prevent the optimizer from using an index [4]. 4. Optimizer Improvements: Recent versions of SQLite (as of 2026) have introduced enhanced optimizations, including EXISTS-to-JOIN transformations, which attempt to automatically convert EXISTS clauses into JOINs when the query planner determines it is more efficient [6]. Recommendations for your schema: - Ensure that the foreign key or join columns (such as artifact_id or raw_id) are explicitly indexed in the target tables [4]. - If you are frequently filtering by last_observed_at_ms, consider a composite index that includes the join/correlation column and the observation timestamp to further speed up the lookups [7]. - Use EXPLAIN QUERY PLAN to verify that your query is using SEARCH TABLE rather than SCAN TABLE for your subqueries or join operations [2][3][4]. If you see SCAN TABLE, the database is likely performing a full table scan, which indicates a missing or ineffective index [2][3].
Citations:
- 1: https://emschwartz.me/short-circuiting-correlated-subqueries-in-sqlite/
- 2: https://dba.stackexchange.com/questions/225298/sqlite-is-it-faster-to-use-select-not-exists-exists-or-in-not-in-in-a-wher
- 3: https://stackoverflow.com/questions/19671738/why-is-sqlite-refusing-to-use-available-indexes-when-adding-a-join
- 4: https://stackoverflow.com/questions/51906826/why-does-this-sqlite-query-not-use-an-index-for-the-correlated-subquery
- 5: Correlated
NOT EXISTS(anti-join idiom) routes to the in-memory interpreter — 0.01s on canonical SQLite becomes ~5s and climbs (verified atmain@ d1caefb5) Dicklesworthstone/frankensqlite#117 - 6: https://sqlite.org/src/info/e33da6d5dc964db8
- 7: https://sqlite.org/expridx.html
Add a composite index for the latest-artifact probe.
idx_raw_artifacts_raw_id supports the LEFT JOIN, but the NOT EXISTS probe also filters by origin, source_path, and source_index before using last_observed_at_ms/artifact_id. Add an index covering (raw_id, origin, source_path, source_index, last_observed_at_ms) for the latest-artifact probe, or use an indexed windowed/lateral query with the same index coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@polylogue/storage/raw_failure_lifecycle.py` around lines 143 - 160, Add a
composite index covering raw_id, origin, source_path, source_index, and
last_observed_at_ms for the latest-artifact lookup used by latest_artifact_join,
and ensure it is created through the existing schema/index initialization path.
Problem: automated review identified stale evidence boundaries, an incomplete historical-chain check, recovery reporting loss, and raw-failure samples that did not prioritize unexplained rows or cover the no-artifact route.\n\nWhat changed: bind receipt serialization through one local, constrain legacy identity rewrites, validate the adoption-floor chain before empty-history returns, cache manifest maps, retain recovery accounting, document lease ownership, prioritize unexplained samples in SQL, and add focused regression coverage.\n\nAlternatives rejected: a new raw_artifacts source index would require a numbered durable migration and target-version change, so it remains outside this admission hardening change.\n\nVerification: devtools test tests/unit/daemon/test_raw_failure_sample.py tests/unit/storage/test_durable_change_train.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt (84 passed). The two rebuild-index tests remain pre-existing failures in the mmap-budget fixture path.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba754f5b3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if actual.user_version > train.target_version: | ||
| if train.tier not in manifests_by_tier: | ||
| manifests_by_tier[train.tier] = _released_train_manifests_by_target(manifest_root, train.tier) | ||
| _require_released_train_chain( |
There was a problem hiding this comment.
When a source tier is live at v29 but only its released v29 manifest remains, actual.user_version == train.target_version, so this condition skips _require_released_train_chain() and the equal-version continuity check admits startup despite the missing v27/v28 backup/train authority. Fresh evidence after the earlier chain findings is that the final code still invokes the startup chain validator only for a historical manifest, whereas the maintenance no-op path now checks the chain even when its historical list is empty; apply that same adoption-floor check before verifying the current released train.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Problem: startup continuity validation skipped the adoption-floor chain when the only remaining released manifest matched the live version.\n\nWhat changed: run the cached released-train chain check whenever the live version exceeds the adoption floor, including equal-version current-train validation, and add a regression for a missing earlier train.\n\nVerification: devtools test tests/unit/storage/test_durable_change_train.py (48 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ff240617
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _prove_and_release_persisted_train(archive_root, manifest_path, train) | ||
| record_reconciled(manifest_path) | ||
|
|
||
| for manifest_path in manifest_paths: |
There was a problem hiding this comment.
When an existing source tier is already above the adoption floor but .maintenance-state/durable-change-trains is absent or empty, this added loop executes zero times (and the earlier missing-directory branch returns immediately), so daemon bootstrap admits the archive with none of the required released backup/train evidence. Fresh evidence beyond the existing partial-chain comments is this zero-manifest case, which never calls _require_released_train_chain; distinguish fresh initialization from an existing durable tier and enforce the adoption-floor chain for the latter even when no manifests are discovered.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Problem: startup admitted an existing durable tier above the adoption floor when the durable change-train manifest directory was absent or empty.\n\nWhat changed: enforce the released-train chain after lifecycle recovery so manifestless tiers fail closed without masking persisted indeterminate failures, and add a regression test.\n\nVerification: direnv exec . devtools test tests/unit/storage/test_durable_change_train.py (49 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: enforcing released train chains on every existing durable tier also rejected freshly bootstrapped archives that start at the current schema without migration history.\n\nWhat changed: record a version- and archive-identity-bound bootstrap receipt, accept that receipt only while the durable tier remains at its direct-bootstrap version with no train manifests, and keep manifestless legacy tiers fail-closed.\n\nVerification: direnv exec . devtools test tests/unit/storage/test_durable_change_train.py (50 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: the fresh-bootstrap exception must remain bound to the archive it authenticated.\n\nWhat changed: add regression coverage proving a tampered bootstrap receipt is rejected during startup reconciliation.\n\nVerification: direnv exec . devtools test tests/unit/storage/test_durable_change_train.py (51 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc8fb78929
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| manifests_by_tier[tier] = _released_train_manifests_by_target(manifest_root, tier) | ||
| tier_manifest_paths = tuple(manifest_root.glob(f"{tier.value}-*.json")) | ||
| if fresh_bootstrap_versions.get(tier) == current_version and not tier_manifest_paths: | ||
| continue |
There was a problem hiding this comment.
When an archive was initialized by the previous release at source schema v29, it has neither the newly introduced .bootstrap marker nor released v27–v29 manifests. On its first startup after this upgrade, fresh_bootstrap_versions is empty, this exemption cannot fire, and _require_released_train_chain() rejects the otherwise valid durable archive before initialization can record a marker. Add an authenticated transition for archives that predate the marker rather than requiring evidence that older builds could not create.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
| if payload.get("archive_identity_digest") != ArchiveIdentity.resolve(archive_root).authority_identity_digest: | ||
| raise DurableChangeTrainError("fresh durable bootstrap marker archive identity mismatch") |
There was a problem hiding this comment.
When a freshly bootstrapped archive later follows the supported index reset/rebuild flow, replacing or deleting index.db changes authority_identity_digest because it includes active_generation and every non-audit tier. This newly introduced .bootstrap comparison then raises before bootstrap can recreate the rebuildable index, even though source.db and user.db are unchanged. Persist and compare a durable-only identity for this marker.
AGENTS.md reference: AGENTS.md:L193-L201
Useful? React with 👍 / 👎.
| _require_released_train_chain( | ||
| tier, | ||
| manifests_by_tier[tier], | ||
| current_version=current_version, |
There was a problem hiding this comment.
When a marker-backed fresh source archive at v29 later applies its first numbered migration to v30, it legitimately has only the v30 released manifest because v27–v29 were supplied directly by canonical bootstrap. On the next startup the exact-version/no-manifest exemption no longer applies, but this call still requires every manifest from the adoption floor, so the successfully migrated archive is rejected for missing v27–v29. Treat the authenticated marker version as the chain's baseline and require manifests only above it.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
Problem: the bootstrap receipt exception rejected pre-marker current-schema archives, depended on rebuildable index identity, and restarted train chains at the global adoption floor after the first migration.\n\nWhat changed: authenticate a one-time pre-marker adoption against current canonical durable schemas, bind receipts to source/user identity, and use the recorded bootstrap version as the chain floor for later migrations. Missing or replaced durable tiers remain fail-closed.\n\nVerification: direnv exec . devtools test tests/unit/storage/test_durable_change_train.py (54 passed).\n\nCo-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
polylogue/storage/sqlite/durable_change_train.py (1)
1921-1926: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the cached live evidence on the released-train path.
Line 1846 populates
forward_version_evidenceduring startup reconciliation. Line 1896 reuses it. This call does not, so_verify_released_train_live_tierre-captures the database evidence, re-runsPRAGMA integrity_check, and rebuilds both schema inventories against the same unchanged file. The docstring at lines 1842-1843 already states that the caller-held lease covers both operations, so the cached evidence is valid here.♻️ Proposed change
+ cached = forward_version_evidence.get(tier) forward_version_receipt = _verify_released_train_live_tier( archive_root, live, train, current_target_version=runtime_target_version, + actual_evidence=cached.actual if cached is not None else None, + integrity_check=cached.integrity_check if cached is not None else None, + live_inventory=cached.live_inventory if cached is not None else None, + canonical_inventory=cached.canonical_inventory if cached is not None else None, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@polylogue/storage/sqlite/durable_change_train.py` around lines 1921 - 1926, Update the released-train path around _verify_released_train_live_tier to pass the startup-reconciled forward_version_evidence instead of recapturing live database evidence. Reuse the cached evidence populated during startup reconciliation, while preserving the existing lease coverage and returned receipt behavior.
♻️ Duplicate comments (1)
polylogue/storage/sqlite/durable_change_train.py (1)
1093-1105: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low valueThe docstring still claims
apply_evidenceis immutable.The gate at lines 1095-1098 now restricts the rewrite to the legacy-authority-digest case. The docstring at line 889 still states that the original migration evidence remains immutable in
apply_evidence. Lines 1099-1105 rewriteapply_evidence.post.archive_identity_digest.Correct that sentence so the documented contract matches the code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@polylogue/storage/sqlite/durable_change_train.py` around lines 1093 - 1105, Update the docstring for the migration flow containing retained_apply_evidence to remove the claim that apply_evidence remains immutable. Document that its post.archive_identity_digest may be rewritten only when the legacy-authority-digest gate matches, while preserving the rest of the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/storage/raw_failure_lifecycle.py`:
- Around line 185-195: The lifecycle query in raw_failure_lifecycle.py lines
185-195 must use the same row-selection contract as daemon status, so classified
failures cannot be excluded by unexplained failures filling the cap; align its
ordering or return classifications for exactly the rows status displays. Add the
regression scenario in tests/unit/daemon/test_raw_failure_sample.py lines
573-614 with more rows than the cap, older unexplained failures, and a newer
classified failure, then assert _raw_failure_info() preserves the classified
lifecycle.
In `@polylogue/storage/sqlite/archive_tiers/bootstrap.py`:
- Around line 316-339: Move the durable-state probes and computation of
fresh_durable_bootstrap/pre_marker_adoption in initialize_active_archive_root to
after OwnedArchiveLocation.acquire is obtained, then use those locked values for
reconciliation and bootstrap recording. Update _record_fresh_durable_bootstrap
to refuse writing when .bootstrap or any durable-train manifest already exists,
preserving required released-train validation for state published concurrently.
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 2136-2147: In
polylogue/storage/sqlite/durable_change_train.py:2103-2125, record each tier
validated by the per-tier loop in a validated_tiers set; in the per-manifest
block at 2136-2147, retain the chain check only when train.tier is not in that
set, preserving validation for missing tier files and avoiding duplicate checks.
In tests/unit/storage/test_durable_change_train.py:1445-1478, add a comment
explaining that the absent source.db selects the per-manifest validation path.
- Around line 1689-1700: Extract the duplicated chain-floor calculation into a
shared helper such as _chain_floor(tier, bootstrap_versions), consistently
applying max(DURABLE_MIGRATION_ADOPTION_FLOORS[tier], recorded bootstrap version
with the adoption floor as fallback). Replace the inline expressions at the
current admission site and the corresponding sites near lines 2124 and 2143,
preserving identical behavior across all three paths.
- Around line 334-342: The bootstrap marker must authenticate its recorded
versions as well as the archive identity. In
polylogue/storage/sqlite/durable_change_train.py:334-342, compute the marker
digest from the format, durable identity, and versions together; in
polylogue/storage/sqlite/durable_change_train.py:378-382, update
_durable_identity_digest or add a dedicated helper, and use it in
_fresh_durable_bootstrap_versions verification. Add a regression test extending
test_fresh_bootstrap_receipt_rejects_archive_identity_mismatch that alters the
recorded version and asserts startup rejects the marker.
- Around line 345-359: Update _fresh_durable_bootstrap_versions to resolve
archive_root immediately at function entry, before loading or validating the
marker and before calling ArchiveIdentity.resolve for the durable identity
digest. Preserve the existing marker validation behavior while ensuring the
digest uses the normalized root.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1319-1328: Update
test_released_train_chain_can_start_at_bootstrap_floor to derive floor from
DURABLE_MIGRATION_ADOPTION_FLOORS[ArchiveTier.SOURCE] and set current_version to
floor + 1, matching the pattern in
test_forward_receipt_checks_missing_chain_before_empty_history. Remove the
hardcoded 29 and 30 values while preserving the existing released-train setup
and assertion path.
---
Outside diff comments:
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 1921-1926: Update the released-train path around
_verify_released_train_live_tier to pass the startup-reconciled
forward_version_evidence instead of recapturing live database evidence. Reuse
the cached evidence populated during startup reconciliation, while preserving
the existing lease coverage and returned receipt behavior.
---
Duplicate comments:
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 1093-1105: Update the docstring for the migration flow containing
retained_apply_evidence to remove the claim that apply_evidence remains
immutable. Document that its post.archive_identity_digest may be rewritten only
when the legacy-authority-digest gate matches, while preserving the rest of the
contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f962e0bf-43cb-4af0-8a47-8e8843a53015
📒 Files selected for processing (7)
polylogue/cli/commands/maintenance/_migrate_tier.pypolylogue/storage/raw_failure_lifecycle.pypolylogue/storage/sqlite/archive_tiers/bootstrap.pypolylogue/storage/sqlite/durable_change_train.pytests/unit/cli/test_archive_maintenance_cli.pytests/unit/daemon/test_raw_failure_sample.pytests/unit/storage/test_durable_change_train.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec37aecfe9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise DurableChangeTrainError(f"invalid fresh durable bootstrap marker: {marker_path}") from exc | ||
| if not isinstance(payload, dict) or payload.get("format") != _FRESH_DURABLE_BOOTSTRAP_FORMAT: | ||
| raise DurableChangeTrainError(f"fresh durable bootstrap marker format mismatch: {marker_path}") | ||
| if payload.get("durable_identity_digest") != _durable_identity_digest(ArchiveIdentity.resolve(archive_root)): |
There was a problem hiding this comment.
When the configured archive root is a symlink, the initial bootstrap records the digest after archive_root.resolve(), but the next startup validates it using the unresolved configured path. Because _durable_identity_digest() includes configured_root, the same source/user files produce a different digest and every subsequent startup fails with a durable identity mismatch. Resolve the root consistently before recomputing the marker digest.
Useful? React with 👍 / 👎.
| marker_root.joinpath(_FRESH_DURABLE_BOOTSTRAP_MARKER).write_text( | ||
| json.dumps(payload, sort_keys=True, separators=(",", ":")), | ||
| encoding="utf-8", | ||
| ) |
There was a problem hiding this comment.
If the process or host stops during this write_text, the marker can remain empty or partially written. The next startup treats its presence as authoritative, skips fresh/pre-marker classification, and then fails permanently while parsing it as JSON; the archive cannot self-recover even though its durable databases were initialized successfully. Write the marker through a flushed temporary file followed by os.replace and directory fsync, as is done for the train manifests.
Useful? React with 👍 / 👎.
| if current_version > DURABLE_MIGRATION_ADOPTION_FLOORS[tier]: | ||
| _require_released_train_chain( |
There was a problem hiding this comment.
When an operator runs ops maintenance migrate-tier audit against the existing current v1 audit database without --initialize-missing, there is no sidecar, so execution now enters _forward_version_receipt_for_current_tier(). That helper indexes DURABLE_MIGRATION_ADOPTION_FLOORS[tier], but the mapping contains only source and user, causing an uncaught KeyError instead of the normal no-pending-migration result. Avoid requesting a forward receipt for tiers without change-train adoption floors.
Useful? React with 👍 / 👎.
| payload = { | ||
| "format": _FRESH_DURABLE_BOOTSTRAP_FORMAT, | ||
| "durable_identity_digest": _durable_identity_digest(ArchiveIdentity.resolve(archive_root)), | ||
| "versions": versions, |
There was a problem hiding this comment.
When a marker-backed archive loses source.db or user.db and the operator uses the supported migrate-tier <tier> --initialize-missing recovery route, the new database has a different inode but this marker continues to attest the old source/user durable_id. The command reports successful initialization, yet every subsequent startup fails the marker identity check, and no production path updates or replaces the marker. Refresh the authenticated marker as part of explicit missing-tier initialization or provide a verified identity-transition receipt.
Useful? React with 👍 / 👎.
Problem The durable train review found races and unauthenticated bootstrap versions. Raw failure status also selected a different sample set from the lifecycle projection. What changed Move bootstrap classification under the archive ownership lock, authenticate marker versions, centralize chain floors, reuse cached forward evidence, and preserve per-manifest validation. Make status consume the lifecycle-selected raw IDs and order classified failures before unexplained rows. Verification Ref #3875 Passed: direnv exec . devtools test tests/unit/daemon/test_raw_failure_sample.py tests/unit/storage/test_durable_change_train.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt Result: 94 passed. Co-Authored-By: Claude <noreply@anthropic.com>
Problem Fresh durable bootstrap markers were written directly to their final path, and audit-tier no-op maintenance could enter a source/user-only train-floor lookup. What changed Publish the marker through a flushed temporary file and directory fsync. Skip forward-version train-floor validation for the audit tier, which has no durable change-train adoption floor, and cover that route with a focused test. Verification Ref #3875 Passed: direnv exec . devtools test tests/unit/daemon/test_raw_failure_sample.py tests/unit/storage/test_durable_change_train.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt Result: 95 passed. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06788ed31e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (root / archive_tier_spec(ArchiveTier.SOURCE).filename).is_file() | ||
| and durable_tier_exists | ||
| and not has_durable_train_state | ||
| and not has_bootstrap_marker | ||
| ) |
There was a problem hiding this comment.
When a post-adoption archive loses its durable-change-train directory, these absence checks classify it as a legacy pre-marker archive and skip reconciliation; _adopt_pre_marker_durable_bootstrap() then checks only the current schema and writes a new marker whose floor is the live version. Deleting the manifests therefore erases the required released-train chain and admits unreceipted durable contents without the verified migration/backup evidence this change otherwise enforces. Fresh evidence in this revision is that legacy status is inferred solely from missing state; require independently authenticated legacy provenance rather than absence alone.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
| pre_marker_adoption = ( | ||
| (root / archive_tier_spec(ArchiveTier.SOURCE).filename).is_file() | ||
| and durable_tier_exists | ||
| and not has_durable_train_state | ||
| and not has_bootstrap_marker |
There was a problem hiding this comment.
When a pre-marker archive still has source.db but user.db is missing, this source-only condition selects adoption, and the subsequent initialization loop silently creates an empty user.db before the marker is recorded. Startup then treats that replacement as the authenticated baseline instead of reporting loss of the irreplaceable assertions tier; require all durable tiers to exist and be safe before adoption, leaving missing-tier recovery to the explicit maintenance path.
AGENTS.md reference: AGENTS.md:L118-L121
Useful? React with 👍 / 👎.
Problem Pre-marker adoption inferred legacy provenance from missing train state and could recreate a missing user tier before recording a new baseline. What changed Require the pre-existing train directory and every durable tier before adoption. Missing train state or a missing user tier now follows the fail-closed chain path and leaves the archive untouched. Add regressions for both loss cases. Verification Ref #3875 Passed: direnv exec . devtools test tests/unit/storage/test_durable_change_train.py tests/unit/daemon/test_raw_failure_sample.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt Result: 97 passed. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 424c773f04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| initialize_archive_database(root / spec.filename, spec.tier) | ||
| if fresh_durable_bootstrap: | ||
| from polylogue.storage.sqlite.durable_change_train import _record_fresh_durable_bootstrap | ||
|
|
||
| _record_fresh_durable_bootstrap(root) |
There was a problem hiding this comment.
On a brand-new archive, if a later initialize_archive_database call raises after source.db has been created—for example, embeddings initialization cannot load sqlite-vec—or the process exits during this loop, _record_fresh_durable_bootstrap is never reached. The next invocation sees an existing durable tier but no marker or train state, enters startup reconciliation, and rejects the current source schema for lacking its released-train chain, so even fixing the original initialization failure cannot resume bootstrap without manually deleting files. Persist an in-progress bootstrap receipt before creating the first durable file, or otherwise recognize and safely recover this partial-fresh-bootstrap state.
Useful? React with 👍 / 👎.
Problem: A failure while creating a later archive tier could leave source.db and other fresh files without durable bootstrap evidence. The next startup treated that partial fresh archive as an established archive with missing train authority.\n\nWhat changed: Persist an authenticated root-bound bootstrap intent before creating tier files. Startup validates and resumes an incomplete fresh bootstrap, publishes the completed marker atomically, and fails closed on tampered intents or conflicting train state. Add recovery and tamper regressions.\n\nCompatibility/migration: Existing completed bootstrap markers and pre-marker adoption remain unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06e9cdbe6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise DurableChangeTrainError( | ||
| f"{train.tier.value} durable tier schema inventory changed during forward admission" | ||
| ) | ||
| expected_inventory = canonical_inventory or _canonical_schema_inventory(train.tier, actual.user_version) |
There was a problem hiding this comment.
When upgrading a source archive through multiple train slots (for example, live v28 with released v27/v28 manifests under a v29 runtime), reconciliation verifies the historical v27 train here before applying v29. _canonical_schema_inventory(..., actual.user_version) sets user_version to 28 but constructs the database from the latest registered SOURCE_DDL, which already contains v29's raw_failure_disposition_receipts; the legitimate v28 inventory therefore appears to be missing canonical objects and migration is blocked before its v29 sidecar can run. Validate an intermediate live version against its matching released train/schema proof rather than relabeling the latest DDL.
AGENTS.md reference: AGENTS.md:L189-L192
Useful? React with 👍 / 👎.
| if has_pending_bootstrap: | ||
| _validate_fresh_durable_bootstrap_intent(root) |
There was a problem hiding this comment.
If the process or host stops after the completed .bootstrap marker is published but before .bootstrap.pending is durably removed, both files remain. After upgrading to a runtime with newer durable schema versions, this unconditional intent validation compares the pending receipt's old target versions with the new ARCHIVE_VERSION_BY_TIER values and raises before the completed marker can establish the legitimate migration baseline, blocking both startup and the numbered upgrade. When a completed marker exists, validate it first and treat a matching pending file as crash residue rather than requiring the old intent to target the current runtime.
Useful? React with 👍 / 👎.
1 participant
Summary
Harden durable change-train admission for later-version databases, bound failure sampling at SQL selection, and reuse durable evidence across historical manifests.
Problem
The admission path accepted a structurally valid later-version database without proving archive identity or historical schema continuity. Lifecycle sampling also fetched the full failure population, and startup reconciliation recaptured durable evidence for each historical manifest.
Solution
Forward-version admission now requires immutable archive identity and frozen historical DDL evidence, with explicit historical/current target fields in the receipt. Failure lifecycle summaries use complete aggregate queries while bounded samples use
LIMIT. Startup reconciliation captures evidence once per durable tier and reuses it across the historical train checks. File-backed regressions cover unrelated archive identity, sample bounds, and evidence reuse.Verification
direnv exec . devtools test tests/unit/storage/test_durable_change_train.py tests/unit/daemon/test_raw_failure_sample.py tests/unit/cli/test_archive_maintenance_cli.py::test_migrate_tier_cli_exposes_forward_version_receipt-> 99 passed.direnv exec . devtools verify --quick-> all 24 steps passed on06e9cdbe.quick-gate-> pending for06e9cdbe.Review disposition
mmap_budget_within_cgroup_limitandrebuild_index_offline_prefetch_warmfixture path; they are outside this PR's changed surface.Follow-ups
Live migration remains under
polylogue-9qnzyandpolylogue-a7gmk. Ref polylogue-dcrmm.Summary by CodeRabbit
Bug Fixes
Reliability