Repository navigation
fix(observability): classify 'does not support tools' provider 400 as user-state (Sentry TAURI-RUST-35 + 10 siblings) - #2796
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExtended is_provider_config_rejection_message to classify upstream responses containing the exact substring "does not support tools" as provider config-rejections, and added unit tests covering positive and negative tool-related message variants across wrapper shapes and providers. ChangesProvider Config Rejection Classifier Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
CodeGhost21
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Clean fix. Single additive change to the PHRASES array in is_provider_config_rejection_message — the anchor phrase "does not support tools" is stable across every observed provider prefix (cloud / ollama / custom_openai), wrapper shape (streaming / non-streaming), and type token (invalid_request_error / api_error). The existing body.to_ascii_lowercase() call means capitalisation variants are free.
Test coverage is thorough: 5 verbatim wire shapes pin each meaningful variant, and the polarity guard correctly rejects near-miss phrases that must still reach Sentry. The diff-cover gate passed, all Rust core tests green, fmt + clippy clean.
Breaking risk is zero — purely additive. No API, type, or schema changes. The 11 Sentry issues (~459 events) should drain to zero on the next release.
Auto-corrected: reviewer policy requires COMMENT state, not APPROVE. Clean PR moved to manual approval queue.
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Clean fix. Single additive change to the PHRASES array in is_provider_config_rejection_message — the anchor phrase "does not support tools" is stable across every observed provider prefix (cloud / ollama / custom_openai), wrapper shape (streaming / non-streaming), and type token (invalid_request_error / api_error). The existing body.to_ascii_lowercase() call means capitalisation variants are free.
Test coverage is thorough: 5 verbatim wire shapes pin each meaningful variant, and the polarity guard correctly rejects near-miss phrases that must still reach Sentry. The diff-cover gate passed, all Rust core tests green, fmt + clippy clean.
Breaking risk is zero — purely additive. No API, type, or schema changes. The 11 Sentry issues (~459 events) should drain to zero on the next release.
✅ Ready for approval.
d74b31e to
85cc3e6
Compare
|
Actionable comments posted: 0 |
… user-state (Sentry TAURI-RUST-35 + 9 siblings)
Add `"does not support tools"` to the `is_provider_config_rejection_message`
PHRASES array. The user picks a model that doesn't implement tool calling
(e.g. `gemma3:1b-it-qat`, `qwen2.5:0.5b`, `phi3.5:mini`), the agent
harness sends a tool spec anyway, and the upstream rejects with
`{"error":{"message":"<model id> does not support tools",
"type":"invalid_request_error",…}}`. Pure user-config error — the
remediation is "pick a tool-capable model in Settings → AI → LLM";
Sentry has no actionable path.
One body substring drops the entire long-tail family. Because the error
includes the model id, each distinct (provider prefix × model id) combo
produced its own Sentry fingerprint — currently 10 unresolved sibling
issues totaling ~458 events on the latest production release as of
2026-05-28:
| shortId | events | provider |
|-----------------|--------|----------|
| TAURI-RUST-35 | 307 | cloud |
| TAURI-RUST-DF | 83 | cloud |
| TAURI-RUST-123 | 25 | cloud |
| TAURI-RUST-4K7 | 19 | ollama |
| TAURI-RUST-4FS | 10 | cloud |
| TAURI-RUST-4F6 | 5 | cloud |
| TAURI-RUST-2YA | 4 | cloud |
| TAURI-RUST-4KR | 3 | ollama |
| TAURI-RUST-4KH | 1 | cloud |
| TAURI-RUST-4KY | 1 | ollama |
The anchor is the exact `"does not support tools"` substring — stable
across all observed wrapper shapes (`cloud` / `ollama` / `custom_openai`
× `streaming API error` / `API error`) and across all observed model
ids. The matcher is already case-insensitive (`body.to_ascii_lowercase()`
upstream), so capitalisation variants are covered for free.
Tests added in `provider::config_rejection::tests`:
- `detects_does_not_support_tools_family` — verbatim TAURI-RUST-35 wire
shape (cloud + ollama prefix + non-streaming sibling + bare body
variants).
- `does_not_classify_unrelated_tools_phrases_as_config_rejection` —
polarity guard pinning near-miss phrases that must STILL escalate
(real tool execution failures, generic "tools" mentions, reversed
phrasing).
- [x] `cargo test detects_does_not_support_tools_family` — passes (4 wire shapes)
- [x] `cargo test does_not_classify_unrelated_tools_phrases` — passes (polarity)
- [x] `cargo test config_rejection` — 8 tests pass, 0 regressions
- [x] `cargo check --bin openhuman-core` — passes
- [x] `cargo fmt --check` — clean
…or does-not-support-tools Sentry issue 5664 (TAURI-RUST-4Z0, ollama/deepseek-r1:8b) is the 11th fingerprint of the same "does not support tools" root cause this PR classifies. Its envelope carries `"type":"api_error"` rather than the `"invalid_request_error"` seen in the other siblings — add it as a test case so the matcher can never be narrowed to require a specific `type` token. The `"does not support tools"` body substring stays the only anchor. No production code change — the existing PHRASES entry already matches this body (verified). Test-only strengthening.
85cc3e6 to
deb82ab
Compare
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
PR #2796 — fix(observability): classify 'does not support tools' provider 400 as user-state
Walkthrough
Clean, single-phrase addition to is_provider_config_rejection_message. The anchor "does not support tools" covers all 11 Sentry issues in the family (cloud / ollama / custom_openai prefixes, streaming / non-streaming, invalid_request_error / api_error type tokens) with one string. Case-insensitive match already in place.
Changes
| File | Summary |
|---|---|
src/openhuman/inference/provider/config_rejection.rs |
Add "does not support tools" phrase; 2 new tests |
Verified / looks good
- Phrase placed at end of PHRASES array with clear comment linking the 10 sibling Sentry issues.
detects_does_not_support_tools_family: 5 verbatim wire shapes — cloud/ollama prefixes, streaming/non-streaming, bare body, and TAURI-RUST-4Z0's distincttype:"api_error"envelope (pinning that the matcher doesn't require a specific type token).does_not_classify_unrelated_tools_phrases_as_config_rejection: polarity contract covers tool-execution failure, generic "tools" mention, reversed phrasing, and empty body.- Merge conflict resolved (PR #2809's
detects_insufficient_balance_402_familytest was added to the same area concurrently — both tests preserved). - All CI checks passed (30/30).
… user-state (Sentry TAURI-RUST-35 + 10 siblings) (tinyhumansai#2796)\n
Summary
Add
"does not support tools"tois_provider_config_rejection_message's PHRASES array (src/openhuman/inference/provider/config_rejection.rs). The user picks a model that doesn't implement tool calling, the agent harness sends a tool spec anyway, and the upstream rejects with{"error":{"message":"<model id> does not support tools",...}}. Pure user-config — Sentry has no actionable path.Why this drops 11 Sentry issues, not 1
Because the rejection message includes the literal model id, each
(provider prefix × model id)combination produced a distinct Sentry fingerprint. The same root cause is currently split across 11 unresolved Sentry issues totalling ~459 events (snapshot 2026-05-28):gemma3:1b-it-qat)deepseek-r1:8b,type=api_error)Anchor token: the exact substring
"does not support tools". It's stable across every observed wrapper shape (cloud/ollama/custom_openai×streaming API error/API error), every observed model id, and both observedtypetokens (invalid_request_errorandapi_error— TAURI-RUST-4Z0). A single phrase covers the whole long tail and any future model that hits the same upstream rejection class.The matcher upstream is already case-insensitive (
body.to_ascii_lowercase()before substring search), so capitalisation variants are covered for free.Tests added (
provider::config_rejection::tests)detects_does_not_support_tools_family— 5 verbatim wire shapes: TAURI-RUST-35 (cloud, gemma3), TAURI-RUST-4K7 (ollama prefix), the non-streamingAPI errorsibling, a bare body without the wrapper, and TAURI-RUST-4Z0 (ollama / deepseek-r1:8b with the distincttype:"api_error"envelope — pins that the matcher never requires a specifictypetoken).does_not_classify_unrelated_tools_phrases_as_config_rejection— polarity guard pinning near-miss phrases that must STILL escalate to Sentry (real tool execution failures, generic "tools" mentions, reversed phrasing).Test plan
cargo test detects_does_not_support_tools_family— passes (5 wire shapes)cargo test does_not_classify_unrelated_tools_phrases— passes (polarity)cargo test config_rejection— 8 tests pass, 0 regressionscargo check --manifest-path Cargo.toml --bin openhuman-core— passescargo fmt --check— cleanPost-merge observation: all 11 listed Sentry issues should drop to ~0 events on releases ≥ this fix. Any new "does not support tools" event on a newer release means the matcher missed a wrapper variant — investigate the wire shape, not the model id.
Summary by CodeRabbit
New Features
Tests