Advance v18 evidence and receipt projection#92
Conversation
📝 WalkthroughWalkthroughThis PR implements evidence posture/domain types for Continuum, hardens patch commit success via CAS-backed writer-ref advancement with post-CAS visibility verification, projects git-warp TickReceipt into Continuum receipt-family Receipt facts, updates package exports, enhances test persistence mocks to model CAS, and adds tests and design/retro docs for these behaviors. ChangesV18 Evidence Posture, Patch Commit Visibility, and Receipt Projection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Release Preflight
If you tag this commit as |
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)
test/unit/domain/WarpGraph.strands.test.ts (1)
1-1440:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSplit this test file under the 800 LOC cap.
This touched test file is ~1440 LOC, which exceeds the policy limit for tests and keeps sludge in place. Please split it into smaller focused test files before merge.
As per coding guidelines, "Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)" and "Use the anti-sludge policy; if a patch compiles but violates the policy, the patch is still wrong."
🤖 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 `@test/unit/domain/WarpGraph.strands.test.ts` around lines 1 - 1440, The test file is too large (~1440 LOC); split it into multiple focused test files and extract shared helpers to a test utilities module. Move helper functions (hexSha, createMockPersistence, simulatePatchCommit) and common imports (WarpCore, Dot, VersionVector, createStateReader, exportCoordinateComparisonFact, exportCoordinateTransferPlanFact, buildStrandBraidRef/buildStrandOverlayRef) into a new test helper file and import them from each smaller spec; then split the "WarpCore strand foundation" describe block into separate specs such as: 1) strand-creation-and-lifecycle (tests using createStrand/getStrand/listStrands/dropStrand), 2) materialization-and-observers (materializeCoordinate/materialize/materializeStrand/observer/materializeStrand receipts/ceilings), 3) patching-and-overlays (patchStrand/getStrandPatches/patchesForStrand/patchStrand persistence), 4) comparison-and-transfer (compareStrand/compareCoordinates/planCoordinateTransfer/planStrandTransfer and exportCoordinate*), and 5) intents-and-ticks (queueStrandIntent/listStrandIntents/tickStrand/intent conflict tests). Ensure each new file imports the shared helpers and sets up the same beforeEach (persistence = createMockPersistence(); graph = await WarpCore.open(...)) and keep test names and referenced symbols (simulatePatchCommit, graph.patchStrand, graph.materializeStrand, graph.compareStrand, graph.planStrandTransfer, graph.queueStrandIntent, graph.tickStrand) unchanged so assertions and behavior remain identical.
🧹 Nitpick comments (3)
docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md (1)
37-37: 💤 Low valueClarify "slice 7" reference.
The text references "slice 7 CAS visibility hardening," but the current cycle is 0152 and the previous CAS visibility work was cycle 0151. If "slice 7" refers to an internal counting system, consider using the cycle number instead for consistency.
🤖 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 `@docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md` at line 37, The phrase "slice 7 CAS visibility hardening" is ambiguous; update the line in v18-same-writer-race-witness.md to use the canonical cycle identifier (e.g., "cycle 0151 CAS visibility hardening" or "cycle 0152" as appropriate) or add a brief parenthetical explaining that "slice 7" is an internal counter, so readers can map it to the cycle number; replace the literal "slice 7" with the chosen clearer reference throughout the document to maintain consistency.docs/method/retro/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.md (1)
58-58: ⚡ Quick winAdd final newline.
The file is missing a final newline. This issue also appears in the other three retro files in this layer (0152, 0153, 0154).
🤖 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 `@docs/method/retro/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.md` at line 58, The file v18-patch-commit-visibility.md is missing a final newline; open the document (and the other retro files named 0152-v18-*, 0153-v18-*, 0154-v18-*) and add a single trailing newline character at end-of-file so the file ends with a blank line; save with your editor/CI settings ensuring files end with newline to prevent the missing-eol issue going forward.src/domain/continuum/ContinuumEvidenceStatus.ts (1)
36-40: ⚡ Quick winReplace inline posture/runtime literals with named constants.
Line 38 and Line 39 embed protocol tokens directly; moving them to shared constants avoids drift and keeps the model aligned with the anti-sludge rule.
♻️ Proposed refactor
+const TRANSLATED_SUBSTRATE = 'translated-substrate'; +const SOURCE_RUNTIME_GIT_WARP = 'git-warp'; + static translatedGitWarp(fields: TranslatedGitWarpEvidenceFields): ContinuumEvidenceStatus { return new ContinuumEvidenceStatus({ - posture: 'translated-substrate', - sourceRuntime: 'git-warp', + posture: TRANSLATED_SUBSTRATE, + sourceRuntime: SOURCE_RUNTIME_GIT_WARP, basisRef: fields.basisRef, summary: fields.summary, }); }As per coding guidelines, “Avoid magic strings and numbers; use named constants instead.”
🤖 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 `@src/domain/continuum/ContinuumEvidenceStatus.ts` around lines 36 - 40, The method translatedGitWarp in ContinuumEvidenceStatus currently uses magic string literals for posture and sourceRuntime; create named constants (e.g., POSTURE_TRANSLATED_SUBSTRATE and SOURCE_RUNTIME_GIT_WARP) and replace the inline values in translatedGitWarp with those constants to avoid drift and follow the anti-sludge rule; add the constants near the top of the file or import them from the shared constants module and reference them in the ContinuumEvidenceStatus constructor call.
🤖 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 `@src/domain/services/PatchCommitter.ts`:
- Around line 199-202: The thrown WriterError in PatchCommitter (the block that
throws 'WRITER_COMMIT_NOT_VISIBLE' when a commit is written but not visible) is
missing expectedSha/actualSha fields; update the throw to populate expectedSha
with options.newCommitSha and actualSha with visibleSha (or null/undefined if
not present) on the WriterError instance so the error carries the expected vs
actual commit SHAs for triage.
- Around line 185-194: The catch currently treats every failure from
compareAndSwapRef as a WRITER_CAS_CONFLICT; instead detect whether the caught
error actually represents a CAS mismatch versus a transport/auth/storage error
(inspect the thrown error's type/code/message coming from
options.persistence.compareAndSwapRef), and only build and throw the WriterError
with code 'WRITER_CAS_CONFLICT' (using options.persistence.readRef to populate
actualSha and attaching expectedSha) when it's a true CAS mismatch; for non-CAS
errors, do not convert them into a conflict—either rethrow the original error or
wrap it in a different error class—so update the catch in PatchCommitter (the
method that calls options.persistence.compareAndSwapRef) to branch on the error
identity before creating the WriterError.
In `@test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts`:
- Around line 8-63: Add unit tests that cover the missing validation branches in
ContinuumEvidenceStatus and posture normalization in ContinuumEvidencePosture:
1) assert that creating a ContinuumEvidenceStatus with empty/undefined
sourceRuntime, basisRef, or summary throws the expected validation error
messages; 2) assert that passing an existing ContinuumEvidencePosture instance
into ContinuumEvidenceStatus (instead of a string) normalizes correctly (e.g.,
new ContinuumEvidencePosture('continuum-native') used for posture) and preserves
behaviors like isContinuumNative()/isTranslatedSubstrate(); and 3) include tests
for translatedGitWarp() still rejecting a nativeWitnessRef and for
ContinuumEvidencePosture throwing on unknown values—target the
ContinuumEvidenceStatus constructor, translatedGitWarp, and
ContinuumEvidencePosture symbols so all previously-uncovered branches are
exercised.
In `@test/unit/domain/services/PatchBuilder.content.test.ts`:
- Around line 31-49: The test file exceeds the 800 LOC limit because a large
inline mock/setup block is expanding the file; extract the reusable mocked repo
behavior into a separate test helper and replace the inline Map+vi.fn mocks with
a factory import. Specifically, move the refs Map and functions (readRef,
updateRef, compareAndSwapRef, showNode, writeBlob, writeTree,
commitNodeWithTree) into a new helper (e.g., createRepoMock) that returns those
functions and import it into PatchBuilder.content.test.ts, then replace the
inline declarations with a call to createRepoMock() to keep the test file under
the ceiling and reuse the mock across spec files.
In `@test/unit/domain/services/PatchBuilder.test.ts`:
- Around line 31-49: The test file exceeds the project's test-file size limit;
split the large PatchBuilder.test.ts into smaller suites by grouping related
tests (e.g., "write operations", "commit and refs", "CAS behavior") and moving
the shared mock factory (the Map-backed refs and mock functions readRef,
compareAndSwapRef, updateRef, writeBlob, writeTree, commitNodeWithTree,
showNode) into a new test helper module that each new test file imports; create
multiple new test files that each import the helper, copy only relevant tests
into each file, and update imports/vi mocks to use the centralized mock factory
so behavior (especially compareAndSwapRef’s CAS logic and readRef
mockImplementation updates) remains consistent across the split files.
In `@test/unit/domain/warp/Writer.test.ts`:
- Around line 24-38: The test file Writer.test.ts exceeds the 800 LOC limit;
trim or split it by extracting common setup (the in-memory refs Map and mocked
functions readRef, updateRef, compareAndSwapRef) into a shared test helper
module and move large, independent test suites into separate test files;
specifically factor out the refs Map and the vi.fn mocks (readRef, updateRef,
compareAndSwapRef) into a new helper (e.g., writerTestHelpers) and import it
from smaller test files, or split long describe blocks into multiple files so
each file stays under 800 LOC while preserving tests that reference readRef,
updateRef, compareAndSwapRef and the refs state.
In `@test/unit/domain/WarpGraph.test.ts`:
- Around line 43-64: The test file WarpGraph.test.ts exceeds the max test-file
size and must be split; identify logical test groups (e.g., unit tests for
compareAndSwapRef/readRef behavior, blob/tree/commit operations, and
higher-level WarpGraph scenarios) and move them into separate files (for example
WarpGraph.compareAndSwapRef.test.ts, WarpGraph.blobTree.test.ts,
WarpGraph.integration.test.ts). Extract the common mock setup (the refs Map,
readRef, and the vi.fn implementations such as compareAndSwapRef, updateRef,
writeBlob, writeTree, commitNode, commitNodeWithTree) into a shared test helper
module (e.g., warpGraphTestUtils) that exports the mocked store and factory
functions used by tests, then update existing tests to import those helpers and
only contain the tests for one concern per file so each new test file stays
under the 800 LOC limit; keep function names readRef, compareAndSwapRef,
updateRef, writeBlob, writeTree, commitNode, commitNodeWithTree, and showNode to
match existing references.
---
Outside diff comments:
In `@test/unit/domain/WarpGraph.strands.test.ts`:
- Around line 1-1440: The test file is too large (~1440 LOC); split it into
multiple focused test files and extract shared helpers to a test utilities
module. Move helper functions (hexSha, createMockPersistence,
simulatePatchCommit) and common imports (WarpCore, Dot, VersionVector,
createStateReader, exportCoordinateComparisonFact,
exportCoordinateTransferPlanFact, buildStrandBraidRef/buildStrandOverlayRef)
into a new test helper file and import them from each smaller spec; then split
the "WarpCore strand foundation" describe block into separate specs such as: 1)
strand-creation-and-lifecycle (tests using
createStrand/getStrand/listStrands/dropStrand), 2) materialization-and-observers
(materializeCoordinate/materialize/materializeStrand/observer/materializeStrand
receipts/ceilings), 3) patching-and-overlays
(patchStrand/getStrandPatches/patchesForStrand/patchStrand persistence), 4)
comparison-and-transfer
(compareStrand/compareCoordinates/planCoordinateTransfer/planStrandTransfer and
exportCoordinate*), and 5) intents-and-ticks
(queueStrandIntent/listStrandIntents/tickStrand/intent conflict tests). Ensure
each new file imports the shared helpers and sets up the same beforeEach
(persistence = createMockPersistence(); graph = await WarpCore.open(...)) and
keep test names and referenced symbols (simulatePatchCommit, graph.patchStrand,
graph.materializeStrand, graph.compareStrand, graph.planStrandTransfer,
graph.queueStrandIntent, graph.tickStrand) unchanged so assertions and behavior
remain identical.
---
Nitpick comments:
In
`@docs/method/retro/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.md`:
- Line 58: The file v18-patch-commit-visibility.md is missing a final newline;
open the document (and the other retro files named 0152-v18-*, 0153-v18-*,
0154-v18-*) and add a single trailing newline character at end-of-file so the
file ends with a blank line; save with your editor/CI settings ensuring files
end with newline to prevent the missing-eol issue going forward.
In
`@docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md`:
- Line 37: The phrase "slice 7 CAS visibility hardening" is ambiguous; update
the line in v18-same-writer-race-witness.md to use the canonical cycle
identifier (e.g., "cycle 0151 CAS visibility hardening" or "cycle 0152" as
appropriate) or add a brief parenthetical explaining that "slice 7" is an
internal counter, so readers can map it to the cycle number; replace the literal
"slice 7" with the chosen clearer reference throughout the document to maintain
consistency.
In `@src/domain/continuum/ContinuumEvidenceStatus.ts`:
- Around line 36-40: The method translatedGitWarp in ContinuumEvidenceStatus
currently uses magic string literals for posture and sourceRuntime; create named
constants (e.g., POSTURE_TRANSLATED_SUBSTRATE and SOURCE_RUNTIME_GIT_WARP) and
replace the inline values in translatedGitWarp with those constants to avoid
drift and follow the anti-sludge rule; add the constants near the top of the
file or import them from the shared constants module and reference them in the
ContinuumEvidenceStatus constructor call.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81f7fad6-c360-46bf-ae0f-34689e986704
📒 Files selected for processing (40)
docs/BEARING.mddocs/design/0150-v18-evidence-posture/v18-evidence-posture.mddocs/design/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.mddocs/design/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.mddocs/design/0153-v18-receipt-family-projection/v18-receipt-family-projection.mddocs/design/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.mddocs/method/retro/0150-v18-evidence-posture/v18-evidence-posture.mddocs/method/retro/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.mddocs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.mddocs/method/retro/0153-v18-receipt-family-projection/v18-receipt-family-projection.mddocs/method/retro/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.mdindex.tssrc/domain/continuum/ContinuumEvidencePosture.tssrc/domain/continuum/ContinuumEvidenceStatus.tssrc/domain/continuum/ContinuumReceipt.tssrc/domain/continuum/ContinuumReceiptFamilyProjection.tssrc/domain/continuum/ContinuumReceiptProjector.tssrc/domain/errors/WriterError.tssrc/domain/services/PatchCommitter.tstest/benchmark/detachedReadBenchmark.fixture.tstest/helpers/WarpGraphMockPersistence.tstest/unit/domain/WarpCore.snapshotHashStability.test.tstest/unit/domain/WarpGraph.conflicts.test.tstest/unit/domain/WarpGraph.invalidation.test.tstest/unit/domain/WarpGraph.observerBoundary.test.tstest/unit/domain/WarpGraph.patchCount.test.tstest/unit/domain/WarpGraph.sameWriterRace.test.tstest/unit/domain/WarpGraph.strands.test.tstest/unit/domain/WarpGraph.test.tstest/unit/domain/WarpGraph.worldline.test.tstest/unit/domain/WarpGraph.writerInvalidation.test.tstest/unit/domain/continuum/ContinuumEvidenceStatus.test.tstest/unit/domain/continuum/ContinuumReceiptProjection.test.tstest/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.tstest/unit/domain/index.exports.test.tstest/unit/domain/services/PatchBuilder.cas.test.tstest/unit/domain/services/PatchBuilder.content.test.tstest/unit/domain/services/PatchBuilder.test.tstest/unit/domain/services/PatchCommitter.visibility.test.tstest/unit/domain/warp/Writer.test.ts
| } catch (err) { | ||
| const actualSha = await options.persistence.readRef(options.writerRef); | ||
| const error = new WriterError( | ||
| 'WRITER_CAS_CONFLICT', | ||
| 'Commit failed: writer ref was updated by another process. Re-materialize and retry.', | ||
| err instanceof Error ? err : undefined, | ||
| ); | ||
| error.expectedSha = options.expectedParentSha; | ||
| error.actualSha = actualSha; | ||
| throw error; |
There was a problem hiding this comment.
Do not collapse all CAS failures into conflict errors.
Line 185 currently converts every compareAndSwapRef failure into WRITER_CAS_CONFLICT. That can misclassify transport/auth/storage failures as race conflicts and trigger incorrect retry behavior.
🐛 Suggested fix
} catch (err) {
- const actualSha = await options.persistence.readRef(options.writerRef);
- const error = new WriterError(
- 'WRITER_CAS_CONFLICT',
- 'Commit failed: writer ref was updated by another process. Re-materialize and retry.',
- err instanceof Error ? err : undefined,
- );
- error.expectedSha = options.expectedParentSha;
- error.actualSha = actualSha;
- throw error;
+ const actualSha = await options.persistence.readRef(options.writerRef);
+ if (actualSha !== options.expectedParentSha) {
+ const error = new WriterError(
+ 'WRITER_CAS_CONFLICT',
+ 'Commit failed: writer ref was updated by another process. Re-materialize and retry.',
+ err instanceof Error ? err : undefined,
+ );
+ error.expectedSha = options.expectedParentSha;
+ error.actualSha = actualSha;
+ throw error;
+ }
+ throw new WriterError(
+ 'PERSIST_WRITE_FAILED',
+ 'Failed to advance writer ref via compare-and-swap',
+ err instanceof Error ? err : undefined,
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err) { | |
| const actualSha = await options.persistence.readRef(options.writerRef); | |
| const error = new WriterError( | |
| 'WRITER_CAS_CONFLICT', | |
| 'Commit failed: writer ref was updated by another process. Re-materialize and retry.', | |
| err instanceof Error ? err : undefined, | |
| ); | |
| error.expectedSha = options.expectedParentSha; | |
| error.actualSha = actualSha; | |
| throw error; | |
| } catch (err) { | |
| const actualSha = await options.persistence.readRef(options.writerRef); | |
| if (actualSha !== options.expectedParentSha) { | |
| const error = new WriterError( | |
| 'WRITER_CAS_CONFLICT', | |
| 'Commit failed: writer ref was updated by another process. Re-materialize and retry.', | |
| err instanceof Error ? err : undefined, | |
| ); | |
| error.expectedSha = options.expectedParentSha; | |
| error.actualSha = actualSha; | |
| throw error; | |
| } | |
| throw new WriterError( | |
| 'PERSIST_WRITE_FAILED', | |
| 'Failed to advance writer ref via compare-and-swap', | |
| err instanceof Error ? err : undefined, | |
| ); | |
| } |
🤖 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 `@src/domain/services/PatchCommitter.ts` around lines 185 - 194, The catch
currently treats every failure from compareAndSwapRef as a WRITER_CAS_CONFLICT;
instead detect whether the caught error actually represents a CAS mismatch
versus a transport/auth/storage error (inspect the thrown error's
type/code/message coming from options.persistence.compareAndSwapRef), and only
build and throw the WriterError with code 'WRITER_CAS_CONFLICT' (using
options.persistence.readRef to populate actualSha and attaching expectedSha)
when it's a true CAS mismatch; for non-CAS errors, do not convert them into a
conflict—either rethrow the original error or wrap it in a different error
class—so update the catch in PatchCommitter (the method that calls
options.persistence.compareAndSwapRef) to branch on the error identity before
creating the WriterError.
| throw new WriterError( | ||
| 'WRITER_COMMIT_NOT_VISIBLE', | ||
| `Commit ${options.newCommitSha} was written but ${options.writerRef} points at ${visibleSha ?? '(none)'}`, | ||
| ); |
There was a problem hiding this comment.
Populate SHA context on visibility failures.
Line 199 creates WRITER_COMMIT_NOT_VISIBLE without setting expectedSha/actualSha, which drops useful triage data.
🔎 Suggested fix
if (visibleSha !== options.newCommitSha) {
- throw new WriterError(
+ const error = new WriterError(
'WRITER_COMMIT_NOT_VISIBLE',
`Commit ${options.newCommitSha} was written but ${options.writerRef} points at ${visibleSha ?? '(none)'}`,
);
+ error.expectedSha = options.newCommitSha;
+ error.actualSha = visibleSha;
+ throw error;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new WriterError( | |
| 'WRITER_COMMIT_NOT_VISIBLE', | |
| `Commit ${options.newCommitSha} was written but ${options.writerRef} points at ${visibleSha ?? '(none)'}`, | |
| ); | |
| if (visibleSha !== options.newCommitSha) { | |
| const error = new WriterError( | |
| 'WRITER_COMMIT_NOT_VISIBLE', | |
| `Commit ${options.newCommitSha} was written but ${options.writerRef} points at ${visibleSha ?? '(none)'}`, | |
| ); | |
| error.expectedSha = options.newCommitSha; | |
| error.actualSha = visibleSha; | |
| throw error; | |
| } |
🤖 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 `@src/domain/services/PatchCommitter.ts` around lines 199 - 202, The thrown
WriterError in PatchCommitter (the block that throws 'WRITER_COMMIT_NOT_VISIBLE'
when a commit is written but not visible) is missing expectedSha/actualSha
fields; update the throw to populate expectedSha with options.newCommitSha and
actualSha with visibleSha (or null/undefined if not present) on the WriterError
instance so the error carries the expected vs actual commit SHAs for triage.
| describe('ContinuumEvidenceStatus', () => { | ||
| it('marks git-warp evidence as translated substrate evidence', () => { | ||
| const status = ContinuumEvidenceStatus.translatedGitWarp({ | ||
| basisRef: PATCH_BASIS_REF, | ||
| summary: 'git-warp patch receipt projected into receipt-family shape', | ||
| }); | ||
|
|
||
| expect(status.posture.toString()).toBe('translated-substrate'); | ||
| expect(status.sourceRuntime).toBe('git-warp'); | ||
| expect(status.basisRef).toBe(PATCH_BASIS_REF); | ||
| expect(status.nativeWitnessRef).toBeUndefined(); | ||
| expect(status.isTranslatedSubstrate()).toBe(true); | ||
| expect(status.isContinuumNative()).toBe(false); | ||
| expect(Object.isFrozen(status)).toBe(true); | ||
| }); | ||
|
|
||
| it('accepts native Continuum evidence only with an explicit native witness reference', () => { | ||
| const status = new ContinuumEvidenceStatus({ | ||
| posture: 'continuum-native', | ||
| sourceRuntime: 'git-warp', | ||
| basisRef: PATCH_BASIS_REF, | ||
| nativeWitnessRef: NATIVE_WITNESS_REF, | ||
| summary: 'receipt-family value was produced through native Continuum witnesshood', | ||
| }); | ||
|
|
||
| expect(status.posture.toString()).toBe('continuum-native'); | ||
| expect(status.nativeWitnessRef).toBe(NATIVE_WITNESS_REF); | ||
| expect(status.isContinuumNative()).toBe(true); | ||
| expect(status.isTranslatedSubstrate()).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects native Continuum evidence without a native witness reference', () => { | ||
| expect(() => new ContinuumEvidenceStatus({ | ||
| posture: 'continuum-native', | ||
| sourceRuntime: 'git-warp', | ||
| basisRef: PATCH_BASIS_REF, | ||
| summary: 'missing witness', | ||
| })).toThrow('nativeWitnessRef'); | ||
| }); | ||
|
|
||
| it('rejects translated substrate evidence that carries a native witness reference', () => { | ||
| expect(() => new ContinuumEvidenceStatus({ | ||
| posture: 'translated-substrate', | ||
| sourceRuntime: 'git-warp', | ||
| basisRef: PATCH_BASIS_REF, | ||
| nativeWitnessRef: NATIVE_WITNESS_REF, | ||
| summary: 'translated evidence cannot claim native witnesshood', | ||
| })).toThrow('translated substrate evidence must not carry nativeWitnessRef'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('ContinuumEvidencePosture', () => { | ||
| it('rejects unknown posture values', () => { | ||
| expect(() => new ContinuumEvidencePosture('fixture-only')).toThrow('Continuum evidence posture'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add missing branch tests to meet slice-complete coverage.
This suite does not currently exercise all touched validation branches (for example empty sourceRuntime/basisRef/summary, and posture normalization when a ContinuumEvidencePosture instance is provided), so the refactor-slice coverage gate is not yet fully satisfied.
✅ Suggested additional tests
+ it('rejects empty required string fields', () => {
+ expect(() => new ContinuumEvidenceStatus({
+ posture: 'translated-substrate',
+ sourceRuntime: '',
+ basisRef: PATCH_BASIS_REF,
+ summary: 'x',
+ })).toThrow('sourceRuntime');
+ });
+
+ it('accepts a posture instance via normalizePosture', () => {
+ const posture = new ContinuumEvidencePosture('translated-substrate');
+ const status = new ContinuumEvidenceStatus({
+ posture,
+ sourceRuntime: 'git-warp',
+ basisRef: PATCH_BASIS_REF,
+ summary: 'ok',
+ });
+ expect(status.posture).toBe(posture);
+ });As per coding guidelines, “**/*.test.{ts,tsx}: Touched code in refactor slices must reach 100% test coverage before the slice is considered done.”
🤖 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 `@test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts` around lines 8 -
63, Add unit tests that cover the missing validation branches in
ContinuumEvidenceStatus and posture normalization in ContinuumEvidencePosture:
1) assert that creating a ContinuumEvidenceStatus with empty/undefined
sourceRuntime, basisRef, or summary throws the expected validation error
messages; 2) assert that passing an existing ContinuumEvidencePosture instance
into ContinuumEvidenceStatus (instead of a string) normalizes correctly (e.g.,
new ContinuumEvidencePosture('continuum-native') used for posture) and preserves
behaviors like isContinuumNative()/isTranslatedSubstrate(); and 3) include tests
for translatedGitWarp() still rejecting a nativeWitnessRef and for
ContinuumEvidencePosture throwing on unknown values—target the
ContinuumEvidenceStatus constructor, translatedGitWarp, and
ContinuumEvidencePosture symbols so all previously-uncovered branches are
exercised.
| const refs = new Map(); | ||
| const readRef = vi.fn(async (ref) => refs.get(ref) || null); | ||
| return { | ||
| readRef: vi.fn().mockResolvedValue(null), | ||
| readRef, | ||
| showNode: vi.fn(), | ||
| writeBlob: vi.fn().mockResolvedValue('d'.repeat(40)), | ||
| writeTree: vi.fn().mockResolvedValue('e'.repeat(40)), | ||
| commitNodeWithTree: vi.fn().mockResolvedValue('f'.repeat(40)), | ||
| updateRef: vi.fn().mockResolvedValue(undefined), | ||
| updateRef: vi.fn(async (ref, sha) => { | ||
| refs.set(ref, sha); | ||
| }), | ||
| compareAndSwapRef: vi.fn(async (ref, newOid, expectedOid) => { | ||
| const current = refs.get(ref) || null; | ||
| if (current !== expectedOid) { | ||
| throw new Error(`CAS mismatch on ${ref}`); | ||
| } | ||
| refs.set(ref, newOid); | ||
| readRef.mockImplementation(async (nextRef) => refs.get(nextRef) || null); | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Reduce this touched test file below the 800 LOC ceiling.
This file is currently over the test-file size limit and should be split before merge. As per coding guidelines, "Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)" and "Use the anti-sludge policy; if a patch compiles but violates the policy, the patch is still wrong."
🤖 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 `@test/unit/domain/services/PatchBuilder.content.test.ts` around lines 31 - 49,
The test file exceeds the 800 LOC limit because a large inline mock/setup block
is expanding the file; extract the reusable mocked repo behavior into a separate
test helper and replace the inline Map+vi.fn mocks with a factory import.
Specifically, move the refs Map and functions (readRef, updateRef,
compareAndSwapRef, showNode, writeBlob, writeTree, commitNodeWithTree) into a
new helper (e.g., createRepoMock) that returns those functions and import it
into PatchBuilder.content.test.ts, then replace the inline declarations with a
call to createRepoMock() to keep the test file under the ceiling and reuse the
mock across spec files.
| const refs = new Map(); | ||
| const readRef = vi.fn(async (ref) => refs.get(ref) || null); | ||
| return { | ||
| readRef: vi.fn().mockResolvedValue(null), | ||
| readRef, | ||
| showNode: vi.fn(), | ||
| writeBlob: vi.fn().mockResolvedValue('a'.repeat(40)), // Valid 40-char hex OID | ||
| writeTree: vi.fn().mockResolvedValue('b'.repeat(40)), | ||
| commitNodeWithTree: vi.fn().mockResolvedValue('c'.repeat(40)), | ||
| updateRef: vi.fn().mockResolvedValue(undefined), | ||
| updateRef: vi.fn(async (ref, sha) => { | ||
| refs.set(ref, sha); | ||
| }), | ||
| compareAndSwapRef: vi.fn(async (ref, newOid, expectedOid) => { | ||
| const current = refs.get(ref) || null; | ||
| if (current !== expectedOid) { | ||
| throw new Error(`CAS mismatch on ${ref}`); | ||
| } | ||
| refs.set(ref, newOid); | ||
| readRef.mockImplementation(async (nextRef) => refs.get(nextRef) || null); | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Split this test suite to comply with the repository test-file size rule.
The file is significantly above the allowed 800 LOC and should be decomposed as part of this touched change. As per coding guidelines, "Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)" and "Use the anti-sludge policy; if a patch compiles but violates the policy, the patch is still wrong."
🤖 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 `@test/unit/domain/services/PatchBuilder.test.ts` around lines 31 - 49, The
test file exceeds the project's test-file size limit; split the large
PatchBuilder.test.ts into smaller suites by grouping related tests (e.g., "write
operations", "commit and refs", "CAS behavior") and moving the shared mock
factory (the Map-backed refs and mock functions readRef, compareAndSwapRef,
updateRef, writeBlob, writeTree, commitNodeWithTree, showNode) into a new test
helper module that each new test file imports; create multiple new test files
that each import the helper, copy only relevant tests into each file, and update
imports/vi mocks to use the centralized mock factory so behavior (especially
compareAndSwapRef’s CAS logic and readRef mockImplementation updates) remains
consistent across the split files.
| const refs = new Map(); | ||
| const readRef = vi.fn(async (ref) => refs.get(ref) || null); | ||
| return { | ||
| readRef: vi.fn(), | ||
| updateRef: vi.fn(), | ||
| readRef, | ||
| updateRef: vi.fn(async (ref, sha) => { | ||
| refs.set(ref, sha); | ||
| }), | ||
| compareAndSwapRef: vi.fn(async (ref, newOid, expectedOid) => { | ||
| const current = refs.get(ref) || null; | ||
| if (current !== expectedOid) { | ||
| throw new Error(`CAS mismatch on ${ref}`); | ||
| } | ||
| refs.set(ref, newOid); | ||
| readRef.mockImplementation(async (nextRef) => refs.get(nextRef) || null); | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Trim or split this touched test file to get under the 800 LOC limit.
This file is slightly over the allowed test-file size, so it still violates policy and should be reduced before merge. As per coding guidelines, "Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)" and "Use the anti-sludge policy; if a patch compiles but violates the policy, the patch is still wrong."
🤖 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 `@test/unit/domain/warp/Writer.test.ts` around lines 24 - 38, The test file
Writer.test.ts exceeds the 800 LOC limit; trim or split it by extracting common
setup (the in-memory refs Map and mocked functions readRef, updateRef,
compareAndSwapRef) into a shared test helper module and move large, independent
test suites into separate test files; specifically factor out the refs Map and
the vi.fn mocks (readRef, updateRef, compareAndSwapRef) into a new helper (e.g.,
writerTestHelpers) and import it from smaller test files, or split long describe
blocks into multiple files so each file stays under 800 LOC while preserving
tests that reference readRef, updateRef, compareAndSwapRef and the refs state.
| const refs = new Map<string, string>(); | ||
| const readRef = vi.fn(async (ref: string) => refs.get(ref) || null); | ||
| return { | ||
| readRef: vi.fn(), | ||
| readRef, | ||
| showNode: vi.fn(), | ||
| writeBlob: vi.fn(), | ||
| writeTree: vi.fn(), | ||
| readBlob: vi.fn(), | ||
| readTreeOids: vi.fn(), | ||
| commitNode: vi.fn(), | ||
| commitNodeWithTree: vi.fn(), | ||
| updateRef: vi.fn(), | ||
| updateRef: vi.fn(async (ref: string, sha: string) => { | ||
| refs.set(ref, sha); | ||
| }), | ||
| compareAndSwapRef: vi.fn(async (ref: string, newOid: string, expectedOid: string | null) => { | ||
| const current = refs.get(ref) || null; | ||
| if (current !== expectedOid) { | ||
| throw new Error(`CAS mismatch on ${ref}`); | ||
| } | ||
| refs.set(ref, newOid); | ||
| readRef.mockImplementation(async (nextRef: string) => refs.get(nextRef) || null); | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Split this test file to satisfy the max test-file size policy.
This touched file is far above the 800 LOC cap and should be decomposed before merge. As per coding guidelines, "Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)" and "Use the anti-sludge policy; if a patch compiles but violates the policy, the patch is still wrong."
🤖 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 `@test/unit/domain/WarpGraph.test.ts` around lines 43 - 64, The test file
WarpGraph.test.ts exceeds the max test-file size and must be split; identify
logical test groups (e.g., unit tests for compareAndSwapRef/readRef behavior,
blob/tree/commit operations, and higher-level WarpGraph scenarios) and move them
into separate files (for example WarpGraph.compareAndSwapRef.test.ts,
WarpGraph.blobTree.test.ts, WarpGraph.integration.test.ts). Extract the common
mock setup (the refs Map, readRef, and the vi.fn implementations such as
compareAndSwapRef, updateRef, writeBlob, writeTree, commitNode,
commitNodeWithTree) into a shared test helper module (e.g., warpGraphTestUtils)
that exports the mocked store and factory functions used by tests, then update
existing tests to import those helpers and only contain the tests for one
concern per file so each new test file stays under the 800 LOC limit; keep
function names readRef, compareAndSwapRef, updateRef, writeBlob, writeTree,
commitNode, commitNodeWithTree, and showNode to match existing references.
|
@codex self-review findings from the branch diff against
Please confirm whether you agree these should be fixed before merge. |
|
Resolved the self-review findings in
Validation after the fix:
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts (1)
8-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the remaining constructor/normalization branch tests to complete this slice.
This suite still misses touched branches in
ContinuumEvidenceStatus: required-string rejection (sourceRuntime,basisRef,summary) and posture-instance normalization (posturepassed asContinuumEvidencePosture).Proposed test additions
describe('ContinuumEvidenceStatus', () => { + it('rejects empty required fields', () => { + expect(() => new ContinuumEvidenceStatus({ + posture: 'participant-runtime', + sourceRuntime: '', + basisRef: PATCH_BASIS_REF, + summary: 'ok', + })).toThrow('sourceRuntime'); + + expect(() => new ContinuumEvidenceStatus({ + posture: 'participant-runtime', + sourceRuntime: 'git-warp', + basisRef: '', + summary: 'ok', + })).toThrow('basisRef'); + + expect(() => new ContinuumEvidenceStatus({ + posture: 'participant-runtime', + sourceRuntime: 'git-warp', + basisRef: PATCH_BASIS_REF, + summary: '', + })).toThrow('summary'); + }); + + it('accepts an existing posture instance', () => { + const posture = new ContinuumEvidencePosture('participant-runtime'); + const status = new ContinuumEvidenceStatus({ + posture, + sourceRuntime: 'git-warp', + basisRef: PATCH_BASIS_REF, + summary: 'instance posture', + }); + expect(status.posture).toBe(posture); + });As per coding guidelines, “
**/*.test.{ts,tsx}: Touched code in refactor slices must reach 100% test coverage before the slice is considered done”.🤖 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 `@test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts` around lines 8 - 57, Add tests in ContinuumEvidenceStatus.test.ts to cover the missing constructor/normalization branches: (1) assert that constructing ContinuumEvidenceStatus throws when required string fields are missing or empty for sourceRuntime, basisRef, and summary (use new ContinuumEvidenceStatus({...}) and expect(...).toThrow with the relevant field name); and (2) assert posture normalization when passing a ContinuumEvidencePosture instance (create a ContinuumEvidencePosture value and pass it as posture to new ContinuumEvidenceStatus and expect status.posture.toString() equals the expected string and isParticipantRuntime/isContinuumWitnessed behave correctly). Reference ContinuumEvidenceStatus and ContinuumEvidencePosture in the new tests.
🧹 Nitpick comments (1)
src/domain/continuum/ContinuumEvidenceStatus.ts (1)
35-41: ⚡ Quick winExtract posture/runtime literals into constants.
Inline literals for posture/runtime and policy messages make drift easier during future renames. Please centralize them as named constants.
Refactor sketch
+const POSTURE_PARTICIPANT_RUNTIME = 'participant-runtime'; +const SOURCE_RUNTIME_GIT_WARP = 'git-warp'; +const ERR_PARTICIPANT_WITH_WITNESS = + 'participant runtime evidence must not carry continuumWitnessRef'; + static gitWarpParticipant(fields: GitWarpParticipantEvidenceFields): ContinuumEvidenceStatus { return new ContinuumEvidenceStatus({ - posture: 'participant-runtime', - sourceRuntime: 'git-warp', + posture: POSTURE_PARTICIPANT_RUNTIME, + sourceRuntime: SOURCE_RUNTIME_GIT_WARP, basisRef: fields.basisRef, summary: fields.summary, }); } ... if (posture.isParticipantRuntime() && continuumWitnessRef !== undefined) { - throw new WarpError('participant runtime evidence must not carry continuumWitnessRef', 'E_VALIDATION'); + throw new WarpError(ERR_PARTICIPANT_WITH_WITNESS, 'E_VALIDATION'); }As per coding guidelines, “Avoid magic strings and numbers; use named constants instead”.
Also applies to: 85-90
🤖 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 `@src/domain/continuum/ContinuumEvidenceStatus.ts` around lines 35 - 41, Extract the repeated literal strings used for evidence posture, sourceRuntime and any policy message strings into named constants and use those constants in the factory methods; specifically replace inline literals in ContinuumEvidenceStatus.gitWarpParticipant and the similar literals around the 85-90 block with constants (e.g., POSTURE_PARTICIPANT_RUNTIME, SOURCE_RUNTIME_GIT_WARP, and any POLICY_* message constants) declared near the top of the ContinuumEvidenceStatus file so all factory methods reference the constants instead of hard-coded strings.
🤖 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.
Duplicate comments:
In `@test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts`:
- Around line 8-57: Add tests in ContinuumEvidenceStatus.test.ts to cover the
missing constructor/normalization branches: (1) assert that constructing
ContinuumEvidenceStatus throws when required string fields are missing or empty
for sourceRuntime, basisRef, and summary (use new ContinuumEvidenceStatus({...})
and expect(...).toThrow with the relevant field name); and (2) assert posture
normalization when passing a ContinuumEvidencePosture instance (create a
ContinuumEvidencePosture value and pass it as posture to new
ContinuumEvidenceStatus and expect status.posture.toString() equals the expected
string and isParticipantRuntime/isContinuumWitnessed behave correctly).
Reference ContinuumEvidenceStatus and ContinuumEvidencePosture in the new tests.
---
Nitpick comments:
In `@src/domain/continuum/ContinuumEvidenceStatus.ts`:
- Around line 35-41: Extract the repeated literal strings used for evidence
posture, sourceRuntime and any policy message strings into named constants and
use those constants in the factory methods; specifically replace inline literals
in ContinuumEvidenceStatus.gitWarpParticipant and the similar literals around
the 85-90 block with constants (e.g., POSTURE_PARTICIPANT_RUNTIME,
SOURCE_RUNTIME_GIT_WARP, and any POLICY_* message constants) declared near the
top of the ContinuumEvidenceStatus file so all factory methods reference the
constants instead of hard-coded strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7b05a23-4623-4725-a26a-0ac21c53636f
📒 Files selected for processing (21)
CHANGELOG.mddocs/BEARING.mddocs/design/0146-v18-continuum-compatibility-charter/v18-continuum-compatibility-charter.mddocs/design/0147-v18-continuum-contract-matrix/v18-continuum-contract-matrix.mddocs/design/0148-v18-warp-optic-realization-map/v18-warp-optic-realization-map.mddocs/design/0150-v18-evidence-posture/v18-evidence-posture.mddocs/design/0153-v18-receipt-family-projection/v18-receipt-family-projection.mddocs/design/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.mddocs/method/retro/0145-push-pr-review-merge/push-pr-review-merge.mddocs/method/retro/0150-v18-evidence-posture/v18-evidence-posture.mddocs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.mddocs/method/retro/0153-v18-receipt-family-projection/v18-receipt-family-projection.mddocs/method/retro/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.mdsrc/domain/continuum/ContinuumEvidencePosture.tssrc/domain/continuum/ContinuumEvidenceStatus.tssrc/domain/warp/PatchSession.tstest/unit/domain/continuum/ContinuumEvidenceStatus.test.tstest/unit/domain/continuum/ContinuumReceiptProjection.test.tstest/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.tstest/unit/domain/index.exports.test.tstest/unit/domain/services/PatchCommitter.visibility.test.ts
✅ Files skipped from review due to trivial changes (8)
- docs/method/retro/0150-v18-evidence-posture/v18-evidence-posture.md
- docs/method/retro/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.md
- docs/method/retro/0145-push-pr-review-merge/push-pr-review-merge.md
- CHANGELOG.md
- docs/design/0148-v18-warp-optic-realization-map/v18-warp-optic-realization-map.md
- docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md
- docs/BEARING.md
- docs/design/0153-v18-receipt-family-projection/v18-receipt-family-projection.md
Summary
This advances the v18 Continuum compatibility work through the first five execution slices after the opening PR:
TickReceiptfacts into the Continuum receipt family shape while keeping evidence posture separate.warp-ttd-oriented smoke test against the generated Continuum receipt-family artifact fixture.The language and modeling keep git-warp as an independent Continuum participant. Continuum is treated as a protocol for exchanging witnessed causal history.
Validation
npm run test:localnpm run test:coverage:cinpm run lintnpm run typecheck:src -- --pretty falsenpm run typecheck:test -- --pretty falsenpm run typecheck:policynpm run typecheck:surfacenpm run lint:sludgenpm run lint:quarantine-graduatenpm run lint:mdgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation