v18 Continuum opening campaign#91
Open
flyingrobots wants to merge 9 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds v18 Continuum compatibility docs, domain types and validators for artifact authority/family/descriptor, a JSON adapter converting Wesley manifests/fixtures into validated ContinuumArtifactDescriptor, an ingestion policy that gates acceptance, public exports, fixtures, and unit tests. ChangesV18 Continuum Artifact Ingestion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Member
Author
|
@codex Self-review pass after clean-worktree check and Issue count: Critical 0, Major 3, Minor 2.
|
Member
Author
|
@coderabbitai review please |
🧩 Analysis chainI'll gather the relevant files before starting the full review. 🏁 Script executed: Length of output: 57081 🏁 Script executed: Length of output: 7949
Review: v18 Continuum Opening Campaign✅ All five self-review items confirmed🔴 Major 1 — Shape and authority are not bound togetherConfirmed. // ContinuumArtifactAuthority.ts
isGeneratedAuthority(): boolean {
return (
this.value === GENERATED_ARTIFACT_AUTHORITY || // 'generated-artifact'
this.value === GENERATED_FIXTURE_AUTHORITY // 'generated-fixture'
);
}
The intended invariant — that Wesley manifests carry 🔴 Major 2 — Caller context can override parsed artifact kindConfirmed. Both parsing branches delegate // wesleyManifestFields() [~line 131]
artifactKind: context.artifactKind ?? WESLEY_REALIZATION_MANIFEST_KIND,
// parseContinuumFamilyFixture() [~line 161]
artifactKind: context.artifactKind ?? CONTINUUM_FIXTURE_KIND,The parsed shape determines which branch runs, but the emitted descriptor's 🔴 Major 3 — Wesley generated inventory can be empty or internally inconsistentConfirmed, with a nuance. // readGeneratedLegs() [~line 258]
const names = Object.freeze(Object.keys(generatedLegs).sort()); // {} → []
// No minimum-leg check followsSeveral existing test fixtures that use The
🟡 Minor 4 — Machine-specific absolute path in fixtureConfirmed. "canonicalSchemaPath": "/Users/james/git/continuum/schemas/continuum-receipt-family.graphql"The field is 🟡 Minor 5 — Verification prose documents a known-nonzero exit commandConfirmed. The design doc at lines 109–115 states the targeted Additional observation —
|
| # | Severity | Status |
|---|---|---|
| 1 | Major | Confirmed — generated-artifact / generated-fixture are both admitted for any shape; policy has no shape-authority coupling |
| 2 | Major | Confirmed — context.artifactKind can produce a descriptor whose artifactKind contradicts the parsed JSON shape |
| 3 | Major | Confirmed — empty generatedLegs: {} passes silently; positive artifactCount is never cross-checked against actual files.length |
| 4 | Minor | Confirmed — absolute workstation path in shared fixture |
| 5 | Minor | Confirmed — nonzero-exit command in verification prose |
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/continuum/ContinuumArtifactIngestionPolicy.ts`:
- Around line 13-19: assertGeneratedAuthority currently only checks
descriptor.hasGeneratedAuthority() and therefore allows mismatched
kind/authority pairs; update it to validate that the descriptor's actual
authority exactly matches the expected generated authority for its shape/kind.
In practice, inside assertGeneratedAuthority use ContinuumArtifactDescriptor's
identifying property (e.g., kind or shape) to determine the expected generated
authority (compute or call a helper like
expectedGeneratedAuthorityForKind(kind)) and then check both
descriptor.hasGeneratedAuthority() and
descriptor.authority.equals(expectedAuthority); if either fails throw the
existing ContinuumArtifactAuthorityError with a message that includes
descriptor.familyId, descriptor.kind (or shape) and the actual vs expected
authority so mismatches like Wesley manifest + generated-fixture or fixture +
generated-artifact are rejected. Ensure you reference and update
assertGeneratedAuthority, ContinuumArtifactDescriptor, and the thrown
ContinuumArtifactAuthorityError only.
In `@src/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.ts`:
- Around line 133-135: The parsed JSON must determine artifactKind and authority
rather than letting context override them: in ContinuumArtifactJsonFileAdapter's
JSON parsing logic (the block that sets generatedBy, artifactKind, targets using
readStringArray/readString), read the artifactKind and authority from the source
JSON shape and bind artifactKind to that value (do not default to
context.artifactKind), and validate that context.authority (if provided) matches
the parsed authority; if they mismatch, throw a validation/error. Apply the same
enforcement where similar parsing occurs (the other parsing blocks around the
indicated ranges 158-163 and 166-177).
- Around line 258-303: readGeneratedLegs currently accepts empty generatedLegs
and doesn't validate artifactCount against the actual files, allowing
inconsistent sealed manifests; update readGeneratedLegs (and readGeneratedFiles
usage) so that after calling readOptionalNumber(leg, 'artifactCount') and
collecting files via readGeneratedFiles(leg, name) you validate: if
artifactCount is present and not equal to files.length throw an
AdapterValidationError with a clear message referencing the leg name, and
additionally reject an entirely empty generatedLegs map by throwing an
AdapterValidationError when names.length === 0; use the existing
AdapterValidationError type and the functions readOptionalNumber,
readGeneratedFiles, and rejectUnknownKeys to locate where to insert these checks
in readGeneratedLegs.
In `@test/fixtures/continuum/receipt-family-wesley-realization-manifest.json`:
- Line 4: The fixture contains a machine-specific absolute "canonicalSchemaPath"
value which breaks portability; edit the
receipt-family-wesley-realization-manifest.json fixture to either remove the
optional "canonicalSchemaPath" field entirely or replace its value with a
repo-relative path (e.g., "./schemas/continuum-receipt-family.graphql") so the
fixture is consistent across contributors/CI and no host-specific absolute path
remains.
In `@test/unit/domain/continuum/ContinuumArtifactIngestionPolicy.test.ts`:
- Line 14: The test uses an environment-specific fixture value for
sourceSchemaPath ('~/git/continuum/...') which should be replaced with a
repo-neutral constant; update the test ContinuumArtifactIngestionPolicy.test.ts
to import or reference the shared test fixture constant (e.g.,
SCHEMA_FIXTURE_PATH or a suite-level RELATIVE_SCHEMA_PATH) and replace all
occurrences of sourceSchemaPath in the file (including the other instances
noted) with that constant so the path is repo-relative and portable across
environments.
In `@test/unit/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.test.ts`:
- Around line 276-349: Add three regression tests to the
ContinuumArtifactJsonFileAdapter test suite: (1) create a fixture JSON loaded
via ContinuumArtifactJsonFileAdapter.loadString with authority
'generated-artifact' and attempt to load a Wesley manifest JSON with authority
'generated-fixture' using loadString, asserting it throws AdapterValidationError
(shape-binding/authority mismatch); (2) add a test that constructs a sealed
Wesley manifest where manifest.artifactCount !== manifest.files.length and
assert ContinuumArtifactJsonFileAdapter.loadString throws AdapterValidationError
(ensure explicit artifactCount must match files.length); and (3) add a test that
creates a sealed Wesley manifest with generatedLegs = [] and assert loadString
throws AdapterValidationError (empty generatedLegs rejected for sealed
manifests). Use existing fixture JSON constants and the
ContinuumArtifactJsonFileAdapter class and its loadString/loadFile calls to
locate where to add these tests.
🪄 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: 47103933-1949-48c5-89f1-9178d67e2899
⛔ Files ignored due to path filters (4)
docs/design/0124-v17-release-blocker-dag.dotis excluded by!**/*.dotdocs/design/0124-v17-release-blocker-dag.svgis excluded by!**/*.svgdocs/design/0124-v17-release-blocker-status.csvis excluded by!**/*.csvdocs/design/continuum-categories.pdfis excluded by!**/*.pdf
📒 Files selected for processing (30)
docs/BEARING.mddocs/VISION.mddocs/design/0124-v17-release-blocker-dag.mddocs/design/0145-push-pr-review-merge/push-pr-review-merge.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/0149-v18-continuum-artifact-ingestion/v18-continuum-artifact-ingestion.mddocs/design/continuum-categories.texdocs/method/backlog/README.mddocs/method/backlog/WORKLOADS.mddocs/method/backlog/bad-code/RELEASE_TRIAGE.mddocs/method/backlog/v18.0.0/PROTO_echo-shaped-edge-records.mddocs/method/backlog/v18.0.0/PROTO_echo-shaped-node-records.mddocs/method/backlog/v18.0.0/README.mddocs/method/retro/0145-push-pr-review-merge/push-pr-review-merge.mdindex.tssrc/domain/continuum/ContinuumArtifactAuthority.tssrc/domain/continuum/ContinuumArtifactDescriptor.tssrc/domain/continuum/ContinuumArtifactIngestionPolicy.tssrc/domain/continuum/ContinuumFamilyId.tssrc/domain/errors/ContinuumArtifactAuthorityError.tssrc/domain/errors/index.tssrc/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.tstest/fixtures/continuum/receipt-family-generated-artifact.jsontest/fixtures/continuum/receipt-family-wesley-realization-manifest.jsontest/unit/domain/continuum/ContinuumArtifactIngestionPolicy.test.tstest/unit/domain/errors/index.test.tstest/unit/domain/index.exports.test.tstest/unit/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.test.ts
Comment on lines
+13
to
+19
| assertGeneratedAuthority(descriptor: ContinuumArtifactDescriptor): void { | ||
| if (descriptor.hasGeneratedAuthority()) { | ||
| return; | ||
| } | ||
| throw new ContinuumArtifactAuthorityError( | ||
| `Continuum family ${descriptor.familyId.toString()} must be loaded from a generated artifact or fixture, not ${descriptor.authority.toString()}`, | ||
| ); |
There was a problem hiding this comment.
Authority validation is too coarse and permits kind/authority mismatches.
Current policy accepts any “generated” authority and does not enforce the expected authority for each descriptor shape/kind. This can admit contradictory descriptors (e.g., Wesley manifest + generated-fixture, Continuum fixture + generated-artifact).
💡 Proposed tightening
+const EXPECTED_AUTHORITY_BY_KIND = Object.freeze({
+ 'wesley.realization.manifest.v1': 'generated-artifact',
+ 'continuum.family.fixture': 'generated-fixture',
+} as const);
+
export default class ContinuumArtifactIngestionPolicy {
ingest(descriptor: ContinuumArtifactDescriptor): ContinuumArtifactDescriptor {
- this.assertGeneratedAuthority(descriptor);
+ this.assertGeneratedAuthority(descriptor);
return descriptor;
}
assertGeneratedAuthority(descriptor: ContinuumArtifactDescriptor): void {
- if (descriptor.hasGeneratedAuthority()) {
- return;
- }
- throw new ContinuumArtifactAuthorityError(
- `Continuum family ${descriptor.familyId.toString()} must be loaded from a generated artifact or fixture, not ${descriptor.authority.toString()}`,
- );
+ const expected = EXPECTED_AUTHORITY_BY_KIND[
+ descriptor.artifactKind as keyof typeof EXPECTED_AUTHORITY_BY_KIND
+ ];
+ if (expected === undefined || descriptor.authority.toString() !== expected) {
+ throw new ContinuumArtifactAuthorityError(
+ `Continuum family ${descriptor.familyId.toString()} requires authority ${expected ?? 'known-kind-authority'} for ${descriptor.artifactKind}, not ${descriptor.authority.toString()}`,
+ );
+ }
}
}🤖 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/ContinuumArtifactIngestionPolicy.ts` around lines 13 -
19, assertGeneratedAuthority currently only checks
descriptor.hasGeneratedAuthority() and therefore allows mismatched
kind/authority pairs; update it to validate that the descriptor's actual
authority exactly matches the expected generated authority for its shape/kind.
In practice, inside assertGeneratedAuthority use ContinuumArtifactDescriptor's
identifying property (e.g., kind or shape) to determine the expected generated
authority (compute or call a helper like
expectedGeneratedAuthorityForKind(kind)) and then check both
descriptor.hasGeneratedAuthority() and
descriptor.authority.equals(expectedAuthority); if either fails throw the
existing ContinuumArtifactAuthorityError with a message that includes
descriptor.familyId, descriptor.kind (or shape) and the actual vs expected
authority so mismatches like Wesley manifest + generated-fixture or fixture +
generated-artifact are rejected. Ensure you reference and update
assertGeneratedAuthority, ContinuumArtifactDescriptor, and the thrown
ContinuumArtifactAuthorityError only.
| function makeDescriptor(authority: string): ContinuumArtifactDescriptor { | ||
| return new ContinuumArtifactDescriptor({ | ||
| familyId: 'receipt-family', | ||
| sourceSchemaPath: '~/git/continuum/schemas/continuum-receipt-family.graphql', |
There was a problem hiding this comment.
Use a repo-neutral schema path fixture string.
The ~/git/continuum/... path is environment-shaped and makes fixture semantics less portable. Replace with a stable repo-relative token/path constant used across the suite.
🧩 Suggested cleanup
+const RECEIPT_SCHEMA_PATH = 'schemas/continuum/continuum-receipt-family.graphql';
+
function makeDescriptor(authority: string): ContinuumArtifactDescriptor {
return new ContinuumArtifactDescriptor({
familyId: 'receipt-family',
- sourceSchemaPath: '~/git/continuum/schemas/continuum-receipt-family.graphql',
+ sourceSchemaPath: RECEIPT_SCHEMA_PATH,
@@
- sourceSchemaPath: '~/git/continuum/schemas/continuum-receipt-family.graphql',
+ sourceSchemaPath: RECEIPT_SCHEMA_PATH,
@@
- sourceSchemaPath: '~/git/continuum/schemas/continuum-receipt-family.graphql',
+ sourceSchemaPath: RECEIPT_SCHEMA_PATH,Also applies to: 77-77, 104-104, 113-113, 135-135, 145-145
🤖 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/ContinuumArtifactIngestionPolicy.test.ts` at line
14, The test uses an environment-specific fixture value for sourceSchemaPath
('~/git/continuum/...') which should be replaced with a repo-neutral constant;
update the test ContinuumArtifactIngestionPolicy.test.ts to import or reference
the shared test fixture constant (e.g., SCHEMA_FIXTURE_PATH or a suite-level
RELATIVE_SCHEMA_PATH) and replace all occurrences of sourceSchemaPath in the
file (including the other instances noted) with that constant so the path is
repo-relative and portable across environments.
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.ts (1)
1-542: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this adapter to satisfy source size and cohesion policy.
This module is over the source LOC cap and currently combines multiple peer concerns (context boundary validation, shape parsing, Wesley leg inventory parsing, and descriptor field assembly) in a single file. Please split it into smaller modules before merge.
As per coding guidelines
Maximum file size: 500 LOC (source), 800 LOC (test), 300 LOC (scripts)andsrc/**/*.{ts,tsx}: One file per class, type, or object; split files that accumulate peer concepts.🤖 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/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.ts` around lines 1 - 542, The file exceeds the LOC/cohesion policy by bundling multiple concerns; split the adapter into focused modules: keep ContinuumArtifactJsonFileAdapter and its public methods in one file (ContinuumArtifactJsonFileAdapter with loadFile/loadString), move parsing entry-points and high-level dispatch (parseJson, parseDescriptorFields, parseWesleyRealizationManifest, parseContinuumFamilyFixture, descriptorFields) into a separate parser module, and extract Wesley-specific inventory logic (readSealedIntegrity, readGeneratedLegs, readGeneratedLegNames, readGeneratedLegFiles, validateGeneratedLegEnvelope, readGeneratedFiles) and the various JSON validation helpers (requireJsonObject, isJsonObject, rejectUnknownKeys, readRequiredString/readOptionalString/readStringArray/etc.) into their own validator/inventory modules; ensure exports/imports wire the modules back into ContinuumArtifactJsonFileAdapter and update references to symbols like ContinuumArtifactIngestionPolicy, ContinuumArtifactDescriptorFields, WESLEY_REALIZATION_MANIFEST_KIND, CONTINUUM_FIXTURE_KIND, and LOAD_CONTEXT_KEYS so tests and policy ingestion behavior remain unchanged.
🤖 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.
Outside diff comments:
In `@src/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.ts`:
- Around line 1-542: The file exceeds the LOC/cohesion policy by bundling
multiple concerns; split the adapter into focused modules: keep
ContinuumArtifactJsonFileAdapter and its public methods in one file
(ContinuumArtifactJsonFileAdapter with loadFile/loadString), move parsing
entry-points and high-level dispatch (parseJson, parseDescriptorFields,
parseWesleyRealizationManifest, parseContinuumFamilyFixture, descriptorFields)
into a separate parser module, and extract Wesley-specific inventory logic
(readSealedIntegrity, readGeneratedLegs, readGeneratedLegNames,
readGeneratedLegFiles, validateGeneratedLegEnvelope, readGeneratedFiles) and the
various JSON validation helpers (requireJsonObject, isJsonObject,
rejectUnknownKeys, readRequiredString/readOptionalString/readStringArray/etc.)
into their own validator/inventory modules; ensure exports/imports wire the
modules back into ContinuumArtifactJsonFileAdapter and update references to
symbols like ContinuumArtifactIngestionPolicy,
ContinuumArtifactDescriptorFields, WESLEY_REALIZATION_MANIFEST_KIND,
CONTINUUM_FIXTURE_KIND, and LOAD_CONTEXT_KEYS so tests and policy ingestion
behavior remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b8254e4-902d-4933-b78d-eef97c5f8fb8
📒 Files selected for processing (5)
docs/BEARING.mddocs/design/0149-v18-continuum-artifact-ingestion/v18-continuum-artifact-ingestion.mdsrc/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.tstest/fixtures/continuum/receipt-family-wesley-realization-manifest.jsontest/unit/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/fixtures/continuum/receipt-family-wesley-realization-manifest.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
authorityfields and local/handwritten mirror authority.Supersedes #90; this PR replaces the prefixed branch with
v18-continuum-opening.Validation
npm run lintnpm run typecheck:srcnpm run typecheck:testnpm run typecheck:surfacenpm run typecheck:policynpm run lint:sludgenpm run lint:quarantine-graduatenpx --yes markdownlint-cli docs/BEARING.md docs/design/0146-v18-continuum-compatibility-charter/v18-continuum-compatibility-charter.md docs/design/0149-v18-continuum-artifact-ingestion/v18-continuum-artifact-ingestion.mdnpx vitest run test/unit/domain/continuum/ContinuumArtifactIngestionPolicy.test.ts test/unit/infrastructure/adapters/ContinuumArtifactJsonFileAdapter.test.ts test/unit/domain/index.exports.test.ts test/unit/domain/errors/index.test.tsnpm run test:local(447 files, 6821 tests)npm run test:coverage:ci(92.14% lines; threshold 91.90%)Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests