## Summary
- Replace `response.json()` with read-as-text + `serde_json::from_str` in `list_configured_models_from_config` so the response body is preserved when JSON decoding fails.
- Append a sanitized + truncated body snippet to the `[providers][list_models] failed to parse JSON` error so the failure is diagnosable from the log/Sentry line alone.
- Add three async unit tests: HTML body returns a diagnostic snippet, empty body still surfaces the parse error, and a valid `/models` response still lists models (regression guard for the new text-then-parse path).
## Problem
Sentry issue **TAURI-RUST-12** — `[providers][list_models] failed to parse JSON: error decoding response body` — 376 events / 14d on the `tauri-rust` project.
`response.json()` in [src/openhuman/inference/provider/ops.rs:125](src/openhuman/inference/provider/ops.rs:125) (pre-change) consumes the body in the process of decoding it. When the decode fails — typically because the server returned HTML from a captive portal / corporate proxy login page, an upstream load-balancer 502 served as HTML with `200 OK`, or a wrong-path endpoint returning a non-JSON response — the body is gone by the time we format the error, so Sentry receives `error decoding response body` with no payload context.
We can't fix this server-side. We *can* stop discarding the diagnostic information at the call site so users and devs can identify the real cause from the error string instead of guessing.
## Solution
`src/openhuman/inference/provider/ops.rs`:
- After the `status.is_success()` check, call `response.text().await` instead of `response.json()`. The text path returns the raw body verbatim, which we can then both parse *and* embed in the diagnostic message.
- `serde_json::from_str(&raw_body)` reproduces the previous decode behaviour exactly — same JSON parser, same `serde_json::Error` shape. On failure, the closure sanitizes the body via the existing `sanitize_api_error` helper and truncates it through the existing `crate::openhuman::util::truncate_with_ellipsis(_, 300)` before appending it as `(body: …)`.
- Adds an explicit error for `response.text()` failure (`failed to read response body`) — a transport-layer concern distinct from JSON parsing.
**Design choices**
- Re-use the existing `sanitize_api_error` (strips ANSI / control chars, caps at `MAX_API_ERROR_CHARS`) and `truncate_with_ellipsis` helpers — same sanitization the non-2xx branch already applies a few lines above. No new redaction policy.
- Keep the canonical error prefix `[providers][list_models] failed to parse JSON:` so any existing log greps / Sentry classifiers continue to match.
- 300-character snippet cap matches the existing non-2xx branch's `truncated` cap and the codebase convention for "include enough for triage, not enough to flood logs."
- No change to the JSON parser, no change to what counts as a valid `/models` response, and no change to error semantics — the new branch returns `Err(...)` in exactly the same shape and code path as before. Callers see one extra clause appended to the message string.
- Body is only read on the success path (`status.is_success()`). The non-2xx branch already had its own `response.text()` + sanitize chain, untouched.
## Submission Checklist
- Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement)
- **Diff coverage ≥ 80%** — pending local `pnpm test:rust` run
- Coverage matrix updated — `N/A: diagnostic-only change, no new feature row`
- All affected feature IDs from the matrix are listed in the PR description under `## Related`
- No new external network dependencies introduced (uses existing axum-based mock pattern from `spawn_openrouter_probe_server`)
- Manual smoke checklist updated — `N/A: no release-cut surface touched`
- Linked issue closed via `Closes #NNN` — `N/A: Sentry-tracked issue, no GitHub issue yet`
## Impact
- **Runtime**: desktop (Rust core). No mobile / web / CLI surface change.
- **Performance**: negligible — one extra `String` allocation for the body (length already bounded by reqwest's response size limits) and one extra `serde_json::from_str` instead of `response.json()`'s internal equivalent. Happy path serialization cost is identical.
- **Security**: no new surface. Body is sanitized via the same helper the non-2xx branch already trusts; `truncate_with_ellipsis(_, 300)` caps the leak window. No PII redaction policy changes.
- **Migration / compatibility**: none. RPC schema, return type, error-string prefix all preserved. Callers that previously matched on `"failed to parse JSON"` still match — only a `(body: …)` suffix is added.
## Related
- Closes: Sentry [TAURI-RUST-12](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/100/?project=4&referrer=issue-list&statsPeriod=14d)
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved model-listing response parsing and diagnostics: parsing now uses the raw response text and, on failure, error messages include a sanitized, truncated snippet of the body to aid troubleshooting. Non-2xx handling and subsequent response validation remain unchanged.
* **Tests**
* Added tests covering HTML responses, empty bodies, and valid model-listing payloads.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2838?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
## Summary
* Filter unpaired `AssistantToolCalls` and orphan `ToolResults` out of the wire payload in `NativeToolDispatcher::to_provider_messages` so a bisected tool cycle can no longer reach the provider.
* Strip trailing `assistant` messages that carry `tool_calls` without paired tool responses from a resumed transcript in `bound_cached_transcript_messages` (symmetric to the existing leading-orphan strip).
* Add `assistant_message_has_tool_calls` helper that peeks into the JSON-encoded `assistant` ChatMessage content to detect the tool_calls field at the `ChatMessage` boundary.
* Add 5 regression tests for `NativeToolDispatcher::to_provider_messages`: paired cycle, trailing unpaired, mid-history unpaired, orphan ToolResults, and multiple paired cycles back-to-back.
## Problem
Sentry issue **TAURI-RUST-7** — `OpenHuman API error (400 Bad Request): {"success":false,"error":"400 An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'. (insufficient tool messages following tool_calls message)"}` — 589 events / 14d on the `tauri-rust` project.
The OpenAI chat-completions contract requires every `assistant{tool_calls}` message to be immediately followed by `tool` messages — one for every `tool_call_id`. Any other ordering produces `400 An assistant message with 'tool_calls' must be followed by tool messages`.
Two reachable code paths in our harness produce a bisected pair:
1. **Mid-turn abort / resume**. `ConversationMessage::AssistantToolCalls` is pushed into `Agent::history` before the tool runner finishes. If the turn aborts (user cancel, error, process kill, max-iter cap) before the matching `ConversationMessage::ToolResults` is appended, history ends on a tool_calls with no follow-up. The next turn submits the whole history and the backend rejects it.
2. **Cached transcript bound**. `bound_cached_transcript_messages` (`turn.rs:1660`) keeps the tail of a resumed transcript. The existing leading-orphan strip handles a window that opens on a stray `tool` message, but it never stripped a *trailing* assistant tool_calls. A resume from a mid-cycle snapshot ships an unpaired tool_calls and the backend rejects.
`trim_history` already had a leading-orphan guard ([`[turn.rs:1637](https://chatgpt.com/c/src/openhuman/agent/harness/session/turn.rs:1637)`](src/openhuman/agent/harness/session/turn.rs:1637)). The symmetric trailing / unpaired case was uncovered.
## Solution
### `src/openhuman/agent/dispatcher.rs` — Native dispatcher only
`to_provider_messages` is the boundary that serialises `ConversationMessage` → `ChatMessage` (wire format). The Native dispatcher is the only impl affected — XML and PFormat dispatchers render tool results into a `user` role with inline `<tool_result>` tags, so the tool_calls/tool ordering rule doesn't apply to them and their impls are untouched.
Two-pass pairing in a single loop:
```rust
let mut paired_indices: Vec<usize> = Vec::with_capacity(history.len());
for (i, msg) in history.iter().enumerate() {
match msg {
AssistantToolCalls { .. } => {
// Keep only if next is ToolResults.
if matches!(history.get(i + 1), Some(ToolResults(_))) {
paired_indices.push(i);
} else { log::debug!("dropping unpaired AssistantToolCalls at index {i}"); }
}
ToolResults(_) => {
// Keep only if the previous *emitted* index is a kept AssistantToolCalls.
let preceded_by_kept = i > 0
&& matches!(history.get(i - 1), Some(AssistantToolCalls { .. }))
&& paired_indices.last() == Some(&(i - 1));
if preceded_by_kept { paired_indices.push(i); }
else { log::debug!("dropping orphan ToolResults at index {i}"); }
}
Chat(_) => paired_indices.push(i),
}
}
```
The key invariant is `paired_indices.last() == Some(&(i - 1))` — that captures emitted-not-raw lookbehind. A bisected assistant tool_calls is dropped, which means the now-orphan tool results that physically followed it are also dropped, even though `history[i-1]` is still `AssistantToolCalls`. Symmetric drop in a single pass.
Second pass: `flat_map` over `paired_indices` runs the existing serialisation logic verbatim (assistant tool_calls → JSON-encoded ChatMessage; tool results → one tool ChatMessage per result). No serialisation change — only the input set is filtered.
### `src/openhuman/agent/harness/session/turn.rs` — cached transcript bound
This layer operates on `Vec<ChatMessage>`, not `ConversationMessage`. Can't pattern-match on enum variants because the dispatcher's serialisation packs both content and tool_calls into a single JSON-encoded string in the assistant `ChatMessage.content` (see `dispatcher.rs:484-490`). To detect tool_calls at this boundary the helper peeks inside the JSON:
```rust
fn assistant_message_has_tool_calls(msg: &ChatMessage) -> bool {
if msg.role != "assistant" { return false; }
let Ok(value) = serde_json::from_str::<serde_json::Value>(&msg.content) else { return false; };
value.get("tool_calls").and_then(|tc| tc.as_array())
.map(|arr| !arr.is_empty()).unwrap_or(false)
}
```
Non-assistant role → false. Non-JSON content (a plain text reply) → false. Missing field or empty array → false. Message kept in all those cases.
Then a `pop_while` after the existing leading-orphan strip:
```rust
while bounded.last().map(assistant_message_has_tool_calls).unwrap_or(false) {
bounded.pop();
dropped_tail += 1;
}
```
Symmetric to the existing leading-orphan strip — both ends of the bounded window now end on a clean turn boundary.
## Design choices
- Filter at the wire boundary, not at write time. Both write sites (turn loop, transcript restore) push into history without coordinating with each other. Centralising the guard at the serialisation boundary catches every code path that ends up shipping to a provider, present and future.
- Native dispatcher only. XML / PFormat dispatchers render tool results into user role with inline tags and aren't subject to the OpenAI tool ordering rule. Leaving their `to_provider_messages` untouched avoids unrelated behaviour change.
- What we drop the backend would have rejected anyway. Every dropped message would have triggered the same 400. Dropping client-side turns a hard failure into a recoverable turn — the rest of the well-formed history still flies.
- `log::debug!` on drop. Visibility into how often the guard fires without flooding warn-level logs.
-JSON-content inspection helper isolated and tested by the integration tests above. A `serde_json::from_str` parse failure means the content isn't the dispatcher's JSON envelope, so the message is a plain text assistant reply and must be kept — false return is correct.
## Submission Checklist
- Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
- Diff coverage ≥ 80% — pending local pnpm test:rust run
- Coverage matrix updated — N/A: bug-fix behaviour-only change, no new feature row
- All affected feature IDs from the matrix are listed in the PR description under ## Related
- No new external network dependencies introduced
- Manual smoke checklist updated — N/A: no release-cut surface touched
- Linked issue closed via Closes #NNN — N/A: Sentry-tracked issue, no GitHub issue yet
## Impact
- Runtime: desktop (Rust core). No mobile / web / CLI surface change.
- Performance: one extra `Vec<usize>` allocation sized to `history.len()` in the dispatcher and one O(n) pass over the bounded transcript tail. Happy-path cost is two extra `matches!` checks per message. No measurable overhead on the chat hot path.
- Security: none — no new network surface, no new inputs trusted, no auth path touched.
- Migration / compatibility: none. Trait signatures, RPC schemas, and dispatcher output for fully-paired histories are unchanged. Previously-failing turns that hit the backend's 400 now succeed with a `log::debug!` line per dropped orphan.
## Related
- Closes: [TAURI-RUST-7](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/47/?project=4&referrer=issue-list&statsPeriod=14d)
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Sanitized provider-bound message history to keep only well-formed assistant opener + matching tool-results (exact ID-set match, order-insensitive); orphaned or mismatched tool messages are dropped.
* Hardened transcript trimming to strip partial/native tool-call envelopes at window edges so turns end on clean boundaries and log the count of stripped envelopes.
* **Tests**
* Expanded regression suite covering pairing semantics, orphan/drop cases, ordering tolerance, and transcript-bounding behavior.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2840?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
