feat: native AI-assisted Magic Rewrite for character, persona, and lorebook fields#1184
Closed
kevin-ho wants to merge 2 commits into
Closed
feat: native AI-assisted Magic Rewrite for character, persona, and lorebook fields#1184kevin-ho wants to merge 2 commits into
kevin-ho wants to merge 2 commits into
Conversation
…rebook fields (Pasta-Devs#1183) - Add POST /api/magic-rewrite/generate endpoint using configured LLM connections - Connection resolution: rewrite-specific → default chat → agent default - New 'Default for Rewrite' per-connection toggle in Connection Settings - MagicRewritePanel component with instructions, context selectors, diff preview - Integrated into LorebookFormFields and shared ExpandedTextarea component - Covers character editor, persona editor, chat settings, and lorebook entries - DB migration adds default_for_rewrite column with mutual-exclusion enforcement
github-actions
Bot
added
feature
📝 WalkthroughWalkthroughAdds an end-to-end Magic Rewrite feature: a new DB flag and schema, storage enforcement for a single default rewrite connection, a server route POST /api/magic-rewrite/generate that calls the selected LLM provider, a client-side MagicRewritePanel component, and editor integrations to run/apply rewrites. ChangesMagic Rewrite
Sequence DiagramsequenceDiagram
participant User
participant ExpandedTextarea
participant MagicRewritePanel
participant MagicRewriteAPI
participant Storage
participant LLMProvider
User->>ExpandedTextarea: Open editor
ExpandedTextarea->>ExpandedTextarea: Sync value to local state
User->>ExpandedTextarea: Enter ✨ Magic Rewrite mode
ExpandedTextarea->>MagicRewritePanel: Show panel with current text
User->>MagicRewritePanel: Enter instruction & select context
User->>MagicRewritePanel: Click Generate
MagicRewritePanel->>MagicRewriteAPI: POST /api/magic-rewrite/generate (text,instruction,context)
MagicRewriteAPI->>Storage: getDefaultForRewrite()
Storage-->>MagicRewriteAPI: return connection (with apiKey)
MagicRewriteAPI->>LLMProvider: chatComplete(system+messages, params)
LLMProvider-->>MagicRewriteAPI: rewritten text
MagicRewriteAPI-->>MagicRewritePanel: { text, finishReason, usage }
MagicRewritePanel->>MagicRewritePanel: compute diff / show before-after
User->>MagicRewritePanel: Click Apply
MagicRewritePanel->>ExpandedTextarea: update local text & exit rewrite mode
ExpandedTextarea->>User: commit final text via onChange when closed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/components/lorebooks/LorebookFormFields.tsx (1)
247-251:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBlock Escape-to-close while Magic Rewrite mode is active.
Line 247–251 closes immediately even during rewrite mode, which bypasses the Back/Apply workflow advertised in Line 308 and can drop generated rewrite output.
Suggested fix
- if (e.key === "Escape") { + if (e.key === "Escape" && !magicRewriteMode) { onChange(local); onCommit?.(); onClose(); } ... - }, [onClose, onChange, onCommit, local]); + }, [onClose, onChange, onCommit, local, magicRewriteMode]);Also applies to: 308-308
🤖 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/client/src/components/lorebooks/LorebookFormFields.tsx` around lines 247 - 251, The Escape key handler currently always calls onChange(local), onCommit and onClose which closes the form even when Magic Rewrite mode is active; update the handler to check the Magic Rewrite mode flag (e.g., isMagicRewrite or isRewriteMode in component state/props) and only perform the close/commit sequence when that flag is false. Concretely, modify the block around the Escape check (the code using e.key === "Escape" that calls onChange(local), onCommit?.(), onClose()) to early-return or ignore Escape when the Magic Rewrite boolean is true so the Back/Apply workflow (the rewrite flow) is not bypassed.
🧹 Nitpick comments (1)
packages/server/src/services/storage/connections.storage.ts (1)
224-260: ⚡ Quick winAh, a most curious omission in your duplication apparatus!
The
duplicate()method explicitly setsdefaultForAgents: "false"on line 240 to prevent inheritance of privileged status, yet the newdefaultForRewritefield receives no such explicit handling. While the database default shall suffice functionally, this inconsistency diminishes the elegance of the experimental design!For clarity and to match the established pattern, explicitly set this field alongside its sibling.
🔬 Proposed refinement to maintain consistency
isDefault: "false", useForRandom: source.useForRandom, defaultForAgents: "false", + defaultForRewrite: "false", enableCaching: source.enableCaching,🤖 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/storage/connections.storage.ts` around lines 224 - 260, The duplicate(id: string) method currently omits explicitly setting the defaultForRewrite field when inserting the copied connection; update the db.insert in connections.storage.ts inside duplicate() to include defaultForRewrite (set to "false" to match defaultForAgents handling) so the new record explicitly mirrors the intended non-default rewrite flag instead of relying on DB defaults.
🤖 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/client/src/components/connections/ConnectionEditor.tsx`:
- Around line 1736-1739: The tooltip/help text for the "Default for Rewrite"
control in ConnectionEditor.tsx currently stops at "default chat connection" but
omits the final fallback to the agent default; update the help string used for
the field labeled "Default for Rewrite" (the help prop in the ConnectionEditor
component) to explicitly describe the full fallback chain: use the
rewrite-specific connection, otherwise the default chat connection, and if that
is not set fall through to the agent default (and mention any further global
default if applicable).
- Around line 417-418: The bug is that defaultForRewrite
(localDefaultForRewrite) is still included when saving an image_generation
provider connection; update the object assembly in ConnectionEditor so
defaultForRewrite is only added for providers that support rewrites. Concretely,
when building the payload that currently sets defaultForAgents:
localDefaultForAgents, defaultForRewrite: localDefaultForRewrite, guard adding
defaultForRewrite (or set it to false/omit it) when the connection/provider type
equals 'image_generation' (or otherwise lacks rewrite capability) so that
defaultForRewrite is not persisted for image generation providers.
In `@packages/client/src/components/ui/ExpandedTextarea.tsx`:
- Around line 22-24: ExpandedTextarea currently keeps magicRewriteMode and
magicRewriteResult across open/close cycles; update the component to clear these
when the overlay is closed by calling setMagicRewriteMode(false) and
setMagicRewriteResult("") in the overlay close path (e.g., the component's
onClose handler or a useEffect that watches the overlay's open state), and
ensure the same reset is applied wherever the duplicate state hooks at lines
~26-28 are used so reopening always starts with a clean rewrite state.
In `@packages/client/src/components/ui/MagicRewritePanel.tsx`:
- Line 93: LocalStorage access in MagicRewritePanel (the initial state using
PROMPT_KEY and later reads/writes around setInstruction) must be guarded to
avoid throws in restricted browsers; update the useState initializer and any
subsequent window.localStorage.getItem/setItem calls to first check for
availability (typeof window !== "undefined" && window.localStorage) and wrap
accesses in try/catch, falling back to an empty string or no-op when errors
occur, so all reads (the initializer using PROMPT_KEY) and writes (the update
logic near lines 105-106) safely handle blocked storage without throwing.
In `@packages/server/src/routes/magic-rewrite.routes.ts`:
- Around line 92-94: The handler currently logs the full error via app.log.error
but returns error.message to clients; change it to keep the full error in server
logs (use app.log.error({ err: error }, "Magic Rewrite generation failed")) and
instead return a stable, generic message in reply.status(500).send (e.g., {
error: "Magic Rewrite generation failed" }) so no raw provider/internal error
text is exposed; ensure you remove or replace any use of error.message in the
response while preserving the existing log call.
- Line 34: Replace the direct call to magicRewriteSchema.parse(req.body) (which
can throw) with a non-throwing validation using
magicRewriteSchema.safeParse(req.body); check the result object (result.success)
and if false respond with res.status(400).json(...) containing the validation
errors and return early so the handler doesn't proceed; if success, use
result.data as the input variable for the existing logic. This change touches
the usage of magicRewriteSchema and the input variable in the route handler.
- Line 92: Replace the inline use of app.log.error with the shared Pino logger:
import the default logger from ../lib/logger.js (if not already imported) and
change the call in the Magic Rewrite generation error path to
logger.error(error, "Magic Rewrite generation failed") so the error object is
passed first and the project-wide logging contract is followed; also remove or
stop using app.log in this module to keep logging consistent.
---
Outside diff comments:
In `@packages/client/src/components/lorebooks/LorebookFormFields.tsx`:
- Around line 247-251: The Escape key handler currently always calls
onChange(local), onCommit and onClose which closes the form even when Magic
Rewrite mode is active; update the handler to check the Magic Rewrite mode flag
(e.g., isMagicRewrite or isRewriteMode in component state/props) and only
perform the close/commit sequence when that flag is false. Concretely, modify
the block around the Escape check (the code using e.key === "Escape" that calls
onChange(local), onCommit?.(), onClose()) to early-return or ignore Escape when
the Magic Rewrite boolean is true so the Back/Apply workflow (the rewrite flow)
is not bypassed.
---
Nitpick comments:
In `@packages/server/src/services/storage/connections.storage.ts`:
- Around line 224-260: The duplicate(id: string) method currently omits
explicitly setting the defaultForRewrite field when inserting the copied
connection; update the db.insert in connections.storage.ts inside duplicate() to
include defaultForRewrite (set to "false" to match defaultForAgents handling) so
the new record explicitly mirrors the intended non-default rewrite flag instead
of relying on DB defaults.
🪄 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: 96baea19-32c4-4522-babb-7108eead7d48
📒 Files selected for processing (11)
packages/client/src/components/connections/ConnectionEditor.tsxpackages/client/src/components/lorebooks/LorebookFormFields.tsxpackages/client/src/components/ui/ExpandedTextarea.tsxpackages/client/src/components/ui/MagicRewritePanel.tsxpackages/server/src/db/migrate.tspackages/server/src/db/schema/connections.tspackages/server/src/routes/index.tspackages/server/src/routes/magic-rewrite.routes.tspackages/server/src/services/storage/connections.storage.tspackages/shared/src/schemas/connection.schema.tspackages/shared/src/types/connection.ts
- Block Escape-to-close during Magic Rewrite mode in LorebookFormFields - Add defaultForRewrite: 'false' to duplicate() method for consistency - Reset magicRewriteMode/magicRewriteResult when ExpandedTextarea closes - Guard localStorage access with try/catch in MagicRewritePanel - Use safeParse instead of parse for request validation - Use shared Pino logger instead of app.log; return generic error to client - Exclude defaultForRewrite from image_generation provider save payload - Update tooltip to describe full fallback chain
Collaborator
|
@kevin-ho Hi! Thanks for putting this together. I think the overall direction is a good fit, native Magic Rewrite supports the core authoring workflows around characters, personas, prompts, and lorebooks. I do think the first version should be a little narrower before we merge it. This PR includes the core rewrite flow, but it also makes a few longer-term product decisions at the same time like a rewrite-specific default connection, a DB migration, new connection settings UI, richer context selection, chat-history context, and broad integration through shared editor surfaces. For V1, I’d like to keep the merge focused on the smallest useful native rewrite workflow. Specifically, rewrite the current editor text from user instructions using the existing default chat/agent connection fallback, with preview/apply/back behavior. Then we can handle rewrite-specific defaults, chat-history context, and richer context controls as follow-up PRs once the basic workflow has landed. Would you be able to submit a new PR that does just those things for now? |
9 tasks
Contributor
Author
|
Superseded by #1238 — slimmed-down V1 per the feedback here. Closes this out. |
Contributor
Author
|
Closing in favor of the scoped V1 PR → #1238 |
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
3 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 #1183
Why this change
Writing and maintaining long-form text entries — character descriptions, persona definitions, lorebook entries, system prompts — is one of the more tedious parts of setting up a roleplay session. Other RP platforms offer AI-assisted rewriting that lets users iteratively refine these fields with natural language instructions: "add more detail about their childhood," "convert to XML format," "make this more token-efficient."
Currently Marinara has no built-in mechanism for this. A Marinara app extension (
magic-rewrite-extension) exists as a proof of concept, but it requires users to manually enter API keys because it cannot access the LLM connections already configured in the app. This PR ships the feature natively, reusing Marinara's existing connection infrastructure so users can leverage their configured endpoints with no additional credential management.What changed
packages/shared/src/types/connection.ts+packages/shared/src/schemas/connection.schema.ts: adddefaultForRewriteboolean field to the shared connection type and Zod create schema.packages/server/src/db/schema/connections.ts+packages/server/src/db/migrate.ts: adddefault_for_rewritetext column to theapi_connectionstable with a migration entry.packages/server/src/services/storage/connections.storage.ts: addgetDefaultForRewrite()method and mutual-exclusion enforcement in create/update — only one non-image-generation connection can hold the rewrite default at a time.packages/server/src/routes/magic-rewrite.routes.ts: newPOST /api/magic-rewrite/generateroute. Acceptstext,instruction, and optionalcontext(character, lorebook, chat history). Resolves connection via rewrite-specific → default chat → agent default chain. UsescreateLLMProvider()andchatComplete()with a focused system prompt that returns only rewritten text.packages/client/src/components/ui/MagicRewritePanel.tsx: new panel component with instruction textarea, context selectors (character card, lorebook entries, chat history with configurable message count up to 250), generate button with loading spinner, and a word-level diff preview showing before/after with changed spans highlighted. Supports persistent instruction recall vialocalStorage.packages/client/src/components/lorebooks/LorebookFormFields.tsx: adds ✨ Rewrite button in the lorebook description editor modal (bottom-right, left of Done). Clicking it enters rewrite mode with Back/Apply controls.packages/client/src/components/ui/ExpandedTextarea.tsx: adds ✨ Rewrite button in the shared full-screen editor header. This automatically makes the feature available in the character editor, persona editor, and chat settings drawer — all of which use this shared component.packages/client/src/components/connections/ConnectionEditor.tsx: adds "Default for Rewrite" toggle beneath the existing "Default for Agents" toggle, with tooltip explaining the fallback behavior. Hidden for image-generation provider connections.Validation
pnpm checkpasses locallyCONTRIBUTING.mdManual verification notes
localStorage.Docs and release impact
UI evidence (if applicable)