fix(llm): enhance fallback logic to handle empty completions and impr…

[Abhiiishek44]

…ove metadata handling

Summary

Updates stream_llm_with_fallback() so empty or metadata-only streams correctly move to the next fallback model.

A candidate is now accepted only after it produces text, reasoning, or a completed tool call. Buffered metadata is discarded when switching models.

Added regression tests for [DONE]-only streams, metadata-only streams, valid output, tool calls, and cases where all candidates return empty responses.

Target branch

• This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release. If your PR is on main by accident, click "Edit" on this PR and change the base.

Linked Issue

Fixes #5489

Type of Change

• Bug fix (non-breaking — fixes a confirmed issue)
• New feature (non-breaking — adds new behaviour)
• Breaking change (changes or removes existing behaviour)
• Refactor / cleanup (behaviour unchanged)
• Documentation only
• CI / tooling / configuration

Checklist

• I searched open issues and open PRs — this is not a duplicate.
• This PR targets dev
• My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
• I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.

How to Test

1. Run the focused fallback tests:

   pytest -q tests/test_llm_core_fallback.py

2. Run the broader LLM core test suite:

   pytest -q tests/test_llm_core*.py

3. Verify the following cases:

   • A primary stream containing only [DONE] invokes the fallback.
   • Usage metadata followed by [DONE] invokes the fallback and discards the primary metadata.
   • Text, reasoning, whitespace deltas, and completed tool calls prevent switching.
   • If every candidate returns no substantive output, one clear terminal error is emitted.

Visual / UI changes — REQUIRED if you touched anything that renders

Anything that changes what the UI looks like — buttons, icons, padding, colors, fonts, spacing, layout, CSS, HTML, SVG, or any static/js/ module that draws to the DOM — needs all of the following. PRs that change rendering without these WILL be closed.

• Screenshot or short clip of the change in the running app, attached below. Mobile screenshot too if the change affects mobile.
• Style match: the change uses Odysseus's existing visual language. Specifically:
  • Reuse existing CSS variables (--red, --fg, --bg, --card, --border, etc.) — do not introduce new color values, font sizes, or spacing units.
  • Reuse existing button/input/card/border classes. Don't invent parallel styling.
  • No Unicode emoji in UI or code. Use inline SVG (matching the monochrome icon style already in static/index.html) or plain text.
  • Monospaced font (Fira Code) for primary UI text. Don't override.
  • Dark theme is the default; any light-mode work must be wired through the existing theme system, not hard-coded.
• No new component patterns. If a similar widget already exists in the app, extend it instead of writing a parallel one.
• I am not an LLM agent submitting a bulk PR. If you are, please open an issue describing the problem first — bulk auto-generated PRs that don't match the project's visual style are closed on sight, even when the underlying fix is correct.

Screenshots / clips

[RonitSachdev]

I ran into this same bug and verified this patch's approach end-to-end against a live SSE endpoint: a [DONE]-only primary and a metadata-only (usage + [DONE]) primary both correctly fall through to the backup, and a healthy primary is unaffected. The core approach here looks right — metadata no longer commits a candidate, a failed candidate's usage/model_actual chunks don't leak into the fallback's response, and surfacing a terminal error when every candidate comes back empty is a nice improvement over a silent blank.

One thing I noticed reviewing the diff:

tool_call_delta chunks are classified as metadata and get buffered. The substantive check only accepts non-empty text deltas and completed tool_calls:

substantive = (
    isinstance(delta, str) and bool(delta)
) or (
    event_data.get("type") == "tool_calls"
    and bool(event_data.get("calls"))
)

But stream_llm also emits {"type": "tool_call_delta", ...} chunks — the live-streamed tool arguments for create_document / update_document / edit_document (the arg_delta yields in stream_llm). With this patch, a response that is purely a native doc-tool call buffers every arg_delta in pending_metadata until the final tool_calls event arrives, so document content stops streaming live in the UI and instead lands in one burst at the end of the response.

Suggestion — count streamed tool arguments as substantive output too:

or event_data.get("type") in ("tool_call_delta", "tool_calls")

Pre-patch, tool_call_delta chunks were always passed straight through to the client, so treating them as substantive preserves the existing live-streaming behavior (and once a chunk has reached the client, switching candidates would risk duplicated output anyway, which is the same reason text deltas commit the candidate).

[Abhiiishek44]

@RonitSachdev You were right about tool_call_delta. I verified the behavior, and those chunks were being buffered until the final tool_calls event, which caused document tool output to arrive in one burst instead of streaming progressively.

I’ve updated the fallback logic to treat tool_call_delta as substantive output. The candidate is now committed as soon as streamed tool arguments begin, and the chunks are forwarded immediately, preserving the existing live-streaming behavior.