feat(weave): carry alice bio 10 -> 11 second payload weave slice#5
Merged
feat(weave): carry alice bio 10 -> 11 second payload weave slice#5
Conversation
- add the narrow second payload-state weave path for an already woven payload artifact - create alice/bio/_history001/_s0002 and advance alice/bio/_knop/_inventory to _s0002 while keeping _mesh/_inventory unchanged - treat a second payload weave as eligible only when the working payload differs from the latest historical snapshot - add unit, integration, and black-box CLI coverage for 11-alice-bio-v2-woven - document the carried 11 slice and update the weave behavior and codebase overview notes
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new "secondPayloadWeave" slice to the weave workflow, implementing planner and runtime logic to create a second historical snapshot for an alice/bio payload, updating inventories and generating corresponding artifact pages; updates docs and tests to cover the new slice. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI"
participant Runtime as "Weave Runtime\n(load candidates, files)"
participant Planner as "Core Planner\n(planSecondPayloadWeave)"
participant FS as "Filesystem\n(payload & history files)"
participant Renderer as "Renderers\n(pages, TTL)"
CLI->>Runtime: request weave for designatorPaths
Runtime->>FS: load working payload + historical snapshot (if present)
Runtime->>Planner: classify slice -> secondPayloadWeave
Planner->>Planner: planSecondPayloadWeave (create _s0002 paths, inventory updates)
Planner->>Renderer: generate woven knop inventory TTL + pages
Renderer->>FS: write new snapshot TTLs and index.html pages
Planner->>Runtime: return WeavePlan (created/updated files)
Runtime->>CLI: report wovenDesignatorPaths & updatedPaths
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/weave/weave.ts (1)
319-357:⚠️ Potential issue | 🟡 MinorRun
deno fmton this block.CI is already failing
fmt:checkhere, so this hunk will block merge until the standard Deno formatting pass is applied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/weave/weave.ts` around lines 319 - 357, The code block around latestHistoricalSnapshotPath, currentPayloadTurtle, and latestHistoricalSnapshotTurtle is not formatted to Deno standards and is failing fmt:check; run the Deno formatter (deno fmt) or reformat this section so spacing, indentation, and line breaks for the Deno.readTextFile calls, the ternary that sets latestHistoricalSnapshotPath (using toPayloadHistoricalSnapshotPath and currentKnopInventoryTurtle), and the WeaveRuntimeError message construction (using toPayloadHistoricalSnapshotPath) conform to deno fmt output.
🧹 Nitpick comments (1)
src/core/weave/weave.ts (1)
221-231: Consider consolidating duplicate payload artifact validation.Lines 221-225 and 227-231 perform identical checks with the same error message. These could be combined:
♻️ Optional refactor
- if (slice === "firstPayloadWeave" && !candidate.payloadArtifact) { - throw new WeaveInputError( - `Payload weave candidate ${candidate.designatorPath} is missing working payload state.`, - ); - } - - if (slice === "secondPayloadWeave" && !candidate.payloadArtifact) { + if ( + (slice === "firstPayloadWeave" || slice === "secondPayloadWeave") && + !candidate.payloadArtifact + ) { throw new WeaveInputError( `Payload weave candidate ${candidate.designatorPath} is missing working payload state.`, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/weave/weave.ts` around lines 221 - 231, The two identical checks for payloadArtifact under slice values "firstPayloadWeave" and "secondPayloadWeave" should be consolidated into a single conditional: check if slice is either "firstPayloadWeave" or "secondPayloadWeave" and if candidate.payloadArtifact is falsy, then throw the WeaveInputError with the same message referencing candidate.designatorPath; update the code around the existing slice check (the block containing the current throw using WeaveInputError and candidate.payloadArtifact) to use a combined condition (e.g., slice === "firstPayloadWeave" || slice === "secondPayloadWeave") so the duplicate branches are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@documentation/notes/wd.task.2026.2026-04-05_0903-weave-alice-bio-v2-woven.md`:
- Around line 27-29: The doc overstates which pages are regenerated; update the
note so it only claims creation/presence of the new _s0002 pages (e.g.,
alice/bio/_s0002... and related inventory entries) instead of saying
alice/index.html, alice/bio/index.html, or existing history landing pages are
rewritten; align the wording with the tests that enforce this behavior (see
tests/integration/weave_test.ts and tests/e2e/weave_cli_test.ts) and make the
same tightening for the other occurrences noted (around the current blocks at
lines 71-79 and 164-167).
In `@tests/e2e/weave_cli_test.ts`:
- Around line 39-45: The test disables text comparisons by setting
compareTextFiles: false in the call to assertWeaveTransitionMatchesManifest for
the manifest "11-alice-bio-v2-woven.jsonld", which turns manifest
compareMode==="text" checks into mere existence stats and can mask drift;
restore text comparison for this slice (remove or set compareTextFiles: true) so
the fixture is fully validated, and if only a few files are intentionally
unstable, instead add explicit exemptions for those filenames in the compare
routine (or pass a dedicated allowlist option) rather than disabling all text
comparisons; apply the same change for the other similar invocations referenced
around lines 116-120.
In `@tests/integration/weave_test.ts`:
- Around line 191-266: The new integration test block (Deno.test titled
"executeWeave materializes the second alice bio payload weave slice" in
tests/integration/weave_test.ts) is not formatted per project style; run the
repository formatter (deno fmt) over that file or the whole repo and commit the
resulting changes so the test hunk matches CI's fmt:check expectations.
---
Outside diff comments:
In `@src/runtime/weave/weave.ts`:
- Around line 319-357: The code block around latestHistoricalSnapshotPath,
currentPayloadTurtle, and latestHistoricalSnapshotTurtle is not formatted to
Deno standards and is failing fmt:check; run the Deno formatter (deno fmt) or
reformat this section so spacing, indentation, and line breaks for the
Deno.readTextFile calls, the ternary that sets latestHistoricalSnapshotPath
(using toPayloadHistoricalSnapshotPath and currentKnopInventoryTurtle), and the
WeaveRuntimeError message construction (using toPayloadHistoricalSnapshotPath)
conform to deno fmt output.
---
Nitpick comments:
In `@src/core/weave/weave.ts`:
- Around line 221-231: The two identical checks for payloadArtifact under slice
values "firstPayloadWeave" and "secondPayloadWeave" should be consolidated into
a single conditional: check if slice is either "firstPayloadWeave" or
"secondPayloadWeave" and if candidate.payloadArtifact is falsy, then throw the
WeaveInputError with the same message referencing candidate.designatorPath;
update the code around the existing slice check (the block containing the
current throw using WeaveInputError and candidate.payloadArtifact) to use a
combined condition (e.g., slice === "firstPayloadWeave" || slice ===
"secondPayloadWeave") so the duplicate branches are removed.
🪄 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: e86091ea-b743-4688-abde-7f4d5cfe63d5
⛔ Files ignored due to path filters (1)
documentation/notes/wd.conv.2026.2026-04-05_0842-implement-wd-task-2026-2026-04-04-1553-weave-alice-bio-re-codex.mdis excluded by!documentation/notes/wd.conv.*
📒 Files selected for processing (9)
documentation/notes/wd.codebase-overview.mddocumentation/notes/wd.spec.2026-04-03-weave-behavior.mddocumentation/notes/wd.task.2026.2026-04-04_2019-update-alice-bio-payload.mddocumentation/notes/wd.task.2026.2026-04-05_0903-weave-alice-bio-v2-woven.mdsrc/core/weave/weave.tssrc/core/weave/weave_test.tssrc/runtime/weave/weave.tstests/e2e/weave_cli_test.tstests/integration/weave_test.ts
documentation/notes/wd.task.2026.2026-04-05_0903-weave-alice-bio-v2-woven.md
Show resolved
Hide resolved
Comment on lines
+39
to
+45
| Deno.test("weave matches the manifest-scoped alice bio v2 woven fixture as a black-box CLI run", async () => { | ||
| await assertWeaveTransitionMatchesManifest({ | ||
| manifestName: "11-alice-bio-v2-woven.jsonld", | ||
| expectedStdoutFragment: "Wove 1 designator path", | ||
| compareTextFiles: false, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Keep slice 11 under fixture text comparison.
documentation/notes/wd.spec.2026-04-03-weave-behavior.md, Line 134 still treats the 11 HTML as fixture-aligned output. With compareTextFiles: false, every manifest compareMode === "text" check becomes an existence-only stat(), so this new acceptance path can pass even when current/history pages drift from the settled fixture. If only a few files are intentionally unstable, exempt those explicitly instead of disabling all text comparisons for the transition.
Also applies to: 116-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/weave_cli_test.ts` around lines 39 - 45, The test disables text
comparisons by setting compareTextFiles: false in the call to
assertWeaveTransitionMatchesManifest for the manifest
"11-alice-bio-v2-woven.jsonld", which turns manifest compareMode==="text" checks
into mere existence stats and can mask drift; restore text comparison for this
slice (remove or set compareTextFiles: true) so the fixture is fully validated,
and if only a few files are intentionally unstable, instead add explicit
exemptions for those filenames in the compare routine (or pass a dedicated
allowlist option) rather than disabling all text comparisons; apply the same
change for the other similar invocations referenced around lines 116-120.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This was referenced Apr 7, 2026
Merged
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 by CodeRabbit
New Features
Documentation
Tests