Docs: reconcile living v1 pages with implemented recovery - #136
Merged
Merged
Conversation
Replace stale future-work claims with current initialization, admission, publication, restart, and recovery evidence. Keep historical issue references and distinguish process death from power loss. Red/green documentation contracts pass in debug and release after reproducing stale claims. Pinned fmt, Clippy, documentation/source checks, dependency audit and policy checks passed. Refs #69, #132.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 28 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
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 |
Owner
Author
|
Code Lawyer current-head findings (c009895). Cc @codex.
These are documentation/integration defects; neither finding demonstrates a runtime recovery defect. Independent agy review is in progress before remediation. |
|
To use Codex here, create an environment for this repo. |
Owner
Author
Adversarial Code Review: PR 136 (
|
| Runtime Behavior | Production Path | Test / Repository Task Path | Rule Parity & Verification |
|---|---|---|---|
| Store Initialization | FilesystemPlatformAdmission::initialize -> FilesystemInitializationStorage::admit -> initialize_store |
FilesystemPlatformAdmission::initialize_unchecked_for_tests; RepositoryInitializationStorage::admit_unchecked |
Both production and test paths execute the identical 6-phase state machine (KEEP-RECOVERY-002) and root synchronization. Test/task path bypasses only the strict ext4 platform profile via lenient capability opening. |
| Store Reopen | FilesystemPlatformAdmission::reopen -> filesystem_platform_profile::open -> reopen_root |
FilesystemPlatformAdmission::reopen_unchecked_for_tests; FilesystemVersionTwoAdmission::reopen (v2) |
Both verify exact root directory contents (writer.lock, staging, segments, catalogs, and regular HEAD). Mutates nothing. v2 reopen additionally enforces device/inode coordinates against migration.intent (PR 137). |
| Catalog Publisher Opening | FilesystemCatalogPublisher::open consuming FilesystemPlatformAdmission |
FilesystemCatalogPublisher::open_unchecked_for_repository_tasks; open_unchecked_for_tests |
Production requires FilesystemPlatformAdmission whose private fields can only be initialized by Keep. Repository task path uses feature = "repository-tasks". Both pin root, staging, segments, and catalogs without following links. |
| Stage Residue Refusal during Publication | publish_catalog_generation -> filesystem_catalog_current::verify_current |
tests/catalog_filesystem_publication.rs |
Invariant parity verified: unowned current.seg, any head.next, or any current.cat causes immediate refusal (ErrorKind::AlreadyExists) before mutation. Empty immutable pools required when HEAD is absent. |
| Recovery Stage Fingerprinting | FilesystemRecoveryInventoryReader::fingerprint_stage -> filesystem_recovery_stage::fingerprint -> fingerprint_recovery_stage |
tests/recovery_stage_fingerprint.rs |
Streaming 8 KiB buffer under KEEP:RECOVERY:STAGE\0 BLAKE3 domain. Verified zero heap byte materialization. Verified post-read namespace and entry checks. |
| Stage Byte Admission & Assessment | admit_recovery_stage_bytes -> assess_recovery_stage |
tests/recovery_stage_assessment/ |
Materialized bytes admitted only when stage, length, and recomputed fingerprint match prior evidence. Dispatches to pure whole-byte classifiers (classify_recovery_segment_stage, classify_recovery_catalog_stage, classify_recovery_next_head_stage). |
| Segment Continuation | execute_recovery_segment_resume -> FilesystemRecoverySegmentResumer::open_reusable |
tests/recovery_segment_resume.rs |
Only path that materializes stage bytes on disk via materialize_and_position (filesystem_recovery_stage.rs:57). Re-fingerprints and positions handle at append boundary without rewriting prefix. |
| Next-Head Recovery Finalization | plan_recovery_next_head_finalization -> RecoveryNextHeadFinalizationStorage |
tests/recovery_next_head_finalization.rs |
Enforces candidate snapshot generation, catalog length, and digest agreement with candidate head (KEEP-RECOVERY-017). Synchronizes candidate before atomic rename (KEEP-RECOVERY-018). |
| Living Docs Assertion | living_v1_pages_no_longer_assign_shipped_recovery_to_a_future_issue |
N/A (xtask integration test) | Scans FORMAT_README, PUBLICATION, RECOVERY, and REQUIREMENTS for 7 specific stale planning strings. Statically verified that all 7 assertions pass on the tree. |
2. Merges Audited (SHA and Integration Invariants)
- Merge Commit:
c0098953dceb78650d50db0547e715e71d8bc978 - Parent 1:
0e3dfbe3125831ab9abe527c69024573edc1e6de(docs branch) - Parent 2:
2311d805e9bbcb1743fc5cdc46adb324641b9f14(mainintegrating PR 138) - Merge Base:
f49cff732cf7a6e1b472decba9e4c4130990559e
Invariants verified across merged commits:
- PR 140 (
eec39ba/971f03f— ambient-CPU-independent source law):- Verified that no benchmark source files or CPU model checks were touched or disrupted by PR 136.
- PR 145 (
88f35c4/05f7ef6— forbidden source filenames):- Verified against
xtask/src/source_structure/forbidden_filename.rs:7-17. No prohibited basenames (utils.rs,helpers.rs,common.rs,misc.rs,shared.rs,manager.rs,service.rs,types.rs,models.rs) were added in PR 136.
- Verified against
- PR 135 (
07bf0b8/e43ad0e,b5def4a— bounded reference-staging memory contract):- Staging memory contract tests and laws in
src/reference/remain untouched.
- Staging memory contract tests and laws in
- PR 137 (
200cfc8/b3bd395,a4ff000,b691da7— restart device/inode vs live mount identity):- Version 2 reopen invariant comparing device/inode coordinates across restart remains intact in
src/adapters/filesystem_version_two_admission.rs.
- Version 2 reopen invariant comparing device/inode coordinates across restart remains intact in
- PR 138 (
2311d80/c2414bf..bb1e6f6— partial-prefix migration recovery & 68 crash cases):- Verified that the migration crash cases and transition ledger remain completely untouched.
- Conflict Resolution & Semantic Equivalence:
git diff 2311d80 c009895 -- . ':(exclude)CHANGELOG.md' ':(exclude)docs/formats/segment-store-v1' ':(exclude)xtask/tests/segment_store_implementation_documentation.rs'is completely empty (0 diff).- In
CHANGELOG.md, the merge cleanly incorporated the PR 136 entry alongside the main entries under### Fixed.
3. Constants and Evidence Coordinates Checked
| Constant / Coordinate | Documented Location | Tree Source / Evidence | Binding Threshold & Evaluation |
|---|---|---|---|
Domain Separator KEEP:RECOVERY:STAGE\0 |
docs/formats/segment-store-v1/requirements.md:125, 129 |
src/adapters/recovery/recovery_stage_fingerprinter.rs:10 |
Exact byte sequence: b"KEEP:RECOVERY:STAGE\0" verified. |
Streaming Fingerprint Buffer 8,192 bytes (8 KiB) |
docs/formats/segment-store-v1/requirements.md:125 |
src/adapters/recovery/recovery_stage_fingerprinter.rs:11 |
Constant BUFFER_LENGTH: usize = 8_192. Zero content-sized heap allocation verified. |
Catalog Stage Interruption 176 bytes |
docs/formats/segment-store-v1/publication.md:231 |
xtask/src/durability_crash_matrix/production_protocol/publication_storage.rs:14 |
Constant CATALOG_INTERRUPTION: usize = 176 verified. |
Head Stage Interruption 64 bytes |
docs/formats/segment-store-v1/publication.md:253 |
xtask/src/durability_crash_matrix/production_protocol/publication_storage.rs:15 |
Constant HEAD_INTERRUPTION: usize = 64 verified. |
Crash Restart Byte Limit 1,048,576 bytes (1 MiB) |
KEEP-RECOVERY-021 |
xtask/src/durability_crash_matrix/production_protocol/initialization.rs:14 |
Constant RESTART_BYTE_LIMIT: u64 = 1_048_576 verified. |
| Catalog Pool Digest Fixture | Conformance vectors | tests/catalog.rs:10 |
04b82519b0399baefd0b9c0f32a871052e4c47e3a00226ab03b21661470f7320 verified across 7 separate fixture sites. |
4. Doc Figures and Counts Checked
| Count / Figure | Document Claim | Raw Tree Evidence | Verification Result |
|---|---|---|---|
| v1 Process-Death Crash Cases: 105 | docs/formats/segment-store-v1/README.md:11, requirements.md:138, 195, recovery.md:123, conformance/segment-store/v1/README.md:108 |
35 v1 points (KEEP-CRASH-001–KEEP-CRASH-035) × 3 positions (Before, During, After) = 105 cases. Tested in xtask/tests/durability_crash_documentation.rs:20. |
Verified Exact. |
| v2 Migration Crash Cases: 68 | conformance/segment-store/v2/README.md:95, docs/formats/segment-store-v2/migration-crash.md:7 |
xtask/tests/durability_crash_case_contract.rs:39: assert_eq!(migration.len(), 68) |
Verified Exact. |
| Total Repository Crash Cases: 173 | N/A (Internal matrix bound) | xtask/tests/durability_crash_case_contract.rs:36: assert_eq!(cases.len(), 173) (105 v1 + 68 v2 migration = 173). |
Verified Exact. |
| v1 Recovery Requirements: 21 | docs/formats/segment-store-v1/requirements.md:118-138 |
Table rows KEEP-RECOVERY-001 through KEEP-RECOVERY-021. |
Verified Exact (21 contiguous rows). |
| v1 Crash Points: 35 | docs/formats/segment-store-v1/requirements.md:118 |
KEEP-CRASH-001 through KEEP-CRASH-035 in xtask/src/durability_crash_point.rs. |
Verified Exact (35 points). |
| Forbidden Source Basenames: 9 | CHANGELOG.md:657 |
xtask/src/source_structure/forbidden_filename.rs:7-17 |
Verified Exact (9 names). |
| Stale Claim Test Count: 7 | xtask/tests/segment_store_implementation_documentation.rs:40-51 |
7 tuples tested in for (document, stale_claim). |
Verified Exact. |
5. Repository Standards Compliance
- Pure Rust Project (AGENTS.md line 13): Compliant (no python scripts).
- Code Size & Line Limits (AGENTS.md lines 49-57):
xtask/tests/segment_store_implementation_documentation.rs: 58 physical lines (Target: 200, Max: 500).- New test function
living_v1_pages_no_longer_assign_shipped_recovery_to_a_future_issue: 19 lines (Target: 20, Max: 60). - Maximum nesting depth: 2 (loop + assert).
- Function parameters: 0.
- Deny unwrap, panic, todo (AGENTS.md line 16): Compliant (standard
assert!used in test). - Markdown Formatting & Line Lengths (Documentation Standards §7.2, §8):
- All modified prose lines in markdown files are <= 80 columns.
git diff --check origin/main..HEADexecuted and reported 0 whitespace errors.- Wide table rows use standard
<!-- markdownlint-disable MD013 -->.
- Rust Formatting (rustfmt.toml):
max_width = 100. The two added lines inxtask/tests/segment_store_implementation_documentation.rsare 94 and 88 columns, strictly compliant.
6. Review Execution & Coverage Ledger
In accordance with mandatory protocol:
- Checks Executed:
git diff 2311d80..HEAD(full diff inspection)git logandgit diffon merge commitc009895against both parents0e3dfbeand2311d80git merge-base 0e3dfbe 2311d80git diff --check origin/main..HEAD(whitespace and line-ending verification)- String search and line-length calculation across all modified lines
- Static evaluation of
xtask/tests/segment_store_implementation_documentation.rsagainst all 4 inspected documents
- Checks Inspected Statically (Read-Only):
- Rust source implementations in
src/adapters/filesystem_recovery_inventory_reader.rs,src/adapters/recovery/,src/adapters/filesystem_store_initializer.rs,src/adapters/filesystem_platform_admission.rs,src/adapters/filesystem_catalog_publisher.rs,src/adapters/filesystem_recovery_stage.rs, andxtask/src/durability_crash_matrix/ - All 173 crash case definitions and counts in
xtask/tests/durability_crash_case_contract.rsandconformance/
- Rust source implementations in
- Checks Skipped / Unavailable:
- Host execution of
cargo test/cargo clippy/dockerwas explicitly skipped per reviewer instructions ("Do not run host tests. Primary agent handles Docker validation."). Static inspection is not dynamic execution. - Physical power loss was not simulated; as documented in
recovery.mdandrequirements.md, process-death injection is distinct from host power-loss evidence.
- Host execution of
Verdict
REQUEST CHANGES
Summary of Required Actions Before Approval
- Correct
docs/formats/segment-store-v1/recovery.md:58-62to remove the inaccurate statement that the inventory reader materializes stage bytes. State instead that stage bytes are caller/adapter materialized and admitted against prior fingerprint evidence viaadmit_recovery_stage_bytes. - Clarify in
docs/formats/segment-store-v1/recovery.mdhow transitive publication-view admission is handled (i.e. governed duringhead.nextrecovery underKEEP-RECOVERY-017/018viaCatalogSnapshotverification). - Refine
docs/formats/segment-store-v1/publication.md:137-140to accurately describe that test harnesses obtain the unchecked publisher and wrap it in fault-injecting decorators.
Primary reconciliation: finding 1 is confirmed and deduplicated with the previously posted materialization finding. Finding 3 is confirmed. Finding 2's claim that transitive verification was entirely dropped is superseded by the existing “Leftover next head” section, which already names the exact CatalogSnapshot, planner, executor, filesystem finalizer, and complete transitive current/candidate checks. I will add a local cross-reference and KEEP-RECOVERY-017/018 anchors to improve the opening section's traceability without claiming the behavior was missing. The primary audit also found that the unqualified crash command runs all 173 cases while this v1 page describes only 105; that separate integration finding will be corrected. Cc @codex.
|
To use Codex here, create an environment for this repo. |
Owner
Author
|
Additional Code Lawyer findings while checking #69's acceptance criterion that every living v1 page describes current main. Cc @codex.
The runtime #146 fix stays a separate coherent PR. These changes correct its evidence/status representation, not its acceptance criterion. |
|
To use Codex here, create an environment for this repo. |
Owner
Author
Code Lawyer Activity Summary — current head 153bf05
The full initial agy feedback/checklist was posted before remediation. All fixes are focused commits and published. No review threads exist to resolve; exhausted discovery contains only global comments. Passed: pre-remediation exact-head full Docker workspace debug/release, doctests, fmt, both Clippy profiles, source policy; updated targeted documentation laws debug/release and fmt; updated eight-page Markdown lint (0.23.2). The first attempted Docker copy targeted the loop mount directly and failed before the new tests were installed; it is setup failure, not RED evidence. Actual RED results above came after copying through the container's visible temporary directory. Pending: independent agy repeat review with the mandatory current-head Verification Checklist, clean-bundle dedicated-target full workspace revalidation, and new-head hosted CI. No merge approval is claimed yet. Format/API/runtime bytes are unchanged. No new benchmark or physical power-loss evidence is claimed. Runtime #146 and broader unfinished roadmap requirements remain open. Cc @codex. |
|
To use Codex here, create an environment for this repo. |
Owner
Author
Adversarial Code Review: PR 136 (
|
| Prior Finding ID | Initial Severity | Verified Issue & Root Cause | Remediation Commit & Mechanism | Head Verification Status |
|---|---|---|---|---|
| Finding 1 (Prior agy P2 / Self-finding) | P2 / P4 | docs/formats/segment-store-v1/recovery.md:58-62 stated that the inventory reader materializes stage bytes. FilesystemRecoveryInventoryReader::fingerprint_stage is streaming and allocates zero content memory. Materialization is performed on disk by FilesystemRecoverySegmentResumer::open_reusable via materialize_and_position. |
5f511c6 rewrote the passage to state that classifiers consume caller-supplied bytes, the inventory reader returns fingerprint evidence without retaining stage bytes, and the segment resumer separately materializes its pinned writable stage. Added regression test recovery_documentation_does_not_assign_materialization_to_the_inventory_reader in xtask/tests/segment_store_implementation_documentation.rs:47-53. |
VERIFIED RESOLVED. Doc matches code; test passes. |
| Finding 2 (Prior agy P3) | P3 | docs/formats/segment-store-v1/recovery.md:55-58 silently removed the mention of transitive publication-view admission without linking to its implemented boundary. |
0cdec01 added an explicit cross-reference in recovery.md:71-75 linking transitive publication-view admission for candidate head.next to the "Leftover next head" section and requirements KEEP-RECOVERY-017 / KEEP-RECOVERY-018. |
VERIFIED RESOLVED. Traceability restored and verified against plan_recovery_next_head_finalization. |
| Finding 3 (Prior agy P4) | P4 | docs/formats/segment-store-v1/publication.md:137-140 stated that "fault-injecting decorators obtain an unchecked value". In reality, test harnesses obtain the unchecked publisher and pass it to wrapping decorators. |
4e78c84 refined phrasing: "The crash matrix harness opens an unchecked publisher only behind the repository-tasks Cargo feature, then wraps the publisher in fault-injecting decorators." |
VERIFIED RESOLVED. Strictly matches initialization.rs:38 and publication_storage.rs:17-29. |
| Finding 4 (Primary Self-finding) | P4 | docs/formats/segment-store-v1/recovery.md:121-124 stated that cargo xtask durability-crash-matrix executes 105 canonical cases. With PR 138 merged, the no-argument command executes 173 cases (including 68 v2 migration cases). |
4791ba0 updated recovery.md:130-133 to state that the 105 version-one cases are a subset of the complete command, which also executes version-two migration cases. Added regression test version_one_crash_evidence_is_distinguished_from_the_complete_command in xtask/tests/segment_store_implementation_documentation.rs:11-17. |
VERIFIED RESOLVED. Strictly matches durability_crash_matrix.rs:25-30 and durability_crash_case_contract.rs:36-39. |
| Finding 5 (Primary Ledger Finding) | P4 | In docs/formats/segment-store-v1/requirements.md, KEEP-SEGMENT-008 and 009 cited tests/segment_filesystem_stage.rs, a nonexistent file path. |
c86119a corrected the path to src/adapters/filesystem_segment_stage_tests.rs. |
VERIFIED RESOLVED. Target file exists and contains the four filesystem laws. |
| Finding 6 (Primary Ledger Finding) | P4 | In docs/formats/segment-store-v1/requirements.md, KEEP-SEGMENT-005 claimed implemented status without disclosing the repository-tasks feature escape SealedSegment::map_stage tracked in issue #146. |
153bf05 updated the status column of KEEP-SEGMENT-005 to: "Production API implemented in #15; repository-task escape tracked in #146". |
VERIFIED RESOLVED. Discloses the capability escape accurately without falsely claiming runtime fix #146 here. |
Mandatory Verification Checklist
1. Runtime Paths Traced (file:line to file:line)
| Runtime Behavior | Production Path | Test / Repository Task Path | Rule Parity & Invariant Verification |
|---|---|---|---|
| Store Initialization | FilesystemPlatformAdmission::initialize -> FilesystemInitializationStorage::admit -> initialize_store |
FilesystemPlatformAdmission::initialize_unchecked_for_tests; RepositoryInitializationStorage::admit_unchecked |
Both production and test paths execute the identical 6-phase state machine (KEEP-RECOVERY-002) and root synchronization. Test/task path bypasses only the strict ext4 platform profile via lenient capability opening. |
| Store Reopen | FilesystemPlatformAdmission::reopen -> filesystem_platform_profile::open -> reopen_root |
FilesystemPlatformAdmission::reopen_unchecked_for_tests; FilesystemVersionTwoAdmission::reopen (v2) |
Both verify exact root directory contents (writer.lock, staging, segments, catalogs, and regular HEAD). Mutates nothing. v2 reopen additionally enforces device/inode coordinates against migration.intent (PR 137). |
| Catalog Publisher Opening | FilesystemCatalogPublisher::open consuming FilesystemPlatformAdmission |
FilesystemCatalogPublisher::open_unchecked_for_repository_tasks; open_unchecked_for_tests |
Production requires FilesystemPlatformAdmission whose private fields can only be initialized by Keep. Repository task path uses feature = "repository-tasks". Both pin root, staging, segments, and catalogs without following links. |
| Stage Residue Refusal during Publication | publish_catalog_generation -> filesystem_catalog_current::verify_current |
tests/catalog_filesystem_publication.rs |
Invariant parity verified: unowned current.seg, any head.next, or any current.cat causes immediate refusal (ErrorKind::AlreadyExists) before mutation. Empty immutable pools required when HEAD is absent. |
| Recovery Stage Fingerprinting | FilesystemRecoveryInventoryReader::fingerprint_stage -> filesystem_recovery_stage::fingerprint -> fingerprint_recovery_stage |
tests/recovery_stage_fingerprint.rs |
Streaming 8 KiB buffer under KEEP:RECOVERY:STAGE\0 BLAKE3 domain. Verified zero heap byte materialization. Verified post-read namespace and entry checks. |
| Stage Byte Admission & Assessment | admit_recovery_stage_bytes -> assess_recovery_stage |
tests/recovery_stage_assessment.rs |
Materialized bytes admitted only when stage, length, and recomputed fingerprint match prior evidence. Dispatches to pure whole-byte classifiers (classify_recovery_segment_stage, classify_recovery_catalog_stage, classify_recovery_next_head_stage). |
| Segment Continuation & Materialization | execute_recovery_segment_resume -> FilesystemRecoverySegmentResumer::open_reusable |
tests/recovery_segment_resume.rs |
The only recovery path that materializes stage bytes on disk via materialize_and_position. Re-fingerprints and positions handle at append boundary without rewriting prefix. |
| Next-Head Recovery Finalization | plan_recovery_next_head_finalization -> RecoveryNextHeadFinalizationStorage -> FilesystemRecoveryNextHeadFinalizer |
tests/recovery_next_head_finalization.rs |
Enforces candidate snapshot generation, catalog length, and digest agreement with candidate head (KEEP-RECOVERY-017). Synchronizes candidate before atomic rename (KEEP-RECOVERY-018). |
| Sealed Stage Handle Immutability | SealedSegment::close |
SealedSegment::map_stage (#[cfg(feature = "repository-tasks")]) |
Production public API consumes the sealed stage and exposes no mutable handle (KEEP-SEGMENT-005). Repository-task escape allows mapping internal stage decorators, tracked in #146. |
| Living Documentation Posture | living_v1_pages_no_longer_assign_shipped_recovery_to_a_future_issue |
recovery_documentation_does_not_assign_materialization_to_the_inventory_reader; version_one_crash_evidence_is_distinguished_from_the_complete_command |
Scans FORMAT_README, PUBLICATION, RECOVERY, and REQUIREMENTS for 7 stale planning strings, false inventory materialization claims, and unqualified 105-case scope claims. Statically verified that all assertions evaluate to true. |
2. Merges Audited (SHA and Integration Invariants)
- Merge Commit:
c0098953dceb78650d50db0547e715e71d8bc978 - Parent 1:
0e3dfbe3125831ab9abe527c69024573edc1e6de(Initial PR 136 commit) - Parent 2:
2311d805e9bbcb1743fc5cdc46adb324641b9f14(mainincorporating PR 138) - Merge Base:
f49cff732cf7a6e1b472decba9e4c4130990559e
Invariants verified across merged commits:
- PR 140 (
eec39ba— ambient-CPU-independent source law):- Verified: PR 136 makes zero modifications to
benches/,xtask/src/benchmark/, or CPU model checking code.
- Verified: PR 136 makes zero modifications to
- PR 145 (
88f35c4— forbidden source filenames):- Verified against
xtask/src/source_structure/forbidden_filename.rs:7-17. PR 136 introduces no new files, and none of the nine forbidden basenames (utils.rs,helpers.rs,common.rs,misc.rs,shared.rs,manager.rs,service.rs,types.rs,models.rs) exist in the PR diff.
- Verified against
- PR 135 (
07bf0b8— bounded reference-staging memory contract):- Verified: PR 136 makes zero modifications to
src/reference/or reference memory allocation test suites.
- Verified: PR 136 makes zero modifications to
- PR 137 (
200cfc8— restart device/inode vs live mount identity):- Verified: PR 136 leaves
src/adapters/filesystem_version_two_admission.rsuntouched; device/inode restart verification remains fully active.
- Verified: PR 136 leaves
- PR 138 (
2311d80— partial-prefix migration recovery & 68 crash cases):- Verified: PR 136 leaves migration recovery in
src/adapters/store_migration/andconformance/segment-store/v2/untouched. The 68 migration crash cases are explicitly accounted for in commit4791ba0.
- Verified: PR 136 leaves migration recovery in
- Conflict Resolution & Changelog Invariants:
git diff 2311d80 c009895 -- . ':(exclude)CHANGELOG.md' ':(exclude)docs/formats/segment-store-v1' ':(exclude)xtask/tests/segment_store_implementation_documentation.rs'is completely empty (0 diff).- In
CHANGELOG.md:624-660, merge commitc009895cleanly combined PR 136 entries with main's PR 138 entries under### Fixed, preserving both histories without dropping any entry.
3. Constants and Evidence Coordinates Checked
| Constant / Coordinate | Documented Location | Tree Source / Evidence | Binding Threshold & Evaluation |
|---|---|---|---|
Domain Separator KEEP:RECOVERY:STAGE\0 |
docs/formats/segment-store-v1/requirements.md:125, 129, recovery.md:38, 278 |
src/adapters/recovery/recovery_stage_fingerprinter.rs:10 |
Exact byte literal b"KEEP:RECOVERY:STAGE\0" verified. |
Streaming Fingerprint Buffer 8,192 bytes (8 KiB) |
docs/formats/segment-store-v1/requirements.md:125 |
src/adapters/recovery/recovery_stage_fingerprinter.rs:11 |
Constant BUFFER_LENGTH: usize = 8_192. Zero heap allocation verified. |
Max Recovery Inventory Entry Limit 2,097,152 / 2,097,153 |
docs/formats/segment-store-v1/recovery.md:15-18, catalog.md:169 |
xtask/tests/segment_store_protocol_contract/recovery_laws.rs:8 |
MAX_RECOVERY_INVENTORY_ENTRY_COUNT = 2_097_152. Refusal on first excess entry reports 2,097,153. Verified. |
Catalog Stage Interruption 176 bytes |
docs/formats/segment-store-v1/publication.md:231 |
xtask/src/durability_crash_matrix/production_protocol/publication_storage.rs:14 |
Constant CATALOG_INTERRUPTION: usize = 176 verified. |
Head Stage Interruption 64 bytes |
docs/formats/segment-store-v1/publication.md:253 |
xtask/src/durability_crash_matrix/production_protocol/publication_storage.rs:15 |
Constant HEAD_INTERRUPTION: usize = 64 verified. |
Crash Restart Byte Limit 1,048,576 bytes (1 MiB) |
KEEP-RECOVERY-021 |
xtask/src/durability_crash_matrix/production_protocol/initialization.rs:14 |
Constant RESTART_BYTE_LIMIT: u64 = 1_048_576 verified. |
Publication Head Length 128 bytes |
docs/formats/segment-store-v1/recovery.md:268, 302 |
src/adapters/publication_head.rs |
Constant PUBLICATION_HEAD_LENGTH: usize = 128 verified. |
| Catalog Pool Digest Fixture | Conformance vectors | tests/catalog.rs:10 |
04b82519b0399baefd0b9c0f32a871052e4c47e3a00226ab03b21661470f7320 verified across 7 separate fixture sites. |
4. Doc Figures and Counts Checked
| Count / Figure | Document Claim | Raw Tree Evidence | Verification Result |
|---|---|---|---|
| v1 Process-Death Crash Cases: 105 | docs/formats/segment-store-v1/README.md:11, requirements.md:138, 195, recovery.md:130, conformance/segment-store/v1/README.md:108 |
35 v1 points (KEEP-CRASH-001–KEEP-CRASH-035) × 3 positions (Before, During, After) = 105 cases. Tested in xtask/tests/durability_crash_documentation.rs:20. |
Verified Exact. |
| v2 Migration Crash Cases: 68 | conformance/segment-store/v2/README.md:68-75, docs/formats/segment-store-v2/migration-crash.md:7 |
xtask/tests/durability_crash_case_contract.rs:39: assert_eq!(migration.len(), 68). |
Verified Exact. |
| Total Crash Cases Executed by Default Command: 173 | docs/formats/segment-store-v1/recovery.md:130-133 |
xtask/tests/durability_crash_case_contract.rs:36: assert_eq!(cases.len(), 173) (105 v1 + 68 v2 migration = 173). |
Verified Exact. |
| v1 Recovery Requirements: 21 | docs/formats/segment-store-v1/requirements.md:118-138 |
Table rows KEEP-RECOVERY-001 through KEEP-RECOVERY-021. |
Verified Exact (21 contiguous rows). |
| v1 Crash Points: 35 | docs/formats/segment-store-v1/requirements.md:118 |
KEEP-CRASH-001 through KEEP-CRASH-035 in xtask/src/durability_crash_point.rs. |
Verified Exact (35 points). |
| Forbidden Source Basenames: 9 | CHANGELOG.md:657 |
xtask/src/source_structure/forbidden_filename.rs:7-17. |
Verified Exact (9 names). |
| Stale Claim Test Tuples: 7 | xtask/tests/segment_store_implementation_documentation.rs:56-67 |
7 tuples tested in for (document, stale_claim). |
Verified Exact. |
| PR 136 Changelog Items: 7 | CHANGELOG.md:627-650 |
7 distinct bullet points detailing each doc fix. | Verified Exact. |
5. Repository Standards Compliance
- Pure Rust Project (
AGENTS.md:13): Strictly compliant (no python scripts). - Code Size & Line Limits (
AGENTS.md:49-57):xtask/tests/segment_store_implementation_documentation.rs: 74 physical lines (Target: 200, Max: 500).- Test functions:
version_one_crash_evidence_is_distinguished_from_the_complete_command: 6 lines (Target: 20, Max: 60).recovery_documentation_does_not_assign_materialization_to_the_inventory_reader: 6 lines (Target: 20, Max: 60).living_v1_pages_no_longer_assign_shipped_recovery_to_a_future_issue: 19 lines (Target: 20, Max: 60).
- Maximum nesting depth: 2 (loop + assert).
- Function parameters: 0.
- Deny unwrap, panic, todo (
AGENTS.md:16): Strictly compliant (assert!used in test functions). - Markdown Formatting & Line Lengths:
git diff --check origin/main..HEADexecuted and reported 0 whitespace errors.- All modified prose lines in markdown files are <= 80 columns.
- Wide table rows use standard
<!-- markdownlint-disable MD013 -->.
- Rust Formatting (
rustfmt.toml):max_width = 100. All modified lines inxtask/tests/segment_store_implementation_documentation.rsare <= 93 columns.
- Link Target Validity:
- All markdown file links (
requirements.md,recovery.md,transitions.tsv,rationale.md,segment-store-v2/README.md) exist on disk. - The section anchor
[Leftover next head](#leftover-next-head)matches line 299 inrecovery.md. - All 32 requirement and test source paths cited in
requirements.mdexist on disk.
- All markdown file links (
6. Review Execution & Coverage Ledger
- Checks Executed:
git diff origin/main...HEAD(full diff inspection across all 6 modified files)git logandgit diffon merge commitc009895against parent 1 (0e3dfbe) and parent 2 (2311d80)git merge-base 0e3dfbe 2311d80git log -p -6 HEADfor each of the six remediation commits (5f511c6,4791ba0,4e78c84,0cdec01,c86119a,153bf05)git diff --check origin/main..HEAD(whitespace and line-ending verification: clean)- Diff line length measurement (prose <= 80 cols, code <= 100 cols: clean)
- Target file existence checks for all 32 evidence paths in
requirements.md(clean) - Tarball inspection of
pr136-green1.tarthroughpr136-green6.tarin scratch directory - Static evaluation of
xtask/tests/segment_store_implementation_documentation.rsacross all 4 living documents
- Checks Inspected Statically (Read-Only):
- All living v1 pages:
README.md,segment.md,catalog.md,publication.md,recovery.md,requirements.md, andrationale.md - Conformance files:
conformance/segment-store/v1/README.md,conformance/segment-store/v2/README.md, andconformance/segment-store/v1/transitions.tsv - Underlying Rust implementations:
src/adapters/filesystem_recovery_inventory_reader.rs,src/adapters/recovery/,src/adapters/filesystem_recovery_stage.rs,src/adapters/filesystem_recovery_segment_resume_storage.rs,src/adapters/filesystem_catalog_publisher.rs,src/adapters/sealed_segment.rs,src/adapters/filesystem_segment_stage_tests.rs, andxtask/src/durability_crash_matrix/ - Crash matrix counts and case generation in
xtask/tests/durability_crash_case_contract.rs
- All living v1 pages:
- Checks Skipped / Unavailable:
- Host execution of tests (
cargo test,cargo clippy, Docker) was omitted per reviewer instructions ("Do not run host tests. Primary agent handles Docker validation."). Static inspection is not dynamic execution. - Physical host power loss simulation was not performed (the repository's process-death crash matrix proves application crash recovery, not physical power-loss or hardware write-tearing evidence, as explicitly acknowledged in
recovery.md:162-164andrequirements.md:110-112).
- Host execution of tests (
Verdict
APPROVE
Primary Code Lawyer final Activity Summary and merge judgment
This supersedes pending gates in the earlier Activity Summary. All six focused findings are addressed at 153bf05. The transitive-view concern was reconciled as an overview traceability improvement, since the existing later section already described that implementation. Runtime #146 remains open and is now disclosed accurately.
Passed on a clean Git-bundle clone at this exact SHA with its own Cargo target directory in Docker: full workspace/all-feature debug and release suites including doctests; formatting; debug and release workspace/all-target/all-feature Clippy with -D warnings; source-structure policy. Updated eight-page Markdown lint passes. All four hosted jobs pass in CI run 36964001601, including documentation/refusal/link/whitespace gates, fuzz smoke and dependency policy. agy completed with APPROVE and the full mandatory checklist above. Exhausted GitHub review discovery has no reviews and no review threads; CodeRabbit remains explicitly rate-limited, not an approval.
MERGE GATE: OPEN under the user's explicit authorization for clean agy review plus green validation in place of the unavailable skill-only two-reviewer/cooldown gates. Actual repository protections remain enforced. No runtime/format/API changes or new benchmark, crash-matrix execution, or physical power-loss evidence are claimed for this documentation PR. Cc @codex.
|
To use Codex here, create an environment for this repo. |
This was referenced Oct 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The living v1 format pages described implemented initialization, admission, and recovery as future work. This PR reconciles those claims with main's implementation and existing requirement/test anchors, while retaining historical issue references.
Evidence and approach
Checked the production initialize/reopen producers, gated repository-harness admission, canonical publication checks, recovery classifiers, and KEEP-RECOVERY-001–021 ledger. The documentation now names implemented behavior and the 105-case process-death matrix, without treating it as power-loss evidence. Whole-byte stage classification is described as the existing design rather than an absent streaming implementation.
The new documentation contract fails against main with the stale future-recovery claim and passes after correction in debug and release. Existing implementation-posture law also passes.
Validation
Pinned Rust 1.96.0: targeted documentation contracts in debug and release, formatting, workspace/all-target/all-feature Clippy with warnings denied, documentation integrity, source structure, dependency audit and policy checks. Existing runtime evidence was inspected; no fresh crash-matrix run is claimed for this prose change.
Compatibility and scope
No runtime, format, API, identity, durability, recovery, performance, or security changes. Reorganizing unrelated documentation and implementing v2 behavior are excluded. A new ADR or benchmark is unnecessary because this records existing decisions and evidence. Leaving stale future claims was rejected because it misstates the source boundary for migration work.
This branch starts directly at origin/main. Closes #69. Refs #132.