Skip to content

Advance v18 evidence and receipt projection#92

Open
flyingrobots wants to merge 7 commits into
mainfrom
v18-evidence-receipt-projection
Open

Advance v18 evidence and receipt projection#92
flyingrobots wants to merge 7 commits into
mainfrom
v18-evidence-receipt-projection

Conversation

@flyingrobots
Copy link
Copy Markdown
Member

@flyingrobots flyingrobots commented May 22, 2026

Summary

This advances the v18 Continuum compatibility work through the first five execution slices after the opening PR:

  • Add a domain-level Continuum evidence posture/status model and export it.
  • Enforce writer patch commit visibility by advancing writer refs through CAS and rereading the visible tip.
  • Add a same-writer race witness showing one winner, one CAS conflict, and deterministic materialization.
  • Project git-warp TickReceipt facts into the Continuum receipt family shape while keeping evidence posture separate.
  • Add a warp-ttd-oriented smoke test against the generated Continuum receipt-family artifact fixture.
  • Align older test fixtures and mocks with CAS ref visibility semantics.

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:local
  • npm run test:coverage:ci
  • npm run lint
  • npm run typecheck:src -- --pretty false
  • npm run typecheck:test -- --pretty false
  • npm run typecheck:policy
  • npm run typecheck:surface
  • npm run lint:sludge
  • npm run lint:quarantine-graduate
  • npm run lint:md
  • git diff --check
  • pre-push IRONCLAD M9 hook: link check, static gates, consumer typecheck, surface validator, Markdown gates, and all unit tests

Summary by CodeRabbit

  • New Features

    • Introduced Continuum evidence posture/status models, receipt-family projection and projector, and package exports for these artifacts.
    • Commit flow now enforces atomic writer-ref advancement with post-CAS visibility verification.
  • Bug Fixes

    • Strengthened patch commit visibility guarantees and surfaced visibility failures with a clear error code.
  • Tests

    • Added same-writer race unit test, projector/smoke tests, and visibility/committer CAS coverage.
  • Documentation

    • Extensive design and retro docs updating evidence posture, patch visibility, receipt projection, and compatibility guidance.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

📝 Walkthrough

Walkthrough

This 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.

Changes

V18 Evidence Posture, Patch Commit Visibility, and Receipt Projection

