Release v18.1.0 prep and reify Optic runtime noun - #666
Conversation
Reify Optic as a first-class runtime read-intent noun for node, node-property, neighborhood, and traversal reads. Add coordinate, aperture, basis, and support posture classes; lower fluent WorldlineOptic scopes into frozen Optic values; and require checkpoint-tail execution to consume the reified target and support posture. Export the new runtime nouns, add consumer/export/runtime-noun tests, update the v18.1.0 release evidence, and move public docs from target-language to transition-language for the implemented Optic surface. Refs #665
Drop unit-script tests that assert README and documentation corpus wording/shape instead of runtime behavior. The stable local suite now covers behavior without requiring README phrasing to preserve implementation-detail assertions.
Remove Gate D suffix exchange from the v18.1.0 active goalpost language and record that #663 is closed as a release-scope correction, not as proof of live Continuum Gate D interop. Continuum #33 and Echo #591 remain the live stack gates for bidirectional network suffix exchange evidence. Closes #663
Fix the package-root consumer smoke test after reified Optic added ContinuumEvidencePosture usage.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesOptic Reification and Fluent Read Chain Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/domain/services/optic/Optic.ts (1)
239-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract optic-kind literals into named constants.
supportRuleMatchesTargetandisExactEntityOpticKindcurrently hardcode optic-kind strings; centralizing them as constants reduces typo/drift risk as the optic surface evolves.♻️ Proposed refactor
+const NODE_OPTIC_KIND = 'node'; +const NODE_PROPERTY_OPTIC_KIND = 'node-property'; +const NEIGHBORHOOD_OPTIC_KIND = 'neighborhood'; +const TRAVERSAL_OPTIC_KIND = 'traversal'; + function supportRuleMatchesTarget( opticKind: OpticKindValue, supportRule: OpticSupportRule, ): boolean { if (isExactEntityOpticKind(opticKind)) { return supportRule.isExactEntity(); } - if (opticKind === 'neighborhood') { + if (opticKind === NEIGHBORHOOD_OPTIC_KIND) { return supportRule.isNeighborhood(); } - if (opticKind === 'traversal') { + if (opticKind === TRAVERSAL_OPTIC_KIND) { return isTraversalSupportRule(supportRule); } return false; } function isExactEntityOpticKind(opticKind: OpticKindValue): boolean { - return opticKind === 'node' || opticKind === 'node-property'; + return opticKind === NODE_OPTIC_KIND || opticKind === NODE_PROPERTY_OPTIC_KIND; }As per coding guidelines, "Use named constants instead of magic strings or numbers in TypeScript code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/services/optic/Optic.ts` around lines 239 - 253, Extract the hardcoded optic-kind string literals into named constants to reduce typo and drift risk. In the functions supportRuleMatchesTarget and isExactEntityOpticKind, replace the hardcoded strings 'neighborhood', 'traversal', 'node', and 'node-property' with named constants. Define these constants at the module level or in a suitable shared location, then update both functions to use these constants instead of the literal strings.Source: Coding guidelines
src/domain/services/optic/WorldlineOptic.ts (1)
50-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
OpticSupportRule.globalDiscoveryRefused()instead of a raw string.
'global-discovery-refused'is a magic string and duplicates a protocol value that already has a domain constructor.♻️ Proposed fix
+import OpticSupportRule from './OpticSupportRule.ts'; ... optic: Optic.traversal({ ...this._posture, startNodeId, - supportRule: 'global-discovery-refused', + supportRule: OpticSupportRule.globalDiscoveryRefused(), }),As per coding guidelines, "Use named constants instead of magic strings or numbers in TypeScript code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/services/optic/WorldlineOptic.ts` around lines 50 - 54, The supportRule property in the optic.traversal() call within WorldlineOptic is using a magic string 'global-discovery-refused' instead of the proper domain constructor. Replace the string value with OpticSupportRule.globalDiscoveryRefused() to follow the coding guidelines that require using named constants instead of magic strings. This ensures the code uses the domain constructor that already encapsulates this protocol value.Source: Coding guidelines
src/domain/services/ProjectionHandle.ts (1)
142-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract one shared builder for captured-coordinate
WorldlineOptic.
_coordinateOptic()and_coordinateExactReadOptic()now duplicate the same coordinate-source wrapping +coordinatePosturewiring. Pull that block into one private helper to prevent future drift.As per coding guidelines, "Do not leave helper corridors, fake shape trust, or transitional duplication behind at the end of a slice. No sludge."
Also applies to: 232-239
🤖 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/ProjectionHandle.ts` around lines 142 - 157, The _coordinateOptic() method and _coordinateExactReadOptic() method both contain duplicate code for creating a WorldlineOptic with the same coordinate-source wrapping pattern and coordinatePosture configuration. Extract this shared logic into a new private helper method (for example, _createCapturedCoordinateOptic) that takes the necessary parameters like source, checkpointSha, and frontier, then constructs and returns the WorldlineOptic with CoordinateCheckpointTailOpticSource and OpticCoordinatePosture.capturedCoordinate(). Replace the duplicate blocks in both _coordinateOptic() and _coordinateExactReadOptic() with calls to this new helper method.Source: Coding guidelines
🤖 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/optic/NodeOptic.ts`:
- Around line 12-18: The NodeOptic constructor accepts any Optic without
validating that it has kind 'node', causing errors to occur later in the read()
method instead of at construction time. Add a validation check in the
constructor after assigning the optic to this._optic to verify that
options.optic has opticKind === 'node', throwing an appropriate error if this
invariant is not met (mirroring the pattern used in TraversalOptic). This
enforces validation at the constructor boundary as required by the coding
guidelines.
---
Nitpick comments:
In `@src/domain/services/optic/Optic.ts`:
- Around line 239-253: Extract the hardcoded optic-kind string literals into
named constants to reduce typo and drift risk. In the functions
supportRuleMatchesTarget and isExactEntityOpticKind, replace the hardcoded
strings 'neighborhood', 'traversal', 'node', and 'node-property' with named
constants. Define these constants at the module level or in a suitable shared
location, then update both functions to use these constants instead of the
literal strings.
In `@src/domain/services/optic/WorldlineOptic.ts`:
- Around line 50-54: The supportRule property in the optic.traversal() call
within WorldlineOptic is using a magic string 'global-discovery-refused' instead
of the proper domain constructor. Replace the string value with
OpticSupportRule.globalDiscoveryRefused() to follow the coding guidelines that
require using named constants instead of magic strings. This ensures the code
uses the domain constructor that already encapsulates this protocol value.
In `@src/domain/services/ProjectionHandle.ts`:
- Around line 142-157: The _coordinateOptic() method and
_coordinateExactReadOptic() method both contain duplicate code for creating a
WorldlineOptic with the same coordinate-source wrapping pattern and
coordinatePosture configuration. Extract this shared logic into a new private
helper method (for example, _createCapturedCoordinateOptic) that takes the
necessary parameters like source, checkpointSha, and frontier, then constructs
and returns the WorldlineOptic with CoordinateCheckpointTailOpticSource and
OpticCoordinatePosture.capturedCoordinate(). Replace the duplicate blocks in
both _coordinateOptic() and _coordinateExactReadOptic() with calls to this new
helper method.
🪄 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: 9cc53e6c-0814-48f1-ab2c-9f4a368e19e8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
CHANGELOG.mdREADME.mddocs/GLOSSARY.mddocs/design/0275-v18-reified-optics/v18-reified-optics.mddocs/releases/v18.1.0/README.mddocs/topics/bounded-reads.mddocs/topics/observers.mddocs/topics/optics.mdeslint.config.tsexamples/README.mdexamples/bounded-reads.tsexamples/observers.tsexamples/optics.tsindex.tsjsr.jsonpackage.jsonpackages/warp-adapters/package.jsonpackages/warp-kernel/package.jsonpackages/warp-orset/package.jsonsrc/domain/services/ProjectionHandle.tssrc/domain/services/optic/CheckpointTailWitnessLocator.tssrc/domain/services/optic/NeighborhoodOptic.tssrc/domain/services/optic/NodeOptic.tssrc/domain/services/optic/NodePropertyOptic.tssrc/domain/services/optic/Optic.tssrc/domain/services/optic/OpticAperturePosture.tssrc/domain/services/optic/OpticBasisPosture.tssrc/domain/services/optic/OpticCoordinatePosture.tssrc/domain/services/optic/OpticSupportRule.tssrc/domain/services/optic/TraversalOptic.tssrc/domain/services/optic/WorldlineOptic.tstest/type-check/consumer.tstest/unit/domain/index.exports.test.tstest/unit/domain/services/optic/Optic.test.tstest/unit/domain/services/optic/TraversalOptic.test.tstest/unit/domain/services/optic/WorldlineOptic.test.tstest/unit/scripts/architecture-doc-shape.test.tstest/unit/scripts/documentation-corpus-graph.test.tstest/unit/scripts/public-api-cost-inventory.test.tstest/unit/scripts/runtime-noun-doc-graph.test.ts
💤 Files with no reviewable changes (3)
- test/unit/scripts/documentation-corpus-graph.test.ts
- test/unit/scripts/architecture-doc-shape.test.ts
- test/unit/scripts/public-api-cost-inventory.test.ts
|
@codex Self-review findings for PR #666. Pre-review gate passed: local worktree was clean (
Issue count: Critical 1, High 1, Medium 1, Low 2. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/unit/domain/services/optic/TraversalOptic.test.ts`:
- Around line 49-53: In the TraversalOptic.test.ts test assertion block that
validates the error with code 'E_OPTIC_SCHEMA', the test is currently asserting
against context.cause but the actual rejection path emits context.reason
instead. Update the assertion to check context.reason with the value
'requires-global-scan' to match the actual error contract being emitted by the
traversal-support rejection path.
🪄 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: 7a1f3bcb-1751-4762-b8df-ec9dc05f98f0
📒 Files selected for processing (9)
README.mddocs/releases/v18.1.0/README.mdexamples/observers.tsexamples/optics.tspatches/@mapbox+node-pre-gyp+2.0.3.patchpatches/README.mdsrc/domain/services/optic/CheckpointTailWitnessLocator.tstest/unit/domain/services/optic/TraversalOptic.test.tstest/unit/scripts/dependency-hygiene.test.ts
💤 Files with no reviewable changes (3)
- patches/README.md
- test/unit/scripts/dependency-hygiene.test.ts
- patches/@mapbox+node-pre-gyp+2.0.3.patch
✅ Files skipped from review due to trivial changes (1)
- docs/releases/v18.1.0/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/observers.ts
- src/domain/services/optic/CheckpointTailWitnessLocator.ts
- README.md
|
@codex Code Lawyer self-audit additions after thread-aware review.
These are added to the same fix queue as the unresolved CodeRabbit thread and global review findings. |
Release Preflight
If you tag this commit as |
|
@codex Activity Summary — Code Lawyer pass on PR #666.
Validation run locally after fixes:
Thread state after this pass: all fetched review threads resolved. |
Release Preflight
If you tag this commit as |
Release Preflight
If you tag this commit as |
|
@codex Code Lawyer self-audit found one additional fix before merge.
|
|
@codex Activity Summary — Code Lawyer follow-up pass on PR #666.
Validation run locally after the fix:
Thread state before push: all fetched review threads resolved. Awaiting GitHub CI and CodeRabbit on |
Release Preflight
If you tag this commit as |
Summary
v18.1.0release packet, version lockstep, changelog entry, examples, topic docs, and README accuracy pass.Opticas an exported runtime-backed read-intent noun for fluent node, node-property, neighborhood, and traversal reads.Refs #665.
Refs #663.
Validation
npm run lintnpm run typechecknpm run typecheck:consumernpm run test:localnpx vitest run test/unit/domain/index.exports.test.ts test/unit/domain/services/optic/Optic.test.ts test/unit/domain/services/optic/WorldlineOptic.test.ts test/unit/domain/services/optic/TraversalOptic.test.tsnpx vitest run test/conformance/v17CheckpointTailOpticReadBasis.test.ts test/conformance/v18FirstUseOpticsHonesty.test.ts test/conformance/v18NeighborhoodOpticReadBasis.test.ts test/conformance/v18TraversalOpticReadBasis.test.tsnpx vitest run test/unit/scripts/runtime-noun-doc-graph.test.tsnpx markdownlint-cli README.md CHANGELOG.md docs/GLOSSARY.md docs/topics/optics.md docs/design/0275-v18-reified-optics/v18-reified-optics.md docs/releases/v18.1.0/README.mdnpm run lint:md:code -- README.md docs/topics docs/design/0275-v18-reified-optics/v18-reified-optics.md docs/releases/v18.1.0/README.md CHANGELOG.md examples/README.mdnpm run release:guard -- --tag v18.1.0 --stage prep-prnpm run test:local.Summary by CodeRabbit
Release Notes
Opticas a runtime-backed noun for bounded optic reads, with posture/support-rule typesOpticobjects for node, node-property, neighborhood, and traversal readsE_OPTIC_FAILURE_SCHEMA)