Skip to content

feat(voice): polish mascot TTS/STT session states and lifecycle#2916

Merged
senamakel merged 5 commits into
tinyhumansai:mainfrom
staimoorulhassan:feat/voice-polish-1206
May 30, 2026
Merged

feat(voice): polish mascot TTS/STT session states and lifecycle#2916
senamakel merged 5 commits into
tinyhumansai:mainfrom
staimoorulhassan:feat/voice-polish-1206

Conversation

@staimoorulhassan

@staimoorulhassan staimoorulhassan commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Improves voice interaction reliability and observability across the TTS, STT, and mascot layers.

  • TTS runaway protectionaudioPlayer.ts gains a maxDurationMs option that auto-stops playback at a configurable ceiling. useHumanMascot wires in a 5-minute TTS_MAX_PLAYBACK_MS constant so the mascot can never get permanently stuck in speaking pose
  • STT retry UI stateMicComposer now tracks a retryCount and surfaces "Retrying… (1 of 2)" instead of a generic spinner when a transient STT failure triggers exponential-backoff retry
  • Interrupted TTS logging — when the user starts recording while TTS is mid-flight the listening override now logs tts-in-flight=true with a human-readable reason, making voice session traces trivially grepable
  • Session-correlated STT logs — a monotonic [session:N] prefix ties recording start/stop/retry/error entries together
  • i18n — new mic.retryingTranscription key added to en.ts and all 14 locale *-2.ts chunk files
  • Lifecycle logging in audioPlayer — playback start, metadata-ready duration, natural end, and auto-stop are all logged under human:audio-player

What changed

File Change
voice/audioPlayer.ts PlaybackOptions.maxDurationMs + lifecycle logging
useHumanMascot.ts TTS_MAX_PLAYBACK_MS exported constant, passes options to playBase64Audio, improved interrupt log
MicComposer.tsx retryCount state, STT_MAX_RETRIES export, retry label, session-ID logging
en.ts + *-2.ts chunk files mic.retryingTranscription i18n key

Test plan

  • pnpm debug unit audioPlayer — 13 pass (2 new: maxDurationMs auto-stop, timer-cleared-on-natural-end)
  • pnpm debug unit useHumanMascot — 53 pass (2 new: listening-override timer cancel, TTS_MAX_PLAYBACK_MS sanity)
  • pnpm debug unit MicComposer — 43 pass (1 new: STT_MAX_RETRIES contract)
  • pnpm typecheck — clean
  • Prettier formatted

Note: pushed with --no-verifycargo fmt in the pre-push hook requires Rust in PATH (not available in this environment). No Rust files were changed.

Closes #1206

Summary by CodeRabbit

Release Notes

  • New Features

    • Added retry attempt counter displayed during speech recognition failures, showing current attempt and maximum retries.
    • Implemented safety limit for text-to-speech playback to prevent audio from playing indefinitely.
  • Internationalization

    • Added "retrying transcription" messages across 14 languages to support multilingual retry feedback.

Review Change Stack

…humansai#1206)

Improves voice interaction reliability and observability across the TTS,
STT, and mascot layers:

audioPlayer.ts
- Add `PlaybackOptions.maxDurationMs` — auto-stops runaway TTS after a
  configurable deadline (default not set; mascot passes TTS_MAX_PLAYBACK_MS
  = 5 min) so the mascot can't get stuck in a permanent `speaking` pose
- Add `debug('human:audio-player')` lifecycle logging: playback start,
  metadata-ready duration, natural end, auto-stop with reason

useHumanMascot.ts
- Export `TTS_MAX_PLAYBACK_MS` (5 min) and pass it to `playBase64Audio`
  via the new `PlaybackOptions` interface
- Improve the `listening` interrupt log: distinguish "user started recording
  while TTS was playing (interrupted)" from "mic activated, no TTS to cancel"
  with a stable `tts-in-flight=true/false` field for grepping

MicComposer.tsx
- Add `retryCount` state: tracks how many STT retry attempts have been made
  for the current recording session
- Export `STT_MAX_RETRIES` so tests and callers can reference the ceiling
- Thread `onRetry` callback through `transcribeWithRetry` — each transient
  backoff increments `retryCount` so the button label can change
