Skip to content

feat: fdb image - #2373

Merged
halajohn merged 8 commits into
mainfrom
feat/fdb-image
Oct 27, 2026
Merged

halajohn merged 8 commits into
mainfrom
feat/fdb-image

Conversation

@FaithfulinLOVE

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Review

1. [Blocker] close_stream close is treated as an unexpected disconnect (ASR guide §5.3 / §8.3 MUST)
deepgram_asr_python/recognition.py:332, deepgram_asr_python/extension.py:583

CloseStream makes Deepgram drain and then close the socket. Nothing marks that close as expected, so on_close reports disconnected, logs "closed unexpectedly", and goes through _handle_reconnect() (0.5s+ backoff) after every finalize. Two things follow from that:

  • Audio sent after CloseStream but before the server closes reaches a stream that is already ending, so Deepgram drops it. ASRBufferConfigModeKeep does not help because is_connected() still returns true and the frame gets "sent".
  • If a frame lands while the socket is closing, send_audio_frame raises ConnectionClosed, which calls on_error, which starts a second reconnect next to the one from on_close. §9.1 prohibits that.

The guide's "Close connection" mode needs an expected-close flag. With that flag set, on_close should finish the finalize and reconnect for the next turn without going through the unexpected-failure path. While the close is pending, it should also gate or buffer audio.

2. [Blocker] close_stream finalize has no bounded completion path (§8 MUST)
deepgram_asr_python/extension.py:425-430, agora_mind_bridge_python/extension.py:449-456

In close_stream mode, the only thing that sends asr_finalize_end is a Deepgram Metadata frame. Several cases never produce one:

  • recognition.stop() returns early when the socket is not connected ("Recognition not started").
  • The socket drops before Metadata arrives.
  • The send fails.

In all of these, asr_finalize_end never arrives. The bridge then keeps _finalize_id set forever, which means:

  • every later speech_final gets buffered and never submitted (or self._finalize_id is not None), and
  • every later finalize_audio returns "Audio finalize is pending".

The session stays stuck until it is reset. To fix it, add a finalize timeout in Deepgram, and also call _finalize_end() in on_close and in the disconnected path. A bridge-side timeout that flushes _pending_asr_text and clears _finalize_id would also help.

3. Duplicate finals without timing are concatenated into the input
agora_mind_bridge_python/extension.py:484-496

Before this PR, a repeated final with no usable start_ms/duration_ms (asr_final_key returns None) was dropped by comparing it against _last_final_text. Now it is appended to _pending_asr_text first, and only the joined text is compared later. Two identical finals for "Book the flight" therefore become "Book the flight Book the flight". Dedupe against the last buffered fragment when key is None, and add a test that sends a duplicate final without timing.

4. Test coverage gaps in deepgram_asr_python
deepgram_asr_python/tests/

This PR adds a vendor finalize mode, routes Metadata into on_result, and moves _finalize_end() after send_asr_result for the other modes. None of that has standalone extension tests. The only coverage is the FDB runtime fake. Per §13, add:

  • a finalize test that sends CloseStream, gets Metadata, and checks that asr_finalize_end echoes the same finalize_id and comes after the final result,
  • the disconnected, no-Metadata and timeout cases,
  • a test that the expected close after CloseStream does not count as an unexpected reconnect.

ASR checklist

Item Result
Lifecycle N/A (no lifecycle hook changes)
Connection state Fail: the expected close after CloseStream reports disconnected as a failure and reconnects through the unexpected path (item 1)
Buffering Fail: audio sent after CloseStream is lost (item 1)
Finalize Fail: no bounded completion for close_stream (item 2). Moving finalize_end after the result in the other modes is fine.
Reconnect Fail: two reconnect chains can start (error + close) (item 1)
Result shape Pass (the Metadata frame is consumed, no empty result is emitted)
Metrics N/A
Tests Fail: no Deepgram standalone tests for the new mode (item 4)

@github-actions

Copy link
Copy Markdown
  1. [P1] Drain queued PCM before sending the terminal CloseStream message — recognition.py:334–336.

    The base class's on_audio_frame() only enqueues frames for a separate consumer, while asr_finalize calls finalize() directly. With burst file uploads or a slow audio send/dumper, this path can send CloseStream while earlier PCM is still queued. Deepgram then acknowledges only the audio already sent, and the bridge submits a truncated transcript; remaining frames cannot be processed on that stream. Put finalization behind the audio queue's drain boundary and prevent further sends to the closing client. Add a regression that delays audio sends and requests finalization immediately after uploading several frames. The new runtime test waits for ASR results before finalizing, which avoids this race. This violates the ASR guide §6.2 MUST serialize send/finalize operations and is a merge blocker.

  2. [P1] Add a bounded completion path when Metadata cannot arrive — extension.py:428–430.

    In close_stream mode, final Results no longer complete the handshake, so this Metadata callback is the only acknowledgement path. If finalization happens while disconnected, recognition.stop() simply returns; if the connection drops after Results but before Metadata, on_close() only reconnects. Neither path completes the request, and there is no timeout. The bridge retains _finalize_id, rejects subsequent finalize_audio commands, and suppresses submission even for later natural speech boundaries. Track a finalize context with exactly one completion on disconnect, send failure, or timeout, and cover these cases in tests. This violates the ASR guide §8.1 MUST provide bounded, exactly-once completion and is a merge blocker.

  3. [P2] Flush buffered next-turn audio when the post-finalize connection opens — property.json:185.

    Enabling close_stream makes every explicit finalization close the provider connection and enter the existing reconnect delay (at least 0.5 seconds). Audio arriving during that gap goes into the base class's buffered_frames, which is flushed only when another audio frame arrives after reconnection. Deepgram's on_open() does not flush it. If the next finite clip arrives entirely during recovery, it remains unsent indefinitely; even a finalize sent after reconnection closes an empty stream. Flush retained frames from the ready callback, preserving each frame's metadata, and add a two-cycle test with the second clip arriving during reconnect and no subsequent audio. This violates the ASR guide §6.3 MUST flush buffered audio without requiring another frame and is a merge blocker.

ASR checklist (changed paths):

Area Result
Lifecycle Fail — next-turn delivery after finalize (#3)
Connection state Fail — closing stream remains available to audio sends (#1)
Buffering Fail — queued/retained PCM can be stranded (#1, #3)
Finalize Fail — no bounded failure completion (#2)
Reconnect Fail — recovery does not flush finite next-turn input (#3)
Result shape Pass by inspection — schema preserved; result precedes completion
Metrics N/A — no dedicated metrics changes
Tests Fail for coverage — missing queued-send, failure/timeout, and repeated close-stream-cycle regressions

Validation: source review against the checked-out ASR guide and the documented ten_ai_base 0.7.62 implementation; git diff --check passed. PR-head scripts/tests and dependency installation were not run, as requested.

@halajohn
halajohn merged commit d093e46 into main Oct 27, 2026
6 of 33 checks passed
@halajohn
halajohn deleted the feat/fdb-image branch October 27, 2026 16:32
@github-actions

Copy link
Copy Markdown

Review

1. close_stream finalize has no bounded completion path (merge blocker, ASR guide §8.1 MUST)
deepgram_asr_python/extension.py (on_result Metadata branch and _handle_asr_result): in close_stream mode, the only thing that emits asr_finalize_end is the provider's Metadata frame. Several common paths never produce that frame:

  • recognition.stop() returns early when not self.is_connected() (for example during a reconnect backoff, or after the previous CloseStream closed the socket and the reconnect hasn't finished). Nothing is sent, and last_finalize_timestamp stays set.
  • The socket drops (ConnectionClosedError, network error) after CloseStream is sent but before Metadata arrives. on_close runs and reconnects, but finalize never completes.
  • ws.send(CloseStream) fails.

The guide requires that "disconnected state … timeout, or finalize send failure MUST still have a bounded completion path." The bridge makes this worse. agora_mind_bridge_python/extension.py keeps self._finalize_id set until a matching asr_finalize_end arrives, and it has no timeout. Once that event is lost:

  • every later finalize_audio is rejected with Audio finalize is pending
  • _handle_asr_result returns early on self._finalize_id is not None, so natural speech_final turns are buffered forever and never submitted until the session is reset

Suggested fix:

  • In Deepgram, call _finalize_end() when stop() is a no-op, when the send fails, and in on_close if a close_stream finalize is still outstanding (last_finalize_timestamp != 0).
  • Add a finalize timeout as a backstop.
  • In the bridge, add a timeout on _finalize_id that flushes pending text and clears the ID, so the bridge can't wedge.

2. Expected CloseStream close is treated as an unexpected disconnect
In extension.py, after close_stream finalize the vendor closes the socket normally. on_close then logs "closed unexpectedly", emits on_disconnected, and reconnects through backoff on every file-end finalize. The guide (§9.2) says an expected close MUST NOT be handled like a failure. Please do one of these:

  • record that a close is expected after CloseStream, and reconnect immediately or lazily without the unexpected-close path
  • document why the reconnect is intended and confirm that audio sent during the backoff window is kept by the Keep buffer

3. No Deepgram unit tests for the new mode
deepgram_asr_python/tests/ has no coverage for:

  • finalize_mode: close_stream sending CloseStream
  • Metadata triggering exactly one asr_finalize_end with the same finalize_id
  • Metadata being ignored in other modes
  • the change that moves _finalize_end() after send_asr_result for flush_api / mute_pkg

The new example runtime test uses a mock server that always sends Metadata, so it can't catch issue 1. Please add tests for the disconnected and no-Metadata cases.

4. finalize_audio acknowledges without checking delivery
agora_mind_bridge_python/extension.py on_cmd: the result of send_data(asr_finalize) is ignored, so the bridge returns audio.finalize.accepted and sets _finalize_id even when routing fails. With issue 1 present, that permanently blocks the session. Check the send error, and clear _finalize_id and return an error if the send failed.

ASR checklist (deepgram_asr_python)

Item Result
Lifecycle N/A
Connection state Fail (expected close after CloseStream goes through the unexpected-close + reconnect path, issue 2)
Buffering Pass (unchanged Keep buffer; please confirm it covers the reconnect gap)
Finalize Fail (no bounded completion when Metadata never arrives, issue 1). Putting the final result before finalize_end is an improvement
Reconnect Fail (a reconnect on every file finalize, issue 2)
Result shape Pass (speech_final stays under metadata.asr_info)
Metrics Pass (finalize latency log unchanged)
Tests Fail (no extension-level tests for close_stream / Metadata, issue 3)

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.

2 participants