Fix reasoning summaries and assigned issue sweep - #4234
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds FPS-aware ComfyUI video workflows, fixes scoped regex rendering and overnight schedule resolution, handles completed OpenAI reasoning summaries, and enables prompt-section injection for Director agents. ChangesComfyUI video generation
Conversation rendering and reasoning summaries
Overnight schedule resolution
Agent prompt-section editor behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
SpicyMarinara
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
added
feature
![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 (2)
packages/client/src/components/connections/ConnectionEditor.tsx (1)
485-488: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winA most elegant new set of variables, yet the instruction manual was left unrevised.
The KNOWN_SUBS list now recognizes
%length_s%,%fps%, and%duration_seconds%— a fine addition to the experiment's parameters. Yet the accompanying help text (ui.connections.connectioneditor.pasteAComfyuiVideoWorkflowInApiFormatUse, en.json line 3607) still instructs the subject to use only%length%"for the 16 fps frame count," with no mention of the new placeholders. A test subject who reads only the manual, and not the validation warnings, will remain blissfully unaware of the new FPS control.Do update the help string to mention
%fps%,%length_s%, and%duration_seconds%alongside%length%.Also applies to: 1872-1877
🤖 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/components/connections/ConnectionEditor.tsx` around lines 485 - 488, Update the en.json help text for ui.connections.connectioneditor.pasteAComfyuiVideoWorkflowInApiFormatUse to document %fps%, %length_s%, and %duration_seconds% alongside %length%, while preserving the existing guidance.scripts/regressions/timezone.regression.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid coverage of the headline scenario — consider extending to
.previous/.nextwhile the fixture is already assembled.The two assertions correctly pin down the Sunday-night-shift-at-2am and Monday-night-shift-at-23:30 cases — I traced the algorithm by hand and it resolves exactly as asserted. Since
overnightScheduleis already built andgetAdjacentScheduleBlocksreturnsprevious/nexttoo, a couple of extraassert.equallines checking those fields on this same fixture would cheaply harden coverage for the adjacency data the PR objectives say downstream intent logic depends on.🧪 Suggested additional assertions
assert.equal(getAdjacentScheduleBlocks(overnightSchedule, mondayAtTwo).current?.activity, "Sunday night shift"); +assert.equal(getAdjacentScheduleBlocks(overnightSchedule, mondayAtTwo).next?.activity, "Monday night shift"); const mondayAtTwentyThreeThirty = new Date(2026, 6, 13, 23, 30, 0); assert.deepEqual(getCurrentStatus(overnightSchedule, mondayAtTwentyThreeThirty), { status: "dnd", activity: "Monday night shift", }); assert.equal( getAdjacentScheduleBlocks(overnightSchedule, mondayAtTwentyThreeThirty).current?.activity, "Monday night shift", ); +assert.equal( + getAdjacentScheduleBlocks(overnightSchedule, mondayAtTwentyThreeThirty).previous?.activity, + "Sunday night shift", +);Also applies to: 67-92
🤖 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 `@scripts/regressions/timezone.regression.ts` around lines 10 - 11, Extend the existing regression coverage around the overnightSchedule fixture and getAdjacentScheduleBlocks result to assert the returned previous and next adjacency fields. Add focused equality assertions for both fields alongside the current Sunday and Monday status assertions, preserving the existing fixture and expected status behavior.
🤖 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/chat/ConversationView.tsx`:
- Line 1280: Propagate the same messageDepth calculation used by the primary
ConversationMessage into the regenerationDraftMessage and liveStreamMessage
render paths. Use each render’s item.index with totalMessageCount so
depth-scoped regex receives the correct depth during regeneration and streaming,
while preserving the existing calculation for the primary message.
In `@packages/client/src/localization/locales/ko.json`:
- Around line 3366-3372: Update the Korean localization for
ui.connections.videogenerationdefaultspanel.comfyuiReceivesDimensionsDurationFpsAndFrameCount
to describe %length% as the configured frame count, not a calculated frame
count, and revise the legacy guidance near the corresponding ComfyUI translation
entry to remove the fixed 16 FPS description and reflect the %length%,
%length_s%, and %fps% contract.
In `@packages/shared/src/utils/conversation-presence.ts`:
- Around line 126-153: Update parseScheduleBlockMinutes and the
getAdjacentScheduleBlocks flow so malformed ScheduleBlock.time values are
explicitly reported through the existing warning or error mechanism before being
skipped. Preserve strict time validation and current handling of valid ranges,
while ensuring invalid ranges cannot silently fall back to the online/free-time
status.
---
Nitpick comments:
In `@packages/client/src/components/connections/ConnectionEditor.tsx`:
- Around line 485-488: Update the en.json help text for
ui.connections.connectioneditor.pasteAComfyuiVideoWorkflowInApiFormatUse to
document %fps%, %length_s%, and %duration_seconds% alongside %length%, while
preserving the existing guidance.
In `@scripts/regressions/timezone.regression.ts`:
- Around line 10-11: Extend the existing regression coverage around the
overnightSchedule fixture and getAdjacentScheduleBlocks result to assert the
returned previous and next adjacency fields. Add focused equality assertions for
both fields alongside the current Sunday and Monday status assertions,
preserving the existing fixture and expected status behavior.
🪄 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: 41373eec-fcef-47a7-833a-8cd9cb375eb6
📒 Files selected for processing (23)
packages/client/src/components/agents/AgentEditor.tsxpackages/client/src/components/chat/ConversationMessage.tsxpackages/client/src/components/chat/ConversationView.tsxpackages/client/src/components/connections/ConnectionEditor.tsxpackages/client/src/components/presets/PresetEditor.tsxpackages/client/src/localization/locales/en.jsonpackages/client/src/localization/locales/ko.jsonpackages/server/src/routes/connections.routes.tspackages/server/src/routes/gallery.routes.tspackages/server/src/routes/game.routes.tspackages/server/src/routes/sprites.routes.tspackages/server/src/services/conversation/schedule.service.tspackages/server/src/services/generation/media-connection-fallback.tspackages/server/src/services/llm/providers/openai.provider.tspackages/server/src/services/video/game-video-runtime.tspackages/server/src/services/video/video-generation.tspackages/shared/src/constants/video-generation-defaults.tspackages/shared/src/types/video-generation-defaults.tspackages/shared/src/utils/conversation-presence.tsscripts/regressions/open-issues.regression.tsscripts/regressions/prompt.regression.tsscripts/regressions/provider-compat.regression.tsscripts/regressions/timezone.regression.ts
| chatCharacterIds={chatCharIds} | ||
| messageIndex={item.index + 1} | ||
| messageOrderIndex={item.index} | ||
| messageDepth={Math.max(0, totalMessageCount - 1 - item.index)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Two specimens left untouched by the messageDepth injection — the draft and live-stream renders.
The new messageDepth wiring only reaches the primary per-item ConversationMessage (line 1280). The regenerationDraftMessage render (~1294-1320) and the liveStreamMessage render (~1325-1351) omit it entirely, leaving depth undefined for depth-scoped regex while a turn is streaming or regenerating. It self-corrects once the real message persists with the correct depth, so this is a transient inconsistency rather than a lasting wound — but an easy one to stitch shut.
🩹 Suggested remedy
+ const depth = Math.max(0, totalMessageCount - 1 - item.index);
return (
<Fragment key={item.key}>
<ConversationMessage
key={msg.id}
message={displayMsg as any}
...
- messageDepth={Math.max(0, totalMessageCount - 1 - item.index)}
+ messageDepth={depth}
...
/>
{regenerationDraftMessage && (
<ConversationMessage
key={regenerationDraftMessage.id}
...
chatCharacterIds={chatCharIds}
+ messageDepth={depth}
hasDraftInput={hasDraftInput}
...
/>
)}
</Fragment>
); {liveStreamMessage && (
<ConversationMessage
key={liveStreamMessage.id}
message={liveStreamMessage as any}
isStreaming
isGrouped={false}
hideActions
+ messageDepth={0}
onDelete={onDelete}Also applies to: 1294-1320, 1325-1351
🤖 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/components/chat/ConversationView.tsx` at line 1280,
Propagate the same messageDepth calculation used by the primary
ConversationMessage into the regenerationDraftMessage and liveStreamMessage
render paths. Use each render’s item.index with totalMessageCount so
depth-scoped regex receives the correct depth during regeneration and streaming,
while preserving the existing calculation for the primary message.
| "ui.connections.videogenerationdefaultspanel.comfyuiReceivesDimensionsDurationFpsAndFrameCount": "ComfyUI는 워크플로 플레이스홀더를 통해 크기, 초 단위 길이, FPS 및 계산된 프레임 수를 받습니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDefaultsForGoogleAiStudioVeoVideo": "Google AI Studio Veo 동영상 생성에 적용되는 연결별 기본값입니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDefaultsForOpenrouterAsynchronousVideoGeneration": "OpenRouter 비동기 동영상 생성에 적용되는 연결별 기본값입니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDefaultsForSceneVideoGenerationDurationIs": "장면 동영상 생성에 적용되는 연결별 기본값입니다. 길이는 Omni 프롬프트에 삽입됩니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDefaultsForSeedance20AsynchronousVideo": "Seedance 2.0 비동기 동영상 생성에 적용되는 연결별 기본값입니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDefaultsForXaiSceneVideoGeneration": "xAI 장면 동영상 생성에 적용되는 연결별 기본값입니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDimensionsAndDurationForLocalComfyuiVideo": "로컬 ComfyUI 동영상 워크플로에 적용되는 연결별 크기와 길이입니다.", | ||
| "ui.connections.videogenerationdefaultspanel.connectionScopedDimensionsAndDurationForLocalComfyuiVideo": "로컬 ComfyUI 동영상 워크플로에 적용되는 연결별 크기, 길이 및 프레임 속도입니다.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe %length% as the configured frame count.
This translation says “calculated frame count,” but the new contract retains %length% as the configured frame count while adding %length_s% and %fps%. Also update the legacy Korean guidance at Line 3229, which still describes a fixed 16 FPS workflow.
🤖 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/ko.json` around lines 3366 - 3372,
Update the Korean localization for
ui.connections.videogenerationdefaultspanel.comfyuiReceivesDimensionsDurationFpsAndFrameCount
to describe %length% as the configured frame count, not a calculated frame
count, and revise the legacy guidance near the corresponding ComfyUI translation
entry to remove the fixed 16 FPS description and reflect the %length%,
%length_s%, and %fps% contract.
| function parseScheduleBlockMinutes(block: ScheduleBlock): { start: number; end: number } | null { | ||
| if (!block || typeof block.time !== "string") return null; | ||
| const [startStr, endStr] = block.time.split("-"); | ||
| if (!startStr || !endStr) return null; | ||
| const [shRaw, smRaw] = startStr.split(":"); | ||
| const [ehRaw, emRaw] = endStr.split(":"); | ||
| const sh = Number(shRaw); | ||
| const sm = Number(smRaw); | ||
| const eh = Number(ehRaw); | ||
| const em = Number(emRaw); | ||
| if ( | ||
| !Number.isInteger(sh) || | ||
| !Number.isInteger(sm) || | ||
| !Number.isInteger(eh) || | ||
| !Number.isInteger(em) || | ||
| sh < 0 || | ||
| sh > 23 || | ||
| eh < 0 || | ||
| eh > 23 || | ||
| sm < 0 || | ||
| sm > 59 || | ||
| em < 0 || | ||
| em > 59 | ||
| ) { | ||
| return null; | ||
| } | ||
| return { start: sh * 60 + sm, end: eh * 60 + em }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether ScheduleBlock.time is validated at write-time (schema/zod), and whether "24:00" or similar formats are used anywhere in the codebase.
rg -n '"24:00"' --type=ts
rg -nP -A5 '\btime\s*:\s*z\.' --type=ts
ast-grep run --pattern 'interface ScheduleBlock { $$$ }' --lang typescriptRepository: Pasta-Devs/Marinara-Engine
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -u
echo "== git files matching conversation/schedule =="
git ls-files | rg -n 'conversation|schedule|presence|schema|zod' || true
echo "== locate conversation-presence.ts =="
fd -a 'conversation-presence\.ts$' . || true
echo "== outline target if present =="
TARGET="$(fd 'conversation-presence\.ts$' . | head -n1)"
if [ -n "${TARGET:-}" ]; then
ast-grep outline "$TARGET" --view expanded || true
echo "== target sections =="
sed -n '1,240p' "$TARGET" | nl -ba
fi
echo "== ScheduleBlock definitions/usages =="
rg -n "ScheduleBlock|parseScheduleBlockMinutes|addBlocks|setPresence|presence" --type=ts --type=tsx . || trueRepository: Pasta-Devs/Marinara-Engine
Length of output: 8652
🌐 Web query:
genshin impact Il Dottore Fatui The Second character quote tone
💡 Result:
Il Dottore, the Second of the Eleven Fatui Harbingers in Genshin Impact, is characterized by a tone that is consistently cold, calculated, and devoid of traditional empathy [1]. His dialogue often reflects a detached, scientific curiosity that views other living beings—including gods and humans—merely as test subjects or variables in his experiments [1][2]. Key aspects of his tone and personality include: Detached and Clinical: Dottore speaks with an unsettling lack of malice, which makes his complete disregard for morality and human life even more chilling [1]. He maintains a professional, almost academic demeanor even when discussing horrific experiments [1][3]. Superiority and Arrogance: He displays profound arrogance, often dismissing the cognitive habits of others—including the God of Wisdom—as limited or disappointing [4]. He considers himself above conventional morality and the judgment of laws, inviting others to praise his magnificence and purity [5]. Scientific Obsession: His tone is frequently analytical, focused on themes of time, cognition, and the elevation of humans to the level of gods [4][2]. He is not interested in power for the sake of power, but in the realization of his heretical scientific goals, regardless of the ethical cost [1][2]. Variation Across Segments: It is important to note that because Dottore consists of multiple "segments" (clones created at different stages of his life), his tone can vary slightly between iterations [6]. While the version encountered in the Sumeru arc projects a steady, calculated aura, other versions—such as those seen in the official prequel manga—can appear more manic and volatile [6]. Example Quotes reflecting his tone: - "Observation is the first step of any experiment, but observing the current world doesn't satisfy me. It lacks an important dimension - that of time" [4][5]. - "Oh, no. I certainly have my own convictions. They just don't fit your standards, that's all" [4][5]. - "My ideals have no stains... Only laws and the Tribunal can judge someone. They can judge even me. So praise my magnificence and purity" [5]. His character represents a departure from more emotionally complex antagonists in the game, often cited as one of the most irredeemably evil figures due to his history of experimenting on children and his total lack of remorse [7][3].
Citations:
- 1: https://exploregenshin.gg/il-dottore/
- 2: https://genshin-impact.fandom.com/wiki/Dottore
- 3: https://sg.news.yahoo.com/dottore-genshin-impact-first-truly-evil-irredeemable-villain-151805260.html
- 4: https://www.thyquotes.com/il-dottore-the-doctor/
- 5: https://www.thyquotes.com/genshin-impact/quote-60688/
- 6: https://www.hoyolab.com/article/9181676
- 7: https://www.hoyolab.com/article/37888778
🏁 Script executed:
#!/bin/bash
set -u
echo "== conversation-presence relevant lines =="
TARGET="./packages/shared/src/utils/conversation-presence.ts"
sed -n '1,260p' "$TARGET"
echo "== ScheduleBlock schema/type references =="
rg -n "ScheduleBlock|DaySchedule|WeekSchedule|character schema|character.schema|presence" packages/shared packages/server packages/client docs/conversation/schedules.md 2>/dev/null || true
echo "== files around character schema =="
sed -n '1,240p' packages/shared/src/schemas/character.schema.ts 2>/dev/null || trueRepository: Pasta-Devs/Marinara-Engine
Length of output: 50382
Handle malformed schedule time ranges explicitly.
parseScheduleBlockMinutes silently drops any ScheduleBlock.time that fails strict 00:00-23:59 parsing, and getAdjacentScheduleBlocks skips that specimen entirely. Since getCurrentStatus only rejects an existing current block, out-of-range ranges remove a scheduled slot and fall back to "online" / "free time" for that moment. Make invalid time values surface as warnings/errors rather than silent disappearances.
🤖 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/shared/src/utils/conversation-presence.ts` around lines 126 - 153,
Update parseScheduleBlockMinutes and the getAdjacentScheduleBlocks flow so
malformed ScheduleBlock.time values are explicitly reported through the existing
warning or error mechanism before being skipped. Preserve strict time validation
and current handling of valid ranges, while ensuring invalid ranges cannot
silently fall back to the online/free-time status.
1 participant
Why
This PR combines the existing official OpenAI reasoning-summary fix with the current issue sweep assigned to
@SpicyMarinara:%length%to an implicit 16 FPS and exposed no duration-seconds or FPS macros.For #4240, the maintainer's important follow-up says the reported first-generation preprocessing/context symptom was resolved by reinstalling Marinara Engine. This PR deliberately does not rewrite that resolved orchestration path; it addresses only the remaining reproducible editor defects.
Closes #4233.
Closes #4236.
Closes #4237.
Closes #4238.
Closes #4239.
Closes #4240.
Intended change
%length_s%plus%fps%, while preserving legacy%length%as the configured frame count.Automated evidence
pnpm check— passed (one pre-existingNoodleHomehook warning, no errors).pnpm localization:check— passed.pnpm regression:timezone— passed.pnpm regression:prompt— passed.pnpm regression:issues— passed.pnpm regression:providers— passed.pnpm smoke:ui— 149 passed, 83 skipped; six unrelated existing accent-variable/tracker assertions failed and reproduced in an isolated one-worker run. The sweep-related browser paths passed.Manual verification
https://api.openai.com/v1save reasoning and show the brain icon without duplicate summaries.%length_s%,%fps%, and%length%using the saved FPS across connection test, Gallery, Roleplay, and Game generation paths.Summary by CodeRabbit
New Features
Bug Fixes