Skip to content

fix(onboarding): notify user when voice intro falls back due to#233

Open
maciej-konczal wants to merge 3 commits into
HamedMP:mainfrom
maciej-konczal:fix/onboarding-gemini-hint
Open

fix(onboarding): notify user when voice intro falls back due to#233
maciej-konczal wants to merge 3 commits into
HamedMP:mainfrom
maciej-konczal:fix/onboarding-gemini-hint

Conversation

@maciej-konczal
Copy link
Copy Markdown

Gemini live does not on onboarding without Gemini API Key

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

@maciej-konczal is attempting to deploy a commit to the Finna Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR surfaces a user-visible notice when Gemini Live is unavailable or fails during onboarding, replacing the previous silent fallback to text mode. The gateway now sends a typed notice message on three paths (no API key, connection failure, mid-session disconnect), the shell hook tracks and exposes it, and the OnboardingScreen renders it as a dismissible informational banner.

  • types.ts: New notice discriminated-union variant with a code: "gemini_unavailable" enum and a freeform message string.
  • ws-handler.ts: sendGeminiVoiceNotice helper added; notices are emitted before each mode_change: text fallback; notices and errors are cleared at the start of each new session.
  • Tests: All three fallback paths and the no-notice text-mode path are covered by new unit tests; existing mocks are refactored into shared helpers.

Confidence Score: 5/5

Safe to merge — the change is additive (new message type, new state field, new banner) with no modifications to existing data paths or auth flows.

All three fallback paths are covered by tests, the schema extension is backwards-compatible, and the notice/error state is correctly cleared on retry. The only finding is that the "could not connect" copy is reused for mid-session disconnects where the voice channel had already been established, which is a UX wording inaccuracy rather than a functional defect.

The two-line wording change in ws-handler.ts (separate constant for mid-session disconnect vs. initial connection failure) is the only thing worth a second look before merge.

Important Files Changed

Filename Overview
packages/gateway/src/onboarding/types.ts Adds notice discriminated-union variant with code limited to gemini_unavailable and a freeform message string; well-structured schema addition.
packages/gateway/src/onboarding/ws-handler.ts Adds notice helpers and sends gemini_unavailable notice on three fallback paths; the disconnect-path reuses the "could not connect" message even for mid-session drops.
shell/src/hooks/useOnboarding.ts Adds notice state, handles the new notice message type, and clears both notice and error when start() is called — the previous stale-notice gap is now resolved.
shell/src/components/OnboardingScreen.tsx Renders the informational notice banner at the bottom of the screen, conditionally hidden when an error is already shown; mutually exclusive display avoids z-index overlap.
tests/gateway/onboarding/ws-handler.test.ts Adds tests for all three new fallback paths (no key, connect failure, mid-session disconnect) plus a negative case confirming text-mode start produces no notice; existing tests refactored to use a shared mock helper.
.env.example Expands the Gemini key comment to document both features it enables and directs users to the API key page.

Sequence Diagram

sequenceDiagram
    participant Shell
    participant Gateway as ws-handler
    participant Gemini as Gemini Live

    Shell->>Gateway: "start { audioFormat: "pcm16" }"

    alt No GEMINI_API_KEY
        Gateway-->>Shell: "notice { code: "gemini_unavailable" }"
        Gateway-->>Shell: "mode_change { mode: "text" }"
    else Key present — connect attempt
        Gateway->>Gemini: connect()
        alt connect() throws
            Gemini-->>Gateway: Error
            Gateway-->>Shell: "notice { code: "gemini_unavailable", "could not connect" }"
            Gateway-->>Shell: "mode_change { mode: "text" }"
        else Connected, then mid-session drop
            Gemini-->>Gateway: disconnected event
            Gateway-->>Shell: "notice { code: "gemini_unavailable", "could not connect" }"
            Gateway-->>Shell: "mode_change { mode: "text" }"
        else Connected successfully
            Gateway-->>Shell: voice onboarding proceeds normally
        end
    end
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/gateway/src/onboarding/ws-handler.ts:42-43
The `GEMINI_VOICE_CONNECT_FAILED_NOTICE` constant ("Voice intro could not connect…") is reused for mid-session disconnections, where the voice channel was already established. A user who heard several seconds of the voice intro and then lost the connection would see "could not connect", which is misleading. Keeping separate messages for the two paths makes the notification accurate for both.

```suggestion
const GEMINI_VOICE_CONNECT_FAILED_NOTICE =
  "Voice intro could not connect — continue in text below, or try again in a moment.";
const GEMINI_VOICE_DISCONNECTED_NOTICE =
  "Voice intro disconnected — continuing in text below.";
```

### Issue 2 of 2
packages/gateway/src/onboarding/ws-handler.ts:235-240
With the separate constant above, the `disconnected` handler should use `GEMINI_VOICE_DISCONNECTED_NOTICE` so the user sees an accurate message when the session drops mid-flow rather than during initial connection.

```suggestion
    client.on("disconnected", () => {
      if (gemini !== client) return;
      if (sm.current !== "done") {
        if (audioMode) {
          sendGeminiVoiceNotice(send, GEMINI_VOICE_DISCONNECTED_NOTICE);
        }
```

Reviews (3): Last reviewed commit: "fix(onboarding): gemini setup updates" | Re-trigger Greptile

Comment on lines +336 to +338
case "notice":
setNotice(msg.message as string);
break;
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.

P2 Stale notice persists across reconnect/restart

notice is set on receipt but never cleared. If handleStart is called a second time (e.g., the user retries with a valid key), the old gemini_unavailable banner will remain visible until a new notice arrives or the component unmounts. error has the same pattern, but a stale "voice unavailable" message while voice is actually working would be misleading. Clearing notice (and error) at the start of a new session would match the intent.

Prompt To Fix With AI
This is a comment left during a code review.
Path: shell/src/hooks/useOnboarding.ts
Line: 336-338

Comment:
**Stale notice persists across reconnect/restart**

`notice` is set on receipt but never cleared. If `handleStart` is called a second time (e.g., the user retries with a valid key), the old `gemini_unavailable` banner will remain visible until a new `notice` arrives or the component unmounts. `error` has the same pattern, but a stale "voice unavailable" message while voice is actually working would be misleading. Clearing `notice` (and `error`) at the start of a new session would match the intent.

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant