Repository navigation
feat: expose capability API 1.3 host services - #3693
Merged
SpicyMarinara merged 27 commits intoJul 16, 2026
Merged
SpicyMarinara merged 27 commits into
SpicyMarinara merged 27 commits into
Conversation
…-runtime-logging # Conflicts: # packages/client/src/components/chat/ChatSettingsDrawer.tsx # packages/server/src/routes/chats.routes.ts
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change introduces Capability API 1.2 runtime contracts and server hosts, adds client capability loading and recovery states, persists checkpoint snapshots, expands prompt preview lore processing, improves map-generation guidance, and enforces timestamp and spatial-storage boundaries. ChangesCapability API runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UI
participant CapabilityElement
participant ModuleLoader
participant ClientPackage
UI->>CapabilityElement: render capability
CapabilityElement->>ModuleLoader: observe package module
ModuleLoader->>ClientPackage: import custom element
ClientPackage-->>CapabilityElement: dispatch runtime error
CapabilityElement-->>UI: render stopped state
UI->>CapabilityElement: select Try again
CapabilityElement->>ModuleLoader: retry module attempt
ModuleLoader-->>UI: render ready capability
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
Collaborator
|
Pookie, shouldn't this be pulled to Agents? |
Collaborator
|
Okay, all good, but not to main. Needs to be changed to staging. |
thetopham
changed the title
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 3693 --repo Pasta-Devs/Marinara-Engine --json number,baseRefName,headRefName,url --jq '{number, baseRefName, headRefName, url}'Length of output: 472
|
thetopham
marked this pull request as draft
July 16, 2026 15:08
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
coderabbitai
Bot
added
the
refactor
thetopham
marked this pull request as ready for review
July 16, 2026 15:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/server/src/services/spatial-context/owner-turn.ts (1)
63-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffRemove vestigial
_dbparameters from the public interfaces.Ah, fascinating! You have successfully excised the database dependency from the underlying capability providers. However, you have left these vestigial
_dbappendages dangling in the exported wrappers. While scientifically harmless for the moment, such evolutionary leftovers clutter our glorious specimen. If you find the resources for a minor operational procedure, consider amputating these unused parameters across the codebase to achieve absolute perfection!
packages/server/src/services/spatial-context/owner-turn.ts#L63-L66: Sever the_dbparameter from the signature and update the upstream invocations.packages/server/src/services/storage/spatial-context.storage.ts#L55-L56: Excise the unused_dbargument from this storage factory.packages/server/src/services/spatial-context/projection.ts#L46-L50: Remove the superfluous_dbparameter from the projection resolution wrapper.packages/server/src/services/spatial-context/state-resolution.ts#L46-L51: Amputate the unused_db: SpatialReadConnectionparameter from this interface.packages/server/src/services/spatial-context/state-resolution.ts#L63-L66: Perform the same excision formaterializeAssistantSpatialState.🤖 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 `@packages/server/src/services/spatial-context/owner-turn.ts` around lines 63 - 66, Remove the unused database parameters from the exported wrappers and update all callers accordingly: packages/server/src/services/spatial-context/owner-turn.ts lines 63-66 (commitSpatialOwnerTurn), packages/server/src/services/storage/spatial-context.storage.ts lines 55-56 (storage factory), packages/server/src/services/spatial-context/projection.ts lines 46-50 (projection resolution wrapper), and packages/server/src/services/spatial-context/state-resolution.ts lines 46-51 and 63-66 (state-resolution interface and materializeAssistantSpatialState). Preserve the existing provider, storage, projection, and state-materialization behavior while eliminating the obsolete arguments throughout upstream invocations.
🤖 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 `@packages/server/src/routes/chats.routes.ts`:
- Around line 2312-2318: Normalize generationLorebookTokenBudget once near
generationTriggers using the resolver’s finite and non-negative validation.
Reuse that normalized budget when invoking each of the three lorebook-generation
paths, including both no-preset branches and the preset branch, so preset
selection does not alter the preview’s lore budget.
In
`@packages/server/src/services/capability-packages/capability-persistence.service.ts`:
- Around line 162-179: Make the composite operations in replaceBootstrap and
replaceAtAnchor atomic by wrapping each delete-and-store sequence in
db.transaction(...), returning the transaction result. Also wrap the message
creation and initial swipe creation flow around the methods at lines 208-234 in
a transaction, preserving operation order and existing behavior; nested
transactions are supported.
---
Nitpick comments:
In `@packages/server/src/services/spatial-context/owner-turn.ts`:
- Around line 63-66: Remove the unused database parameters from the exported
wrappers and update all callers accordingly:
packages/server/src/services/spatial-context/owner-turn.ts lines 63-66
(commitSpatialOwnerTurn),
packages/server/src/services/storage/spatial-context.storage.ts lines 55-56
(storage factory), packages/server/src/services/spatial-context/projection.ts
lines 46-50 (projection resolution wrapper), and
packages/server/src/services/spatial-context/state-resolution.ts lines 46-51 and
63-66 (state-resolution interface and materializeAssistantSpatialState).
Preserve the existing provider, storage, projection, and state-materialization
behavior while eliminating the obsolete arguments throughout upstream
invocations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 262be224-7432-4986-8fd6-44fa93b0b470
📒 Files selected for processing (32)
docs/development/optional-agent-packages.mde2e/core-flows.e2e.tspackages/client/src/components/capabilities/CapabilityElement.tsxpackages/client/src/components/chat/ChatInput.tsxpackages/client/src/components/game/GameInput.tsxpackages/client/src/components/game/GameMap.tsxpackages/client/src/components/layout/AppShell.tsxpackages/client/src/hooks/use-capability-packages.tspackages/server/src/db/schema/checkpoints.tspackages/server/src/routes/chats.routes.tspackages/server/src/routes/game.routes.tspackages/server/src/routes/generate/generate-route-utils.tspackages/server/src/services/capability-packages/capability-language-model.service.tspackages/server/src/services/capability-packages/capability-module-runtime.service.tspackages/server/src/services/capability-packages/capability-persistence.service.tspackages/server/src/services/capability-packages/capability-resources.service.tspackages/server/src/services/game/checkpoint.service.tspackages/server/src/services/game/map.service.tspackages/server/src/services/import/import-timestamps.tspackages/server/src/services/mari-db/mari-db.service.tspackages/server/src/services/spatial-context/owner-turn.tspackages/server/src/services/spatial-context/projection.tspackages/server/src/services/spatial-context/state-resolution.tspackages/server/src/services/storage/chats.storage.tspackages/server/src/services/storage/game-state.storage.tspackages/server/src/services/storage/spatial-context.storage.tspackages/shared/src/index.tspackages/shared/src/schemas/capability-package.schema.tspackages/shared/src/types/capability-runtime.tsscripts/regressions/capability-package-lifecycle.regression.tsscripts/regressions/prompt.regression.tsscripts/regressions/spatial-context.regression.ts
Comment on lines
+2312
to
+2318
| const generationTriggers = Array.from(new Set([chatMode, "chat"])); | ||
| const forcedLorebookEntryIds = | ||
| ownerSpatialProjection && | ||
| (ownerSpatialProjection.ownerMode === chatMode || | ||
| (ownerSpatialProjection.ownerMode === "roleplay" && chatMode === "visual_novel")) | ||
| ? ownerSpatialProjection.lorebookEntryIds | ||
| : []; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
A curious inconsistency: normalize the lorebook budget once.
The no-preset branches ignore generationLorebookTokenBudget, while the preset branch accepts it without the resolver’s finite/non-negative validation. Compute one normalized budget near generationTriggers and pass it to all three paths; otherwise adding/removing a preset changes the preview’s lore content.
Proposed fix
const generationTriggers = Array.from(new Set([chatMode, "chat"]));
+const lorebookTokenBudget = resolveLorebookTokenBudget({
+ lorebookTokenBudget:
+ chatMeta.lorebookTokenBudget ?? chatMeta.generationLorebookTokenBudget,
+});
-tokenBudget: resolveLorebookTokenBudget(chatMeta),
+tokenBudget: lorebookTokenBudget,
-tokenBudget: resolveLorebookTokenBudget(chatMeta),
+tokenBudget: lorebookTokenBudget,
-lorebookTokenBudget:
- typeof (chatMeta.lorebookTokenBudget ?? chatMeta.generationLorebookTokenBudget) === "number"
- ? ((chatMeta.lorebookTokenBudget ?? chatMeta.generationLorebookTokenBudget) as number)
- : undefined,
+lorebookTokenBudget,Also applies to: 2389-2389, 2426-2426, 2508-2511
🤖 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 `@packages/server/src/routes/chats.routes.ts` around lines 2312 - 2318,
Normalize generationLorebookTokenBudget once near generationTriggers using the
resolver’s finite and non-negative validation. Reuse that normalized budget when
invoking each of the three lorebook-generation paths, including both no-preset
branches and the preset branch, so preset selection does not alter the preview’s
lore budget.
Comment on lines
+162
to
+179
| async replaceBootstrap(input) { | ||
| await db | ||
| .delete(spatialContextSnapshots) | ||
| .where(and(eq(spatialContextSnapshots.chatId, input.chatId), eq(spatialContextSnapshots.messageId, ""))); | ||
| return store.create(input); | ||
| }, | ||
| async replaceAtAnchor(input) { | ||
| await db | ||
| .delete(spatialContextSnapshots) | ||
| .where( | ||
| and( | ||
| eq(spatialContextSnapshots.chatId, input.chatId), | ||
| eq(spatialContextSnapshots.messageId, input.messageId), | ||
| eq(spatialContextSnapshots.swipeIndex, input.swipeIndex), | ||
| ), | ||
| ); | ||
| return store.create(input); | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make composite persistence operations atomic.
These methods are callable outside transaction(). An insertion failure after deletion can erase the prior spatial snapshot; a swipe insertion failure can leave an orphaned message. Wrap each complete operation in db.transaction(...)—nested transactions are already supported.
- Lines 162-179: atomically delete and replace the snapshot.
- Lines 208-234: atomically create the message and initial swipe.
Also applies to: 208-234
🤖 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
`@packages/server/src/services/capability-packages/capability-persistence.service.ts`
around lines 162 - 179, Make the composite operations in replaceBootstrap and
replaceAtAnchor atomic by wrapping each delete-and-store sequence in
db.transaction(...), returning the transaction result. Also wrap the message
creation and initial swipe creation flow around the methods at lines 208-234 in
a transaction, preserving operation order and existing behavior; nested
transactions are supported.
thetopham
marked this pull request as draft
July 16, 2026 16:13
1 task
thetopham
marked this pull request as ready for review
July 16, 2026 16:26
…al-maps-api-1.2-engine-2.3
…-1.2-engine-2.3' into release/hierarchical-maps-api-1.2-engine-2.3
This was referenced Jul 16, 2026
5 tasks
7 of 10 tasks
7 of 10 tasks
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
2 participants
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.
Linked issue
Closes #3690
Why this change
stagingexposes capability API 1.0, so it rejects downloadable packages that require the generic API 1.3 host contract.Pasta-Devs/Marinara-Agents.What changed
Validation
pnpm checkpasses locallyCONTRIBUTING.mdManual verification notes
d1308698c:pnpm check— passed.pnpm regression:spatial— passed.pnpm regression:prompt— passed.git diff --check— passed.staging(29f8b7d6e). The earlier chat settings conflict was resolved in favor of staging's package-owned settings placement; the server route resolution retains both API 1.3 game-state parsing and staging's speaker-tag cleanup. The latest staging sync merged without additional conflicts.7d5c09975abfd28e7e5e6645ff3b0bdeef86e1d7is merged with the CodeRabbit correctness fixes at current headd1308698c.Docs and release impact
This PR targets
stagingand does not directly prepare or publish amainrelease. The paired package work is Pasta-Devs/Marinara-Agents#50 and Pasta-Devs/Marinara-Agents#52.UI evidence (if applicable)
No screenshots are attached. Engine supplies generic contribution hosts and package loading/recovery states; all Hierarchical Maps presentation belongs to Marinara Agents. Known real-device mobile issues are tracked separately in #3691 and Pasta-Devs/Marinara-Agents#51.
Summary by CodeRabbit
New Features
Bug Fixes