Resolve open Engine issues and Prose Guardian regression#4122
Merged
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 ComfyUI LoRA discovery and workflow support, ElevenLabs-generated game audio with caching, explicit sidecar model load/unload controls, scene-analysis audio modes, message-cache corrections, accessibility semantics, and regression coverage. ChangesClient and generation correctness
ComfyUI LoRA configuration and propagation
ElevenLabs game-audio generation
Sidecar model load and unload controls
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 task
SpicyMarinara
marked this pull request as ready for review
July 26, 2026 07:42
Collaborator
Author
|
@coderabbitai review |
✅ 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: 8
🧹 Nitpick comments (1)
packages/client/src/components/game/GameSurface.tsx (1)
2600-2619: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo "scoped asset map" flavors, one silently stale.
Ah, a fascinating specimen of divergent evolution!
scopedAssetMap(theuseMemo) spreadsgeneratedAudioAssetsRef.currentat compute time, but its dependency array (assetManifest?.assets,gameAssetExcludedFolders) can never include a ref — so it never re-includes newly generated audio tags oncegenerateGameAudioAssetmutates the ref.getScopedAssetMap()recomputes fresh every call and is correctly used everywhere generated-audio freshness matters today (handleSegmentEnter,applySceneResult,processSceneRef). No active bug in this diff, but the stale twin is left in place as bait for the next unfortunate experimenter who reaches forscopedAssetMapexpecting a freshly generated tag.A brief comment near the
scopedAssetMapdefinition noting it excludes fresh generated-audio tags (or consolidating the two into one helper) would spare a future colleague an... unpleasant surprise.🤖 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/game/GameSurface.tsx` around lines 2600 - 2619, Add a brief comment beside the scopedAssetMap useMemo documenting that its generatedAudioAssetsRef contents are captured only at memoization time and may exclude newly generated audio tags; direct callers requiring fresh generated assets should use getScopedAssetMap().
🤖 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 345: Update the localProvider-change cleanup effect to also reset
remoteLoras, alongside remoteModels and fetchError, so LoRA results from the
previous provider are cleared whenever the provider or base URL changes.
In `@packages/client/src/components/game/GameSurface.tsx`:
- Around line 2620-2675: Wrap the api.post call in generateGameAudioAsset with
the existing withTimeout helper, using the established timeout and
error-handling pattern from runGameAssetGeneration, handleGenerateSceneVideo, or
previewTurnStoryboardPrompts. Preserve the current successful asset registration
and null-return behavior when generation times out or fails.
In `@packages/client/src/hooks/use-background-autonomous.ts`:
- Around line 198-223: Update the text_rewrite handling in the background stream
event loop to apply any string editedText regardless of rewriteApplied,
including the no-rewrite fallback. When updating the latest assistant message,
mirror use-generate.ts by removing extra.postProcessingPending while preserving
other extra fields, and continue ignoring events without valid editedText or
without a latest assistant message.
In `@packages/client/src/localization/locales/en.json`:
- Line 6370: Inspect usages of the localization key
ui.panels.sidecarcard.download and remove it if it is an unused composition
fragment; otherwise replace its value with the intended complete, properly
capitalized download label consistent with neighboring sidecarcard translations,
without retaining the trailing comma.
In `@packages/server/src/routes/connections.routes.ts`:
- Around line 792-801: Ensure the shared ComfyUI LoRA lookup using
fetchComfyLoaderModelNames is executed for both image_generation and
video_generation connections. Update the models handler and its provider split
so the video branch also returns loras, while preserving the existing model and
LoRA mapping behavior.
In `@packages/server/src/routes/tts.routes.ts`:
- Around line 191-201: Change the asset-generation flow around
buildAssetManifest() so cache-miss writes do not synchronously rescan and
rewrite the full manifest for every generated asset. Debounce or batch rebuild
requests, preserving eventual manifest updates while allowing multiple
generations in one turn to share a single rebuild and avoiding blocking the
event loop per asset.
In `@packages/server/src/services/sidecar/scene-postprocess.ts`:
- Line 267: Update the per-beat music assignment in postProcessSceneResult to
sanitize and retain out.music only when ctx.generateMusic &&
!ctx.useSpotifyMusic; otherwise leave it undefined. Also update
buildSceneAnalyzerSystemPrompt so generatedAudio uses ctx.generateSoundEffects
|| (ctx.generateMusic && !ctx.useSpotifyMusic), matching
buildSceneAnalyzerUserPrompt.
In `@packages/server/src/services/sidecar/sidecar-process.service.ts`:
- Around line 205-208: Update unload() so manuallyUnloaded is assigned inside
the same lifecycle-lock-protected operation as stop(), preventing a concurrent
restart from overwriting the unload intent before shutdown completes.
---
Nitpick comments:
In `@packages/client/src/components/game/GameSurface.tsx`:
- Around line 2600-2619: Add a brief comment beside the scopedAssetMap useMemo
documenting that its generatedAudioAssetsRef contents are captured only at
memoization time and may exclude newly generated audio tags; direct callers
requiring fresh generated assets should use getScopedAssetMap().
🪄 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: becb3729-5203-439d-bc6a-94e7b548fbca
📒 Files selected for processing (35)
packages/client/src/components/chat/ChatSettingsDrawer.tsxpackages/client/src/components/chat/HomeCreditsModal.tsxpackages/client/src/components/connections/ConnectionEditor.tsxpackages/client/src/components/game/GameSurface.tsxpackages/client/src/components/panels/ConnectionsPanel.tsxpackages/client/src/components/panels/settings/TTSConfigCard.tsxpackages/client/src/hooks/use-background-autonomous.tspackages/client/src/hooks/use-connections.tspackages/client/src/hooks/use-generate.tspackages/client/src/localization/locales/en.jsonpackages/client/src/stores/sidecar.store.tspackages/server/src/routes/connections.routes.tspackages/server/src/routes/gallery.routes.tspackages/server/src/routes/game.routes.tspackages/server/src/routes/generate.routes.tspackages/server/src/routes/generate/retry-agents-route.tspackages/server/src/routes/sidecar.routes.tspackages/server/src/routes/sprites.routes.tspackages/server/src/routes/tts.routes.tspackages/server/src/services/generation/media-connection-fallback.tspackages/server/src/services/generation/prose-guardian-settings.tspackages/server/src/services/image/image-generation.tspackages/server/src/services/sidecar/scene-analyzer.tspackages/server/src/services/sidecar/scene-postprocess.tspackages/server/src/services/sidecar/sidecar-inference.service.tspackages/server/src/services/sidecar/sidecar-process.service.tspackages/server/src/services/video/game-video-runtime.tspackages/server/src/services/video/video-generation.tspackages/shared/src/constants/image-generation-defaults.tspackages/shared/src/constants/video-generation-defaults.tspackages/shared/src/schemas/scene-analysis.schema.tspackages/shared/src/types/image-generation-defaults.tspackages/shared/src/types/tts.tspackages/shared/src/types/video-generation-defaults.tsscripts/regressions/open-issues.regression.ts
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.
Why
Resolve the current open issue set with focused fixes, include the maintainer credits update, restore Prose Guardian rewrites reported against latest staging, and add the requested explicit sidecar VRAM control.
Scope
modifiesTextas the string"true", while retaining a negative control.Validation
pnpm checkpnpm localization:checkpnpm regression:issuespnpm smoke:uiAutomated validation run while preparing this PR:
pnpm check,pnpm regression:issues, andpnpm smoke:ui(130 passed, 68 skipped). The only lint notice is the pre-existingNoodleHome.tsxexhaustive-deps warning.All validation boxes intentionally remain unchecked for human verification.
Summary by CodeRabbit
New Features
Bug Fixes