Skip to content

Wait for Live output before starting interviews - #295

Open
HellHBBD wants to merge 1 commit into
sysprog21:mainfrom
HellHBBD:fix/gemini-live-readiness
Open

HellHBBD wants to merge 1 commit into
sysprog21:mainfrom
HellHBBD:fix/gemini-live-readiness

Conversation

@HellHBBD

@HellHBBD HellHBBD commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Live setup can complete without model output. Use the normal opening generation as one bounded readiness attempt, preserve its events for delivery, and start the interview clock only after the first model output. Startup failures do not retry.

Carry outcomes on the existing interviewer_state message and degraded-start banner. Name provider-specific errors only when the provider identifies them.

Validation

  • ./scripts/test.sh passed after rebasing onto 546f559, with Chromium, commentflow, and actionlint enabled. Startup tests cover setup without output, timeout, a single generation attempt, event preservation, and timer ordering.
  • The offline Rust-server interview check passed with the existing web/audio/test-voice.wav capture fixture.
  • The contributor ran release build and tests successfully at df170a8; the release binary reports that revision.

Browser tests used a temporary Liberation Sans alias; the default-font lobby failure also reproduces on base a3f3e80. Live provider integration was not run.

Closes #159


Summary by cubic

Live setup can finish without model output, so the interview clock can run against a silent provider. This change starts the interview timer and hard deadline only after the first model output—nonempty audio, model text, output transcription, or a tool call—using the opening greeting as a single bounded readiness attempt that never retries. A failed check ends the interview; offline practice that continues stays untimed.

Failure reporting

  • Readiness outcomes publish on the existing interviewer_state control topic with startup: "ready" or startup: "failed" plus a classified reason (quota, billing, credentials, refused, or unavailable), shown in the degraded-start banner.
  • An exhausted key rotation or a cooldown that cannot fit the first-open budget keeps its quota or billing cause instead of degrading into a generic transport timeout.
  • The page sends a bounded startup_received receipt for failed readiness, hashed and published under the same bound as report receipts, and keeps the diagnosis through room teardown.

Socket and client behavior

  • Socket setup keeps its bounded key-failover and restart budget; only readiness generation is a single attempt.
  • Keepalives keep a responsive socket alive while waiting, and a GoAway advisory sent before output replays with its original timestamp and no further generation.
  • Queued opening events are preserved in order for the normal room loop, and the reply-wait clock starts at readiness so replay cannot expire an already answered greeting.
  • Observed usage is accounted once even when readiness publication fails.
  • Editable work before the clock starts and after a failed opening warns on unload until its report persists; early interviewer departure, recording delays, and premature report requests are handled without blocking cleanup or losing context.

Written for commit d89b4a2. Summary will update on new commits.

View guided diff

cubic-dev-ai[bot]

This comment was marked as resolved.

Comment thread src/livekit.rs Outdated
Comment thread src/livekit.rs Outdated
cubic-dev-ai[bot]

This comment was marked as resolved.

jserv

This comment was marked as outdated.

@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 69ca930 to 16e2bce Compare October 14, 2026 02:50
@HellHBBD
HellHBBD requested a review from jserv October 14, 2026 03:07
@jserv
jserv requested review from ColtenOuO and alanhc October 14, 2026 03:08
jserv

This comment was marked as outdated.

@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 16e2bce to 7792ace Compare October 18, 2026 03:53
@HellHBBD
HellHBBD requested a review from jserv October 18, 2026 05:04
jserv

This comment was marked as outdated.

@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch 2 times, most recently from 5fb22be to 763f8e5 Compare October 18, 2026 07:50
@HellHBBD
HellHBBD requested a review from jserv October 18, 2026 08:29
jserv

This comment was marked as outdated.

Comment thread src/livekit.rs
Comment thread src/livekit.rs Outdated
@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 763f8e5 to 4365539 Compare October 21, 2026 11:46

@ColtenOuO ColtenOuO left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes look good to me, before merge this PR, please squash your commit.

@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 4365539 to 63e6cab Compare October 22, 2026 09:50
@HellHBBD
HellHBBD requested review from ColtenOuO and jserv October 22, 2026 10:31
@HellHBBD

Copy link
Copy Markdown
Contributor Author

The browser CI failure also occurs on upstream main at fd7c23c, before the Gemini changes in this PR:
tests/browser/lobby.test.js:6457
problem cards span desktop panels and return home on narrow screens

AssertionError: 57 !== 4

The same test passed at the preceding main commit, 0116b9b, then failed after merging #401 at fd7c23c. This PR's browser job reports the same assertion failure.

Code inspection points to an interaction between #410's layout test and #401's new .problem-entry wrapper. The test temporarily changes .problem-card.hidden and later clears it, but #401's filtering also hides the enclosing .problem-entry. Clearing the button's hidden property therefore does not necessarily make it visible. The subsequent .problem-card:not([hidden]) query still includes buttons inside hidden wrappers; their zero bounding rectangles cause them to be counted as sharing one row.

The layout test needs to account for the wrapper when changing visibility and count cards that are actually rendered. This analysis is based on the CI logs and source comparison; I have not rerun a corrected test yet.

jserv

This comment was marked as outdated.

Comment thread src/livekit.rs Outdated
jserv

This comment was marked as outdated.