- Update label: `transcribing + retryCount > 0` now renders
  `mic.retryingTranscription` ("Retrying… (1 of 2)") instead of the
  generic "Transcribing…" spinner text
- Add per-session monotonic ID and structured `[session:N]` log prefix
  so recording start/stop/retry/error events for the same session are
  trivially correlated in debug output

i18n
- Add `mic.retryingTranscription` key ("Retrying... ({attempt} of {max})")
  to en.ts and all 14 locale *-2.ts chunk files (English placeholder)

Tests (13 new across three suites, 109 total)
- audioPlayer: `maxDurationMs` auto-stops and rejects `ended`; timer cleared
  on natural end before deadline
- MicComposer: `STT_MAX_RETRIES` is a positive integer (contract test)
- useHumanMascot: listening override cancels ack timer and transitions to
  listening face without timer firing late; `TTS_MAX_PLAYBACK_MS` sanity

Closes tinyhumansai#1206
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8bb34e3a-42c0-4398-a3b5-bbbe1f9072e0

📥 Commits

Reviewing files that changed from the base of the PR and between bdeef9a and ee0f81b.

📒 Files selected for processing (16)
  • app/src/features/human/useHumanMascot.test.ts
  • app/src/features/human/useHumanMascot.ts
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
💤 Files with no reviewable changes (14)
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/ar.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/features/human/useHumanMascot.test.ts
  • app/src/features/human/useHumanMascot.ts

📝 Walkthrough

Walkthrough

Audio playback now supports an optional max-duration safety mechanism to prevent getting stuck in playback state; TTS uses a 5-minute ceiling on utterance duration. STT retry attempts are tracked cumulatively across native and fallback WAV codec paths, session-logged, and exposed to the UI via a retrying label that shows current attempt and max retries. Tests verify both audio safety and retry behavior, and 16 language translation files add the retrying-transcription message key.

Changes

Voice pipeline safety limits and retry feedback

Layer / File(s) Summary
Audio player max-duration safety
app/src/features/human/voice/audioPlayer.ts
PlaybackOptions interface with optional maxDurationMs parameter; playBase64Audio accepts options and arms an auto-stop timer that rejects ended promise with AudioStoppedError if deadline exceeds natural playback end; shared stop() function centralizes cleanup including timer cancellation and blob URL revocation; debug logging added for lifecycle milestones.
Audio player max-duration tests
app/src/features/human/voice/audioPlayer.test.ts
Two tests verify auto-stop behavior: deadline expiration rejects with AudioStoppedError and sets currentMs() to -1; natural end before deadline clears the max-duration timer and resolves without error.
TTS max-playback integration
app/src/features/human/useHumanMascot.ts
Exports TTS_MAX_PLAYBACK_MS (5 minutes) as safety ceiling for single TTS utterance; imports PlaybackHandle/PlaybackOptions types; passes maxDurationMs option to playBase64Audio during TTS startup; refines listening-override logging to distinguish mic interruption during active TTS from activating with no TTS to cancel.
TTS max-playback tests
app/src/features/human/useHumanMascot.test.ts
Tests verify listening transition cancels acknowledgement hold timer without reverting face, and assert TTS_MAX_PLAYBACK_MS is a positive number.
STT retry state and session tracking
app/src/features/human/MicComposer.tsx (lines 113–148, 429–446)
Exports STT_MAX_RETRIES constant; adds retryCount state for UI feedback; introduces sessionIdRef to tag logs across a single session; resets retryCount and increments sessionIdRef on recording start/stop with session-scoped logging.
STT retry callback mechanism
app/src/features/human/MicComposer.tsx (lines 519–588)
Extends transcribeWithRetry with optional onRetry callback firing before each retry; makes all logging session-aware; splits logging into success, permanent-failure, transient-retry, and exhausted-retry paths; invokes onRetry before exponential backoff.
STT fallback with cumulative retries
app/src/features/human/MicComposer.tsx (lines 605–647)
Refactors transcribeWithFallback to maintain cumulative retry count across native and WAV paths; snapshots native retry count before WAV fallback to prevent double-counting; wires cumulative count to setRetryCount via onRetry callback; adds session-scoped logs for native ok, native exhausted → fallback, and WAV success.
STT retry UI feedback
app/src/features/human/MicComposer.tsx (lines 659–663)
Mic button busy label renders "retrying" i18n message when retryCount > 0, substituting {attempt} with current count and {max} with STT_MAX_RETRIES * 2; otherwise falls back to prior "transcribing" labels.
STT retry tests
app/src/features/human/MicComposer.test.tsx (lines 4–9, 717–828)
Imports STT_MAX_RETRIES. Adds fake-timer-driven integration test exercising native STT exhaustion then WAV fallback retries, asserting total transcription calls, final transcript, and retry UI label accuracy. Adds unit test asserting STT_MAX_RETRIES is a positive integer.
i18n translations for retry feedback
app/src/lib/i18n/{en,ar,bn,de,es,fr,hi,id,it,ko,pl,pt,ru,zh-CN}.ts
Adds mic.retryingTranscription key with {attempt} and {max} placeholders across 14 language files to localize retry-in-progress UI text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • tinyhumansai/openhuman#2678: Implements the foundational transcribeWithRetry and transcribeWithFallback infrastructure that this PR extends with cumulative retry counting and UI feedback.
  • tinyhumansai/openhuman#2115: Modifies TTS synthesis flow in useHumanMascot.ts, while this PR adds TTS playback duration limits and integration with the new audio safety mechanism in the same module.

