fix(weave): append-only current-only ReferenceCatalog inventory weave - #34
Merged
Conversation
- renderCurrentOnlyReferenceCatalogWovenKnopInventoryTurtle now routes through the existing planInventoryAppend instead of splitting, replacing, and rejoining subject blocks: existing inventory bytes are preserved as an exact prefix, a semantic no-op returns the exact input bytes, and only hasWorkingLocatedFile is treated as single-valued (hasResourcePage is not functional in the ontology) - conflicts now report both requested and existing facts instead of a substring guard - new knop_inventory_renderers_test.ts (kept out of the active-lane weave_test.ts): fail-on-old recorded for all three — the append test failed exact-prefix because the old renderer rejoined blocks; the no-op test failed byte equality because the old renderer reordered page types and dropped a trailing blank line; the conflict test threw nothing because the old substring guard accepted the requested locator appearing in an unrelated ex: predicate while a different working locator was carried - first production consumer migrated onto the append planner per wa.task.2026.2026-05-17-append-onlyish-inventory (brief in that note) Implemented-By: Codex (codex exec) as Kim Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHrFYeUefDr227gLuWWuq1
📝 WalkthroughWalkthroughChangesReferenceCatalog inventory rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Renderer as ReferenceCatalog renderer
participant Planner as planInventoryAppend
participant Inventory as Existing inventory
participant Error as WeaveInputError
Renderer->>Planner: Submit requested facts
Planner->>Inventory: Compare existing facts
alt Conflict
Planner-->>Renderer: Return conflict
Renderer->>Error: Report both conflicting IRIs
else Missing facts
Planner-->>Renderer: Return planned append
Renderer->>Inventory: Combine compacted append
else Complete inventory
Planner-->>Renderer: Return no-op
Renderer-->>Inventory: Preserve bytes unchanged
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
djradon
added a commit
that referenced
this pull request
Aug 2, 2026
… finding Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHrFYeUefDr227gLuWWuq1
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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.
🧹 Nitpick comments (1)
src/core/weave/knop_inventory_renderers.ts (1)
347-364: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded IRI-to-prefix replacement list is brittle to future additions.
This chain manually enumerates every SFLO predicate and class IRI that must be compacted. If a new predicate or class is added to
requestedSettledFactsTurtlein the future but the corresponding.replaceAll()entry is forgotten, the appended Turtle silently keeps the full uncompacted IRI instead of failing a test or a compile check. Extract a small generic compaction helper driven by a name-to-IRI map, so adding a new term to the Turtle template does not require a matching manual.replaceAll()entry to remember.♻️ Proposed generic compaction helper
+const REFERENCE_CATALOG_COMPACTION_TERMS: ReadonlyArray<[string, string]> = [ + [`${RDF_NAMESPACE}type`, "a"], + [`${SFLO_NAMESPACE}ReferenceCatalog`, "sflo:ReferenceCatalog"], + [`${SFLO_NAMESPACE}DigitalArtifact`, "sflo:DigitalArtifact"], + [`${SFLO_NAMESPACE}RdfDocument`, "sflo:RdfDocument"], + [`${SFLO_NAMESPACE}hasWorkingLocatedFile`, "sflo:hasWorkingLocatedFile"], + [`${SFLO_NAMESPACE}hasResourcePage`, "sflo:hasResourcePage"], + [`${SFLO_NAMESPACE}ResourcePage`, "sflo:ResourcePage"], + [`${SFLO_NAMESPACE}LocatedFile`, "sflo:LocatedFile"], +]; + +function compactTurtleTerms(turtle: string): string { + return REFERENCE_CATALOG_COMPACTION_TERMS.reduce( + (text, [iri, compact]) => text.replaceAll(`<${iri}>`, compact), + turtle, + ); +}- let compactAppendTurtle = plan.appendTurtle - .replaceAll(`<${RDF_NAMESPACE}type>`, "a") - .replaceAll( - `<${SFLO_NAMESPACE}ReferenceCatalog>`, - "sflo:ReferenceCatalog", - ) - .replaceAll(`<${SFLO_NAMESPACE}DigitalArtifact>`, "sflo:DigitalArtifact") - .replaceAll(`<${SFLO_NAMESPACE}RdfDocument>`, "sflo:RdfDocument") - .replaceAll( - `<${SFLO_NAMESPACE}hasWorkingLocatedFile>`, - "sflo:hasWorkingLocatedFile", - ) - .replaceAll( - `<${SFLO_NAMESPACE}hasResourcePage>`, - "sflo:hasResourcePage", - ) - .replaceAll(`<${SFLO_NAMESPACE}ResourcePage>`, "sflo:ResourcePage") - .replaceAll(`<${SFLO_NAMESPACE}LocatedFile>`, "sflo:LocatedFile"); + let compactAppendTurtle = compactTurtleTerms(plan.appendTurtle);🤖 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/knop_inventory_renderers.ts` around lines 347 - 364, Replace the hardcoded replaceAll chain used to build compactAppendTurtle with a small generic compaction helper driven by the existing SFLO name-to-IRI mapping, applying each mapping entry to plan.appendTurtle and retaining the RDF type conversion. Ensure future terms added to requestedSettledFactsTurtle are compacted automatically without requiring a separate manual replacement.
🤖 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.
Nitpick comments:
In `@src/core/weave/knop_inventory_renderers.ts`:
- Around line 347-364: Replace the hardcoded replaceAll chain used to build
compactAppendTurtle with a small generic compaction helper driven by the
existing SFLO name-to-IRI mapping, applying each mapping entry to
plan.appendTurtle and retaining the RDF type conversion. Ensure future terms
added to requestedSettledFactsTurtle are compacted automatically without
requiring a separate manual replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c11b5fd-3ede-4a41-98d3-1d5db6729e0f
📒 Files selected for processing (2)
src/core/weave/knop_inventory_renderers.tssrc/core/weave/knop_inventory_renderers_test.ts
djradon
added a commit
that referenced
this pull request
Aug 2, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHrFYeUefDr227gLuWWuq1
This was referenced Aug 2, 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.
Bite 1 of
wa.task.2026.2026-05-17-append-onlyish-inventory— the first production consumer migrated onto the existingplanInventoryAppend. Brief and evidence audit are in that note.What
renderCurrentOnlyReferenceCatalogWovenKnopInventoryTurtlepreviously split the KnopInventory into subject blocks, replaced the ReferenceCatalog block, upserted a page block, and rejoined the whole document. It now routes throughplanInventoryAppend, so the operation:sflo:hasWorkingLocatedFileas single-valued —hasResourcePageis not functional in the core ontology (verified: plainowl:ObjectProperty).The planner itself is untouched; this is a consumer migration.
It also closed a false negative
The conflict test found a defect in the code being replaced: the old substring guard accepted the requested working locator appearing in an unrelated
ex:predicate while a different locator was actually carried — so a genuine conflict passed silently. The new path refuses it.Fail-on-old, all three recorded
New tests live in
src/core/weave/knop_inventory_renderers_test.ts, deliberately kept out ofweave_test.tsto avoid colliding with concurrent lanes. Fulldeno task cianddeno task build:npm-libgreen.Not in this bite
Versioned ReferenceCatalog history, the PageDefinition twin, MeshInventory/payload/batch/progression refactors, and — flagged as its own correctness bite — the
resource_page_policy.tspath that deletes disallowed settled page facts, a sharper violation of the append-onlyish contract than what this fixes.Implemented by Codex (
codex exec) as Kim under the standing grant; reviewed, gated, and landed by the planning seat.🤖 Generated with Claude Code
https://claude.ai/code/session_01LHrFYeUefDr227gLuWWuq1
Summary by CodeRabbit
New Features
Bug Fixes
Tests