Skip to content

fix(agents,ui): Ollama chat HITL — null-content, pending-approval salvage, and approval report#345

Merged
w7-mgfcode merged 3 commits into
devfrom
fix/agents-ollama-null-content
Jun 1, 2026
Merged

fix(agents,ui): Ollama chat HITL — null-content, pending-approval salvage, and approval report#345
w7-mgfcode merged 3 commits into
devfrom
fix/agents-ollama-null-content

Conversation

@w7-mgfcode
Copy link
Copy Markdown
Owner

@w7-mgfcode w7-mgfcode commented Jun 1, 2026

Closes #344. Closes #346.

Three fixes for the agent-chat HITL approval flow on local Ollama, found while dogfooding the Chat page. One PR, three focused commits.

Bug 1 — null-content crash (FallbackExceptionGroup) — #344

Ollama's OpenAI-compatible /v1/chat/completions rejects any message whose content is JSON null and carries no tool_calls with 400 invalid message content type: <nil>. A weak local model emits that shape for an empty assistant turn; PydanticAI replays it on retry, so every retry 400s and the run dies with FallbackExceptionGroup. Not streaming-specific.

Verified (curl → Ollama, qwen3:8b): content=null + tool_calls → 200; content=null no tool_calls400; content="" → 200.

Fix: build_agent_model injects a sanitizing httpx transport into OllamaProvider(http_client=…) that coerces outgoing messages[*].content: null → "" (recomputes Content-Length; generous read timeout for slow local generation).

Bug 2 — pending approval dropped on model misbehavior — #344

A gated tool records deps.pending_action when it fires but does not halt the run. A weak model rambles past the gate and exhausts its retry budget → UnexpectedModelBehavior before agent.run() returns. The handlers emitted a generic "invalid tool call" error without checking deps.pending_action, discarding a valid approval.

Fix: shared _salvage_pending_action; both chat() and stream_chat() UnexpectedModelBehavior handlers now surface deps.pending_action (pending_approval=True / approval_required event) before the generic error.

Bug 3 — approve/reject produced no visible result — #346

The Chat handleApprove/handleReject awaited POST /approve but discarded the ApprovalResponse and only cleared the card. The operator saw nothing — and a failed execution (create_alias → "Run not found", returned status=rejected + result.error) was silent.

Fix: capture the response and append a one-line report for every outcome (executed / approved-but-failed-with-cause / rejected / expired) via a pure, unit-tested formatApprovalReport helper.

Testing

  • Backend: test_base.py (null-content truth table + transport rewrite + Content-Length); test_service.py (gated-tool-then-misbehavior → chat() pending_approval=True, stream_chat() emits approval_required, generic-error path preserved). 69 agents tests pass.
  • Frontend: approval-report.test.ts covers all four outcomes; tsc --noEmit clean, eslint clean, 254/254 vitest pass.
  • Gates: ruff · ruff format · mypy app/ (only pre-existing xgboost optional-dep errors) · pyright app/ (no new errors).
  • Verified live end-to-end on local Ollama (llama3.1:8b primary): no crash; HITL Approve card surfaces; approving a real success run creates the alias; the execution report renders for success and failure.

Notes

Reliable HITL on local Ollama needs a tool-capable model: llama3.1:8b triggers the gate reliably; qwen3:8b often returns empty turns under PromptedOutput. Use one fresh session per approval attempt.

Ollama's OpenAI-compatible /v1/chat/completions rejects any message whose content is JSON null and carries no tool_calls (400 'invalid message content type: <nil>'). A weak local model emits that shape for an empty assistant turn and PydanticAI replays it on retry, so every retry 400s and the run dies with FallbackExceptionGroup. Inject a sanitizing httpx transport into the OllamaProvider client that coerces outgoing content:null to content:"".
…344)

A gated tool (create_alias/archive_run/save_scenario) records deps.pending_action the moment it fires, but does not halt the run. A weak model can ramble past the gate and exhaust its retry budget, so agent.run raises UnexpectedModelBehavior before returning and the post-run approval-surfacing path never runs. The captured approval is valid, so surface it: chat() and stream_chat()'s UnexpectedModelBehavior handlers now check deps.pending_action first and emit the approval (ChatResponse pending_approval / approval_required event) before falling back to the generic error.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08fae0cb-d676-48b0-bd5c-7bb6687e35bb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/agents-ollama-null-content

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @w7-mgfcode, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

)

The chat Approve/Reject handlers awaited POST /approve but discarded the ApprovalResponse and only cleared the card, so a click produced no visible result — and a failed action execution (e.g. create_alias 'Run not found', returned as status=rejected + result.error) was silent. Capture the response and append a one-line report for every outcome (executed / approved-but-failed / rejected / expired) via a pure, unit-tested formatApprovalReport helper.

Closes #346.
@w7-mgfcode w7-mgfcode changed the title fix(agents): Ollama chat HITL — sanitize null content and preserve pending approval (#344) fix(agents,ui): Ollama chat HITL — null-content, pending-approval salvage, and approval report Jun 1, 2026
@w7-mgfcode w7-mgfcode merged commit 7cced50 into dev Jun 1, 2026
8 checks passed
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