Skip to content

fix(chat): guard final_response so empty runs fail cleanly, not KeyError - #251

Merged
OmarB97 merged 2 commits into
mainfrom
fix/chat-final-response-guard-20260629
Jun 29, 2026
Merged

OmarB97 merged 2 commits into
mainfrom
fix/chat-final-response-guard-20260629

Conversation

@OmarB97

@OmarB97 OmarB97 commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Why

AIAgent.chat() (run_agent.py) did return result["final_response"]. run_conversation (forwarded to agent/conversation_loop.py) has several early-return / error paths — interrupted, retries exhausted, policy/billing bail — that return a dict without final_response. When a local model never commits a final turn, chat() crashed with KeyError: 'final_response'.

In MeshBoard's dispatch path the worker runs hermes -z "/goal ...", whose goal loop calls chat(). The KeyError traceback surfaced as harness/hermes_exited_with_stderr and was counted as a 0%-success harness fault in the autonomy metric — but the real condition is simply "no final response produced," which the codebase already treats as a clean failed run elsewhere (hermes -z: no final response was produced; treating the run as failed.).

What changed

  • run_agent.py chat(): return result.get("final_response") or "" — honors the documented -> str contract; a run that produced no final turn returns "" instead of raising.
  • run_agent.py __main__ summary: guard the same result['final_response'] read.
  • tests/test_chat_final_response_guard.py: new.

How to test

uv run pytest tests/test_chat_final_response_guard.py -q

Evidence

$ uv run pytest tests/test_chat_final_response_guard.py -q
...                                                                      [100%]
3 passed in 1.35s

$ python3 -m py_compile run_agent.py   # clean

The three cases: final_response key missing → ""; key present but None""; key present with text → that text.

Risks / gaps

  • Behavior change only on the previously-crashing path: a missing/None final_response now returns "" rather than raising. Callers already treat a falsy final response as "no response" (the __main__ summary only prints when truthy), so this aligns with existing handling. Accepted.
  • Does not change why a local model sometimes produces no final turn (model/transport quality on the local-AI lanes) — that is tracked separately on the MeshBoard side; this PR only stops the missing-key crash from being misclassified as a harness fault. Out of scope here.
  • Full end-to-end hermes -z dispatch was not re-run live (requires a local model lane); the unit test exercises the exact chat() contract that crashed. Reasonable given the change is a one-line defensive guard.

AIAgent.chat() did `return result["final_response"]`. run_conversation has
several early-return / error paths (interrupted, retries exhausted, policy or
billing bail) that omit "final_response", so when a local model never commits
a final turn, chat() crashed with `KeyError: 'final_response'`.

In the MeshBoard dispatch path (`hermes -z "/goal ..."` goal loop), that
traceback surfaced as harness/hermes_exited_with_stderr and was counted as a
0%-success harness fault — when the real condition is just "no final response
produced", which the codebase already treats as a clean failed run elsewhere.

Honor the documented `-> str` contract: `return result.get("final_response")
or ""`. Also guard the __main__ summary print. Empty string flows upstream as
a normal failed run instead of a Python crash, so the autonomy metric classes
it correctly (worker produced nothing) rather than as a launcher/harness bug.

Tests: tests/test_chat_final_response_guard.py — chat() returns "" when the
key is missing or None, and the value when present.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🔎 Lint report: fix/chat-final-response-guard-20260629 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10906 on HEAD, 10904 on base (🆕 +2)

🆕 New issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:3004: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

✅ Fixed issues (1):

Rule Count
invalid-assignment 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5708 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

The --environment jsdom flag in the npm script was not being honored by
vitest v4. The window and requestAnimationFrame globals were not being
defined, causing unhandled ReferenceError exceptions in tests that
import DOM-dependent libraries like react-dom and use-stick-to-bottom.

Add a test.environment: jsdom block to vite.config.ts so vitest picks
up the jsdom environment correctly for all tests in apps/desktop.
@OmarB97
OmarB97 merged commit 4510e26 into main Jun 29, 2026
36 checks passed
@OmarB97
OmarB97 deleted the fix/chat-final-response-guard-20260629 branch June 29, 2026 17:11
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.

1 participant