Next/v0.3.0#21
Conversation
- set DENO_DIR in the test tmp harness so child deno run commands reuse a stable /tmp cache - cover the harness env behavior and document the cache path - record the e2e and full-suite timing improvement docs: vision
…sitory options documentation
- document Accord as a transition checker rather than a Weave executor - record ResourcePage expectations for woven fixture rungs - replace fixture main reset guidance with merge-based accepted-rung provenance
Add a later-payload read model that derives current history, latest state, next ordinal, support policy, and KnopInventory progression from current RDF facts. Classify later ordinal payload candidates without second-state fixture hints, render computed later payload and inventory state paths, and keep exact targets narrow. Add Stagecraft-shaped planner/runtime coverage, condition-specific diagnostics, and the top-level sparqljs import mapping needed by Accord-backed fixture-ladder tests.
- rename generalized later-payload slice symbols from second-payload terminology - remove the unused second-payload inventory shape assertion - preserve state/manifestation block ordering in generalized later-payload inventory rendering - mark Accord acceptance follow-up complete in developer notes
- plan explicit payload targets together in canonical designator-path order with merged shared inventory progression and no-op reruns - add scoped working-payload-file snapshot verification around batch capture with fail-closed diagnostics - add --generated-at to full weave and generate, canonicalizing ISO instants and forcing explicit footer timestamp convergence - document the new CLI surface, app-owned atomicity boundary, and hash-scope decision - tests: deno task test; deno task lint
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR generalizes payload weaving from a fixed "second payload" slice to an RDF-fact-driven "later payload" progression with deterministic multi-target batching and pre-write snapshot verification, adds a ChangesLater Payload Weave Generalization, Batching & Snapshot Verification
--generated-at Timestamp Flag
Knop Source Registry Inventory Append Planner
Test Harness DENO_DIR Isolation
Project Documentation and Release Notes
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant planWeave
participant planExplicitPayloadBatchWeave
participant version_execution as prepareVersionExecution
participant Snapshot as PayloadBatchWorkingFileSnapshot
CLI->>planWeave: multiple --target payload requests
planWeave->>planExplicitPayloadBatchWeave: build ordered batch entries
version_execution->>Snapshot: hash working payload files
Snapshot-->>version_execution: afterInitialHash()
version_execution->>Snapshot: verify() after candidate loading
Snapshot-->>version_execution: afterVerifiedCapture() or reject
planExplicitPayloadBatchWeave-->>planWeave: merged files/pages
planWeave-->>CLI: batched plan or fail-closed error
sequenceDiagram
participant CLI
participant resolveGeneratedAtOption
participant executeWeave
participant generatePreparedPages
CLI->>resolveGeneratedAtOption: parse --generated-at
resolveGeneratedAtOption-->>CLI: {iso, now()}
CLI->>executeWeave: now, updateTimestampOnlyPages=true
executeWeave->>generatePreparedPages: updateTimestampOnlyPages
generatePreparedPages-->>executeWeave: pages written with pinned timestamp
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/core/weave/payload_version_layout.ts (1)
437-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThird near-duplicate single-non-negative-integer scanner.
requireSingleNonNegativeIntegerLiteralWithDiagnosticslargely duplicatesrequireSingleNonNegativeIntegerLiteral(rdf_helpers.ts) andrequireSingleNonNegativeIntegerFact(payload_weave_read_model.ts), differing only in how errors are messaged (single message vs missing/conflict/invalid diagnostics, list vs Set dedup). Consider consolidating into one helper that accepts optional per-case diagnostic messages, so the three copies can't drift (they already differ subtly in dedup semantics). Not blocking.🤖 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/core/weave/payload_version_layout.ts` around lines 437 - 481, The helper logic for scanning a single non-negative integer is duplicated across requireSingleNonNegativeIntegerLiteralWithDiagnostics, requireSingleNonNegativeIntegerLiteral, and requireSingleNonNegativeIntegerFact, and the implementations are already drifting in dedup/error handling. Consolidate the shared parsing and validation into one reusable helper that accepts optional missing/conflict/invalid diagnostic messages, then have the existing functions delegate to it so behavior stays consistent and only the messaging varies.src/core/import/import.ts (1)
753-758: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCatch only the planner’s input error type.
planInventoryAppendonly raisesWeaveInputErrorfor expected Turtle/shape failures; catching everyErrorhere will also relabel unexpected bugs asImportInputError. Narrow this catch toWeaveInputErrorand let other exceptions bubble up.🤖 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/core/import/import.ts` around lines 753 - 758, The catch block in planInventoryAppend is too broad and currently converts any Error into ImportInputError, which masks unexpected bugs. Update the handling around the planner call to catch only WeaveInputError, preserve its message when rethrowing ImportInputError, and let all other exceptions bubble up unchanged so the import path in import.ts only treats expected input/shape failures as user-facing input errors.src/cli/run.ts (2)
1943-2039: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct unit tests for the new date-parsing helpers.
parseIso8601Instant/parseOffsetMinutes/daysInMonth/isLeapYearcurrently appear to be exercised only indirectly via slow e2e CLI tests (tests/e2e/weave_cli_test.ts). A small unit-test module directly targeting these pure functions (leap years, offset signs, boundary hours/minutes/seconds, fractional seconds) would give faster, more precise coverage of this critical input-validation path.🤖 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/cli/run.ts` around lines 1943 - 2039, Add direct unit coverage for the new date-parsing helpers instead of relying only on `tests/e2e/weave_cli_test.ts`. Create focused tests for `parseIso8601Instant`, `parseOffsetMinutes`, `daysInMonth`, and `isLeapYear` that verify leap-year behavior, positive/negative offsets, boundary hour/minute/second values, and fractional-second parsing. Use the helper names to locate the logic in `src/cli/run.ts` and keep the tests narrow and deterministic.
102-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
Temporal.Instant.from()instead of hand-rolled ISO-8601 parsing.The custom regex +
daysInMonth/isLeapYear/offset-math implementation duplicates whatTemporal.Instant.from()already does natively: it strictly validates calendar dates (throws on2026-02-31) and requires an explicit offset/Z, which matches this feature's exact requirements. Temporal has been stable in Deno since 2.7 (Feb 2026), so if the project's toolchain target is at or above that, this ~70-line block could collapse to a few lines with less surface area for date-math bugs.♻️ Sketch using Temporal
-function resolveGeneratedAtOption( - value: string | undefined, - commandName: string, -): { iso: string; now: () => Date } | undefined { - if (value === undefined) { - return undefined; - } - const instant = resolveRequiredOptionValue( - value, - `${commandName} --generated-at requires an ISO 8601 instant`, - (message) => new WeaveInputError(message), - ); - const parsed = parseIso8601Instant(instant); - if (parsed === undefined) { - throw new WeaveInputError( - `${commandName} --generated-at must be an ISO 8601 instant with an explicit UTC offset, such as 2026-07-06T12:34:56Z or 2026-07-06T08:34:56-04:00`, - ); - } - const iso = parsed.toISOString(); - return { - iso, - now: () => new Date(iso), - }; -} +function resolveGeneratedAtOption( + value: string | undefined, + commandName: string, +): { iso: string; now: () => Date } | undefined { + if (value === undefined) { + return undefined; + } + const instant = resolveRequiredOptionValue( + value, + `${commandName} --generated-at requires an ISO 8601 instant`, + (message) => new WeaveInputError(message), + ); + let iso: string; + try { + iso = Temporal.Instant.from(instant).toString(); + } catch { + throw new WeaveInputError( + `${commandName} --generated-at must be an ISO 8601 instant with an explicit UTC offset, such as 2026-01-01T12:34:56Z or 2026-01-01T08:34:56-04:00`, + ); + } + return { iso, now: () => new Date(iso) }; +}Please confirm the project's Deno target/toolchain (deno.json, CI image) is on 2.7+ before adopting this; otherwise
--unstable-temporal/polyfill considerations apply.Also applies to: 1968-2039
🤖 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/cli/run.ts` around lines 102 - 103, The ISO-8601 parsing in run.ts is hand-rolled with regex and date math, but this feature can be simplified by using Temporal.Instant.from() if the Deno toolchain target supports it. Update the parsing path around ISO_8601_INSTANT_PATTERN and the related date-validation/offset logic to rely on Temporal.Instant.from() for strict calendar validation and required offset/Z handling, and first confirm deno.json and CI are on Deno 2.7+ before removing the custom implementation.
🤖 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/runtime/weave/version_execution.ts`:
- Around line 651-786: `resolvePayloadBatchPolicies` is not checking
`resourcePageGenerationConfig` for batch consistency, so
`planExplicitPayloadBatchVersion` can reuse the first target’s
artifact-generation settings across differing targets. Update the comparison in
`resolvePayloadBatchPolicies` to include `resourcePageGenerationConfig` in the
serialized consistency key, and keep the existing `WeaveInputError` path when
any candidate’s scoped config differs from the first one.
---
Nitpick comments:
In `@src/cli/run.ts`:
- Around line 1943-2039: Add direct unit coverage for the new date-parsing
helpers instead of relying only on `tests/e2e/weave_cli_test.ts`. Create focused
tests for `parseIso8601Instant`, `parseOffsetMinutes`, `daysInMonth`, and
`isLeapYear` that verify leap-year behavior, positive/negative offsets, boundary
hour/minute/second values, and fractional-second parsing. Use the helper names
to locate the logic in `src/cli/run.ts` and keep the tests narrow and
deterministic.
- Around line 102-103: The ISO-8601 parsing in run.ts is hand-rolled with regex
and date math, but this feature can be simplified by using
Temporal.Instant.from() if the Deno toolchain target supports it. Update the
parsing path around ISO_8601_INSTANT_PATTERN and the related
date-validation/offset logic to rely on Temporal.Instant.from() for strict
calendar validation and required offset/Z handling, and first confirm deno.json
and CI are on Deno 2.7+ before removing the custom implementation.
In `@src/core/import/import.ts`:
- Around line 753-758: The catch block in planInventoryAppend is too broad and
currently converts any Error into ImportInputError, which masks unexpected bugs.
Update the handling around the planner call to catch only WeaveInputError,
preserve its message when rethrowing ImportInputError, and let all other
exceptions bubble up unchanged so the import path in import.ts only treats
expected input/shape failures as user-facing input errors.
In `@src/core/weave/payload_version_layout.ts`:
- Around line 437-481: The helper logic for scanning a single non-negative
integer is duplicated across
requireSingleNonNegativeIntegerLiteralWithDiagnostics,
requireSingleNonNegativeIntegerLiteral, and requireSingleNonNegativeIntegerFact,
and the implementations are already drifting in dedup/error handling.
Consolidate the shared parsing and validation into one reusable helper that
accepts optional missing/conflict/invalid diagnostic messages, then have the
existing functions delegate to it so behavior stays consistent and only the
messaging varies.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf926f26-4750-4ee9-985c-424d2e88db8e
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
deno.jsondocumentation/notes/release-notes.v0.3.0.mddocumentation/notes/wd.codebase-overview.mddocumentation/notes/wd.decision-log.mddocumentation/notes/wd.testing.fixture-ladder-regeneration.mddocumentation/notes/wd.testing.mddocumentation/notes/wd.todo.mddocumentation/notes/wd.vision.mddocumentation/notes/wu.cli-reference.generate.mddocumentation/notes/wu.cli-reference.integrate.mddocumentation/notes/wu.cli-reference.mddocumentation/notes/wu.cli-reference.target-syntax.mddocumentation/notes/wu.cli-reference.version.mddocumentation/notes/wu.cli-reference.weave.mddocumentation/notes/wu.repository-options.mdsrc/cli/run.tssrc/core/import/import.tssrc/core/import/import_test.tssrc/core/weave/artifact_history_queries.tssrc/core/weave/mesh_inventory_renderers.tssrc/core/weave/payload_renderers.tssrc/core/weave/payload_version_layout.tssrc/core/weave/payload_weave_read_model.tssrc/core/weave/resource_page_builders.tssrc/core/weave/shape_assertions.tssrc/core/weave/slice_classification.tssrc/core/weave/slices.tssrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/weave/candidate_loader.tssrc/runtime/weave/page_generation.tssrc/runtime/weave/prepared_execution.tssrc/runtime/weave/version_execution.tssrc/runtime/weave/weave.tstests/e2e/weave_cli_test.tstests/integration/weave_test.tstests/support/test_tmp.tstests/support/test_tmp_test.ts
💤 Files with no reviewable changes (1)
- src/core/weave/shape_assertions.ts
- reject payload batches when target-scoped resource page generation config differs - replace hand-rolled generated-at instant parsing with Temporal-backed parsing - add deterministic generated-at parser coverage for leap dates, offsets, bounds, and fractions - preserve unexpected import planner exceptions instead of wrapping every Error - consolidate non-negative integer literal diagnostics across weave helpers
1 participant
v0.3.0is a planner-generalization release driven by Stagecraft, Weave's first application-shaped consumer. Later-payload advancement is now derived from current RDF facts instead of carried fixture shapes, so application meshes can advance_s0003and later payload states with current-only support artifacts. The release adds deterministic multi-target payload batches with pre-write input snapshot verification, a--generated-atflag for pinned ResourcePage timestamps, and service-integration documentation for applications that drive Weave programmatically.Summary by CodeRabbit
New Features
--generated-atoption to pin generated page timestamps.Bug Fixes
Documentation
Tests