Conversation
設定画面の半透明バックドロップと外側クリックでのクローズを追加。モデル読み込み中の共通スピナー表示を追加し、AivisSpeech/VOICEVOXの話者リスト更新API呼び出しとレスポンス検証も修正。
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughLive2D・VRM・PNGTuberビューアにModelLoadingOverlayコンポーネントを導入してローディング表示を統一し、VRM/Live2Dのモデル読み込みで競合するリクエストのローディング状態誤解除を防止した。AivisSpeech/VOICEVOX話者更新APIにレスポンス検証を追加し、設定画面のエラーメッセージを改善。設定バックドロップのクリッククローズとテーマCSSも調整した。 Changesモデルローディング表示
話者リスト更新APIとエラーメッセージ
設定バックドロップとテーマスタイル
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant VrmViewer
participant Viewer
participant ModelLoadingOverlay
VrmViewer->>Viewer: loadVrm(url)
Viewer->>Viewer: requestId採番
Viewer->>VrmViewer: onModelLoadingChange(true)
VrmViewer->>ModelLoadingOverlay: 表示
Viewer->>Viewer: モデル読込完了
alt requestIdが最新
Viewer->>VrmViewer: onModelLoadingChange(false)
VrmViewer->>ModelLoadingOverlay: 非表示
else 古いrequestId
Viewer->>Viewer: unLoadVrmのみ実行
end
sequenceDiagram
participant User
participant SettingsVoice
participant UpdateAivisSpeakersAPI
participant AivisServer
User->>SettingsVoice: UpdateSpeakerListクリック
SettingsVoice->>UpdateAivisSpeakersAPI: POST /api/update-aivis-speakers
UpdateAivisSpeakersAPI->>AivisServer: fetch /speakers
AivisServer-->>UpdateAivisSpeakersAPI: レスポンス
alt レスポンス不正またはok:false
UpdateAivisSpeakersAPI-->>SettingsVoice: 500 + errorCode
SettingsVoice->>SettingsVoice: getSpeakerUpdateErrorMessageで表示
else 正常
UpdateAivisSpeakersAPI-->>SettingsVoice: 200
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/components/Live2DComponent.tsx (1)
132-171: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
loadLive2DModelの競合制御が不完全（古いリクエストが新しいモデルを上書きする）
requestIdはfinally内のsetIsModelLoading(false)にのみ使われており、モデル適用処理（stage.addChild/modelRef.current/setModel/homeStore.setState）はrequestIdのチェックなしに実行されます。selectedLive2DPathが短時間に連続変更され、古いリクエストが新しいリクエストより後に解決した場合、古いモデルがステージに追加され「現在のモデル」として上書きされてしまいます（表示上のモデル重複や、ドラッグ・スケール操作が意図しないモデルに適用される等の不具合につながります）。
viewer.ts（VRM側）はthis.model !== modelのチェックで同様の競合を正しく防止しており、本ファイルにも同じ考え方の適用が必要です。🔒 提案する修正
await new Promise((resolve, reject) => { newModel.once('load', () => resolve(true)) newModel.once('error', (e) => reject(e)) setTimeout(() => reject(new Error('Model load timeout')), 10000) }) + // 新しいリクエストが既に開始されている場合、この結果は破棄する + if (requestId !== loadRequestIdRef.current) { + newModel.destroy() + return + } + currentApp.stage.addChild(newModel as unknown as DisplayObject) newModel.anchor.set(0.5, 0.5) setModelPosition(currentApp, newModel)🤖 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/components/Live2DComponent.tsx` around lines 132 - 171, The request cancellation/race handling in loadLive2DModel is incomplete because requestId is only used in the final loading flag reset, so older loads can still overwrite newer ones. Add a requestId/current request check in loadLive2DModel before applying the loaded model (especially around currentApp.stage.addChild, modelRef.current assignment, setModel, and homeStore.setState) and bail out if the request is stale. Keep the existing finally-based loading-state guard, and follow the same stale-model protection pattern used in viewer.ts.
🧹 Nitpick comments (3)
src/pages/api/update-aivis-speakers.ts (1)
62-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winレスポンス検証ロジックの重複。
response.okチェックとArray.isArray検証のロジックがupdate-voicevox-speakers.tsの同等コードとほぼ同一です。共通ヘルパー（例:validateSpeakersResponse(response, serverName)）に切り出すことで重複を削減できます。♻️ 共通化の一例
// src/lib/api-services/validateSpeakersResponse.ts export async function fetchAndValidateSpeakers( response: Response, serverName: string ): Promise<Speaker[]> { if (!response.ok) { throw new Error(`${serverName} server responded with status: ${response.status}`) } const speakers: unknown = await response.json() if (!Array.isArray(speakers)) { throw new Error(`${serverName} speakers response must be an array`) } return speakers as Speaker[] }🤖 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/pages/api/update-aivis-speakers.ts` around lines 62 - 73, The speaker response validation in update-aivis-speakers.ts duplicates the same response.ok and Array.isArray checks used in the VoiceVox flow, so extract that logic into a shared helper such as validateSpeakersResponse or fetchAndValidateSpeakers under the same API services layer. Update the updateAivisSpeakers path to call the shared helper with the AivisSpeech server name, and reuse it from the matching update-voicevox-speakers.ts code so both handlers share one validation implementation.src/__tests__/components/voice.test.tsx (1)
183-247: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVOICEVOX 側の更新テストも追加してください。
src/__tests__/components/voice.test.tsxのdescribe('voicevox settings')に、/api/update-voicevox-speakersの POST 呼び出しとServerSecretAccessDeniedのエラー表示を AIVIS と同様に追加したいです。🤖 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/__tests__/components/voice.test.tsx` around lines 183 - 247, The AIVIS speech settings tests already cover the POST update flow and `ServerSecretAccessDenied` handling, but the equivalent VoiceVox coverage is missing. Add matching cases under `describe('voicevox settings')` in `voice.test.tsx` using the existing `Voice` component and `UpdateSpeakerList` flow: assert that the update action posts to `/api/update-voicevox-speakers`, and add a failure test that stubs the response with `ServerSecretAccessDenied` and verifies the same server-resource access error message is rendered.src/components/settings/index.tsx (1)
58-61: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winキーボードでの閉じる操作も検討を。
バックドロップクリックでの閉じる処理は正しく実装されていますが、Escapeキーでの閉じる操作は追加されていません。他の場所（Headerの閉じるボタン等）で対応済みであれば問題ありませんが、モーダルとしてのアクセシビリティ観点では検討の余地があります。
🤖 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/components/settings/index.tsx` around lines 58 - 61, The theme settings backdrop currently closes only on click, so add keyboard-dismiss support for the modal by handling Escape in the settings component. Update the relevant modal logic in the settings index component, using the existing close path behind handleBackdropClick or the header close action, and wire a keydown listener so pressing Escape triggers the same close behavior while the modal is open.
🤖 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 `@src/components/Live2DComponent.tsx`:
- Around line 132-171: The request cancellation/race handling in loadLive2DModel
is incomplete because requestId is only used in the final loading flag reset, so
older loads can still overwrite newer ones. Add a requestId/current request
check in loadLive2DModel before applying the loaded model (especially around
currentApp.stage.addChild, modelRef.current assignment, setModel, and
homeStore.setState) and bail out if the request is stale. Keep the existing
finally-based loading-state guard, and follow the same stale-model protection
pattern used in viewer.ts.
---
Nitpick comments:
In `@src/__tests__/components/voice.test.tsx`:
- Around line 183-247: The AIVIS speech settings tests already cover the POST
update flow and `ServerSecretAccessDenied` handling, but the equivalent VoiceVox
coverage is missing. Add matching cases under `describe('voicevox settings')` in
`voice.test.tsx` using the existing `Voice` component and `UpdateSpeakerList`
flow: assert that the update action posts to `/api/update-voicevox-speakers`,
and add a failure test that stubs the response with `ServerSecretAccessDenied`
and verifies the same server-resource access error message is rendered.
In `@src/components/settings/index.tsx`:
- Around line 58-61: The theme settings backdrop currently closes only on click,
so add keyboard-dismiss support for the modal by handling Escape in the settings
component. Update the relevant modal logic in the settings index component,
using the existing close path behind handleBackdropClick or the header close
action, and wire a keydown listener so pressing Escape triggers the same close
behavior while the modal is open.
In `@src/pages/api/update-aivis-speakers.ts`:
- Around line 62-73: The speaker response validation in update-aivis-speakers.ts
duplicates the same response.ok and Array.isArray checks used in the VoiceVox
flow, so extract that logic into a shared helper such as
validateSpeakersResponse or fetchAndValidateSpeakers under the same API services
layer. Update the updateAivisSpeakers path to call the shared helper with the
AivisSpeech server name, and reuse it from the matching
update-voicevox-speakers.ts code so both handlers share one validation
implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e2a84abc-2e06-41d6-bc0e-771f6c0d2c11
📒 Files selected for processing (15)
src/__tests__/components/voice.test.tsxsrc/__tests__/pages/api/update-aivis-speakers.test.tssrc/__tests__/pages/api/update-voicevox-speakers.test.tssrc/components/Live2DComponent.tsxsrc/components/PNGTuberComponent.tsxsrc/components/live2DViewer.tsxsrc/components/modelLoadingOverlay.tsxsrc/components/pngTuberViewer.tsxsrc/components/settings/index.tsxsrc/components/settings/voice.tsxsrc/components/vrmViewer.tsxsrc/features/vrmViewer/viewer.tssrc/pages/api/update-aivis-speakers.tssrc/pages/api/update-voicevox-speakers.tssrc/styles/globals.css
1 participant
新機能
改善
バグ修正
テスト
ドキュメント・翻訳
その他
Summary by CodeRabbit
New Features
Bug Fixes