feat(magic-rewrite): port V1 to refactor#1260
Conversation
|
Warning Review limit reached
More reviews will be available in 13 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAh, a most elegant specimen indeed! This pull request introduces a sophisticated text-rewriting apparatus powered by linguistic artifice. A new ChangesMagic Rewrite Integration Framework
Ah, you wish to observe the apparatus in action? Behold: sequenceDiagram
participant User as User
participant ExpandedTextarea as ExpandedTextarea
participant MagicRewritePanel as MagicRewritePanel
participant useMagicRewrite as useMagicRewrite
participant LLMApi as LLMApi
User->>ExpandedTextarea: Click "Magic Rewrite"
activate ExpandedTextarea
ExpandedTextarea->>ExpandedTextarea: Set magicRewriteMode=true
ExpandedTextarea->>MagicRewritePanel: Render with current text
deactivate ExpandedTextarea
User->>MagicRewritePanel: Edit instruction & Click Generate
activate MagicRewritePanel
MagicRewritePanel->>useMagicRewrite: Call generate()
activate useMagicRewrite
useMagicRewrite->>LLMApi: POST messages with instruction
activate LLMApi
LLMApi-->>useMagicRewrite: Rewritten text
deactivate LLMApi
useMagicRewrite->>useMagicRewrite: Compute word-level diff
useMagicRewrite-->>MagicRewritePanel: result updated
deactivate useMagicRewrite
MagicRewritePanel->>MagicRewritePanel: Render Before/After with highlights
MagicRewritePanel-->>User: Show diff preview
deactivate MagicRewritePanel
User->>ExpandedTextarea: Click "Apply"
activate ExpandedTextarea
ExpandedTextarea->>ExpandedTextarea: Update local=result
ExpandedTextarea->>ExpandedTextarea: Set magicRewriteMode=false
ExpandedTextarea->>ExpandedTextarea: Fire onChange(local)
ExpandedTextarea-->>User: Return to editor with new text
deactivate ExpandedTextarea
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The implementation spans multiple new files and non-trivial integration points. The Possibly Related PRs
Poem
🚥 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 review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (1)
src/features/catalog/lorebooks/components/LorebookFormFields.tsx (1)
320-325: ⚡ Quick winAdd
type="button"toExpandedContentModalcontrols in the lorebook editorThe modal buttons (close/back/apply/rewrite/done) in
src/features/catalog/lorebooks/components/LorebookFormFields.tsxomittype;buttondefaults tosubmitonly when nested in a<form>. In this repo,<form>elements only appear inAppDialogRendererandSettingsPanel, so the lorebook editor/modal isn’t currently at risk of accidental submit—but addingtype="button"is a cheap guard against future layout changes. Ah, a prudent little preventative experiment.🧪 Proposed fix
<div className="flex items-center gap-2"> <button + type="button" onClick={handleClose} className="rounded-lg p-1.5 hover:bg-[var(--accent)]" > <X size="1rem" /> </button> @@ <div className="flex items-center gap-2"> <button + type="button" onClick={handleMagicRewriteBack} className="rounded-xl border border-[var(--border)] px-4 py-1.5 text-xs font-medium text-[var(--foreground)] hover:bg-[var(--accent)] active:scale-[0.98]" > Back </button> <button + type="button" onClick={handleMagicRewriteApply} disabled={!magicRewriteResult} className="rounded-xl bg-gradient-to-r from-violet-500 to-fuchsia-500 px-4 py-1.5 text-xs font-medium text-white shadow-md hover:shadow-lg active:scale-[0.98] disabled:cursor-not-allowed disabled:opacity-40" > Apply @@ <div className="flex items-center gap-2"> <button + type="button" onClick={() => setMagicRewriteMode(true)} className="inline-flex items-center gap-1.5 rounded-lg border border-violet-400/30 bg-violet-500/10 px-3 py-1.5 text-xs font-medium text-violet-200 hover:bg-violet-500/20" title="Open Magic Rewrite" > <Sparkles size="0.875rem" /> Rewrite </button> <button + type="button" onClick={handleClose} className="rounded-xl bg-gradient-to-r from-amber-400 to-orange-500 px-4 py-1.5 text-xs font-medium text-white shadow-md hover:shadow-lg active:scale-[0.98]" > Done </button>🤖 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/features/catalog/lorebooks/components/LorebookFormFields.tsx` around lines 320 - 325, The modal control buttons in LorebookFormFields.tsx (e.g., the close button using onClick={handleClose} and other modal controls: back, apply, rewrite, done) are missing explicit types and should be declared as non-submitting controls; update each <button> element used in the ExpandedContentModal controls to include type="button" to prevent accidental form submission (specifically add type="button" to the button rendering the X icon tied to handleClose and to the other modal control buttons referenced in this component).
🤖 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/shared/components/ui/MagicRewritePanel.tsx`:
- Around line 96-107: The button in MagicRewritePanel (the element with
onClick={generate} and disabled={loading}) lacks an explicit type and can
implicitly act as a form submitter; update that button element to include
type="button" to prevent accidental form submissions when this component is
rendered inside a <form>, keeping the onClick handler (generate) and other props
unchanged.
In `@src/shared/hooks/use-magic-rewrite.ts`:
- Around line 90-103: The generate() function leaves a prior successful result
visible if the new llmApi.complete(...) call fails; call setResult("") at the
start of generate (e.g., immediately after setLoading(true)/setError("")) to
clear any previous rewrite before attempting the new request so the UI cannot
show stale text under a new error; ensure generate and the state setters
setResult, setLoading, setError are the places you change.
- Around line 59-71: The current selection picks the default connection even if
it's an image_generation provider and throws before checking other text
connections; update the selection logic used around
storageApi.list<ConnectionRecord>("connections") so it first looks for a default
connection whose provider !== "image_generation" (e.g., connections.find(c =>
(boolish(c.isDefault) || boolish(c.default)) && c.provider !==
"image_generation")), then if none found fall back to the first connection with
provider !== "image_generation" (then finally to connections[0] only if no text
providers exist), and keep computing connectionId from the chosen selected;
ensure you only throw "No default agent connection configured" if no text
connection was found at all and remove the premature image_generation throw that
ignores later text connections.
---
Nitpick comments:
In `@src/features/catalog/lorebooks/components/LorebookFormFields.tsx`:
- Around line 320-325: The modal control buttons in LorebookFormFields.tsx
(e.g., the close button using onClick={handleClose} and other modal controls:
back, apply, rewrite, done) are missing explicit types and should be declared as
non-submitting controls; update each <button> element used in the
ExpandedContentModal controls to include type="button" to prevent accidental
form submission (specifically add type="button" to the button rendering the X
icon tied to handleClose and to the other modal control buttons referenced in
this component).
🪄 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: 1a832110-5911-416a-9d60-a60e1b0ba327
⛔ Files ignored due to path filters (1)
docs/evidence/pr-1260-magic-rewrite/after.pngis excluded by!**/*.png
📒 Files selected for processing (5)
docs/evidence/pr-1260-magic-rewrite/provider-request.jsonsrc/features/catalog/lorebooks/components/LorebookFormFields.tsxsrc/shared/components/ui/ExpandedTextarea.tsxsrc/shared/components/ui/MagicRewritePanel.tsxsrc/shared/hooks/use-magic-rewrite.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Linked issue
Refs #1183
Refs #1238
Why this change
PR #1238 contains the Magic Rewrite V1 feature, but its branch was not based on the active
refactorbranch.This PR ports the Magic Rewrite V1 surface directly onto
refactorand adapts the implementation to the refactor architecture, where editor UI calls the shared LLM gateway instead of adding the old package/server route path.What changed
llmApi.complete.MagicRewritePanelwith instructions, Generate Rewrite, Before/After preview, and word-level diff highlighting.Refactor impact
Primary owner:
Impact areas reviewed:
src/shared/api/llm-api.tsandstorageApi; no old package/server route was restored.llm_completevia the Rust remote runtime.Boundary notes:
src/shared/hooks/use-magic-rewrite.tsstays in shared code and imports only shared API wrappers.Pressure points touched:
src-tauri/src/lib.rscommand registration, mode surface, or import-module changes.Validation
pnpm checkpasses locallypnpm typecheckpasses locallypnpm buildpasses locallypnpm check:architecturepasses locallypnpm check:docspasses locallycargo check --manifest-path src-tauri/Cargo.toml --workspacepasses locallyManual verification notes
pnpm check:architecturesuccessfully inC:\tmp\marinara-pr-1238.pnpm check:frontendsuccessfully.pnpm checksuccessfully after refreshing onto currentupstream/refactor.git diff --check upstream/refactor...HEADsuccessfully.node scratch\verify-pr-1260-magic-rewrite.mjssuccessfully. The harness starts a disposable Rust remote runtime, seeds a default text connection and character, points the app at a fake OpenAI-compatible provider, opens the real character Description expanded editor, runs Magic Rewrite, receives the provider rewrite throughllm_complete, applies it, and captures proof.8f08fcd5and reran local validation.Docs and release impact
README.mdCONTRIBUTING.mddocs/developer/AGENTS.mdAdded scoped PR evidence under
docs/evidence/pr-1260-magic-rewrite/; no user-facing docs or release metadata changed.UI evidence
Runtime proof screenshot:
Provider request proof:
https://github.com/cha1latte/Marinara-Engine/blob/8f08fcd5de42cd273d4a9abf85a6bc81d83c2cd7/docs/evidence/pr-1260-magic-rewrite/provider-request.json