Layer / File(s) Summary
Evidence Posture and Status Domain Models
src/domain/continuum/ContinuumEvidencePosture.ts, src/domain/continuum/ContinuumEvidenceStatus.ts
Adds ContinuumEvidencePosture and ContinuumEvidenceStatus with posture/witness invariants, normalization helpers, gitWarpParticipant factory, and immutability enforcement.
Patch Commit Visibility Contract via CAS
src/domain/services/PatchCommitter.ts, src/domain/errors/WriterError.ts, docs/design/0151-v18-patch-commit-visibility/...
commitPatch advances writer ref using compareAndSwapRef, converts CAS failures into WriterError with expectedSha/actualSha, re-reads the writer ref to assert it equals the returned commit SHA, and throws WRITER_COMMIT_NOT_VISIBLE if not. Tests and design doc record prior RED behavior and the new algorithm.
Receipt Family Projection
src/domain/continuum/ContinuumReceipt.ts, src/domain/continuum/ContinuumReceiptFamilyProjection.ts, src/domain/continuum/ContinuumReceiptProjector.ts, docs/design/0153-v18-receipt-family-projection/...
Implements ContinuumReceipt with runtime validation and immutability, ContinuumReceiptFamilyProjection with artifact conformance and receiptsForHead, and ContinuumReceiptProjector mapping TickReceiptContinuumReceipt (counts admitted/rejected ops, computes input/output ticks).
Package Entry Point Exports
index.ts
Exposes new Continuum evidence/receipt symbols and associated types from the package entry point.
Test Persistence Mock Enhancements
test/helpers/WarpGraphMockPersistence.ts, various test/*
Mocks now maintain in-memory ref state and implement compareAndSwapRef with CAS semantics; tests updated to seed refs via updateRef where appropriate.
New Test Cases and Coverage
test/unit/domain/WarpGraph.sameWriterRace.test.ts, test/unit/domain/services/PatchCommitter.visibility.test.ts, test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts, test/unit/domain/continuum/ContinuumReceiptProjection.test.ts, test/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.ts, test/unit/domain/index.exports.test.ts
Adds same-writer race witness, visibility contract tests, evidence posture and receipt projection unit tests, smoke test wiring projection/evidence posture, and entry-point export assertions.
Design and Completion Documentation
docs/BEARING.md, docs/design/0150-0154/..., docs/method/retro/...
Updates BEARING and adds design/retro entries recording completed tasks, RED/GREEN playback, and SSJS scorecards for the v18 slices.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A rabbit scribbles in the log,
CAS locks click, commits clear fog,
Receipts parade from tick to fact,
Witness rules kept tidy and exact,
Hops of tests make race-claims nod.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 69.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Advance v18 evidence and receipt projection' directly and concisely summarizes the main change: advancing v18 Continuum compatibility work through evidence posture modeling, receipt projection, and visibility enforcement.
Description check ✅ Passed The PR description includes a detailed Summary covering all major changes, a Validation section listing comprehensive test/lint/typecheck commands, and addresses the required template structure, though ADR checks section is missing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v18-evidence-receipt-projection

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

❤️ Share

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

@github-actions
Copy link
Copy Markdown

Release Preflight

  • package version: 17.0.0
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If you tag this commit as v17.0.0, release workflow will publish.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 lift

Split 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 value

Clarify "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 win

Add 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 win

Replace 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d4a0d6 and 5028440.

📒 Files selected for processing (40)
  • docs/BEARING.md
  • docs/design/0150-v18-evidence-posture/v18-evidence-posture.md
  • docs/design/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.md
  • docs/design/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md
  • docs/design/0153-v18-receipt-family-projection/v18-receipt-family-projection.md
  • docs/design/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.md
  • docs/method/retro/0150-v18-evidence-posture/v18-evidence-posture.md
  • docs/method/retro/0151-v18-patch-commit-visibility/v18-patch-commit-visibility.md
  • docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md
  • docs/method/retro/0153-v18-receipt-family-projection/v18-receipt-family-projection.md
  • docs/method/retro/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.md
  • index.ts
  • src/domain/continuum/ContinuumEvidencePosture.ts
  • src/domain/continuum/ContinuumEvidenceStatus.ts
  • src/domain/continuum/ContinuumReceipt.ts
  • src/domain/continuum/ContinuumReceiptFamilyProjection.ts
  • src/domain/continuum/ContinuumReceiptProjector.ts
  • src/domain/errors/WriterError.ts
  • src/domain/services/PatchCommitter.ts
  • test/benchmark/detachedReadBenchmark.fixture.ts
  • test/helpers/WarpGraphMockPersistence.ts
  • test/unit/domain/WarpCore.snapshotHashStability.test.ts
  • test/unit/domain/WarpGraph.conflicts.test.ts
  • test/unit/domain/WarpGraph.invalidation.test.ts
  • test/unit/domain/WarpGraph.observerBoundary.test.ts
  • test/unit/domain/WarpGraph.patchCount.test.ts
  • test/unit/domain/WarpGraph.sameWriterRace.test.ts
  • test/unit/domain/WarpGraph.strands.test.ts
  • test/unit/domain/WarpGraph.test.ts
  • test/unit/domain/WarpGraph.worldline.test.ts
  • test/unit/domain/WarpGraph.writerInvalidation.test.ts
  • test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts
  • test/unit/domain/continuum/ContinuumReceiptProjection.test.ts
  • test/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.ts
  • test/unit/domain/index.exports.test.ts
  • test/unit/domain/services/PatchBuilder.cas.test.ts
  • test/unit/domain/services/PatchBuilder.content.test.ts
  • test/unit/domain/services/PatchBuilder.test.ts
  • test/unit/domain/services/PatchCommitter.visibility.test.ts
  • test/unit/domain/warp/Writer.test.ts

Comment on lines +185 to +194
} 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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
} 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.

Comment on lines +199 to +202
throw new WriterError(
'WRITER_COMMIT_NOT_VISIBLE',
`Commit ${options.newCommitSha} was written but ${options.writerRef} points at ${visibleSha ?? '(none)'}`,
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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.

Comment on lines +8 to +63
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');
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Comment on lines +31 to +49
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);
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Comment on lines +31 to +49
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);
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Comment on lines +24 to +38
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);
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Comment on lines +43 to +64
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);
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

@flyingrobots
Copy link
Copy Markdown
Member Author

@codex self-review findings from the branch diff against origin/main:

Severity File / Lines Infraction Evidence Recommended mitigation prompt
P1 Major src/domain/continuum/ContinuumEvidencePosture.ts:3, src/domain/continuum/ContinuumEvidenceStatus.ts:36-39, docs/BEARING.md:181-186, test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts:9-19 Protocol vocabulary / product semantics drift The new public posture is literally translated-substrate, with docs/tests describing git-warp evidence as translated substrate evidence. That reintroduces the substrate/cold-runtime framing we explicitly stopped: git-warp is a complete Continuum participant, not a lesser translated substrate below Echo. Replace the evidence posture vocabulary with participant-safe terms. Update the runtime class names/method names/docs/tests so git-warp evidence is modeled as git-warp participant evidence or generated-family participant evidence, while still distinguishing it from separately certified Continuum-native witness refs. Remove translated-substrate and translated substrate language from the new public API and BEARING updates.
P2 Medium src/domain/services/PatchCommitter.ts:197-202, src/domain/warp/PatchSession.ts:74-83, test/unit/domain/services/PatchCommitter.visibility.test.ts:82-90 Error contract leak through higher-level Writer API PatchCommitter now throws WRITER_COMMIT_NOT_VISIBLE, but PatchSession._classifyCommitError() only preserves WRITER_CAS_CONFLICT; every other commit error becomes PERSIST_WRITE_FAILED. The only visibility test calls PatchBuilder.commit() directly, so Writer.beginPatch().commit() and Writer.commitPatch() can mask the new semantic error. Add a PatchSession/Writer regression using a post-CAS-drift adapter, then update _classifyCommitError() to preserve WRITER_COMMIT_NOT_VISIBLE with expected/actual ref context instead of collapsing it into PERSIST_WRITE_FAILED.
P3 Minor docs/BEARING.md:57-64 Documentation accuracy in a touched signpost The branch updates BEARING tasks 6-10 but leaves the "Current branch state" block claiming branch v18-continuum-opening, origin/main at 5afdd3eb, and latest closed cycle 0145-push-pr-review-merge. This PR added cycles 0150-0154 on v18-evidence-receipt-projection after origin/main advanced to 6d4a0d6. Update BEARING's current branch-state block as part of the same signpost edit, or remove volatile branch/SHA data from BEARING if it is expected to stale immediately.

Please confirm whether you agree these should be fixed before merge.

@flyingrobots
Copy link
Copy Markdown
Member Author

Resolved the self-review findings in 81cd8320.

Finding Resolution Evidence
P1 Major: translated-substrate / substrate-native framing Replaced the public evidence posture vocabulary with participant-runtime and continuum-witnessed; renamed the git-warp factory/methods/field from translated/native wording to participant/witness wording; updated BEARING, design notes, retros, tests, and CHANGELOG. rg over forbidden terms returned no matches outside ignored build deps; focused Continuum evidence/projection smoke tests passed.
P2 Medium: WRITER_COMMIT_NOT_VISIBLE masked by Writer/PatchSession Added a Writer-level post-CAS visibility regression and preserved WRITER_COMMIT_NOT_VISIBLE in PatchSession._classifyCommitError(). test/unit/domain/services/PatchCommitter.visibility.test.ts; focused suite passed.
P3 Minor: stale BEARING branch-state block Updated BEARING to branch v18-evidence-receipt-projection, origin/main 6d4a0d6, and latest closed cycle 0154-v18-warp-ttd-receipt-smoke. docs/BEARING.md; Markdown lint passed.

Validation after the fix:

  • npx vitest run test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts test/unit/domain/continuum/ContinuumReceiptProjection.test.ts test/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.ts test/unit/domain/index.exports.test.ts test/unit/domain/services/PatchCommitter.visibility.test.ts -> 5 files / 63 tests passed.
  • npm run typecheck:src -- --pretty false
  • npm run typecheck:test -- --pretty false
  • npm run lint
  • npm run lint:md
  • npm run lint:md:code
  • npm run lint:sludge
  • npm run lint:contamination
  • npm run typecheck:policy
  • npm run typecheck:consumer
  • npm run typecheck:surface
  • npm run lint:quarantine-graduate
  • git diff --check
  • Pre-push hook passed: link check, Gates 1-7, and npm run test:local -> 452 files / 6,841 tests passed.

@github-actions
Copy link
Copy Markdown

Release Preflight

  • package version: 17.0.0
  • prerelease: false
  • npm dist-tag on release: latest
  • npm pack dry-run: passed
  • jsr publish dry-run: passed

If you tag this commit as v17.0.0, release workflow will publish.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts (1)

8-57: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add 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 (posture passed as ContinuumEvidencePosture).

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 win

Extract 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5028440 and 81cd832.

📒 Files selected for processing (21)
  • CHANGELOG.md
  • docs/BEARING.md
  • docs/design/0146-v18-continuum-compatibility-charter/v18-continuum-compatibility-charter.md
  • docs/design/0147-v18-continuum-contract-matrix/v18-continuum-contract-matrix.md
  • docs/design/0148-v18-warp-optic-realization-map/v18-warp-optic-realization-map.md
  • docs/design/0150-v18-evidence-posture/v18-evidence-posture.md
  • docs/design/0153-v18-receipt-family-projection/v18-receipt-family-projection.md
  • docs/design/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
  • docs/method/retro/0150-v18-evidence-posture/v18-evidence-posture.md
  • docs/method/retro/0152-v18-same-writer-race-witness/v18-same-writer-race-witness.md
  • docs/method/retro/0153-v18-receipt-family-projection/v18-receipt-family-projection.md
  • docs/method/retro/0154-v18-warp-ttd-receipt-smoke/v18-warp-ttd-receipt-smoke.md
  • src/domain/continuum/ContinuumEvidencePosture.ts
  • src/domain/continuum/ContinuumEvidenceStatus.ts
  • src/domain/warp/PatchSession.ts
  • test/unit/domain/continuum/ContinuumEvidenceStatus.test.ts
  • test/unit/domain/continuum/ContinuumReceiptProjection.test.ts
  • test/unit/domain/continuum/WarpTtdReceiptFamilySmoke.test.ts
  • test/unit/domain/index.exports.test.ts
  • test/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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant