feat(debug): add splash previews - #2051
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds development-only splash scenario previews across typed routes, main-process lifecycle, preload IPC, Vue and fallback renderers, and Debug settings. It also updates provider model metadata and bumps the dimcode registry package from ChangesSplash debug tooling
Model catalog updates
Agent registry update
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DebugSettings
participant DebugClient
participant MainProcess
participant SplashWindow
participant Preload
participant SplashRenderer
DebugSettings->>DebugClient: showSplashScenario(mode)
DebugClient->>MainProcess: invoke typed debug route
MainProcess->>SplashWindow: showDebugScenario(mode)
SplashWindow->>Preload: send debug mode IPC
Preload->>SplashRenderer: replay or deliver mode
SplashRenderer->>SplashRenderer: disable preview unlock actions
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/settings/components/DebugSettings.vue (1)
99-103: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSplash scenario labels won't update on locale change.
splashScenarioscallst(...)once at setup-time and stores plain strings, so the button labels won't react if the user switches locale while this page is mounted — unlike every other translated string in this file, which is re-evaluated live (inline in template or inside click handlers). Wrap it incomputed()so labels stay in sync witht().🌐 Proposed fix
-const splashScenarios: Array<{ mode: SplashDebugMode; label: string }> = [ - { mode: 'loading', label: t('settings.debug.splash.loading') }, - { mode: 'system-unlock', label: t('settings.debug.splash.systemUnlock') }, - { mode: 'unlock', label: t('settings.debug.splash.unlock') } -] +const splashScenarios = computed<Array<{ mode: SplashDebugMode; label: string }>>(() => [ + { mode: 'loading', label: t('settings.debug.splash.loading') }, + { mode: 'system-unlock', label: t('settings.debug.splash.systemUnlock') }, + { mode: 'unlock', label: t('settings.debug.splash.unlock') } +])(also add
computedto theimport { onMounted, ref } from 'vue'line)As per path instructions, "Use vue-i18n for user-facing copy" for
src/renderer/**/*.{vue,ts,tsx}.🤖 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 `@src/renderer/settings/components/DebugSettings.vue` around lines 99 - 103, Update splashScenarios to be a computed value so its labels re-evaluate through t() when the locale changes, while preserving the existing scenario modes and translation keys. Add computed to the Vue imports and update consumers to use the computed result as needed.Source: Path 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.
Inline comments:
In `@src/renderer/splash/loading.vue`:
- Around line 46-50: Update the unlock-hint rendering in the loading component
so the isDebugPreview branch uses an existing vue-i18n message via t/$t instead
of hardcoded text. Add the preview hint to the established i18n messages and
preserve unlockHint for the non-preview branch.
In `@test/renderer/splash/loading.test.ts`:
- Around line 70-81: Extend the existing development-preview test around
debugModeListener('unlock') to assert the secondary Quit/cancel button is
disabled, then trigger its cancellation action and verify window.deepchatSplash
does not receive a cancellation request. Keep the regression coverage limited to
this disabled cancellation contract alongside the existing submission assertion.
---
Nitpick comments:
In `@src/renderer/settings/components/DebugSettings.vue`:
- Around line 99-103: Update splashScenarios to be a computed value so its
labels re-evaluate through t() when the locale changes, while preserving the
existing scenario modes and translation keys. Add computed to the Vue imports
and update consumers to use the computed result as needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: def819f0-5962-45f5-a0f3-6a5d92c364ed
📒 Files selected for processing (20)
docs/features/splash-debug-tooling/spec.mdresources/acp-registry/registry.jsonresources/model-db/providers.jsonsrc/main/app/composition.tssrc/main/app/mainProcess.tssrc/main/app/routes.tssrc/main/app/splashWindow.tssrc/preload/splash-preload.tssrc/renderer/api/DebugClient.tssrc/renderer/settings/components/DebugSettings.vuesrc/renderer/splash/env.d.tssrc/renderer/splash/loading.vuesrc/shared/contracts/routes.tssrc/shared/contracts/routes/debug.routes.tssrc/shared/contracts/splash.tstest/main/app/routes.test.tstest/main/app/splashWindow.display.test.tstest/renderer/api/preloadBoundaries.test.tstest/renderer/components/DebugSettings.test.tstest/renderer/splash/loading.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/main/app/splashWindow.display.test.ts (1)
139-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse scoped DOM/style assertions for both transparency checks.
Both tests validate CSS through broad source-string matching, so they can miss incorrectly scoped rules or fail on harmless formatting changes.
test/main/app/splashWindow.display.test.ts#L139-L148: assert the primary document’sbodystyle/rule directly.test/main/app/splashWindow.display.test.ts#L399-L402: assert the fallback root style through JSDOM while separately preserving the manual-unlock shell’s dark-background contract.As per coding guidelines, tests should focus on observable contracts and remove checks that only test implementation internals before handoff.
🤖 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 `@test/main/app/splashWindow.display.test.ts` around lines 139 - 148, Replace the broad source-string checks in test/main/app/splashWindow.display.test.ts:139-148 with a scoped DOM/style assertion against the primary document body, verifying its background is transparent. At test/main/app/splashWindow.display.test.ts:399-402, assert the fallback root background through JSDOM and separately preserve the manual-unlock shell’s dark-background contract; avoid formatting-sensitive raw source matching at both sites.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@test/main/app/splashWindow.display.test.ts`:
- Around line 139-148: Replace the broad source-string checks in
test/main/app/splashWindow.display.test.ts:139-148 with a scoped DOM/style
assertion against the primary document body, verifying its background is
transparent. At test/main/app/splashWindow.display.test.ts:399-402, assert the
fallback root background through JSDOM and separately preserve the manual-unlock
shell’s dark-background contract; avoid formatting-sensitive raw source matching
at both sites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e167bc35-0e5d-4afe-80cd-58f7a2440cad
📒 Files selected for processing (4)
docs/features/splash-debug-tooling/spec.mdsrc/main/app/splashWindow.tssrc/renderer/splash/index.htmltest/main/app/splashWindow.display.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/features/splash-debug-tooling/spec.md
- src/main/app/splashWindow.ts
Collaborator
Author
|
Addressed the three actionable review items in 41d86a4:\n\n- localized the splash preview hint and bootstrapped the splash renderer with the typed locale bridge;\n- covered disabled Quit/cancel behavior in the splash preview regression test;\n- made splash scenario labels reactive to locale changes, with focused regression coverage.\n\nVerified with format, i18n, lint, full typecheck, and the focused renderer tests. |
Contributor
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 `@src/renderer/splash/main.ts`:
- Around line 7-9: Update the splash initialization around createRendererI18n to
capture its returned languageState, then set document.documentElement.dir to
"rtl" for RTL state and "auto" otherwise before mounting the splash app.
Preserve the existing i18n setup and ensure the direction is applied before
mount.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e08cd9a-efb8-4658-bca1-3ad6e62c5896
📒 Files selected for processing (28)
src/preload/splash-preload.tssrc/renderer/settings/components/DebugSettings.vuesrc/renderer/splash/env.d.tssrc/renderer/splash/loading.vuesrc/renderer/splash/main.tssrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsontest/renderer/api/preloadBoundaries.test.tstest/renderer/components/DebugSettings.test.tstest/renderer/splash/loading.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/renderer/settings/components/DebugSettings.vue
- src/preload/splash-preload.ts
- src/renderer/splash/env.d.ts
- test/renderer/splash/loading.test.ts
- test/renderer/components/DebugSettings.test.ts
- test/renderer/api/preloadBoundaries.test.ts
- src/renderer/splash/loading.vue
Collaborator
Author
|
Addressed the RTL-direction finding in 5f0b600: splash bootstrap now applies the returned languageState direction to document.documentElement.dir before mounting. Added focused RTL and non-RTL bootstrap tests; focused splash tests, web typecheck, and lint pass. |
Contributor
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 `@test/renderer/splash/main.test.ts`:
- Around line 24-26: Update the mount mock setup in the splash bootstrap tests
to capture document.documentElement.dir at the moment mount is invoked, then
assert that captured value rather than the final document direction in both RTL
and non-RTL cases. Keep the existing mount-call assertions 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b96407e7-e6a1-4c5f-bf24-558614a110a0
📒 Files selected for processing (2)
src/renderer/splash/main.tstest/renderer/splash/main.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/splash/main.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.
Summary
UI layout
BEFORE
AFTER
Safety
Validation
pnpm run format:checkpnpm run i18npnpm run lintpnpm run architecture:renderer-baseline:checkpnpm run icons:checkpnpm run typecheckpnpm run test:main— 5,370 passed, 277 skippedpnpm run test:renderer— 1,625 passedpnpm run buildSummary by CodeRabbit