feat: replace dialogueStyle with notes on characters#261
Conversation
- Remove dialogue_style column from characters schema, add notes (text) - Migration 0051: rename dialogue_style to notes (db:generate) - Update Character shared type, validation, service, API types, hooks - Replace dialogueStyle input with notes textarea in CharacterEditDialog - Remove Style badge from CharacterList and LabelPropertiesPanel - Update all test fixtures and DATABASE_SCHEMAS.md
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
React Doctor found 1 new issue in 1 file · 1 warning · score 90 / 100 (Great) · 0 fixed · vs 1 warning
Reviewed by React Doctor for commit |
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesdialogueStyle to notes rename
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/backend/src/lib/__tests__/validation.unit.test.ts (1)
1015-1030: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd boundary tests for the 10,000-char
noteslimit.The rename is correct, but no test asserts the new max-length requirement itself (accepting exactly 10,000 chars, rejecting 10,001+). Since "support notes up to 10,000 characters" is an explicit PR requirement, a boundary test would guard the
optionalString(10000)constraint.✅ Suggested additional tests
it("should accept valid full character payload", () => { ... }); + + it("should accept notes at the 10,000 character limit", () => { + const validData = { + projectId: "550e8400-e29b-41d4-a716-446655440000", + name: "Character Name", + displayName: "Display Name", + renpyTag: "char", + color: "`#FF5733`", + notes: "a".repeat(10000), + }; + const result = createCharacterSchema.safeParse(validData); + expect(result.success).toBe(true); + }); + + it("should reject notes exceeding 10,000 characters", () => { + const invalidData = { + projectId: "550e8400-e29b-41d4-a716-446655440000", + name: "Character Name", + displayName: "Display Name", + renpyTag: "char", + color: "`#FF5733`", + notes: "a".repeat(10001), + }; + const result = createCharacterSchema.safeParse(invalidData); + expect(result.success).toBe(false); + });🤖 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 `@apps/backend/src/lib/__tests__/validation.unit.test.ts` around lines 1015 - 1030, The createCharacterSchema validation tests currently cover a valid payload but do not verify the notes length boundary added by optionalString(10000). Add focused tests in validation.unit.test.ts near the createCharacterSchema suite to assert that notes with exactly 10,000 characters is accepted and 10,001 characters is rejected, keeping the existing valid full character payload test intact.apps/backend/src/services/characters.service.ts (1)
53-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
CharacterSummaryandCharacterDetailare now identical.After this rename both interfaces have the exact same fields (id, name, displayName, renpyTag, color, routeAffiliation, isLoveInterest, isNarrator, notes, conditionalPrefix, avatarUrl). Consider consolidating to avoid future drift.
♻️ Suggested consolidation
-export interface CharacterDetail { - id: string; - name: string; - displayName: string; - renpyTag: string; - color: string; - routeAffiliation: string | null; - isLoveInterest: boolean; - isNarrator: boolean; - notes: string | null; - conditionalPrefix: string | null; - avatarUrl: string | null; -} +export type CharacterDetail = CharacterSummary;Also applies to: 68-80
🤖 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 `@apps/backend/src/services/characters.service.ts` around lines 53 - 65, `CharacterSummary` and `CharacterDetail` now have the same shape, so the service should be consolidated to avoid duplicated definitions and future drift. Update `characters.service.ts` to use a single shared type (or derive one from the other) for the identical fields, and adjust any references in `CharacterSummary`/`CharacterDetail` usage so both names no longer carry separate field lists.apps/frontend/src/components/CharacterEditDialog.tsx (1)
140-163: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo client-side length guard for the 10,000-char notes limit.
The notes textarea and
validateFormdon't enforce the 10,000-character limit mentioned in the PR objectives. Users can type past the backend limit and only find out via a generic save-failure toast, with no inline indication of which field/why.♻️ Suggested guard
+const NOTES_MAX_LENGTH = 10000; + function validateForm(state: CharacterFormState): { valid: boolean; errors: Record<string, string>; } { const errors: Record<string, string> = {}; ... + if (state.notes.length > NOTES_MAX_LENGTH) { + errors.notes = `Notes must be ${NOTES_MAX_LENGTH} characters or fewer`; + } return { valid: Object.keys(errors).length === 0, errors }; }<textarea id="edit-char-notes" rows={4} + maxLength={NOTES_MAX_LENGTH} className="..." placeholder="Backstory, personality notes, voice references..." value={form.notes} onChange={(e) => handleFieldChange("notes", e.target.value)} disabled={isSaving} />Also applies to: 548-561
🤖 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 `@apps/frontend/src/components/CharacterEditDialog.tsx` around lines 140 - 163, Add a client-side length check for the notes field in CharacterEditDialog so the form enforces the 10,000-character limit before save. Update validateForm to validate the notes value in CharacterFormState and set an inline errors.notes message when it exceeds the limit, and ensure the notes textarea in the dialog reflects that constraint so users get immediate feedback instead of a generic save failure.
🤖 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 `@apps/backend/src/lib/__tests__/validation.unit.test.ts`:
- Around line 1015-1030: The createCharacterSchema validation tests currently
cover a valid payload but do not verify the notes length boundary added by
optionalString(10000). Add focused tests in validation.unit.test.ts near the
createCharacterSchema suite to assert that notes with exactly 10,000 characters
is accepted and 10,001 characters is rejected, keeping the existing valid full
character payload test intact.
In `@apps/backend/src/services/characters.service.ts`:
- Around line 53-65: `CharacterSummary` and `CharacterDetail` now have the same
shape, so the service should be consolidated to avoid duplicated definitions and
future drift. Update `characters.service.ts` to use a single shared type (or
derive one from the other) for the identical fields, and adjust any references
in `CharacterSummary`/`CharacterDetail` usage so both names no longer carry
separate field lists.
In `@apps/frontend/src/components/CharacterEditDialog.tsx`:
- Around line 140-163: Add a client-side length check for the notes field in
CharacterEditDialog so the form enforces the 10,000-character limit before save.
Update validateForm to validate the notes value in CharacterFormState and set an
inline errors.notes message when it exceeds the limit, and ensure the notes
textarea in the dialog reflects that constraint so users get immediate feedback
instead of a generic save failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c09bc54-4127-4881-974e-0c3fee283398
📒 Files selected for processing (23)
apps/backend/src/db/migrations/0051_same_blur.sqlapps/backend/src/db/migrations/meta/0051_snapshot.jsonapps/backend/src/db/migrations/meta/_journal.jsonapps/backend/src/db/schema/tables/characters.tsapps/backend/src/lib/__tests__/validation.unit.test.tsapps/backend/src/lib/validation.tsapps/backend/src/routes/__tests__/characters.avatar.integration.test.tsapps/backend/src/routes/__tests__/characters.routes.integration.test.tsapps/backend/src/services/__tests__/characters.service.unit.test.tsapps/backend/src/services/characters.service.tsapps/frontend/src/components/CharacterEditDialog.tsxapps/frontend/src/components/CharacterList.tsxapps/frontend/src/components/__tests__/CharacterDialog.test.tsxapps/frontend/src/components/__tests__/CharacterImportWizard.test.tsxapps/frontend/src/components/__tests__/CharacterList.test.tsxapps/frontend/src/components/script-mode/__tests__/ScriptReferencePanel.test.tsxapps/frontend/src/components/write-mode/LabelPropertiesPanel.tsxapps/frontend/src/components/write-mode/__tests__/DialogueLine.test.tsxapps/frontend/src/hooks/__tests__/useCharacters.test.tsxapps/frontend/src/hooks/useCharacters.tsapps/frontend/src/lib/api/characters.tsdocs/DATABASE_SCHEMAS.mdpackages/shared/src/index.ts
💤 Files with no reviewable changes (1)
- apps/frontend/src/components/write-mode/LabelPropertiesPanel.tsx
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <Label htmlFor="edit-char-notes" className="text-xs"> | ||
| Notes | ||
| </Label> | ||
| <textarea |
There was a problem hiding this comment.
React Doctor · react-doctor/control-has-associated-label (warning)
Blind users can't tell what this control does because screen readers find no label, so add visible text, aria-label, or aria-labelledby.
Fix → Give every interactive control a label screen readers can read.
1 participant
Closes #257
What changed
Replaced the underused
dialogueStylefree-text field (100 chars, no clear purpose) with a largernotestextarea (10,000 chars) for arbitrary author notes per character.Database
dialogue_style→notes(text)db:generate— usesRENAME COLUMNto preserve existing dataAll layers updated
CharacterEditDialog: dialogueStyle input removed, notes textarea added (min-h 250px)CharacterList+LabelPropertiesPanel: Style: badge removedlistCharactersendpoint now includesnotesandconditionalPrefix(fixing a pre-existing bug where conditional prefix badge was always hidden)How verified
dialogueStyle/dialogue_stylereferences@oracle review
Reviewed and approved. Two should-fix items (stale comments, leftover grid-cols-3) addressed before opening PR.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation