fix: trim card names, scope agent context, and render Markdown - #4307
Merged
Conversation
📝 WalkthroughWalkthroughAdds configurable context sources for custom agents, Markdown previews and rendered card text, character-name trimming across validation and storage, and regression coverage for edited-message memory recall. ChangesCustom-agent context controls
Markdown previews and rendering
Character-name normalization
Edited-message memory recall
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentEditor
participant AgentResolution
participant AgentExecutor
participant GenerationContext
AgentEditor->>AgentResolution: persist selected contextSources
AgentResolution->>AgentExecutor: provide isCustomAgent metadata
GenerationContext->>AgentExecutor: provide authorNotes and activatedLorebookEntries
AgentExecutor->>AgentExecutor: compute selected source union
AgentExecutor->>AgentExecutor: include matching prompt context blocks
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
1 task
This was referenced Jul 30, 2026
Collaborator
Author
|
@coderabbitai review |
SpicyMarinara
marked this pull request as ready for review
July 30, 2026 13:45
✅ Action performedReview finished.
|
![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
🤖 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/agents/AgentEditor.tsx`:
- Around line 314-362: Keep CUSTOM_AGENT_CONTEXT_SOURCE_META synchronized with
CUSTOM_AGENT_CONTEXT_SOURCE_IDS by adding a dev-time completeness check that
verifies every shared source ID has exactly one metadata entry. Use the existing
source-ID symbols and preserve the current metadata structure, while making
missing or duplicate entries fail during development rather than silently
omitting an editor toggle.
In `@packages/client/src/components/characters/CharacterEditor.tsx`:
- Around line 1017-1020: Apply the same blur-time name normalization used by the
existing editable name input to the Metadata-tab name input near the metadata
field. Add an onBlur handler that trims formData.name and calls
updateField("name", trimmedName) only when the value changes, ensuring both
inputs keep the shared name state normalized.
In `@packages/server/src/services/agents/agent-executor.ts`:
- Around line 2631-2653: Escape the user-authored text appended in the
chatSummary and authorNotes branches of buildAgentExtras using the existing
escapeXml helper, matching the activatedLorebookEntries block. Keep the
surrounding tags and conditional gating unchanged while ensuring both context
values are escaped before insertion.
In `@packages/server/src/services/storage/characters.storage.ts`:
- Line 97: Update createVersionSnapshot to normalize the parsed existing
character data before serializing and persisting the snapshot. Reuse
normalizeCharacterData on the result of parseCharacterData(existing.data), while
preserving the existing snapshot storage flow.
In `@packages/shared/src/types/agent.ts`:
- Around line 553-573: Update the context-source defaults used by
normalizeCustomAgentContextSources so missing contextSources preserve the full
legacy-enabled behavior, including characters, persona, lorebook entries, chat
summary, author notes, tracker data, and recalled memories. Apply the same
legacy default constant to both server and local duplicate definitions, while
retaining explicit stored boolean values during normalization.
In `@scripts/regressions/open-issues.regression.ts`:
- Around line 520-528: Add focused regression cases in the storage normalization
tests for restoreVersion, resetVersions, and duplicateCharacter, using padded
character names and asserting each path persists or returns the trimmed name.
Keep the existing create coverage unchanged and exercise the storage methods
directly to verify normalization when route validation is bypassed.
In `@scripts/regressions/regeneration-context.regression.ts`:
- Around line 197-212: Strengthen the invalidation test around
chatStorage.updateMessageContent by seeding a second memoryChunks row for
editedChat whose message range does not include createdMessages[2]. Assert
afterEdit retains that unaffected chunk while removing the overlapping
stale-edited-memory row, proving invalidation is targeted rather than deleting
every chunk for the chat.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7fbcafd0-e43b-4f7e-89fd-eadde1a1203b
📒 Files selected for processing (20)
CHANGELOG.mde2e/core-flows.e2e.tspackages/client/src/components/agents/AgentEditor.tsxpackages/client/src/components/characters/CharacterEditor.tsxpackages/client/src/components/characters/CharacterLibraryView.tsxpackages/client/src/components/lorebooks/LorebookEntryRow.tsxpackages/client/src/components/lorebooks/LorebookFormFields.tsxpackages/client/src/components/personas/PersonaEditor.tsxpackages/client/src/components/ui/MacroTextarea.tsxpackages/client/src/localization/locales/en.jsonpackages/server/src/routes/generate.routes.tspackages/server/src/routes/generate/retry-agents-route.tspackages/server/src/services/agents/agent-executor.tspackages/server/src/services/generation/agent-resolution.tspackages/server/src/services/storage/characters.storage.tspackages/shared/src/schemas/character.schema.tspackages/shared/src/types/agent.tsscripts/regressions/open-issues.regression.tsscripts/regressions/prompt.regression.tsscripts/regressions/regeneration-context.regression.ts
SpicyMarinara
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/server/src/services/agents/agent-executor.ts (2)
1268-1302: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude recalled-memory vector context in batched agent prompts.
buildBatchSystemPromptassembles lore/extras from the batch’s selected sources, but the batch call only renders that merged prompt and does not appendbuildCustomAgentVectorContextBlock. Since that helper is only used in the standard single-agent path, agents withrecalledMemoriesenabled won’t get their<vector_context>block in combined batch calls. Add equivalent vector-context assembly for batch configs or merge the block into the batch system prompt.🤖 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/agents/agent-executor.ts` around lines 1268 - 1302, Update buildBatchSystemPrompt to include recalled-memory vector context for batch configs by assembling the equivalent buildCustomAgentVectorContextBlock output and merging it into the generated system prompt. Ensure agents with recalledMemories enabled receive a <vector_context> block in combined calls while preserving existing lore and extras assembly.
83-100: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftMake
isCustomAgentrequired at the prompt boundary.
isCustomAgentis optional, andgetAgentContextSourcestreatsundefinedas a built-in agent, returningALL_AGENT_CONTEXT_SOURCES. A custom-agent config that omits this field will ignore savedsettings.contextSourcesand send unrelated prompt context to the model. Make the discriminator required, with explicitfalsefor built-ins, or assert custom-agent configs before prompt assembly.🤖 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/agents/agent-executor.ts` around lines 83 - 100, Make isCustomAgent required in the prompt-boundary configuration and update all built-in agent call sites to pass false explicitly, ensuring custom-agent configurations cannot omit the discriminator and bypass settings.contextSources. Update getAgentContextSources and related AgentExecConfig types as needed while preserving normalized custom sources and ALL_AGENT_CONTEXT_SOURCES for explicit built-ins.packages/server/src/services/storage/characters.storage.ts (1)
509-518: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winNormalize the pre-restore snapshot as well.
restoredDatais normalized, butcurrentDataremains raw and is serialized at Line [518]. Restoring over a legacy padded name therefore leaves that padded value in version history, and the comparison can create an unnecessary snapshot.Normalize
currentDatabefore both the comparison and insertion:Proposed fix
- const currentData = parseCharacterData(existing.data); + const currentData = normalizeCharacterData(parseCharacterData(existing.data));This keeps the restore path aligned with the PR’s name-normalization objective.
🤖 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/characters.storage.ts` around lines 509 - 518, Normalize currentData before the alreadyMatches comparison and before serializing it in the character card version insert within the restore flow. Use the normalized current value alongside restoredData so legacy padded names do not trigger unnecessary snapshots or remain in version history.
🤖 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 `@scripts/regressions/open-issues.regression.ts`:
- Around line 559-577: Update the restoreVersion regression fixture so the
current character row’s name is padded before restoration, ensuring the
transactional pre-restore snapshot exercises legacy normalization. After calling
restoreVersion, query the inserted characterCardVersions record with source
"restore" and assert its stored JSON name is trimmed, while retaining the
existing restored-card assertion as appropriate.
In `@scripts/regressions/regeneration-context.regression.ts`:
- Around line 248-251: Replace the dynamic RegExp assertions in the regression
test with direct substring assertions using includes for both editedLine and
originalLine checks. Keep the existing positive and negative assertion behavior
around rebuiltEditedChunk.content unchanged.
---
Outside diff comments:
In `@packages/server/src/services/agents/agent-executor.ts`:
- Around line 1268-1302: Update buildBatchSystemPrompt to include
recalled-memory vector context for batch configs by assembling the equivalent
buildCustomAgentVectorContextBlock output and merging it into the generated
system prompt. Ensure agents with recalledMemories enabled receive a
<vector_context> block in combined calls while preserving existing lore and
extras assembly.
- Around line 83-100: Make isCustomAgent required in the prompt-boundary
configuration and update all built-in agent call sites to pass false explicitly,
ensuring custom-agent configurations cannot omit the discriminator and bypass
settings.contextSources. Update getAgentContextSources and related
AgentExecConfig types as needed while preserving normalized custom sources and
ALL_AGENT_CONTEXT_SOURCES for explicit built-ins.
In `@packages/server/src/services/storage/characters.storage.ts`:
- Around line 509-518: Normalize currentData before the alreadyMatches
comparison and before serializing it in the character card version insert within
the restore flow. Use the normalized current value alongside restoredData so
legacy padded names do not trigger unnecessary snapshots or remain in version
history.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 860549b8-5904-4d54-b084-dccbb1187638
📒 Files selected for processing (6)
packages/client/src/components/agents/AgentEditor.tsxpackages/client/src/components/characters/CharacterEditor.tsxpackages/server/src/services/agents/agent-executor.tspackages/server/src/services/storage/characters.storage.tsscripts/regressions/open-issues.regression.tsscripts/regressions/regeneration-context.regression.ts
Collaborator
Author
|
CodeRabbit incremental outside-diff follow-up:
Validation after these changes: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/src/services/agents/agent-executor.ts (1)
1091-1097: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
getBatchContextSources(configs)computation.
executeAgentBatchcomputesbatchContextSourcesfromconfigsat line 1097, thenbuildBatchSystemPrompt(called with the sameconfigs) recomputes the identical union at line 1268. SincegetBatchContextSourcesis pure andconfigsdoesn't change between the two calls, consider threading the already-computed value intobuildBatchSystemPromptinstead of recomputing it.♻️ Proposed refactor
function buildBatchSystemPrompt( configs: AgentExecConfig[], context: AgentContext, renderedTemplates?: RenderedAgentTemplateMap, + contextSources: CustomAgentContextSources = getBatchContextSources(configs), ): string { const parts: string[] = []; - const contextSources = getBatchContextSources(configs);And pass
batchContextSourcesfromexecuteAgentBatch's call site:- const systemPrompt = buildBatchSystemPrompt(configs, context, renderedTemplates); + const batchContextSources = getBatchContextSources(configs); + const systemPrompt = buildBatchSystemPrompt(configs, context, renderedTemplates, batchContextSources);Also applies to: 1262-1268
🤖 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/agents/agent-executor.ts` around lines 1091 - 1097, Remove the redundant getBatchContextSources computation by extending buildBatchSystemPrompt to accept the precomputed batchContextSources value. Pass that value from executeAgentBatch at its call site, and use the parameter inside buildBatchSystemPrompt instead of recomputing the union from configs.
🤖 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.
Outside diff comments:
In `@packages/server/src/services/agents/agent-executor.ts`:
- Around line 1091-1097: Remove the redundant getBatchContextSources computation
by extending buildBatchSystemPrompt to accept the precomputed
batchContextSources value. Pass that value from executeAgentBatch at its call
site, and use the parameter inside buildBatchSystemPrompt instead of recomputing
the union from configs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0fad529a-2af9-49c9-9d63-c1e64f200c16
📒 Files selected for processing (6)
packages/server/src/routes/generate.routes.tspackages/server/src/services/agents/agent-executor.tspackages/server/src/services/storage/characters.storage.tsscripts/regressions/open-issues.regression.tsscripts/regressions/prompt.regression.tsscripts/regressions/regeneration-context.regression.ts
Collaborator
Author
|
Final CodeRabbit follow-up: I am not applying the low-value |
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
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.
Linked issues
Closes #4303
Closes #4304
Closes #4305
Closes #4306
Why this change
The latest issue sweep found four user-visible gaps: pasted Character names retain accidental edge whitespace, edited chat text needs durable Memory Recall protection, custom agents receive context they do not need, and card/lorebook fields have no rendered Markdown view. Each correction stays within the existing subsystem without introducing a parallel framework.
What changes
Validation
Automated evidence run locally:
Manual verification needed
Summary by CodeRabbit