Comment thread src/livekit.rs Outdated
@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 63e6cab to 0ef4e22 Compare October 24, 2026 01:28
Comment thread web/interview.js
}
// A deployment with no live session still offers the old timed practice.
// A failed readiness check never turns a joined interview into that clock.
if (!error?.startup && !state.joinedRoom) {

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.

After a failed startup check, connect() returns null, so init() stops at if (!startup) return; and never reaches updateBeforeUnloadGuard(). Meanwhile the banner tells the candidate "You can still work the problem offline." shouldWarnBeforeUnload only checks endsAt, so a candidate who keeps coding offline can close or reload the tab and lose everything without a prompt. Before this PR, a degraded start always set endsAt and turned the guard on. Turn the guard on for this path without starting the clock, for example with a flag passed to shouldWarnBeforeUnload. The same early return also hits any non-startup failure after state.joinedRoom is set (for example, publishIntegrityEvent throwing). If that is intended, the comment should say so, because it currently mentions only readiness.

Comment thread web/interview.js Outdated
Comment thread web/interview.js
language: state.language,
});
}
const startup = await state.interviewerReadiness.wait;

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.

Outside Strict mode, nodes.end and nodes.pause stay enabled during this wait, which now lasts the whole server-side startup check. Pressing End here sends end_interview to an agent that only defers it until readiness, so when the check then fails the candidate is left on the ending overlay waiting for a report the agent will never send, and when it succeeds they get a report for an interview whose clock never started. Keep both buttons disabled until readiness settles, the way init() already does for Strict, or have endInterview skip the report wait while state.interviewerReadiness is still pending.

@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 0ef4e22 to 35e28ce Compare October 24, 2026 06:03
@HellHBBD
HellHBBD requested a review from jserv October 24, 2026 06:22
Comment thread web/interview.js Outdated
Comment thread web/interview.js
Comment on lines +2258 to +2261
if (
participant?.identity === state.agentIdentity ||
(!state.agentIdentity && isAgent(participant))
)

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.

isolate_local_agent removes duplicate agent participants during startup. That runs before this agent publishes its audio track, so the browser has usually not adopted an agent yet. When the duplicate's ParticipantDisconnected lands, !state.agentIdentity && isAgent(participant) holds, so fail() settles readiness as failed. The candidate then gets the "did not answer the startup check" banner just before the real interviewer would have reported ready. Fail only when no interviewer is left in the room.

Suggested change
if (
participant?.identity === state.agentIdentity ||
(!state.agentIdentity && isAgent(participant))
)
if (
isAgent(participant) &&
!roomInterviewer(roomParticipants(), state.agentIdentity)
)

Live setup can succeed without the model answering. Starting the clock
at setup spends candidate time against a silent provider. Require model
output before timing an interview, and retain untimed practice after a
failed opening without retrying that generation.

Readiness must preserve existing setup failover, keepalive, candidate
media and reconnect grace. Retain provider failure reasons and observed
usage so failed starts remain diagnosable without duplicate generation
or accounting.

Protect editable work before the clock starts and after an untimed
failure. Detect early interviewer departures, keep recording delays
from blocking cleanup, and prevent premature report requests. Keep
Strict failure guidance consistent with its disabled controls. Isolate
startup tests from unrelated model loading and rendering so they
measure startup ordering reliably.

Closes sysprog21#159
@HellHBBD
HellHBBD force-pushed the fix/gemini-live-readiness branch from 35e28ce to d89b4a2 Compare October 24, 2026 07:54

@jserv jserv 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.

Before asking someone to review, you MUST resolve any regressions flagged by GitHub Actions.

Comment thread src/livekit.rs
usage: &mut crate::gemini::TokenUsage,
publication: impl std::future::Future<Output = Result<(), Box<dyn std::error::Error + Send + Sync>>>,
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
let result = tokio::time::timeout(RECONNECT_NOTICE_LIMIT, publication)

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.

RECONNECT_NOTICE_LIMIT is one second, sized for a reconnect notice that retry_cold_open logs and drops on timeout. Here the same bound makes the ready notice fatal: when publish_data takes longer than a second on a congested data channel, an interview whose greeting Gemini already generated (and billed) is torn down, and the failure notice that follows carries unavailable, which the page words as "Gemini did not answer the startup check". Give the ready notice its own bound that fits inside the browser's readiness wait, and do not report a publication failure as a provider failure.

interviewerReadiness: gate,
};
let updates = 0;
const handler = runInNewContext(`(${handlerSource})`, {

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.

The handler now calls roomInterviewer and roomParticipants, but this context still passes only state, isAgent and updateAgentState, so this test and the next one throw ReferenceError: roomInterviewer is not defined in the browser gate. Pass roomInterviewer and a roomParticipants stub that returns who is still in the room, and rewrite the cases for the new rule: an agent's departure fails readiness only when no interviewer remains, so another agent leaving while the pinned one stays settles nothing, and the pinned one leaving fails it only when no other agent is left.

}

async function assertPendingControls(page) {
assert.equal(await page.locator("#end").isDisabled(), true);

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.

init now leaves End enabled outside Strict mode during the readiness wait, so this helper fails both practice-mode tests that call it (false !== true in the browser gate), and its click on End would start the ending flow rather than do nothing. Assert End disabled and inert only when the page is in Strict mode, and for practice mode assert what pressing End before readiness is meant to do, so the test pins the behavior the open thread on End during the startup wait settles on.

@jserv jserv 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.

Rebase latest main branch and resolve conflicts.

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.

Check Gemini API readiness before starting an interview and show quota/billing errors

3 participants