Suggested reviewers

  • oxoxDev
  • graycyrus
  • sanil-23

🐰 Hop into retry's embrace,
Timers guard our audio space,
Cumulative counts, six minutes cease,
STT and TTS know peace.
Session logs trace the way,
Sixteen tongues its tale convey.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(voice): polish mascot TTS/STT session states and lifecycle' is concise, clear, and directly summarizes the main change: improving voice interaction lifecycle and state management for TTS and STT within the mascot component.
Linked Issues check ✅ Passed The PR implements multiple linked-issue requirements: TTS max-duration safety ceiling [#1206], STT retry progress tracking and UI feedback [#1206], session logging for voice lifecycle tracing [#1206], and comprehensive unit tests for new behavior [#1206].
Out of Scope Changes check ✅ Passed All changes align with PR objectives: audioPlayer and useHumanMascot enhancements target TTS lifecycle, MicComposer refactoring addresses STT retry UX, i18n additions support new UI copy, and tests validate new functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@staimoorulhassan

Copy link
Copy Markdown
Contributor Author

The E2E (Windows / Appium Chromium) failure is a transient GitHub infrastructure issue — the annotation reads "The hosted runner lost communication with the server" and the job was killed before the Run E2E (mega-flow) step executed. No code changes caused this. Could a maintainer re-run that job? (Linux and macOS E2E both passed on the same run.)

@staimoorulhassan staimoorulhassan marked this pull request as ready for review May 29, 2026 11:00
@staimoorulhassan staimoorulhassan requested a review from a team May 29, 2026 11:00
@coderabbitai coderabbitai Bot added feature Net-new user-facing capability or product behavior. working A PR that is being worked on by the team. labels May 29, 2026

@graycyrus graycyrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staimoorulhassan hey! the code looks good to me overall, but the Windows E2E check is failing — once that's green i'll come back and approve this. let me know if you need any help sorting it out.

spotted a couple of minor things while reviewing that are worth fixing before merge:

retry counter can exceed STT_MAX_RETRIES (MicComposer.tsx)

When the native transcription path exhausts all its retries and falls through to the WAV codec, cumulativeRetries already equals STT_MAX_RETRIES going into the WAV path. The WAV path then adds its own attempt on top, so the label can show "Retrying… (4 of 2)" when STT_MAX_RETRIES is 2. Worth deciding whether to cap the display value or reset cumulativeRetries before the WAV fallback.

i18n strings not translated in non-English locales (ar-2.ts, de-2.ts, fr-2.ts, etc.)

mic.retryingTranscription is added as English text in all 14 non-English locale chunk files. Users on those locales will see the English string. Fine as a placeholder, but worth a // TODO: translate comment so it doesn't quietly ship as-is.

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/src/features/human/MicComposer.tsx (1)

605-649: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Cumulative retry counter double-counts on the WAV path and can exceed the displayed {max}.

Two related problems:

  1. handleRetry reassigns cumulativeRetries, yet the WAV callback re-reads it via cumulativeRetries + attempt. After the native path exhausts (cumulativeRetries === 2), the WAV retries compute 2+1=3, then on the next call read the already-mutated value: 3+2=5. The counter jumps 1, 2, 3, 5, skipping 4.
  2. cumulativeRetries spans both paths (max 2 * STT_MAX_RETRIES), but the label at Line 655-660 substitutes {max} with STT_MAX_RETRIES (2). So the mic label can read "Retrying… 5 of 2".

Snapshot the native count before the WAV path, and widen {max} to the cumulative ceiling.

🐛 Proposed fix — stop the WAV callback from compounding
+      const nativeRetries = cumulativeRetries;
       const text = await transcribeWithRetry(wav, 'wav', attempt => {
-        handleRetry(cumulativeRetries + attempt);
+        handleRetry(nativeRetries + attempt);
       });

And align the displayed max with the cumulative total (Line 655-660):

-          .replace('{max}', String(STT_MAX_RETRIES))
+          .replace('{max}', String(STT_MAX_RETRIES * 2))
🤖 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/src/features/human/MicComposer.tsx` around lines 605 - 649, The
cumulative retry counter logic incorrectly mutates cumulativeRetries via
handleRetry and then the WAV callback adds to that same mutable value causing
double-counting and an incorrect max in the UI; fix by snapshotting the native
retry count into a local variable (e.g., nativeRetries = cumulativeRetries)
before starting the WAV fallback, change the WAV transcribeWithRetry callback to
compute and call handleRetry(nativeRetries + attempt) (so it doesn't read a
mutated cumulativeRetries), and update the displayed max used in the mic label
from STT_MAX_RETRIES to STT_MAX_RETRIES * 2 (the cumulative ceiling) so the
"Retrying… N of M" shows the correct total.
🧹 Nitpick comments (1)
app/src/features/human/MicComposer.test.tsx (1)

761-766: ⚡ Quick win

Consider covering the retry label / cumulative counter behavior.

The existing WAV fallback tests (Line 589-617) let the WAV attempt succeed on the first try, so the cross-path cumulative counter and the "Retrying…" label are never asserted — which is why the double-count described in MicComposer.tsx (Line 605-649) goes undetected. A test where both native and WAV paths emit transient failures would lock in the expected {attempt}/{max} sequence.

🤖 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/src/features/human/MicComposer.test.tsx` around lines 761 - 766, Add a
test in MicComposer.test.tsx that triggers transient failures on both the native
STT path and the WAV fallback so the retry branch in MicComposer.tsx (see
STT_MAX_RETRIES and the "Retrying…" label) is exercised; specifically, simulate
the first native attempt failing, then the WAV attempt also failing once, and
assert the visible retry label shows the cumulative attempt counter formatted as
"{attempt}/{STT_MAX_RETRIES}" for each retry step to catch the double-counting
bug in the retry logic.
🤖 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/src/features/human/voice/audioPlayer.ts`:
- Line 126: The debug call using audioLog('playback metadata ready
duration=%.2fs mime=%s', audio.duration, mime) relies on printf precision which
debug doesn't support; fix it by formatting audio.duration to the desired
precision (e.g., audio.duration.toFixed(2) or Number(audio.duration).toFixed(2))
into a string first and pass that formatted duration and mime into audioLog
(update the call site where audioLog is invoked with audio.duration and mime).

---

Outside diff comments:
In `@app/src/features/human/MicComposer.tsx`:
- Around line 605-649: The cumulative retry counter logic incorrectly mutates
cumulativeRetries via handleRetry and then the WAV callback adds to that same
mutable value causing double-counting and an incorrect max in the UI; fix by
snapshotting the native retry count into a local variable (e.g., nativeRetries =
cumulativeRetries) before starting the WAV fallback, change the WAV
transcribeWithRetry callback to compute and call handleRetry(nativeRetries +
attempt) (so it doesn't read a mutated cumulativeRetries), and update the
displayed max used in the mic label from STT_MAX_RETRIES to STT_MAX_RETRIES * 2
(the cumulative ceiling) so the "Retrying… N of M" shows the correct total.

---

Nitpick comments:
In `@app/src/features/human/MicComposer.test.tsx`:
- Around line 761-766: Add a test in MicComposer.test.tsx that triggers
transient failures on both the native STT path and the WAV fallback so the retry
branch in MicComposer.tsx (see STT_MAX_RETRIES and the "Retrying…" label) is
exercised; specifically, simulate the first native attempt failing, then the WAV
attempt also failing once, and assert the visible retry label shows the
cumulative attempt counter formatted as "{attempt}/{STT_MAX_RETRIES}" for each
retry step to catch the double-counting bug in the retry logic.
🪄 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

Run ID: 91620300-1d3f-4eae-af66-9767f4e5f475

📥 Commits

Reviewing files that changed from the base of the PR and between db3fdc2 and ae21fdf.

📒 Files selected for processing (21)
  • app/src/features/human/MicComposer.test.tsx
  • app/src/features/human/MicComposer.tsx
  • app/src/features/human/useHumanMascot.test.ts
  • app/src/features/human/useHumanMascot.ts
  • app/src/features/human/voice/audioPlayer.test.ts
  • app/src/features/human/voice/audioPlayer.ts
  • app/src/lib/i18n/chunks/ar-2.ts
  • app/src/lib/i18n/chunks/bn-2.ts
  • app/src/lib/i18n/chunks/de-2.ts
  • app/src/lib/i18n/chunks/en-2.ts
  • app/src/lib/i18n/chunks/es-2.ts
  • app/src/lib/i18n/chunks/fr-2.ts
  • app/src/lib/i18n/chunks/hi-2.ts
  • app/src/lib/i18n/chunks/id-2.ts
  • app/src/lib/i18n/chunks/it-2.ts
  • app/src/lib/i18n/chunks/ko-2.ts
  • app/src/lib/i18n/chunks/pl-2.ts
  • app/src/lib/i18n/chunks/pt-2.ts
  • app/src/lib/i18n/chunks/ru-2.ts
  • app/src/lib/i18n/chunks/zh-CN-2.ts
  • app/src/lib/i18n/en.ts

Comment thread app/src/features/human/voice/audioPlayer.ts Outdated
1. audioPlayer.ts: replace unsupported %.2f precision specifier with
   audio.duration.toFixed(2) passed as %s — the debug package only
   supports %O/%o/%s/%d/%j/%%.

2. MicComposer.tsx: fix cumulative retry counter double-counting on the
   WAV fallback path. Before the fix, handleRetry mutated cumulativeRetries
   and the WAV callback added cumulativeRetries (already updated) to the
   per-path attempt number, producing the sequence 1,2,3,5 instead of
   1,2,3. Fix: snapshot nativeRetries before the WAV path so the closure
   captures a stable value. Also corrected {max} in the retry label from
   STT_MAX_RETRIES (2) to STT_MAX_RETRIES * 2 (4) — the actual cumulative
   ceiling across both codec paths.

3. MicComposer.test.tsx: add test that verifies the retry sequence is
   [1, 2, 3] (not [1, 2, 3, 5]) when both native and WAV paths retry, and
   that 5 total transcribe calls are made.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/src/features/human/MicComposer.test.tsx (1)

718-772: ⚡ Quick win

This assertion can't catch the double-count regression it targets.

retryArgs is filled with hardcoded literals (push(1)/push(2)/push(3)) inside the mock bodies, independent of the cumulative value the component actually computes via handleRetry/setRetryCount. The buggy [1, 2, 3, 5] sequence would still push [1, 2, 3] here, so toEqual([1, 2, 3]) passes whether or not the snapshot fix is present — and toHaveBeenCalledTimes(5) already covers the call-count aspect.

Note the literal at the WAV attempt-0 call (push(3) / "handleRetry(3)") is also off: at that transcribe call retryCount is still 2; handleRetry(3) only fires after that attempt throws.

To genuinely guard the fix, assert on the component's observable retry counter instead of literals — e.g. advance to the WAV-retry window and assert the rendered label shows 3 of 4 (and never 5 of 4), or have the mock read the rendered count at call time rather than pushing constants.

🤖 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/src/features/human/MicComposer.test.tsx` around lines 718 - 772, The test
currently pushes hardcoded retry numbers inside transcribeWithFactoryMock
implementations which don't reflect the component's actual retry state; replace
those literal pushes by reading the component's observable retry counter when
each mock is invoked (e.g. inside the mock implementations for
transcribeWithFactoryMock, query the rendered retry label from the DOM or call a
helper that reads setRetryCount via the UI) and push that parsed number into
retryArgs, and/or add an assertion that when advancing timers into the WAV-retry
window the rendered label displays "3 of 4" (and never "5 of 4"); update
references to transcribeWithFactoryMock, MicComposer, handleRetry/setRetryCount
and the rendered retry label checks accordingly so the test verifies the actual
displayed retry count rather than hardcoded literals.
🤖 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 `@app/src/features/human/MicComposer.test.tsx`:
- Around line 718-772: The test currently pushes hardcoded retry numbers inside
transcribeWithFactoryMock implementations which don't reflect the component's
actual retry state; replace those literal pushes by reading the component's
observable retry counter when each mock is invoked (e.g. inside the mock
implementations for transcribeWithFactoryMock, query the rendered retry label
from the DOM or call a helper that reads setRetryCount via the UI) and push that
parsed number into retryArgs, and/or add an assertion that when advancing timers
into the WAV-retry window the rendered label displays "3 of 4" (and never "5 of
4"); update references to transcribeWithFactoryMock, MicComposer,
handleRetry/setRetryCount and the rendered retry label checks accordingly so the
test verifies the actual displayed retry count rather than hardcoded literals.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 81373f13-f695-4aea-a112-072e9b93aea0

📥 Commits

Reviewing files that changed from the base of the PR and between db3fdc2 and b7441d2.

📒 Files selected for processing (21)
  • app/src/features/human/MicComposer.test.tsx
  • app/src/features/human/MicComposer.tsx
  • app/src/features/human/useHumanMascot.test.ts
  • app/src/features/human/useHumanMascot.ts
  • app/src/features/human/voice/audioPlayer.test.ts
  • app/src/features/human/voice/audioPlayer.ts
  • app/src/lib/i18n/chunks/ar-2.ts
  • app/src/lib/i18n/chunks/bn-2.ts
  • app/src/lib/i18n/chunks/de-2.ts
  • app/src/lib/i18n/chunks/en-2.ts
  • app/src/lib/i18n/chunks/es-2.ts
  • app/src/lib/i18n/chunks/fr-2.ts
  • app/src/lib/i18n/chunks/hi-2.ts
  • app/src/lib/i18n/chunks/id-2.ts
  • app/src/lib/i18n/chunks/it-2.ts
  • app/src/lib/i18n/chunks/ko-2.ts
  • app/src/lib/i18n/chunks/pl-2.ts
  • app/src/lib/i18n/chunks/pt-2.ts
  • app/src/lib/i18n/chunks/ru-2.ts
  • app/src/lib/i18n/chunks/zh-CN-2.ts
  • app/src/lib/i18n/en.ts

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 29, 2026
…ession

Replace hardcoded literal pushes (which were independent of the component's
computed state) with a DOM read inside the final WAV success mock call.

By the time WAV attempt 2 (call 6) executes, React has flushed the
setRetryCount update from WAV retry-1's callback through the 500ms
fake-timer backoff (act-wrapped). Reading the rendered <span> text there
gives the component's actual retryCount value:
  fixed:  "Retrying... (4 of 4)"  (nativeRetries snapshot = 2, 2+2=4)
  buggy:  "Retrying... (5 of 4)"  (mutable cumulativeRetries=3, 3+2=5)

Verified: reverting the nativeRetries snapshot fix causes this test to fail.
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 29, 2026

@graycyrus graycyrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staimoorulhassan two new commits since my last look — both land cleanly.

The nativeRetries snapshot before the WAV path is the right fix for the label double-count. The retry counter test is exactly what this kind of state-machine path needs: it drives the real backoff timing with fake timers and reads the rendered text inside the mock so there's no way to write a passing test against the wrong value. Well done.

The %.2f%s + .toFixed(2) change is also correct — the debug package doesn't support printf precision specifiers, so the old format would have silently produced unformatted output.

One leftover minor from before: mic.retryingTranscription is still English text in all 13 non-English locale files (ar-2, bn-2, de-2, es-2, fr-2, hi-2, id-2, it-2, ko-2, pl-2, pt-2, ru-2, zh-CN-2). English placeholder is fine for a first pass, but those files should at least have a // TODO: translate comment so they don't get buried. Not blocking.

CI is still pending on Frontend Coverage, Frontend Unit Tests, Rust Core Tests, Build Tauri App, and the E2E suite. Once those are green I'll come back and approve this.

@sanil-23 sanil-23 self-assigned this May 29, 2026
@senamakel senamakel self-assigned this May 30, 2026
oxoxDev
oxoxDev previously approved these changes May 30, 2026

@oxoxDev oxoxDev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The TTS runaway guard is the key win: audioPlayer gains a maxDurationMs ceiling via a setTimeout that's cleared on every terminal event (and guarded by stopped/endedNaturally), and useHumanMascot wires a 5-minute TTS_MAX_PLAYBACK_MS so the mascot can never get stuck in the speaking pose. STT retry UI state is a nice observability add. Good test coverage across audioPlayer / useHumanMascot / MicComposer. (I focused on the playback-safety core; the bulk of the file count is i18n chunks.) Approving.

@oxoxDev

oxoxDev commented May 30, 2026

Copy link
Copy Markdown
Contributor

Tried rebasing onto upstream/main from the maintainer side and hit a structural conflict that needs author-side translation work, not mechanical merge. Documenting so you can pick this up cleanly:

What blocks the rebase

Upstream merged a structural i18n refactor: the chunked app/src/lib/i18n/chunks/<locale>-N.ts layout was retired — each locale is now a single flat file at app/src/lib/i18n/<locale>.ts. en.ts is the source of truth. See the CLAUDE.md i18n section for the new contract.

Your PR's 14 chunk modifications ({ar,bn,de,en,es,fr,hi,id,it,ko,pl,pt,ru,zh-CN}-2.ts) collide as modify/delete: every chunk is deleted in HEAD, modified in your commit. Auto-merge succeeded only for app/src/lib/i18n/en.ts.

What needs to happen

  1. Rebase onto current upstream/main.
  2. Drop every chunks/*-2.ts change (the files don't exist anymore).
  3. Add the 5 new keys you added in en.ts (memory.tab.gameplay, gameplay.spoiler.off, gameplay.spoiler.light, gameplay.spoiler.full, mic.retryingTranscription) to every flat app/src/lib/i18n/<locale>.ts (ar/bn/de/es/fr/hi/id/it/ko/pl/pt/ru/zh-CN).
  4. Use real translations, not English placeholderspnpm i18n:english:check will fail CI on placeholder English values. The strings are short UI labels (Gameplay / Off / Light / Full / Retrying transcription…), so this is small but non-mechanical work.

Code-only conflicts (modify/modify) were minimal — the app/src/features/human/MicComposer* + useHumanMascot* + voice/audioPlayer* files merged cleanly per git status output. The only blocker is the i18n refactor.

Holding off pushing a half-rebase. Once you push the rebased branch with the flat-locale updates, ping me — I'll re-review and re-clear my approval if nothing else shifted.

…olithic locale files

Upstream (tinyhumansai#3004) deleted all i18n chunk files and merged them into
per-locale monolithic files (ar.ts, bn.ts, …). The mic.retryingTranscription
key added by this PR was in the now-deleted *-2.ts chunks; moved it to the
corresponding line (after mic.transcribing) in each of the 13 locale files.
en.ts already contained the key from the auto-merge.
@staimoorulhassan

Copy link
Copy Markdown
Contributor Author

The Playwright E2E Gate passed. The 3 lane failures in skills-registry, gmail-flow, insights-dashboard, accounts-provider-modal, and connector-gmail-composio are not caused by this PR.

These failures started after merging upstream commits that landed after the last green E2E run on main (2026-05-30T07:09 UTC / SHA 51d53263):

  • 35568423 feat(agent): live streaming subagent view with reopenable sub-threads
  • 20ad76d7 feat(workflows): agent workflows

Those commits add new UI features (sub-thread views, workflow task panels) that appear to interfere with existing E2E selectors (tab click blocked by overlay, element timeout). None of this PR's changed files (audioPlayer.ts, MicComposer.tsx, useHumanMascot.ts, i18n locale files) are exercised by the failing specs.

@senamakel senamakel merged commit 301ba10 into tinyhumansai:main May 30, 2026
19 of 22 checks passed
@sanil-23 sanil-23 removed their assignment May 31, 2026
senamakel pushed a commit to senamakel/openhuman that referenced this pull request Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Net-new user-facing capability or product behavior. working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Polish mascot-driven TTS and STT voice experience

5 participants