Skip to content

Address staging issue sweep through #4341 - #4340

Merged
SpicyMarinara merged 6 commits into
stagingfrom
agent/issue-sweep-4327-4337
Jul 31, 2026
Merged

Address staging issue sweep through #4341#4340
SpicyMarinara merged 6 commits into
stagingfrom
agent/issue-sweep-4327-4337

Conversation

@SpicyMarinara

@SpicyMarinara SpicyMarinara commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Important

Contributions target staging. Only SpicyMarinara may promote staging to main.

Linked issues

Closes #4327
Closes #4332
Closes #4333
Closes #4334
Closes #4335
Closes #4336
Closes #4337
Closes #4341

Follow-up documentation: #4342
Follow-up browser-suite isolation: #4343

Why this change

The open-issue sweep exposed several small but user-visible gaps across development startup, Windows subprocesses, lorebooks, summaries, provider reasoning, character macros, image agents, and chat ordering. This patch keeps each fix on the existing Engine path and adds focused regression coverage instead of parallel implementations.

What changed

  • Build the shared package before the server dev process and avoid deprecated Windows shell-spawn shapes.
  • Allow embedded lorebooks to stay attached while hidden from the general library.
  • Combine selected summaries into one replacement summary through the configured summary model.
  • Treat custom OpenAI reasoning Off as an explicit omission while retaining encrypted reasoning content when advanced reasoning is enabled.
  • Resolve exact character IDs into bounded card, tracker, and linked-lorebook prompt context.
  • Give custom image-prompt agents their own connection and reference-image settings in generation and retry paths.
  • Separate Recent activity ordering from creation-date Newest and Oldest ordering.
  • Update release notes, contributor guidance, localization, and macro documentation.

Automated verification completed locally

  • pnpm check
  • pnpm regression:prompt
  • pnpm regression:providers
  • pnpm regression:issues
  • git diff --check

The full pnpm smoke:ui run exposed pre-existing shared appearance/background fixture failures: 153 passed, 88 skipped, and 13 failed. A serial failed-only retry still reproduced 12 failures, showing stale live-state assertions in addition to parallel sharing. This is isolated in #4343 and deliberately excluded from this product patch.

Validation

  • pnpm check passes locally
  • Container (Docker / Podman) built and ran without issue
  • Ran the app, clicked through the changes manually
  • Checked edge cases (light + dark mode, mobile viewport, empty states, error paths)
  • Above manual verification completed (describe below)
  • Read and followed CONTRIBUTING.md

Manual verification needed

  • Manually verify hidden embedded lorebooks remain active but disappear from the general library.
  • Manually combine summaries and confirm ordering, message coverage, and replacement behavior.
  • Manually verify a custom provider with reasoning Off and with advanced reasoning enabled.
  • Manually verify known and unknown character-ID macros in an active roleplay.
  • Manually verify custom ComfyUI reference payloads in generation and retry paths.
  • Manually verify Recent, Newest, and Oldest chat ordering on desktop and mobile.
  • Manually verify Windows startup, update, and build flows no longer emit DEP0190.

Docs and release impact

  • No docs changes needed
  • Updated docs (README / CONTRIBUTING / android/README / CHANGELOG) as needed
  • Translated docs on the docs-i18n branch updated to match, or a [docs-i18n] follow-up issue opened
  • Version/release files updated (only if this PR includes a version bump)

Translated macro documentation is tracked in #4342.

Summary by CodeRabbit

  • New Features
    • Added character-ID macros for referenced character context in prompts.
    • Added multi-select summary combination for consolidated summaries.
    • Added visibility controls for embedded lorebooks.
    • Added image-generation settings for custom image-prompt agents.
    • Added “Recent” chat sorting as the default.
  • Bug Fixes
    • Improved reasoning controls for compatible AI providers.
    • Fixed image settings during generation retries.
    • Improved server development startup reliability.
    • Strengthened Windows command handling and update workflows.
  • Documentation
    • Documented character-ID macros and shared-package development requirements.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f199353c-00ee-482f-bf1e-20f8943e1c0f

📥 Commits

Reviewing files that changed from the base of the PR and between b7730a4 and 4891b47.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • CONTRIBUTING.md
  • packages/client/src/localization/locales/en.json
  • packages/server/src/routes/generate.routes.ts
  • scripts/regressions/prompt.regression.ts

📝 Walkthrough

Walkthrough

This PR adds character-ID prompt context, summary combination, embedded lorebook visibility controls, custom image-agent settings, Recent chat sorting, shared development build ordering, shell-free Windows process execution, and corrected reasoning handling for OpenAI-compatible providers.

Changes

Application feature updates

Layer / File(s) Summary
Referenced character prompt context
docs/prompts/macros.md, packages/shared/src/utils/macro-engine.ts, packages/server/src/services/prompt/*, scripts/regressions/prompt.regression.ts
Adds exact character-ID macros and inserts referenced character data and eligible lorebooks into system context.
Summary combination
packages/client/src/components/chat/SummaryPopover.tsx, packages/client/src/hooks/use-chats.ts, packages/server/src/routes/chats.routes.ts, packages/client/src/localization/locales/en.json
Adds multi-entry selection, token-aware condensation, replacement of selected entries, and localized controls.
Embedded lorebook visibility
packages/shared/src/schemas/lorebook.schema.ts, packages/shared/src/types/lorebook.ts, packages/server/src/db/schema/lorebooks.ts, packages/server/src/services/storage/lorebooks.storage.ts, packages/client/src/hooks/use-lorebooks.ts, packages/client/src/components/lorebooks/LorebookAssignmentSection.tsx, packages/client/src/components/characters/CharacterEditor.tsx
Adds hiddenFromLibrary, filters hidden lorebooks from general views, preserves owner assignments, and adds an editor switch.
Custom image-agent settings
packages/client/src/components/agents/AgentEditor.tsx, packages/server/src/routes/generate.routes.ts, packages/server/src/routes/generate/retry-agents-route.ts
Extends image settings to custom image_prompt agents and preserves the originating agent during generation and retry flows.
Recent chat sorting
packages/client/src/lib/chat-recency.ts, packages/client/src/components/layout/ChatSidebar.tsx
Adds creation-time sorting, separates Recent from Newest and Oldest, and makes Recent the default.

Compatibility and development tooling

Layer / File(s) Summary
Shared development build ordering
package.json, CONTRIBUTING.md
Builds shared output before pnpm dev:server and documents the rebuild and restart boundary.
Shell-free process execution
packages/client/scripts/build.mjs, packages/server/src/config/build-info.ts, packages/server/src/routes/updates.routes.ts, packages/server/src/services/mari-db/mari-db.service.ts, scripts/check-tracked-installers.mjs, scripts/ensure-native-deps.mjs
Removes unrestricted Windows shell execution and uses direct or validated cmd.exe invocation.
Provider reasoning handling
packages/server/src/services/llm/providers/openai.provider.ts, packages/client/src/components/ui/GenerationParametersEditor.tsx, packages/client/src/localization/locales/en.json
Gates reasoning parameters by model support and sends explicit disabled reasoning where supported.
Validation and release notes
scripts/regressions/open-issues.regression.ts, scripts/regressions/provider-compat.regression.ts, CHANGELOG.md
Adds regression coverage for the feature and compatibility changes and records them in the changelog.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: bugfix, feature, windows

Suggested reviewers: kolacheee, thetopham

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.16% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately identifies a staging issue sweep and is concise enough for the multi-issue changeset.
Description check ✅ Passed The description includes linked issues, rationale, key changes, validation commands, known test failures, and documentation impact.
Linked Issues check ✅ Passed The implementation and regression tests address the coding objectives for issues #4327, #4332#4337, and #4341.
Out of Scope Changes check ✅ Passed The documentation, localization, release notes, and regression tests support the linked issue objectives without unrelated code changes.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/issue-sweep-4327-4337

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SpicyMarinara SpicyMarinara changed the title Address development, model, and library issue sweep Address staging issue sweep through #4341 Jul 31, 2026
@SpicyMarinara
SpicyMarinara marked this pull request as ready for review July 31, 2026 10:33
@SpicyMarinara

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

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/routes/generate.routes.ts (1)

8259-8283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Same fallback-agent misattribution bug in both routes. Both illustratorAgent (generate.routes.ts) and imagePromptAgent (retry-agents-route.ts) fall back to the built-in Illustrator agent's resolved config even when the current image_prompt result did not come from the Illustrator and the actual agent could not be found by ID. This can leak the Illustrator's imageConnectionId, useAvatarReferences, includeCharacterAppearance, and (in generate.routes.ts) prompt settings onto a different agent's image request.

  • packages/server/src/routes/generate.routes.ts#L8259-L8283: change illustratorAgent = resultAgent ?? fallbackIllustratorAgent to fall back to the Illustrator agent only when result.agentType === "illustrator", mirroring the correct illustratorBackgroundAgent ternary a few lines below.
  • packages/server/src/routes/generate/retry-agents-route.ts#L3194-L3199: apply the identical fix to imagePromptAgent.
🤖 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/generate.routes.ts` around lines 8259 - 8283,
Prevent fallback Illustrator configuration from being applied to unrelated
image-prompt results. In packages/server/src/routes/generate.routes.ts lines
8259-8283, update illustratorAgent to use fallbackIllustratorAgent only when
result.agentType === "illustrator"; in
packages/server/src/routes/generate/retry-agents-route.ts lines 3194-3199, apply
the same condition to imagePromptAgent. Preserve direct resultAgent resolution
and leave illustratorBackgroundAgent unchanged.
🤖 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 `@docs/prompts/macros.md`:
- Line 41: Replace the literal-looking {{CHARACTER_ID}} syntax with one clearly
marked placeholder form in docs/prompts/macros.md lines 41-41 and
packages/shared/src/utils/macro-engine.ts lines 312-316, such as
{{<21-character-card-ID>}}; update the registry syntax field to match the
documentation, and verify any client-side macro picker using this registry
inserts a real 21-character ID rather than the placeholder text.

In `@packages/server/src/routes/chats.routes.ts`:
- Around line 3927-3971: Before the provider.chatComplete call in the
selectedEntries flow, sum each selected entry’s tokenEstimate and reject the
request with a client error when the total exceeds a defined combined-summary
budget. Reuse the existing summary size/context limit symbols where applicable,
and ensure the guard runs before constructing or sending the oversized prompt
while preserving the current valid-request behavior.

In `@packages/server/src/services/prompt/assembler.ts`:
- Around line 358-372: Update the referenced-character scanning call to include
only enabled sections whose groups are enabled, reusing the existing
sectionMap/groupMap checks from hasDialogueExamplesMarker before collecting
section content. Extend sources to also include input.chatSummary,
input.personaDescription, and input.personaFields so character-ID macros in
those values are detected and resolved.

In `@packages/server/src/services/prompt/macro-context.ts`:
- Around line 74-92: Export the character-reference regex from the shared
macro-engine module and remove the local CHARACTER_REFERENCE_ID_PATTERN
definition in extractCharacterReferenceIds. Import and reuse the shared pattern
in macro-context.ts, preserving the existing matchAll extraction behavior and
limits.
- Around line 211-227: Parallelize the character lookups in the active-character
and referenced-character processing around the two loops by mapping IDs to
asynchronous getById operations and awaiting them with Promise.all. Preserve the
existing parsing, source accumulation, active-ID filtering, and referenced
result shape while eliminating sequential database round trips.

---

Outside diff comments:
In `@packages/server/src/routes/generate.routes.ts`:
- Around line 8259-8283: Prevent fallback Illustrator configuration from being
applied to unrelated image-prompt results. In
packages/server/src/routes/generate.routes.ts lines 8259-8283, update
illustratorAgent to use fallbackIllustratorAgent only when result.agentType ===
"illustrator"; in packages/server/src/routes/generate/retry-agents-route.ts
lines 3194-3199, apply the same condition to imagePromptAgent. Preserve direct
resultAgent resolution and leave illustratorBackgroundAgent unchanged.
🪄 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: 243c5ed0-c6c0-4965-bfab-8b0c68710ff9

📥 Commits

Reviewing files that changed from the base of the PR and between 0d94354 and 99e7b3f.

📒 Files selected for processing (34)
  • CHANGELOG.md
  • CONTRIBUTING.md
  • docs/prompts/macros.md
  • package.json
  • packages/client/scripts/build.mjs
  • packages/client/src/components/agents/AgentEditor.tsx
  • packages/client/src/components/characters/CharacterEditor.tsx
  • packages/client/src/components/chat/SummaryPopover.tsx
  • packages/client/src/components/layout/ChatSidebar.tsx
  • packages/client/src/components/lorebooks/LorebookAssignmentSection.tsx
  • packages/client/src/components/ui/GenerationParametersEditor.tsx
  • packages/client/src/hooks/use-chats.ts
  • packages/client/src/hooks/use-lorebooks.ts
  • packages/client/src/lib/chat-recency.ts
  • packages/client/src/localization/locales/en.json
  • packages/server/src/config/build-info.ts
  • packages/server/src/db/schema/lorebooks.ts
  • packages/server/src/routes/chats.routes.ts
  • packages/server/src/routes/generate.routes.ts
  • packages/server/src/routes/generate/retry-agents-route.ts
  • packages/server/src/routes/updates.routes.ts
  • packages/server/src/services/llm/providers/openai.provider.ts
  • packages/server/src/services/mari-db/mari-db.service.ts
  • packages/server/src/services/prompt/assembler.ts
  • packages/server/src/services/prompt/macro-context.ts
  • packages/server/src/services/storage/lorebooks.storage.ts
  • packages/shared/src/schemas/lorebook.schema.ts
  • packages/shared/src/types/lorebook.ts
  • packages/shared/src/utils/macro-engine.ts
  • scripts/check-tracked-installers.mjs
  • scripts/ensure-native-deps.mjs
  • scripts/regressions/open-issues.regression.ts
  • scripts/regressions/prompt.regression.ts
  • scripts/regressions/provider-compat.regression.ts
💤 Files with no reviewable changes (2)
  • scripts/check-tracked-installers.mjs
  • packages/server/src/config/build-info.ts

Comment thread docs/prompts/macros.md Outdated
Comment thread packages/server/src/routes/chats.routes.ts
Comment thread packages/server/src/services/prompt/assembler.ts
Comment thread packages/server/src/services/prompt/macro-context.ts Outdated
Comment thread packages/server/src/services/prompt/macro-context.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

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/routes/chats.routes.ts (2)

3963-3987: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add debug prompt logging for the summary-combination request.

This new model-generation path builds provider messages but does not call logDebugOverride(...) or an equivalent debug callback. Honor debugMode and DEBUG_AGENTS so debug mode shows the final provider prompt.

As per coding guidelines, “New agent, model-generation, image-generation, and prompt-building paths must wire prompt logging, honor debugMode and DEBUG_AGENTS, and use logDebugOverride(...) or an equivalent debug callback so the final provider prompt is visible in debug mode.”

🤖 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 3963 - 3987, Add
prompt-debug logging to the summary-combination request surrounding
provider.chatComplete, using the existing logDebugOverride-equivalent callback
and honoring both debugMode and DEBUG_AGENTS. Log the finalized system and user
messages sent to the provider without changing the summary generation behavior.

Source: Coding guidelines


3969-4003: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Reject concurrent summary-entry changes before replacement.

The provider receives sourceText from the original entries. The metadata callback only verifies that the selected IDs still exist.

If a user edits an entry during generation, this code removes the edited content and replaces it with a summary of stale content. If a user deletes an entry, Line 4002 throws and the route can return HTTP 500.

Capture a fingerprint of each selected entry before generation. In the metadata callback, compare the fresh entries with that fingerprint. Return HTTP 409 when they differ.

🤖 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 3969 - 4003, Capture
fingerprints of the selected entries before calling provider.chatComplete,
including the content used to build sourceText. In the patchMetadata callback,
compare each freshly selected entry against those fingerprints and reject any
added, removed, reordered, or edited entry as a conflict. Handle that conflict
at the route level by returning HTTP 409 instead of allowing the callback error
to become HTTP 500, while preserving the existing replacement flow for unchanged
entries.
packages/server/src/services/prompt/macro-context.ts (1)

75-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Apply the eight-character limit after excluding active IDs.

extractCharacterReferenceIds stops after eight unique IDs. Line 221 removes active IDs only after that limit is applied. If the first eight IDs are active characters, later external references are discarded even though they are the only IDs that this function loads.

Make the extractor exclude active IDs before counting the eight-character limit, or apply the limit after that exclusion.

Also applies to: 221-225

🤖 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/prompt/macro-context.ts` around lines 75 - 92,
Update extractCharacterReferenceIds and its active-ID filtering flow so active
character IDs are excluded before enforcing MAX_REFERENCED_CHARACTERS. Ensure
later non-active references are still collected when earlier matches are active,
while preserving uniqueness and the eight-ID cap for returned external
references.
🤖 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 3939-3955: The combined-summary budget uses
effectiveSummaryMaxTokens, but the provider request uses the larger
summaryMaxTokens. Update the provider.chatComplete call in the selected-summary
combination flow to pass effectiveSummaryMaxTokens, preserving the existing
budget calculation and validation.

In `@packages/server/src/routes/generate.routes.ts`:
- Around line 8273-8278: Rename illustratorAgent to imagePromptAgent throughout
this handler block, including its declaration and all downstream references,
while preserving the existing fallback selection and usesChatIllustratorSettings
behavior. Match the equivalent naming used in retry-agents-route.ts.

In `@packages/server/src/services/prompt/assembler.ts`:
- Around line 358-366: Update the enabledSectionContents collection to skip
sections under the same impersonation condition used by the render loop: when
input.impersonate is true and input.preserveImpersonatePresetSections is not
true, exclude non-marker sections from reference scanning while retaining marker
sections and existing enabled/group checks.

In `@packages/shared/src/utils/macro-engine.ts`:
- Around line 1769-1771: Update the character-reference replacement flow and the
later generic variable pass in the macro engine so unresolved 21-character
character IDs remain unchanged even when a variable has the same name. Track
unresolved IDs from the CHARACTER_REFERENCE_ID_PATTERN replacement and exclude
exact matches from generic variable substitution, including the corresponding
logic near the later replacement block.

In `@scripts/regressions/open-issues.regression.ts`:
- Around line 2578-2584: Replace the source-text assertions in
scripts/regressions/open-issues.regression.ts:2578-2584 with behavioral
regression tests that execute image generation and prompt-review retry using a
custom producing agent, then verify the actual calls use that agent. Also update
scripts/regressions/open-issues.regression.ts:3482-3485 to submit an oversized
summary combination, assert it is rejected, and verify provider.chatComplete is
not called.

---

Outside diff comments:
In `@packages/server/src/routes/chats.routes.ts`:
- Around line 3963-3987: Add prompt-debug logging to the summary-combination
request surrounding provider.chatComplete, using the existing
logDebugOverride-equivalent callback and honoring both debugMode and
DEBUG_AGENTS. Log the finalized system and user messages sent to the provider
without changing the summary generation behavior.
- Around line 3969-4003: Capture fingerprints of the selected entries before
calling provider.chatComplete, including the content used to build sourceText.
In the patchMetadata callback, compare each freshly selected entry against those
fingerprints and reject any added, removed, reordered, or edited entry as a
conflict. Handle that conflict at the route level by returning HTTP 409 instead
of allowing the callback error to become HTTP 500, while preserving the existing
replacement flow for unchanged entries.

In `@packages/server/src/services/prompt/macro-context.ts`:
- Around line 75-92: Update extractCharacterReferenceIds and its active-ID
filtering flow so active character IDs are excluded before enforcing
MAX_REFERENCED_CHARACTERS. Ensure later non-active references are still
collected when earlier matches are active, while preserving uniqueness and the
eight-ID cap for returned external references.
🪄 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: 702c0440-e0eb-4220-889b-44b391fa2eca

📥 Commits

Reviewing files that changed from the base of the PR and between 99e7b3f and a0c5c63.

📒 Files selected for processing (8)
  • docs/prompts/macros.md
  • packages/server/src/routes/chats.routes.ts
  • packages/server/src/routes/generate.routes.ts
  • packages/server/src/routes/generate/retry-agents-route.ts
  • packages/server/src/services/prompt/assembler.ts
  • packages/server/src/services/prompt/macro-context.ts
  • packages/shared/src/utils/macro-engine.ts
  • scripts/regressions/open-issues.regression.ts

Comment thread packages/server/src/routes/chats.routes.ts
Comment thread packages/server/src/routes/generate.routes.ts
Comment thread packages/server/src/services/prompt/assembler.ts
Comment thread packages/shared/src/utils/macro-engine.ts
Comment thread scripts/regressions/open-issues.regression.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (5)
packages/server/src/routes/chats.routes.ts (4)

3972-3987: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Wire this model-generation path into debug prompt logging.

This new provider.chatComplete path does not handle debugMode or DEBUG_AGENTS, and it does not call logDebugOverride(...) or an equivalent callback. Add the shared prompt logging hook so the final system and user messages are visible in debug mode.

As per coding guidelines, new agent, model-generation, image-generation, and prompt-building paths must wire prompt logging, honor debugMode and DEBUG_AGENTS, and use logDebugOverride(...) or an equivalent debug callback so the final provider prompt is visible in debug mode.

🤖 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 3972 - 3987, Update
the model-generation flow containing provider.chatComplete to wire in the shared
prompt-debug logging hook, including logDebugOverride(...) or the established
equivalent callback. Ensure the final system and user messages are logged when
debugMode or DEBUG_AGENTS is enabled, while preserving the existing prompt
content and completion behavior.

Source: Coding guidelines


4005-4006: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve hidden-message ownership for legacy entries.

This code collects only entry.hiddenMessageIds. Older entries can omit that field. The delete path at Lines 1174-1176 explicitly falls back to messageIds. When such entries are combined, the replacement can lose ownership of messages that were hidden by the original entries. Use the same fallback when building hiddenMessageIds.

Also applies to: 4025-4026

🤖 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 4005 - 4006, Update
the hiddenMessageIds aggregation in the selected-entry handling, including the
corresponding logic around the second occurrence, to fall back to each entry’s
messageIds when hiddenMessageIds is absent, matching the delete path’s
legacy-entry behavior while preserving deduplication.

4000-4003: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Reject stale combination results before replacing entries.

The provider combines entries captured before provider.chatComplete. The final callback checks only that the selected IDs still exist. If a selected entry changes while generation runs, the stale summaryText replaces newer content. Compare selected entry versions, timestamps, or content before committing, and return a conflict without writing the stale result.

🤖 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 4000 - 4003, Update
the final callback in the provider combination flow to validate that each
selected entry is unchanged from the entries captured before
provider.chatComplete, not merely that its ID still exists. Compare the
available version, timestamp, or content fields, and return the existing
conflict outcome without replacing entries when any selected entry has changed;
only commit summaryText when all selected entries still match.

3939-3955: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Account for the complete prompt in the input budget.

The new entry-content estimate does not include the resolved summary prompt, entry titles, or combination instructions. The fixed 1,024-token reserve can be exceeded by a custom prompt from CHAT_SUMMARY_PROMPT_SETTINGS_KEY. The provider request can then exceed its context limit after validation succeeds.

Resolve the prompt before the budget check and estimate the complete system and user prompt.

Also applies to: 3963-3971

🤖 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 3939 - 3955, Update
the combined-summary budget validation around effectiveSummaryMaxTokens and
combinedTokenEstimate to resolve the configured summary prompt before checking
the limit, then estimate the complete system and user prompt including the
prompt, entry titles, contents, and combination instructions. Use that full
estimate against the provider context budget instead of relying on the fixed
SUMMARY_COMBINE_PROMPT_RESERVE_TOKENS, while preserving the existing 400
response when the complete request exceeds the budget.
packages/server/src/services/prompt/assembler.ts (1)

370-393: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include groupScenarioOverrideText in referenced-character scanning.

buildPromptMacroContext() adds groupScenarioOverrideText to characterFields.scenario, but buildReferencedCharacterContext() only pulls character data from referencedCharacterSourceFields(data), which uses data.scenario. A character-ID macro in the group scenario override is not found during extraction unless the override is added to sources.

🤖 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/prompt/assembler.ts` around lines 370 - 393,
Update the sources passed to buildReferencedCharacterContext in
buildPromptMacroContext to include input.groupScenarioOverrideText, ensuring
character-ID macros in the group scenario override are scanned alongside the
existing prompt content.
🤖 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/generate.routes.ts`:
- Around line 8717-8720: Update both illustration retry failure payloads near
the image generation error handling to use result.agentType instead of the
hardcoded "illustrator", preserving the originating custom image_prompt agent
for retryAgents(). Also update the no-connection payload’s agent-specific
metadata to reflect the custom agent when applicable, while keeping the
illustration retry target unchanged.

In `@scripts/regressions/prompt.regression.ts`:
- Around line 2053-2061: Update the regression test’s unknownId value used in
the collision assertions to a 21-character identifier containing only letters,
digits, and underscores, while preserving the existing
unresolved-character-reference expectations.

---

Outside diff comments:
In `@packages/server/src/routes/chats.routes.ts`:
- Around line 3972-3987: Update the model-generation flow containing
provider.chatComplete to wire in the shared prompt-debug logging hook, including
logDebugOverride(...) or the established equivalent callback. Ensure the final
system and user messages are logged when debugMode or DEBUG_AGENTS is enabled,
while preserving the existing prompt content and completion behavior.
- Around line 4005-4006: Update the hiddenMessageIds aggregation in the
selected-entry handling, including the corresponding logic around the second
occurrence, to fall back to each entry’s messageIds when hiddenMessageIds is
absent, matching the delete path’s legacy-entry behavior while preserving
deduplication.
- Around line 4000-4003: Update the final callback in the provider combination
flow to validate that each selected entry is unchanged from the entries captured
before provider.chatComplete, not merely that its ID still exists. Compare the
available version, timestamp, or content fields, and return the existing
conflict outcome without replacing entries when any selected entry has changed;
only commit summaryText when all selected entries still match.
- Around line 3939-3955: Update the combined-summary budget validation around
effectiveSummaryMaxTokens and combinedTokenEstimate to resolve the configured
summary prompt before checking the limit, then estimate the complete system and
user prompt including the prompt, entry titles, contents, and combination
instructions. Use that full estimate against the provider context budget instead
of relying on the fixed SUMMARY_COMBINE_PROMPT_RESERVE_TOKENS, while preserving
the existing 400 response when the complete request exceeds the budget.

In `@packages/server/src/services/prompt/assembler.ts`:
- Around line 370-393: Update the sources passed to
buildReferencedCharacterContext in buildPromptMacroContext to include
input.groupScenarioOverrideText, ensuring character-ID macros in the group
scenario override are scanned alongside the existing prompt content.
🪄 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: d968d2c6-7c59-4432-8bc2-35fe4d923ee5

📥 Commits

Reviewing files that changed from the base of the PR and between a0c5c63 and b7730a4.

📒 Files selected for processing (5)
  • packages/server/src/routes/chats.routes.ts
  • packages/server/src/routes/generate.routes.ts
  • packages/server/src/services/prompt/assembler.ts
  • packages/shared/src/utils/macro-engine.ts
  • scripts/regressions/prompt.regression.ts

Comment thread packages/server/src/routes/generate.routes.ts Outdated
Comment thread scripts/regressions/prompt.regression.ts
@SpicyMarinara
SpicyMarinara merged commit 822018b into staging Jul 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant