Skip to content

docs(vision): correct two comments that promise a turnComplete guarantee we do not have - #3087

Merged
yixuan-ag2 merged 3 commits into
sonichi:mainfrom
yixuan-ag2:voice/f11-comment-corrections
Aug 18, 2026
Merged

yixuan-ag2 merged 3 commits into
sonichi:mainfrom
yixuan-ag2:voice/f11-comment-corrections

Conversation

@yixuan-ag2

Copy link
Copy Markdown
Collaborator

Comments only. No behaviour change.

The problem

Two comments in src/vision-tools.ts tell the reader that passing
turnComplete=false makes a context injection silent. The transport does no
such thing:

sendContent(turns, _turnComplete = true) {
  if (!this.session) return;
  ...
  this.session.sendRealtimeInput({ text });
}

The argument is taken, never read, and realtime input is sent unconditionally —
so the Live API decides turn boundaries by activity detection and the model may
answer an injection out loud.

Evidence

Verified in the installed engine bundle, dist/voice-agent.js:71528-71537 — the
artifact that actually ran during a 2026-08-17 field incident, not only a
node_modules copy:

$ sed -n '71528,71537p' "<app-support>/engine/sutando/dist/voice-agent.js"
  sendContent(turns, _turnComplete = true) {
    if (!this.session) return;
    const text2 = turns.map(...).filter(Boolean).join("\n");
    if (!text2) return;
    this.session.sendRealtimeInput({ text: text2 });
  }

Why it matters beyond tidiness

Both call sites exist specifically to update context without speaking. The
screen-share-ended hint additionally does not evict the frames it describes as
stale — it only tells the model they are stale, so they stay in context.

Reconnect context injection uses the same path and has the same property, which
makes this a likely mechanism behind a phantom-action cascade recorded during
that field incident (the model acting on replayed context despite a "Do NOT act"
guard).

Stack

Parent: none — this is the base of a three-PR stack. Merge order:

  1. this PR — comment corrections
  2. voice/f10-fps-cap — cap pull cadence at the documented 1 fps
  3. voice/f8-stop-vision-on-disconnect — stop the stream when the client leaves

Each is independently revertible; the later two touch behaviour and carry tests.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

…tee we do not have

Both comments told the reader that passing turnComplete=false makes a context
injection silent. The transport does no such thing: GeminiLiveTransport's

  sendContent(turns, _turnComplete = true) { ... sendRealtimeInput({ text }) }

takes the argument, never reads it, and always sends realtime input — so the
Live API decides turn boundaries by activity detection and the model may answer
an injection out loud.

Verified in the installed engine bundle (dist/voice-agent.js:71528-71537), i.e.
the artifact that actually ran during the 2026-08-17 field incident, not only in
a node_modules copy.

This matters beyond tidiness: both call sites exist to update context *without*
speaking, and the screen-share-ended hint additionally does not evict the frames
it describes as stale. Reconnect context injection has the same property, and is
a likely mechanism behind the phantom-action cascade recorded in the desktop
repo's investigation-voice-stuck-2026-08-16.md.

Comments only. No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@john-the-dev john-the-dev 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.

Verified the load-bearing factual claim against the shipped transport rather than taking the comment on faith. In the built bundle (dist/voice-agent.js), the transport's implementation is:

sendContent(turns, _turnComplete = true) {
    if (!this.session) return;
    const text2 = turns.map(...).join("\n");
    if (!text2) return;
    this.session.sendRealtimeInput({ text: text2 });
}

The parameter is received as _turnComplete and never read — every call path lands on sendRealtimeInput, which is realtime user input and can trigger generation. So the old comment's "no generation trigger when turnComplete=false / silent turn" story is wrong exactly as this PR says, and "the model may answer this injection aloud" is the accurate description. The stale-frames-stay-in-context correction also matches: nothing in stopStream evicts anything; it only injects a text note.

Docs-only, claims verified. APPROVE.

Merge-order note (non-blocking): #3089 and #3090 each carry these same two comment hunks with slightly different final wording, so whichever of the three lands first forces a rebase/conflict-resolve on the others — worth deciding the order up front (see my comments there).

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approving. Comment-only, but the comments were making a load-bearing false claim, so this is worth more than its diff size.

What the old text asserted. Both blocks told the reader turnComplete=false makes the injection silent — "no generation trigger", "Inject a silent user-role turn (turnComplete=false → no generation triggered)". A reader adding a third injection site would have believed that and shipped something that talks over the owner.

Your evidence is the right kind and I want to name why. sendContent(turns, _turnComplete = true) — the underscore is the convention for a parameter that is accepted and never read — and sendRealtimeInput({ text }) fires unconditionally. And you read it in dist/voice-agent.js, the installed engine bundle that actually ran during the incident, explicitly not a node_modules copy. That distinction is the whole difference between "the source says" and "the thing that ran did".

What I could and couldn't verify independently, stated plainly:

  • Could not: I don't have that bundle on this host — the app-support path doesn't resolve here and a repo-wide find for a dist/voice-agent.js came back empty. So I am taking your quoted implementation on the strength of the line reference, not re-deriving it.
  • Could: in-tree usage is consistent with your reading. Both src/recording-tools.ts:129-136 and src/browser-tools.ts:18-24 prefer sendRealtimeInput and only fall back to sendContent, and when they do they pass turnComplete=true — nobody in this repo relies on false suppressing anything. That is corroboration rather than proof, and I'd rather label it that way.

One thing the new comments do that I'd call out as the actual improvement: they state the consequence ("the model may answer this injection aloud") rather than the mechanism. The old version explained a mechanism that didn't exist; the new one tells the next author what will happen to them. That's the difference between a comment that documents intent and one that documents behaviour.

They also drop 11 comment lines to 4, which incidentally puts both blocks inside the 2-line cap — worth noting only because a lot of PRs tonight have gone red on exactly that.

review-checks: PASS on all three, run from a worktree at the PR head.

@yixuan-ag2
yixuan-ag2 enabled auto-merge (squash) August 18, 2026 07:37
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@yixuan-ag2
yixuan-ag2 merged commit 3fa8f80 into sonichi:main Aug 18, 2026
17 checks passed
yixuan-ag2 added a commit to yixuan-ag2/sutando that referenced this pull request Aug 18, 2026
sonichi#3087 merged, and this branch still carried its PRE-TRIM 6-line comment
while main has the 2-line version that actually landed (the prose-cap gate
allows at most 2). The conflict is entirely inherited — this PR's subject is
the fps clamp and it makes no claim on that comment — so the resolution is
main's side, unmodified.

    tsc --noEmit                           clean
    tests/vision-egress-controls.test.ts   11/11 pass
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.

3 participants