Add image prompt instructions to connections - #4842
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds optional image-prompting instructions to image connections. The client stores and transfers the setting. Server routes apply it to selfie, Illustrator, game, and image-capable agent prompt-generation contexts. ChangesImage prompting instructions
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant ConnectionEditor
participant ConnectionStorage
participant GenerateRoutes
participant AgentPipeline
participant AgentExecutor
participant ImagePromptModel
ConnectionEditor->>ConnectionStorage: Save imagePromptInstructions
GenerateRoutes->>ConnectionStorage: Resolve selected or default image connection
ConnectionStorage-->>GenerateRoutes: Return imagePromptInstructions
GenerateRoutes->>AgentPipeline: Configure per-agent context resolver
AgentPipeline->>AgentExecutor: Execute image-capable agent individually
AgentExecutor->>ImagePromptModel: Send provider-facing image guidance
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
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/gallery.routes.ts (1)
1258-1292: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winExtract the duplicated selfie system-prompt augmentation into a shared helper. Both sites build
selfieSystemPromptWithImageInstructionsfrom an identical template string that readsimagePromptInstructionsoff the image connection and wraps it in an<image_prompting_instructions>block. This is the same logic implemented twice, and it already has a third, differently-worded variant inagent-executor.ts.
packages/server/src/routes/gallery.routes.ts#L1258-L1261: replace the inline template string with a call to a shared helper, e.g.appendImagePromptInstructions(selfieSystemPrompt, imageConn.imagePromptInstructions).packages/server/src/services/generation/conversation-selfie-command-runtime.ts#L204-L207: replace the identical inline template string with the same shared helper call.🤖 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/gallery.routes.ts` around lines 1258 - 1292, Extract the duplicated image-prompt augmentation into a shared appendImagePromptInstructions helper, preserving the existing trimming and <image_prompting_instructions> formatting. Replace the inline template in packages/server/src/routes/gallery.routes.ts:1258-1292 and packages/server/src/services/generation/conversation-selfie-command-runtime.ts:204-220 with calls passing the respective selfie system prompt and image connection instructions; leave agent-executor.ts unchanged unless needed to host or reuse the helper.
🤖 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`:
- Line 724: Update the connection transfer/save path and import validation to
trim imagePromptInstructions and enforce a shared 5,000-character maximum before
persistence or prompt use, including all image-generation paths that read the
direct connection field. Keep the textarea maxLength as a secondary client-side
guard, and reuse one shared limit rather than duplicating literal values.
In `@packages/client/src/localization/locales/en.json`:
- Around line 4058-4060: Update the localization text for
ui.connections.connectioneditor.imagePromptingInstructionsHelp to describe that
the instructions are added to the existing prompt-writer system message and
processed by the configured Illustrator provider and model. Remove claims about
an extra call or default language model, while preserving that an invalid-JSON
retry may trigger a second call.
In `@packages/server/src/routes/generate/retry-agents-route.ts`:
- Around line 4192-4207: Scope _imagePromptInstructions to the image-generating
agent instead of assigning it once to shared agentContext memory. In the batched
retry prompt-executor flow, resolve each image_prompt result’s imageConnectionId
through the corresponding imagePromptAgent settings and load that connection’s
imagePromptInstructions before generating its prompt; ensure custom image agents
never inherit another agent’s instructions.
In `@packages/server/src/services/agents/agent-executor.ts`:
- Around line 2367-2381: Restrict image-prompt instructions to agents that
generate image prompts: in packages/server/src/services/agents/agent-executor.ts
lines 2367-2381, gate both the requiresTerminalUserInstruction contribution and
the <image_prompting_instructions> append in buildAgentMessages using the
appropriate illustrator/image-producing agent-type check. In
packages/server/src/routes/generate.routes.ts lines 3634-3647, only populate
agentContext.memory._imagePromptInstructions when an illustrator agent is
active; otherwise leave it unset, avoiding irrelevant instructions for tracker,
lorebook-keeper, quest, about-me-keeper, spotify, haptic, and custom agents.
---
Outside diff comments:
In `@packages/server/src/routes/gallery.routes.ts`:
- Around line 1258-1292: Extract the duplicated image-prompt augmentation into a
shared appendImagePromptInstructions helper, preserving the existing trimming
and <image_prompting_instructions> formatting. Replace the inline template in
packages/server/src/routes/gallery.routes.ts:1258-1292 and
packages/server/src/services/generation/conversation-selfie-command-runtime.ts:204-220
with calls passing the respective selfie system prompt and image connection
instructions; leave agent-executor.ts unchanged unless needed to host or reuse
the helper.
🪄 Autofix
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: 6d64012b-5b5d-4e36-9335-d5bf3b62e5a8
📒 Files selected for processing (16)
CHANGELOG.mdpackages/client/src/components/connections/ConnectionEditor.tsxpackages/client/src/hooks/use-connections.tspackages/client/src/lib/connection-transfer.tspackages/client/src/localization/locales/en.jsonpackages/server/src/db/schema/connections.tspackages/server/src/routes/gallery.routes.tspackages/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/conversation-selfie-command-runtime.tspackages/server/src/services/generation/illustrator-manual-prompt-generation.tspackages/server/src/services/generation/illustrator-prompt-runtime.tspackages/server/src/services/storage/connections.storage.tspackages/shared/src/schemas/connection.schema.tspackages/shared/src/types/connection.ts
|
Thank you for the PR, @snpNEXT! Please check Coderabbitai's comments and resolve all issues for the PR. I also have a question about the Image prompt Instruction: why are you sending those in a separate call instead of adding them to the call that generates the image prompt itself? |
Working on addressing the reviews as we speak! |
|
Alright, I think I got them all, tested and everything works still. Unsure if I need to do anything to retrigger the bot review, never used it. |
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/routes/generate.routes.ts (1)
3635-3650: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPopulate
agentContext.memory._imagePromptInstructionsfor image-capable custom agents.
buildAgentMessages()enables image prompt instructions forconfig.type === "illustrator"or custom agents withtrigger_image_generation, but this route only sets_imagePromptInstructionswhen a built-inillustratoragent is present. A custom image-capable agent with noillustratortype therefore gets the instruction flag without the shared context. Add the same image-prompt eligibility condition used byisImagePromptRetryAgent()or handle each image-capable agent separately before callingexecuteAgentBatch().🤖 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 3635 - 3650, Update the image-prompt setup around the resolvedAgents check to include custom agents whose configuration enables trigger_image_generation, matching the eligibility condition used by isImagePromptRetryAgent() and buildAgentMessages(). Populate agentContext.memory._imagePromptInstructions using the existing connection-resolution flow before executeAgentBatch(), while preserving the current built-in illustrator behavior.
♻️ Duplicate comments (1)
packages/client/src/localization/locales/en.json (1)
4059-4059: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the forward-looking Noodle claim from the current help text.
The sentence "Some flows, including Noodle, use an additional prompt-writing call to apply them" describes behavior that does not exist in this PR. Per the PR comment thread, the separate-call behavior applies only to a future noodle-agents PR, not to any code shipped here.
Update the text to describe only the current flows (selfie and Illustrator), and mention the Noodle behavior later, when that PR ships.
✏️ Proposed wording
- "ui.connections.connectioneditor.imagePromptingInstructionsHelp": "When set, these instructions are added to the existing prompt-writer system message and processed by the configured Illustrator provider and model before the prompt is sent to this image backend. Some flows, including Noodle, use an additional prompt-writing call to apply them. Useful for tags, quality tokens, negative-prompt conventions, or local/ComfyUI syntax. An invalid-JSON retry may trigger another call.", + "ui.connections.connectioneditor.imagePromptingInstructionsHelp": "When set, these instructions are added to the existing prompt-writer system message and processed by the configured Illustrator provider and model before the prompt is sent to this image backend. Useful for tags, quality tokens, negative-prompt conventions, or local/ComfyUI syntax. An invalid-JSON retry may trigger another call.",🤖 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/localization/locales/en.json` at line 4059, Update the localization value for ui.connections.connectioneditor.imagePromptingInstructionsHelp to remove the claim about Noodle and additional prompt-writing calls, and describe only the currently supported selfie and Illustrator flows while preserving the rest of the guidance.
🤖 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/routes/generate.routes.ts`:
- Around line 3635-3650: Update the image-prompt setup around the resolvedAgents
check to include custom agents whose configuration enables
trigger_image_generation, matching the eligibility condition used by
isImagePromptRetryAgent() and buildAgentMessages(). Populate
agentContext.memory._imagePromptInstructions using the existing
connection-resolution flow before executeAgentBatch(), while preserving the
current built-in illustrator behavior.
---
Duplicate comments:
In `@packages/client/src/localization/locales/en.json`:
- Line 4059: Update the localization value for
ui.connections.connectioneditor.imagePromptingInstructionsHelp to remove the
claim about Noodle and additional prompt-writing calls, and describe only the
currently supported selfie and Illustrator flows while preserving the rest of
the guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aaa15a2d-92be-471c-add7-513cecd8444e
📒 Files selected for processing (12)
CHANGELOG.mdpackages/client/src/components/connections/ConnectionEditor.tsxpackages/client/src/lib/connection-transfer.tspackages/client/src/localization/locales/en.jsonpackages/server/src/routes/gallery.routes.tspackages/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/conversation-selfie-command-runtime.tspackages/server/src/services/generation/image-prompt-instructions.tspackages/shared/src/constants/defaults.tspackages/shared/src/schemas/connection.schema.ts
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/routes/generate.routes.ts (1)
3635-3659: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftScope image-prompt instructions per image-capable agent.
This lookup selects only the first matching agent, but the instructions are stored in shared
agentContext.memory. If an Illustrator agent and a custom agent withtrigger_image_generationare both active, both agents receive the first agent’s connection instructions. The later image handler resolves each result agent’s connection separately, so these values can differ. Illustrator cadence filtering can also remove the selected Illustrator agent while leaving a custom image agent with the wrong instructions.Resolve the instructions at agent execution time using the current agent ID, or enforce that only one image-capable agent can be active.
🤖 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 3635 - 3659, Remove the single-agent lookup and shared assignment around imagePromptAgentForInstructions. Resolve and attach normalized image-prompt instructions during each image-capable agent’s execution, using that agent’s ID and the same per-agent connection resolution as the later image handler, so Illustrator cadence filtering and multiple active image agents receive their own instructions.
🤖 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/routes/generate.routes.ts`:
- Around line 3635-3659: Remove the single-agent lookup and shared assignment
around imagePromptAgentForInstructions. Resolve and attach normalized
image-prompt instructions during each image-capable agent’s execution, using
that agent’s ID and the same per-agent connection resolution as the later image
handler, so Illustrator cadence filtering and multiple active image agents
receive their own instructions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 46e056a9-9b54-4f17-a0fb-9b51f8ec781b
📒 Files selected for processing (2)
packages/client/src/localization/locales/en.jsonpackages/server/src/routes/generate.routes.ts
|
I wanted to keep the changes surgical but the reviews are asking to touch more and more stuff that I feel I shouldn't be. I'll do as it says but yeah if its too much feel free to not include the next few commits or so. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/game.routes.ts`:
- Line 10299: Extract and export the existing compactImagePromptInstructions
helper from packages/server/src/services/sidecar/scene-analyzer.ts:120-120,
preserving its trim, whitespace-collapsing, and
MAX_IMAGE_PROMPT_INSTRUCTIONS_LENGTH truncation behavior. Replace the duplicated
inline normalization at packages/server/src/routes/game.routes.ts:10299-10299,
11182-11182, 12032-12032, and 12445-12445, plus
packages/server/src/services/game/game-asset-generation.ts:994-994 and
1128-1128, with imports and calls to this shared helper.
In `@packages/server/src/routes/generate.routes.ts`:
- Around line 4345-4370: Update the automatic retry path around the direct
executeAgent call to resolve the retry context through
resolveImagePromptAgentContext(agentCfg, retryCtx) first, then pass the resolved
context to executeAgent. Preserve the existing retry behavior while ensuring
Illustrator and custom image agents retain _imagePromptInstructions.
In `@packages/server/src/services/agents/agent-executor.ts`:
- Around line 1100-1106: Update every recursive executeAgentBatch call,
including the request-option grouping branch, to pass the existing
resolveAgentContext argument. Ensure resolver-backed agents receive the resolved
context in all batching paths, while preserving the current behavior for calls
without a resolver.
🪄 Autofix
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: a1bd9ef4-3c8f-40e1-8c9a-d45cd3270a76
📒 Files selected for processing (9)
CHANGELOG.mdpackages/server/src/routes/game.routes.tspackages/server/src/routes/generate.routes.tspackages/server/src/services/agents/agent-executor.tspackages/server/src/services/agents/agent-pipeline.tspackages/server/src/services/game/game-asset-generation.tspackages/server/src/services/sidecar/scene-analyzer.tspackages/shared/src/constants/defaults.tspackages/shared/src/schemas/scene-analysis.schema.ts
|
Okay looks like review bot found everything I think. Still seems to work |
|
All looks good, thank you! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/game/game-asset-generation.ts`:
- Line 994: Remove the raw image instruction append from both the background
provider prompt at
packages/server/src/services/game/game-asset-generation.ts:994-994 and the
scene-illustration provider prompt at
packages/server/src/services/game/game-asset-generation.ts:1128-1128. Route
req.imagePromptInstructions through the text prompt-generation path instead,
ensuring both builders apply the instructions there rather than directly to the
image provider prompt.
🪄 Autofix
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: d0b37aa5-7f40-425e-9fef-6fa2a1f7077b
📒 Files selected for processing (7)
CHANGELOG.mdpackages/client/src/localization/locales/en.jsonpackages/server/src/routes/game.routes.tspackages/server/src/routes/generate.routes.tspackages/server/src/services/agents/agent-executor.tspackages/server/src/services/game/game-asset-generation.tspackages/server/src/services/sidecar/scene-analyzer.ts
| : GAME_BACKGROUND.defaultBuilder(backgroundVars); | ||
| const imagePromptInstructionsLine = req.imagePromptInstructions?.trim() | ||
| ? `User image instructions: ${req.imagePromptInstructions.trim().replace(/\s+/g, " ").slice(0, 5000)}` | ||
| ? `User image instructions: ${compactImagePromptInstructions(req.imagePromptInstructions)}` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Route image instructions through text prompt generation instead of provider prompt construction.
Both builders append the raw instruction block to rawPrompt. This can send the original block directly to the image provider when no dynamic prompt generator is active.
packages/server/src/services/game/game-asset-generation.ts#L994-L994: remove the raw instruction append from the background provider prompt and apply the instructions in the text prompt-generation path.packages/server/src/services/game/game-asset-generation.ts#L1128-L1128: apply the same change to the scene-illustration provider prompt.
📍 Affects 1 file
packages/server/src/services/game/game-asset-generation.ts#L994-L994(this comment)packages/server/src/services/game/game-asset-generation.ts#L1128-L1128
🤖 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/game/game-asset-generation.ts` at line 994,
Remove the raw image instruction append from both the background provider prompt
at packages/server/src/services/game/game-asset-generation.ts:994-994 and the
scene-illustration provider prompt at
packages/server/src/services/game/game-asset-generation.ts:1128-1128. Route
req.imagePromptInstructions through the text prompt-generation path instead,
ensuring both builders apply the instructions there rather than directly to the
image provider prompt.
Important
Contributions target
staging. OnlySpicyMarinaramay promote this repository'sstagingbranch or a same-repositoryhotfix/*branch tomain.Linked issue
Closes N/A (No target issue in mind, will look and edit this if I find one)
Why this change
This allows any finnicky local model to basically work at all, as the default image prompt controls aren't local friendly, and provide limited control per connection. Personally, I found this feature to be a MUST HAVE, I've been using it on my local fork for months and months before finally deciding I really should be contributing back.
What changed
imagePromptInstructionsto connection types, schemas, persistence, transfer, and the connection editor.Validation
pnpm checkpasses locallyCONTRIBUTING.mdManual verification notes
pnpm checkpassed locally.Docs and release impact
docs-i18nbranch updated to match, or a[docs-i18n]follow-up issue openedUI evidence (if applicable)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation