Skip to content

[codex] fix TUI fallback providers for dflash empty responses - #36056

Closed
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:fix/tui-fallback-chain-empty-dflash
Closed

[codex] fix TUI fallback providers for dflash empty responses#36056
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:fix/tui-fallback-chain-empty-dflash

Conversation

@OmarB97

@OmarB97 OmarB97 commented May 31, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Load fallback_providers / legacy fallback_model config in TUI gateway session creation.
  • Pass the resolved fallback chain into AIAgent, matching oneshot CLI behavior.
  • Preserve the full fallback chain for TUI background turns instead of only carrying _fallback_model.
  • Add regression coverage for TUI session construction and background agent kwargs.

Root cause

TUI sessions ignored the configured fallback chain when constructing AIAgent. In local taro/dflash sessions, dflash can occasionally return an empty post-tool response. Hermes then retried the same context but had no fallback available in TUI, despite fallbacks being present in config, so the user saw a terminal No reply result.

Impact

TUI behavior now matches oneshot CLI: if the primary model repeatedly returns empty output, the configured fallback chain can activate instead of ending the turn with no recovery path.

Validation

  • python3 -m pytest tests/test_tui_gateway_server.py -q -k "make_agent or background_agent_kwargs"
  • python3 -m pytest tests/run_agent/test_run_agent.py -q -k empty_response
  • python3 -m pytest tests/run_agent/test_provider_fallback.py tests/cli/test_cli_init.py -q
  • Live taro import canary confirmed _make_agent() passes the configured fallback chain.
  • Live hermes-phone tmux session was restarted after deploying the patched gateway file to the installed taro checkout.

Notes

The full local tests/test_tui_gateway_server.py -q run had one unrelated environment-dependent browser-launch expectation failure on this Mac; the fallback-focused tests passed.

Fork mirror: OmarB97#47

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels May 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #28753 (bug report — TUI sessions don't propagate fallback_model/fallback_providers to AIAgent) and PR #18310 (open, same fix). This PR supersedes #18310 with additional dflash empty-response handling and background-agent kwargs coverage.

@OmarB97

OmarB97 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Review Summary: APPROVE

Focused, correct fix for the TUI dflash empty-response regression. The change is minimal (3 lines of production code + 2 targeted tests) and addresses the root cause: background agents in the TUI gateway were receiving only the first fallback provider instead of the full chain.

What the PR does

tui_gateway/server.py — 2 changes:

  1. _make_agent: Now calls get_fallback_chain(cfg) and passes the full list as fallback_model. Previously, the config-level fallback chain was never wired into the agent constructor.
  2. _background_agent_kwargs: Prefers agent._fallback_chain (full list) over agent._fallback_model (single dict legacy attr). Falls back to _fallback_model when the chain is empty or absent.

tests/test_tui_gateway_server.py — 2 new tests:

  • test_make_agent_passes_fallback_chain_from_config: Verifies the config → _make_agent → AIAgent path passes the full chain.
  • test_background_agent_kwargs_preserves_full_fallback_chain: Verifies the agent → _background_agent_kwargs → runner path prefers the chain.

Correctness analysis

Verified against init_agent (agent/agent_init.py:885-897): the fallback_model parameter already supports both list and dict — it filters valid entries and sets _fallback_chain. The PR leverages this existing capability correctly.

Edge cases handled:

  • Empty chain ([]) → falsy → falls back to _fallback_model or None
  • None chain → or []list([])[] → falsy → same fallback ✓
  • list() creates shallow copy to prevent mutation ✓

Verification

  • Both new tests pass ✓
  • Existing adjacent tests (test_make_agent_handles_null_agent_config, test_background_agent_kwargs_handles_null_agent_config) still pass ✓
  • No other callers of _background_agent_kwargs or _make_agent affected ✓

Non-blocking suggestions

  1. Type hint: fallback_model: Dict[str, Any] in both AIAgent.__init__ and init_agent should be Dict[str, Any] | List[Dict[str, Any]] | None. Pre-existing inaccuracy — init_agent already handles lists, but the hint hasn't been updated. Consider fixing in a follow-up.

  2. Test coverage: No test for the case where _fallback_chain is explicitly set to [] (empty list) and _fallback_model is also None — the result would be None. This is correct behavior but worth a dedicated test assertion for clarity.

@OmarB97

OmarB97 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Obsolete: upstream 4b073d0 independently implemented the identical TUI fallback-provider fix with strictly more coverage (_load_fallback_model + _agent_fallback_model incl. the empty-chain nuance, 102 test lines). Nothing here is lost on main. Closing.

@OmarB97 OmarB97 closed this Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants