Avatar Integration - #44
Open
AaronFrancis05 wants to merge 5 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults 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:
📝 WalkthroughWalkthroughThe change adds configurable AI providers and structured error handling, replaces local avatar rendering with an Akademia iframe bridge, introduces authenticated avatar management APIs and client state, and batches session/chat data access. ChangesAI provider platform
Avatar experience
Session data loading
Estimated code review effort: 5 (Critical) | ~100 minutes Sequence Diagram(s)sequenceDiagram
participant ChatRoute
participant AIEngine
participant ProviderSelector
participant AIProvider
ChatRoute->>AIEngine: analyzeAndGenerateTurn
AIEngine->>ProviderSelector: getAIProvider
ProviderSelector->>AIProvider: generateJSON
AIProvider-->>AIEngine: JSON response
AIEngine-->>ChatRoute: generated turn or categorized error
sequenceDiagram
participant AkademiaIframe
participant AvatarViewport
AkademiaIframe->>AvatarViewport: AKADEMIA_READY message
AvatarViewport->>AkademiaIframe: UPDATE_AVATAR_STATE message
AvatarViewport->>AkademiaIframe: MAKE_AVATAR_SPEAK message
AkademiaIframe->>AvatarViewport: USER_SPOKE message
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx`:
- Around line 25-37: Update handleMessage in
components/roleplay/AkademiaAvatarViewport.tsx (lines 25-37) and
components/roleplay/AvatarViewport.tsx (lines 25-36) to accept messages only
when event.source matches the respective embedded iframe’s contentWindow, in
addition to the existing origin check. Validate event.data before reading type
or text, allowing only the expected message types and appropriate payload shape;
apply the same validation consistently in both handlers.
- Around line 60-64: Replace the unbounded loading spinners in
components/roleplay/AkademiaAvatarViewport.tsx lines 60-64 and
components/roleplay/AvatarViewport.tsx lines 59-63 with shared timeout, retry,
and fallback handling for iframe load or handshake failures. Update each
viewport’s existing readiness/iframe state flow so loading eventually reports
failure, offers retry, and renders the fallback state instead of spinning
indefinitely.
- Around line 33-37: Remove or redact the raw spoken-content logging in the
USER_SPOKE handler of components/roleplay/AkademiaAvatarViewport.tsx at lines
33-37 and the duplicate transcript log in components/roleplay/AvatarViewport.tsx
at lines 34-36, while preserving the event-handling behavior.
- Around line 45-55: The iframe source changes with cameraMode while readiness
remains true, allowing messages before the new document completes its handshake.
In components/roleplay/AkademiaAvatarViewport.tsx lines 45-55 and
components/roleplay/AvatarViewport.tsx lines 44-54, either keep iframeSrc stable
across cameraMode changes or reset the corresponding readiness state whenever
navigation occurs so the existing postMessage effect waits for the new
handshake.
- Around line 67-74: Restrict both avatar iframes by updating the iframe
elements in components/roleplay/AkademiaAvatarViewport.tsx (67-74) and
components/roleplay/AvatarViewport.tsx (66-73) to use a restrictive sandbox
configuration, and remove clipboard-write from their allow attributes unless
clipboard access is required by the embedded applications.
🪄 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
Run ID: 989ff34d-e614-4677-9940-5103d540ccd9
📒 Files selected for processing (2)
components/roleplay/AkademiaAvatarViewport.tsxcomponents/roleplay/AvatarViewport.tsx
Comment on lines
+25
to
+37
| const handleMessage = (event: MessageEvent) => { | ||
| // SECURITY: Verify the message comes from our domain | ||
| if (event.origin !== 'https://ai-avatar.akademia.co.jp') return; | ||
|
|
||
| if (event.data.type === 'AKADEMIA_READY') { | ||
| setIsReady(true); | ||
| } | ||
|
|
||
| if (event.data.type === 'USER_SPOKE') { | ||
| // Optional: If you want Dojo to handle the STT backend logic, | ||
| // you can trigger your handleSend(event.data.text) here. | ||
| console.log('🎤 User spoke to Akademia avatar:', event.data.text); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Bind messages to the specific embedded window, not only its origin.
components/roleplay/AkademiaAvatarViewport.tsx#L25-L37: verifyevent.sourceagainst this iframe and validate the message shape.components/roleplay/AvatarViewport.tsx#L25-L36: apply the same source and payload validation.
📍 Affects 2 files
components/roleplay/AkademiaAvatarViewport.tsx#L25-L37(this comment)components/roleplay/AvatarViewport.tsx#L25-L36
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx` around lines 25 - 37, Update
handleMessage in components/roleplay/AkademiaAvatarViewport.tsx (lines 25-37)
and components/roleplay/AvatarViewport.tsx (lines 25-36) to accept messages only
when event.source matches the respective embedded iframe’s contentWindow, in
addition to the existing origin check. Validate event.data before reading type
or text, allowing only the expected message types and appropriate payload shape;
apply the same validation consistently in both handlers.
Comment on lines
+33
to
+37
| if (event.data.type === 'USER_SPOKE') { | ||
| // Optional: If you want Dojo to handle the STT backend logic, | ||
| // you can trigger your handleSend(event.data.text) here. | ||
| console.log('🎤 User spoke to Akademia avatar:', event.data.text); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove production logging of raw spoken content.
components/roleplay/AkademiaAvatarViewport.tsx#L33-L37: remove or redactevent.data.text.components/roleplay/AvatarViewport.tsx#L34-L36: remove or redact the duplicate transcript log.
📍 Affects 2 files
components/roleplay/AkademiaAvatarViewport.tsx#L33-L37(this comment)components/roleplay/AvatarViewport.tsx#L34-L36
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx` around lines 33 - 37, Remove
or redact the raw spoken-content logging in the USER_SPOKE handler of
components/roleplay/AkademiaAvatarViewport.tsx at lines 33-37 and the duplicate
transcript log in components/roleplay/AvatarViewport.tsx at lines 34-36, while
preserving the event-handling behavior.
Comment on lines
+45
to
+55
| useEffect(() => { | ||
| if (!isReady || !iframeRef.current) return; | ||
|
|
||
| iframeRef.current.contentWindow?.postMessage({ | ||
| type: 'UPDATE_AVATAR_STATE', | ||
| payload: { mode, emotion, gesture, cameraMode } | ||
| }, 'https://ai-avatar.akademia.co.jp'); | ||
| }, [isReady, mode, emotion, gesture, cameraMode]); | ||
|
|
||
| // The URL pointing to your hosted Akademia app | ||
| const iframeSrc = `https://ai-avatar.akademia.co.jp/?mode=avatar-only&camera=${cameraMode}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not reload the iframe without restarting its readiness handshake.
components/roleplay/AkademiaAvatarViewport.tsx#L45-L55: keepsrcstable or reset readiness whenever navigation occurs.components/roleplay/AvatarViewport.tsx#L44-L54: apply the same lifecycle correction.
📍 Affects 2 files
components/roleplay/AkademiaAvatarViewport.tsx#L45-L55(this comment)components/roleplay/AvatarViewport.tsx#L44-L54
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx` around lines 45 - 55, The
iframe source changes with cameraMode while readiness remains true, allowing
messages before the new document completes its handshake. In
components/roleplay/AkademiaAvatarViewport.tsx lines 45-55 and
components/roleplay/AvatarViewport.tsx lines 44-54, either keep iframeSrc stable
across cameraMode changes or reset the corresponding readiness state whenever
navigation occurs so the existing postMessage effect waits for the new
handshake.
Comment on lines
+60
to
+64
| {!isReady && ( | ||
| <div className="absolute inset-0 z-10 flex items-center justify-center bg-dojo-surface/80 backdrop-blur-sm"> | ||
| <div className="h-8 w-8 animate-spin rounded-full border-2 border-dojo-accent border-t-transparent" /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle iframe load and handshake failures.
components/roleplay/AkademiaAvatarViewport.tsx#L60-L64: replace the unbounded spinner with timeout, retry, and fallback behavior.components/roleplay/AvatarViewport.tsx#L59-L63: add the same recovery path.
📍 Affects 2 files
components/roleplay/AkademiaAvatarViewport.tsx#L60-L64(this comment)components/roleplay/AvatarViewport.tsx#L59-L63
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx` around lines 60 - 64, Replace
the unbounded loading spinners in components/roleplay/AkademiaAvatarViewport.tsx
lines 60-64 and components/roleplay/AvatarViewport.tsx lines 59-63 with shared
timeout, retry, and fallback handling for iframe load or handshake failures.
Update each viewport’s existing readiness/iframe state flow so loading
eventually reports failure, offers retry, and renders the fallback state instead
of spinning indefinitely.
Comment on lines
+67
to
+74
| <iframe | ||
| ref={iframeRef} | ||
| src={iframeSrc} | ||
| className="h-full w-full border-none" | ||
| allow="microphone; autoplay; clipboard-write" | ||
| title="Akademia AI Avatar" | ||
| style={{ opacity: isReady ? 1 : 0, transition: 'opacity 0.5s ease' }} | ||
| /> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Next.js docs candidates =="
fd -a "iframe" node_modules/next/dist/docs || true
fd -a "sandbox" node_modules/next/dist/docs || true
echo
echo "== Relevant source files =="
sed -n '1,220p' components/roleplay/AkademiaAvatarViewport.tsx
echo
sed -n '1,220p' components/roleplay/AvatarViewport.tsxRepository: AaronFrancis05/AI-DOJO
Length of output: 5182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Avatar iframe usages =="
rg -n "ai-avatar\.akademia\.co\.jp|<iframe|sandbox=|allow=" components . || true
echo
echo "== Files mentioning the avatar host =="
rg -n "ai-avatar\.akademia\.co\.jp" . || trueRepository: AaronFrancis05/AI-DOJO
Length of output: 2987
Restrict both avatar iframes.
components/roleplay/AkademiaAvatarViewport.tsx#L67-L74components/roleplay/AvatarViewport.tsx#L66-L73
Add a restrictive sandbox and drop clipboard-write unless the embedded app actually needs clipboard access.
🧰 Tools
🪛 React Doctor (0.7.6)
[error] 67-67: An <iframe> with no sandbox is a security hole: the embedded page gets full access to your site.
Add sandbox="" or a curated value so embedded pages cannot get full access to your site by default.
(iframe-missing-sandbox)
📍 Affects 2 files
components/roleplay/AkademiaAvatarViewport.tsx#L67-L74(this comment)components/roleplay/AvatarViewport.tsx#L66-L73
🤖 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 `@components/roleplay/AkademiaAvatarViewport.tsx` around lines 67 - 74,
Restrict both avatar iframes by updating the iframe elements in
components/roleplay/AkademiaAvatarViewport.tsx (67-74) and
components/roleplay/AvatarViewport.tsx (66-73) to use a restrictive sandbox
configuration, and remove clipboard-write from their allow attributes unless
clipboard access is required by the embedded applications.
Source: Linters/SAST tools
…vatar API routes; ai-providers
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
app/api/sessions/[id]/route.ts (1)
80-100: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFold the corrections query into the second
Promise.allto remove a serial round trip.The corrections lookup (Lines 94-100) only depends on
conversationListfrom the firstPromise.all, not onscenario/situationResultused by the second block. Running it serially after the secondPromise.alladds an unnecessary sequential DB round trip. Since it's independent of the second batch's inputs, it can run concurrently with vocab/goals/domain.♻️ Proposed parallelization
- const [vocabItems, goals, domainResult] = await Promise.all([ + const conversationIds = conversationList.map(c => c.id); + const [vocabItems, goals, domainResult, allCorrections] = await Promise.all([ scenario ? db.select().from(vocabulary).where(eq(vocabulary.scenarioId, scenario.id)) : Promise.resolve([]), scenario ? db.select().from(scenarioGoals).where(eq(scenarioGoals.scenarioId, scenario.id)).orderBy(asc(scenarioGoals.sequenceOrder)) : Promise.resolve([]), situationResult ? db.select().from(domains).where(eq(domains.id, situationResult.domainId)).then(r => r[0] ?? null) : Promise.resolve(null), + + conversationIds.length > 0 + ? db.select().from(corrections).where(inArray(corrections.conversationId, conversationIds)) + : Promise.resolve([]), ]); - - const conversationIds = conversationList.map(c => c.id); - const allCorrections = conversationIds.length > 0 - ? await db - .select() - .from(corrections) - .where(inArray(corrections.conversationId, conversationIds)) - : [];🤖 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 `@app/api/sessions/`[id]/route.ts around lines 80 - 100, Move the allCorrections query into the existing second Promise.all alongside vocabItems, goals, and domainResult, using conversationList to build the conditional inArray query; destructure its result with the other values and remove the subsequent serial query block.
🤖 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 `@app/api/chat/route.ts`:
- Around line 52-56: Update the conversation query ordering near
conversationHistory reconstruction to add a deterministic secondary sort key
after conversations.turnNo, using the existing row ordering field that
consistently places the user message before the AI reply. Preserve ascending
turnNo ordering and ensure the resulting history maintains user/assistant
alternation for every provider.
In `@lib/ai-engine.ts`:
- Around line 236-242: Update the JSON parsing in the AI response flow around
provider.generateJSON and parsed to catch malformed rawText, then rethrow an
AIProviderError with the original parse failure and useful context. Import
AIProviderError from ./ai-providers if it is not already available, while
preserving successful parsing behavior.
In `@lib/ai-providers/openai-compatible.ts`:
- Around line 7-18: Update the OpenAI client construction in the
openai-compatible provider to pass a placeholder string when AI_API_KEY is
unset, rather than undefined, so keyless endpoints can initialize; preserve the
existing AI_MODEL validation and configured-key behavior.
In `@lib/auth/avatar-context.tsx`:
- Around line 82-91: Update deleteAvatar so fetchAvatars is called after the
DELETE request succeeds, while retaining the existing catch-path refresh for
failures. Keep the optimistic removal and error handling unchanged.
In `@lib/auth/user-context.tsx`:
- Around line 32-40: Update the avatarSrc state in the user context to
initialize as undefined rather than value?.avatarSrc, so server updates remain
authoritative until an optimistic override is set. In the merged value
construction, replace the nullish fallback with an explicit undefined check so
null preserves an optimistic avatar clear while undefined falls back to
value.avatarSrc.
---
Nitpick comments:
In `@app/api/sessions/`[id]/route.ts:
- Around line 80-100: Move the allCorrections query into the existing second
Promise.all alongside vocabItems, goals, and domainResult, using
conversationList to build the conditional inArray query; destructure its result
with the other values and remove the subsequent serial query block.
🪄 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
Run ID: fa04a5c9-ca8a-41ae-842c-d7738ea72b94
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
app/api/chat/route.tsapp/api/sessions/[id]/route.tsapp/api/user/avatar/route.tsapp/api/user/avatars/[id]/route.tsapp/api/user/avatars/[id]/select/route.tsapp/api/user/avatars/route.tscomponents/roleplay/AvatarViewport.tsxlib/ai-engine.tslib/ai-providers/anthropic.tslib/ai-providers/azure-openai.tslib/ai-providers/gemini.tslib/ai-providers/groq.tslib/ai-providers/index.tslib/ai-providers/openai-compatible.tslib/ai-providers/types.tslib/auth/avatar-context.tsxlib/auth/user-context.tsxpackage.json
Comment on lines
+52
to
+56
| db | ||
| .select() | ||
| .from(conversations) | ||
| .where(eq(conversations.sessionId, numericSessionId)) | ||
| .orderBy(asc(conversations.turnNo)), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add a deterministic tie-break to the conversation ordering.
User and AI rows are inserted with the same turnNo (Lines 117 and 150), but this query orders only by turnNo. Row order within a turn is therefore not guaranteed, so the reconstructed conversationHistory (Lines 93-96) can place the AI reply before the user message. This corrupts context for all providers and violates Anthropic's required user/assistant alternation. Add a secondary sort key.
🛠️ Proposed fix
db
.select()
.from(conversations)
.where(eq(conversations.sessionId, numericSessionId))
- .orderBy(asc(conversations.turnNo)),
+ .orderBy(asc(conversations.turnNo), asc(conversations.id)),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| db | |
| .select() | |
| .from(conversations) | |
| .where(eq(conversations.sessionId, numericSessionId)) | |
| .orderBy(asc(conversations.turnNo)), | |
| db | |
| .select() | |
| .from(conversations) | |
| .where(eq(conversations.sessionId, numericSessionId)) | |
| .orderBy(asc(conversations.turnNo), asc(conversations.id)), |
🤖 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 `@app/api/chat/route.ts` around lines 52 - 56, Update the conversation query
ordering near conversationHistory reconstruction to add a deterministic
secondary sort key after conversations.turnNo, using the existing row ordering
field that consistently places the user message before the AI reply. Preserve
ascending turnNo ordering and ensure the resulting history maintains
user/assistant alternation for every provider.
Comment on lines
+236
to
+242
| const provider = await getAIProvider(); | ||
| const rawText = await provider.generateJSON(systemInstruction, [ | ||
| ...conversationHistory, | ||
| { role: 'user' as const, parts: [{ text: userContent }] } | ||
| ]; | ||
|
|
||
| const response = await ai.models.generateContent({ | ||
| model: 'gemini-2.5-flash', | ||
| contents, | ||
| config: { | ||
| systemInstruction: systemInstruction, | ||
| responseMimeType: 'application/json' | ||
| } | ||
| }); | ||
|
|
||
| if (!response.text) { | ||
| throw new Error('Received an empty response back from the Gemini API system.'); | ||
| } | ||
| { role: 'user', content: userContent }, | ||
| ]); | ||
|
|
||
| const parsed = JSON.parse(response.text) as AIResponseAnalysis; | ||
| const parsed = JSON.parse(rawText) as AIResponseAnalysis; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard JSON.parse against malformed provider output.
rawText is not guaranteed to be valid JSON — the Anthropic provider has no native JSON mode and relies on prompt + fence stripping, so a stray non-JSON response throws a raw SyntaxError here. That error is not an AIProviderError, so app/api/chat/route.ts skips its structured branches and returns a generic 500 with no diagnostic context. Wrap the parse and rethrow as an AIProviderError.
🛡️ Proposed fix
- const parsed = JSON.parse(rawText) as AIResponseAnalysis;
+ let parsed: AIResponseAnalysis;
+ try {
+ parsed = JSON.parse(rawText) as AIResponseAnalysis;
+ } catch (err) {
+ throw new AIProviderError(provider.name, `Provider returned non-JSON response: ${rawText.slice(0, 500)}`, err);
+ }Import AIProviderError from ./ai-providers if not already available.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const provider = await getAIProvider(); | |
| const rawText = await provider.generateJSON(systemInstruction, [ | |
| ...conversationHistory, | |
| { role: 'user' as const, parts: [{ text: userContent }] } | |
| ]; | |
| const response = await ai.models.generateContent({ | |
| model: 'gemini-2.5-flash', | |
| contents, | |
| config: { | |
| systemInstruction: systemInstruction, | |
| responseMimeType: 'application/json' | |
| } | |
| }); | |
| if (!response.text) { | |
| throw new Error('Received an empty response back from the Gemini API system.'); | |
| } | |
| { role: 'user', content: userContent }, | |
| ]); | |
| const parsed = JSON.parse(response.text) as AIResponseAnalysis; | |
| const parsed = JSON.parse(rawText) as AIResponseAnalysis; | |
| const provider = await getAIProvider(); | |
| const rawText = await provider.generateJSON(systemInstruction, [ | |
| ...conversationHistory, | |
| { role: 'user', content: userContent }, | |
| ]); | |
| let parsed: AIResponseAnalysis; | |
| try { | |
| parsed = JSON.parse(rawText) as AIResponseAnalysis; | |
| } catch (err) { | |
| throw new AIProviderError( | |
| provider.name, | |
| `Provider returned non-JSON response: ${rawText.slice(0, 500)}`, | |
| err, | |
| ); | |
| } |
🤖 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 `@lib/ai-engine.ts` around lines 236 - 242, Update the JSON parsing in the AI
response flow around provider.generateJSON and parsed to catch malformed
rawText, then rethrow an AIProviderError with the original parse failure and
useful context. Import AIProviderError from ./ai-providers if it is not already
available, while preserving successful parsing behavior.
Comment on lines
+7
to
+18
| const apiKey = process.env.AI_API_KEY; | ||
| const modelName = process.env.AI_MODEL; | ||
| const jsonMode = process.env.AI_JSON_MODE !== 'off'; | ||
|
|
||
| if (!modelName) { | ||
| throw new AIProviderError('openai-compatible', 'AI_MODEL is required'); | ||
| } | ||
|
|
||
| const client = new OpenAI({ | ||
| baseURL, | ||
| apiKey: apiKey ?? undefined, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
openai node SDK v6 constructor throws when apiKey missing
💡 Result:
In OpenAI Node SDK v6, the constructor strictly enforces the presence of credentials to prevent configuration errors. Starting in v6.36.0, the client throws an error if an API key is missing (or not provided via other supported authentication methods like workload identity or admin API keys) [1][2]. If you encounter this error, it is because the SDK failed to detect a valid credential in the environment variables (e.g., OPENAI_API_KEY) or the constructor options [1][3]. To resolve this: 1. Ensure the environment variable is correctly set in your environment [4]. 2. Pass the apiKey explicitly in the constructor: const openai = new OpenAI({ apiKey: 'your-key-here' }); 3. If you are using a custom base URL that does not require an API key, you can pass an empty string to satisfy the validator: const openai = new OpenAI({ apiKey: '', baseURL: 'https://...' }); [3][5] Note that a regression related to this validation (which incorrectly flagged empty strings as missing) was addressed in subsequent patches [5]. Ensure you are using the latest version of the SDK for the most stable behavior [6].
Citations:
- 1: apiKey became mandatory in v6.36.0 openai/openai-node#1957
- 2: https://github.com/openai/openai-node/blob/e228aaadf3ebb4dd457dc7814402653c667411ec/CHANGELOG.md
- 3: Library throws error if apiKey isn't provided, even for custom base URLs openai/openai-node#1464
- 4: https://developers.openai.com/api/docs/libraries
- 5: fix(client): allow explicit empty apiKey string openai/openai-node#1961
- 6: https://github.com/openai/openai-node/blob/HEAD/CHANGELOG.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file ==\n'
git ls-files lib/ai-providers/openai-compatible.ts
printf '\n== package refs ==\n'
rg -n '"openai"|openai' package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb 2>/dev/null || true
printf '\n== provider file ==\n'
cat -n lib/ai-providers/openai-compatible.tsRepository: AaronFrancis05/AI-DOJO
Length of output: 2442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== next docs guide candidates ==\n'
fd -t f 'openai' node_modules/next/dist/docs 2>/dev/null || trueRepository: AaronFrancis05/AI-DOJO
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package manager files ==\n'
git ls-files | rg '^(package.json|pnpm-lock.yaml|package-lock.json|yarn.lock|bun.lockb)$'Repository: AaronFrancis05/AI-DOJO
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== provider =='
cat -n lib/ai-providers/openai-compatible.ts
echo '== package refs =='
rg -n '"openai"|openai' package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb 2>/dev/null || trueRepository: AaronFrancis05/AI-DOJO
Length of output: 2385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Need results for provider file and package version.'Repository: AaronFrancis05/AI-DOJO
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== provider file =='
cat -n lib/ai-providers/openai-compatible.ts
echo
echo '== openai version refs =='
rg -n '"openai"|openai' package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb 2>/dev/null || true
echo
echo '== next docs candidates =='
fd -t f 'openai' node_modules/next/dist/docs 2>/dev/null || trueRepository: AaronFrancis05/AI-DOJO
Length of output: 2426
Use a placeholder API key for keyless endpoints. apiKey: apiKey ?? undefined still fails client construction when neither AI_API_KEY nor OPENAI_API_KEY is set. Fall back to a dummy string (for example '') so local OpenAI-compatible servers can start.
🤖 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 `@lib/ai-providers/openai-compatible.ts` around lines 7 - 18, Update the OpenAI
client construction in the openai-compatible provider to pass a placeholder
string when AI_API_KEY is unset, rather than undefined, so keyless endpoints can
initialize; preserve the existing AI_MODEL validation and configured-key
behavior.
Comment on lines
+82
to
+91
| const deleteAvatar = useCallback(async (id: number) => { | ||
| const deleted = avatars.find(a => a.id === id); | ||
| setAvatars(prev => prev.filter(a => a.id !== id)); | ||
| try { | ||
| const res = await fetch(`/api/user/avatars/${id}`, { method: 'DELETE' }); | ||
| if (!res.ok) throw new Error('Failed'); | ||
| } catch { | ||
| await fetchAvatars(); | ||
| } | ||
| }, [avatars, fetchAvatars]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Refresh avatar state after successful deletion.
When the currently selected avatar is deleted, the server automatically assigns isSelected: true to a fallback avatar. However, the client's optimistic update only removes the deleted item and does not refresh the list on success. This leaves the UI without a selected avatar until the page is manually reloaded.
Ensure fetchAvatars is called after the deletion request completes successfully.
🐛 Proposed fix
const deleteAvatar = useCallback(async (id: number) => {
const deleted = avatars.find(a => a.id === id);
setAvatars(prev => prev.filter(a => a.id !== id));
try {
const res = await fetch(`/api/user/avatars/${id}`, { method: 'DELETE' });
if (!res.ok) throw new Error('Failed');
- } catch {
- await fetchAvatars();
- }
+ } finally {
+ await fetchAvatars();
+ }
}, [avatars, fetchAvatars]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const deleteAvatar = useCallback(async (id: number) => { | |
| const deleted = avatars.find(a => a.id === id); | |
| setAvatars(prev => prev.filter(a => a.id !== id)); | |
| try { | |
| const res = await fetch(`/api/user/avatars/${id}`, { method: 'DELETE' }); | |
| if (!res.ok) throw new Error('Failed'); | |
| } catch { | |
| await fetchAvatars(); | |
| } | |
| }, [avatars, fetchAvatars]); | |
| const deleteAvatar = useCallback(async (id: number) => { | |
| const deleted = avatars.find(a => a.id === id); | |
| setAvatars(prev => prev.filter(a => a.id !== id)); | |
| try { | |
| const res = await fetch(`/api/user/avatars/${id}`, { method: 'DELETE' }); | |
| if (!res.ok) throw new Error('Failed'); | |
| } finally { | |
| await fetchAvatars(); | |
| } | |
| }, [avatars, fetchAvatars]); |
🤖 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 `@lib/auth/avatar-context.tsx` around lines 82 - 91, Update deleteAvatar so
fetchAvatars is called after the DELETE request succeeds, while retaining the
existing catch-path refresh for failures. Keep the optimistic removal and error
handling unchanged.
Comment on lines
+32
to
+40
| const [avatarSrc, setAvatarSrc] = useState<string | null | undefined>(value?.avatarSrc); | ||
|
|
||
| const handleSetAvatarSrc = useCallback((src: string | null) => { | ||
| setAvatarSrc(src); | ||
| }, []); | ||
|
|
||
| const merged = value | ||
| ? { ...value, avatarSrc: avatarSrc ?? value.avatarSrc } | ||
| : null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Refactor local state to support optimistic clearing and server updates.
Currently, avatarSrc is initialized with the server's value?.avatarSrc. This duplicates state, which causes two issues:
- Stale state on navigation: If the server pushes an updated
valueafter a navigation or mutation, the local state will override it with the stale initial value. - Cannot clear avatar: Because of the nullish coalescing operator (
??), callingsetAvatarSrc(null)will fall back tovalue.avatarSrc, making it impossible to optimistically clear the avatar.
Initialize the local state as undefined so it only acts as an optimistic override, and use a strict !== undefined check to merge it.
♻️ Proposed fix
- const [avatarSrc, setAvatarSrc] = useState<string | null | undefined>(value?.avatarSrc);
+ const [avatarSrc, setAvatarSrc] = useState<string | null | undefined>();
const handleSetAvatarSrc = useCallback((src: string | null) => {
setAvatarSrc(src);
}, []);
const merged = value
- ? { ...value, avatarSrc: avatarSrc ?? value.avatarSrc }
+ ? { ...value, avatarSrc: avatarSrc !== undefined ? avatarSrc : value.avatarSrc }
: null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [avatarSrc, setAvatarSrc] = useState<string | null | undefined>(value?.avatarSrc); | |
| const handleSetAvatarSrc = useCallback((src: string | null) => { | |
| setAvatarSrc(src); | |
| }, []); | |
| const merged = value | |
| ? { ...value, avatarSrc: avatarSrc ?? value.avatarSrc } | |
| : null; | |
| const [avatarSrc, setAvatarSrc] = useState<string | null | undefined>(); | |
| const handleSetAvatarSrc = useCallback((src: string | null) => { | |
| setAvatarSrc(src); | |
| }, []); | |
| const merged = value | |
| ? { ...value, avatarSrc: avatarSrc !== undefined ? avatarSrc : value.avatarSrc } | |
| : null; |
🤖 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 `@lib/auth/user-context.tsx` around lines 32 - 40, Update the avatarSrc state
in the user context to initialize as undefined rather than value?.avatarSrc, so
server updates remain authoritative until an optimistic override is set. In the
merged value construction, replace the nullish fallback with an explicit
undefined check so null preserves an optimistic avatar clear while undefined
falls back to value.avatarSrc.
This was referenced Jul 20, 2026
Merged
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(app)/session/[sessionId]/page.tsx (1)
349-358: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResolve double audio playback and state transitions.
If the Akademia iframe avatar now handles its own speech and audio generation (via the new
MAKE_AVATAR_SPEAKcommand triggered by thetextToSpeakprop), invoking the localspeakWithVisemesandttsSpeakconcurrently will result in overlapping, out-of-sync audio.
- If the iframe bridge includes audio generation, remove these local TTS calls (and make the corresponding removal in
handleReplay). You will also need a mechanism (like anAVATAR_STOPPED_SPEAKINGmessage from the iframe) to know when to safely revertavatarModeto'idle'.- If the iframe only handles visual lip-sync without emitting audio, the local TTS can stay, but please verify this behavior.
🤖 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 `@app/`(app)/session/[sessionId]/page.tsx around lines 349 - 358, Update the speech flow around the shown aiText block and handleReplay to avoid local TTS when the Akademia iframe bridge generates audio; remove speakWithVisemes and ttsSpeak calls, and use the iframe’s AVATAR_STOPPED_SPEAKING message or equivalent completion signal to transition avatarMode from talking to idle. Ensure textToSpeak/MAKE_AVATAR_SPEAK remains the single speech trigger and preserve idle handling when no text is available.
🧹 Nitpick comments (2)
components/roleplay/AvatarViewport.tsx (2)
60-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract hardcoded avatar domain to an environment variable.
The domain
https://ai-avatar.akademia.co.jpis hardcoded here and in multiplepostMessage/MessageEventlisteners (lines 30, 44, and 56). This will break integration testing in lower environments (e.g.,localhostor staging) if the avatar bridge is hosted elsewhere during development.Consider extracting this to an environment variable like
process.env.NEXT_PUBLIC_AVATAR_ORIGIN.🤖 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 `@components/roleplay/AvatarViewport.tsx` around lines 60 - 61, Replace the hardcoded avatar origin in AvatarViewport, including iframeSrc and the postMessage/MessageEvent origin checks, with a shared process.env.NEXT_PUBLIC_AVATAR_ORIGIN value. Ensure all bridge communication consistently uses the configured origin across environments.
29-33: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd defensive optional chaining for
event.data.
event.datacan sometimes benullor a primitive string (e.g., from browser extensions injecting scripts), which would throw aTypeErrorwhen accessing.type. Adding optional chaining prevents unexpected crashes.💡 Proposed defensive checks
- if (event.data.type === 'AKADEMIA_READY') setIsReady(true); - if (event.data.type === 'USER_SPOKE') console.log('🎤 User spoke:', event.data.text); + if (event.data?.type === 'AKADEMIA_READY') setIsReady(true); + if (event.data?.type === 'USER_SPOKE') console.log('🎤 User spoke:', event.data?.text);🤖 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 `@components/roleplay/AvatarViewport.tsx` around lines 29 - 33, Update the handleMessage function to use optional chaining when accessing event.data.type, so null, undefined, or primitive message payloads are ignored without throwing while preserving the existing READY and USER_SPOKE handling.
🤖 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 `@components/roleplay/AvatarViewport.tsx`:
- Around line 47-58: Replace the bare textToSpeak prop and effect in
AvatarViewport with a speakEvent object containing a unique id, text, and
language; track the last handled id, buffer pending events until isReady, and
post each event once with its supplied language. Update page.tsx to emit
speakEvent only for newly generated AI messages or explicit replays, not when
initializing from historical messages, while allowing identical text with
different ids to replay.
---
Outside diff comments:
In `@app/`(app)/session/[sessionId]/page.tsx:
- Around line 349-358: Update the speech flow around the shown aiText block and
handleReplay to avoid local TTS when the Akademia iframe bridge generates audio;
remove speakWithVisemes and ttsSpeak calls, and use the iframe’s
AVATAR_STOPPED_SPEAKING message or equivalent completion signal to transition
avatarMode from talking to idle. Ensure textToSpeak/MAKE_AVATAR_SPEAK remains
the single speech trigger and preserve idle handling when no text is available.
---
Nitpick comments:
In `@components/roleplay/AvatarViewport.tsx`:
- Around line 60-61: Replace the hardcoded avatar origin in AvatarViewport,
including iframeSrc and the postMessage/MessageEvent origin checks, with a
shared process.env.NEXT_PUBLIC_AVATAR_ORIGIN value. Ensure all bridge
communication consistently uses the configured origin across environments.
- Around line 29-33: Update the handleMessage function to use optional chaining
when accessing event.data.type, so null, undefined, or primitive message
payloads are ignored without throwing while preserving the existing READY and
USER_SPOKE handling.
🪄 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
Run ID: d4149902-be2c-44c2-b8d2-c273c8ea5027
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
app/(app)/session/[sessionId]/page.tsxcomponents/roleplay/AvatarViewport.tsxpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Comment on lines
+47
to
+58
| // 3. 🆕 NEW: Send text to speak when it changes | ||
| useEffect(() => { | ||
| for (const mesh of meshes) { | ||
| if (mesh.morphTargetDictionary) { | ||
| const keys = Object.keys(mesh.morphTargetDictionary); | ||
| console.log(`[MorphTargetController] ${mesh.name}: ${keys.length} targets, sample:`, keys.slice(0, 5)); | ||
|
|
||
| // Check that expected ARKit shapes exist (for warning purposes) | ||
| const missingArkit = ['jawOpen', 'mouthClose', 'mouthSmileLeft', 'mouthFunnel', 'browInnerUp'] | ||
| .filter(s => !mesh.morphTargetDictionary![s] && ARKIT_INDEX[s] !== undefined); | ||
| if (missingArkit.length > 0 && !MISSING_SHAPE_WARNED.has(`mesh:${mesh.name}`)) { | ||
| MISSING_SHAPE_WARNED.add(`mesh:${mesh.name}`); | ||
| if (keys.every(k => /^\d+$/.test(k))) { | ||
| console.log(`[MorphTargetController] "${mesh.name}" uses numeric targets — using positional ARKIT order`); | ||
| } else { | ||
| logDevWarning(`"${mesh.name}" missing shapes: ${missingArkit.join(', ')}`); | ||
| } | ||
| if (textToSpeak && iframeRef.current?.contentWindow) { | ||
| iframeRef.current.contentWindow.postMessage({ | ||
| type: 'MAKE_AVATAR_SPEAK', | ||
| payload: { | ||
| text: textToSpeak, | ||
| language: 'en' // Adjust if Dojo uses dynamic languages | ||
| } | ||
| } | ||
| }, 'https://ai-avatar.akademia.co.jp'); | ||
| } | ||
| }, [meshes]); | ||
|
|
||
| const timeRef = useRef(0); | ||
| const blinkTimer = useRef(0); | ||
| const nextBlink = useRef(2 + Math.random() * 4); | ||
| const blinkWeight = useRef(0); | ||
| const targetVisemeShapes = useRef<VisemeShapeMap>({}); | ||
| const currentVisemeShapes = useRef<VisemeShapeMap>({}); | ||
| const prevVisemeId = useRef(-1); | ||
| const fadingVisemeKeys = useRef<Set<string>>(new Set()); | ||
|
|
||
| const targetEmotionShapes = useMemo<EmotionShapeMap>(() => { | ||
| if (emotion && EMOTION_SHAPES[emotion]) return EMOTION_SHAPES[emotion]; | ||
| return {}; | ||
| }, [emotion]); | ||
|
|
||
| useFrame((_, delta) => { | ||
| try { | ||
| timeRef.current += delta; | ||
|
|
||
| const visemeId = mode === 'talking' ? getCurrentViseme() : -1; | ||
| if (visemeId !== prevVisemeId.current && visemeId >= 0) { | ||
| // Track keys from previous viseme so they fade out | ||
| for (const k of Object.keys(targetVisemeShapes.current)) { | ||
| fadingVisemeKeys.current.add(k); | ||
| } | ||
| targetVisemeShapes.current = VISEME_SHAPES[visemeId] ?? {}; | ||
| prevVisemeId.current = visemeId; | ||
| } else if (visemeId < 0 && prevVisemeId.current >= 0) { | ||
| // Speech just ended — transfer active keys to fading set | ||
| for (const k of Object.keys(targetVisemeShapes.current)) { | ||
| fadingVisemeKeys.current.add(k); | ||
| } | ||
| targetVisemeShapes.current = {}; | ||
| prevVisemeId.current = -1; | ||
| } | ||
|
|
||
| // Remove fully-faded keys | ||
| for (const k of fadingVisemeKeys.current) { | ||
| const cur = currentVisemeShapes.current[k as keyof VisemeShapeMap] ?? 0; | ||
| if (cur < 0.01) fadingVisemeKeys.current.delete(k); | ||
| } | ||
|
|
||
| const allShapeKeys = new Set([ | ||
| ...Object.keys(targetVisemeShapes.current), | ||
| ...Object.keys(targetEmotionShapes), | ||
| ...fadingVisemeKeys.current, | ||
| ]); | ||
|
|
||
| // Blink logic | ||
| let currentBlink = blinkWeight.current; | ||
| if (mode === 'idle' || mode === 'listening') { | ||
| blinkTimer.current += delta; | ||
| if (blinkTimer.current >= nextBlink.current) { | ||
| blinkWeight.current = 1; | ||
| blinkTimer.current = 0; | ||
| nextBlink.current = 2 + Math.random() * 5; | ||
| } | ||
| } else { | ||
| blinkWeight.current = 0; | ||
| } | ||
| if (currentBlink > 0) blinkWeight.current = Math.max(0, currentBlink - delta * 6); | ||
| const blink = Math.sin(Math.max(0, Math.min(1, blinkWeight.current)) * Math.PI); | ||
|
|
||
| for (const mesh of meshes) { | ||
| if (!mesh.morphTargetInfluences) continue; | ||
| const isEyelash = mesh.name === 'AvatarEyelashes'; | ||
| const isHead = mesh.name === 'AvatarHead'; | ||
|
|
||
| if (isHead) { | ||
| for (const key of allShapeKeys) { | ||
| const visemeTarget = targetVisemeShapes.current[key as keyof VisemeShapeMap] ?? 0; | ||
| const emotionTarget = targetEmotionShapes[key as keyof EmotionShapeMap] ?? 0; | ||
| const combined = Math.max(visemeTarget, emotionTarget); | ||
| const current = currentVisemeShapes.current[key as keyof VisemeShapeMap] ?? 0; | ||
| const smoothed = lerp(current, combined, Math.min(1, delta * 16)); | ||
| (currentVisemeShapes.current as Record<string, number>)[key] = smoothed; | ||
| setShapeWeight(mesh, key, smoothed); | ||
| } | ||
| } | ||
|
|
||
| if (isEyelash) { | ||
| // Use dictionary lookup for blink indices | ||
| const blinkIdx = mesh.morphTargetDictionary?.['eyeBlinkLeft'] ?? 7; | ||
| const blinkIdx2 = mesh.morphTargetDictionary?.['eyeBlinkRight'] ?? 8; | ||
| setEyelashWeight(mesh, blinkIdx, blink); | ||
| setEyelashWeight(mesh, blinkIdx2, blink); | ||
| if (targetEmotionShapes.browInnerUp) { | ||
| const browUpIdx = mesh.morphTargetDictionary?.['browInnerUp'] ?? 2; | ||
| setEyelashWeight(mesh, browUpIdx, targetEmotionShapes.browInnerUp); | ||
| } | ||
| if (targetEmotionShapes.browDownLeft || targetEmotionShapes.browDownRight) { | ||
| const browDown = Math.max(targetEmotionShapes.browDownLeft ?? 0, targetEmotionShapes.browDownRight ?? 0); | ||
| const bdLeftIdx = mesh.morphTargetDictionary?.['browDownLeft'] ?? 0; | ||
| const bdRightIdx = mesh.morphTargetDictionary?.['browDownRight'] ?? 1; | ||
| setEyelashWeight(mesh, bdLeftIdx, browDown); | ||
| setEyelashWeight(mesh, bdRightIdx, browDown); | ||
| } | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error('[MorphTargetController] frame error:', err); | ||
| } | ||
| }); | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /* ── AutoCamera ───────────────────────────────────────────────────────────── | ||
| Frames the camera after the model is grounded. | ||
| ────────────────────────────────────────────────────────────────────────── */ | ||
| function AutoCamera({ scene, cameraMode, onFramed }: { | ||
| scene: THREE.Group; | ||
| cameraMode: 'front' | 'over-shoulder'; | ||
| onFramed?: () => void; | ||
| }) { | ||
| const { camera } = useThree(); | ||
| const framed = useRef(false); | ||
|
|
||
| useEffect(() => { | ||
| if (!scene || framed.current) return; | ||
|
|
||
| let rafId: number; | ||
| let attempts = 0; | ||
| const MAX_ATTEMPTS = 60; | ||
|
|
||
| const tryFrame = () => { | ||
| attempts += 1; | ||
| const box = new THREE.Box3().setFromObject(scene); | ||
| const boxSize = box.getSize(new THREE.Vector3()); | ||
| const isFinite3 = (v: THREE.Vector3) => | ||
| Number.isFinite(v.x) && Number.isFinite(v.y) && Number.isFinite(v.z); | ||
|
|
||
| const boxValid = isFinite3(box.min) && isFinite3(box.max) && isFinite3(boxSize) | ||
| && boxSize.y >= 0.1 && boxSize.y <= 100; | ||
| }, [textToSpeak]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
Address race condition, history auto-play, and hardcoded language.
There are several critical flaws with triggering speech by watching a bare textToSpeak string:
- Dropped Messages: It does not wait for
isReady. IftextToSpeakchanges before the iframe script initializes, thepostMessageis dropped. - Unintended History Playback: If you try to fix the dropped messages by simply adding
isReadyto the dependency array, the avatar will automatically speak the last historical message on page load (because the parent initializestextToSpeakwith the latest historical turn). - Repeated Messages Ignored: If the AI outputs the exact same text twice in a row, the
textToSpeakstring doesn't change, meaning this effect will not re-trigger. - Hardcoded Language: The payload hardcodes
'en'instead of dynamically using the session's target language.
Recommendation: Instead of passing a textToSpeak string, update the component's props to accept an explicit event object (e.g., speakEvent: { id: number, text: string, lang: string } | null).
- In
page.tsx, only set this event for newly generated AI messages or explicit replays. - In
AvatarViewport, use the unique ID to detect repeats, buffer the event if!isReady, and pass the correct language to the iframe.
🤖 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 `@components/roleplay/AvatarViewport.tsx` around lines 47 - 58, Replace the
bare textToSpeak prop and effect in AvatarViewport with a speakEvent object
containing a unique id, text, and language; track the last handled id, buffer
pending events until isReady, and post each event once with its supplied
language. Update page.tsx to emit speakEvent only for newly generated AI
messages or explicit replays, not when initializing from historical messages,
while allowing identical text with different ids to replay.
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
2 participants
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 by CodeRabbit