fix: keep LM Studio (llama.cpp) on-model instead of falling back on grammar errors

[rsegrest]

Problem

With a local LM Studio / llama.cpp provider as the primary model, every agent
turn that loaded certain tool schemas failed at request time with

400 — Failed to initialize samplers: failed to parse grammar

and, after 3 retries, silently fell through to the cloud fallback model. The
network and server were fine — LM Studio was rejecting the grammar llama.cpp
builds from the tool schemas.

Root cause

An MCP tool schema (in our case apple-notes' batch-delete-notes /
batch-move-notes) carried "maxLength": 2000 on array-item strings.
llama.cpp expands that into a GBNF rule repeating a char class up to 2000×
inside array repetition; the generated grammar is too large/malformed and the
whole request 400s. (maxItems alone and top-level maxLength are fine — it's
large maxLength nested in array items.)

Hermes already had a reactive recovery for llama.cpp grammar rejections, but it
never helped here, for two independent reasons:

1. Streaming path — LM Studio's error surfaces as a bare APIError with
   status_code=None (not BadRequestError/400), so the classifier's
   status_code == 400 guard never matched and the error wasn't recognized as
   a grammar failure at all.
2. Non-stream path — it was recognized, but the strip only removed
   pattern/format, and these schemas have neither, so 0 keywords were
   stripped and it fell through to fallback.

Changes

• agent/error_classifier.py — recognize the grammar error when
  status_code is 400 or None (fixes the streaming path). The phrases
  are llama.cpp-specific and safe to match without a status code.
• tools/schema_sanitizer.py — broaden the reactive strip to also remove
  maxLength / minLength / maxItems / minItems, not just
  pattern / format.
• agent/conversation_loop.py — recovery log wording.

Cloud providers still receive the full schema hints; the strip runs only after
a llama.cpp backend actually rejects the request.

Verification

End-to-end against a live LM Studio server with the real apple-notes tool set,
both transports:

Path	attempt 1	recovery	attempt 2
non-stream	BadRequestError 400 → classified	stripped 93 keywords	succeeds, no fallback
stream	APIError sc=None → now classified	stripped 93 keywords	succeeds, no fallback

🤖 Generated with Claude Code

[teknium1]

Thanks for the focused regression fix. Current main recognizes the existing llama.cpp phrases at agent/error_classifier.py:737-752, but not LM Studio's failed to parse grammar wording; that prevents the recovery branch at agent/conversation_loop.py:3086-3111 from stripping unsupported schema keywords and retrying. The added predicate and exact HTTP-400 regression test are consistent with the established recovery contract.

Automated hermes-sweeper review.

[rsegrest]

Thanks for the review. Adding test coverage for the second commit in b50b0c945 — it changed two behaviors that a3312add4's test doesn't reach, and I'd left them unpinned:

1. The streaming path. The predicate was relaxed from status_code == 400 to status_code in (400, None). On a non-streaming request the SDK raises BadRequestError/400, but on a streaming request LM Studio's identical error arrives as a bare APIError with status_code=None — so the 400 gate missed every streamed request, the recovery branch never ran, and the turn fell through to the fallback model. test_llama_cpp_grammar_error_without_status_code pins it, and test_unrelated_error_without_status_code_is_not_grammar guards the relaxed gate against swallowing unrelated status-code-less errors.

2. The broadened strip. Beyond pattern/format, the reactive strip now removes the bounded-repetition keywords maxLength/minLength/maxItems/minItems — llama.cpp expands those literally into GBNF, and a large bound is what actually produced the 400 here ("maxLength": 2000 on array items, from apple-notes' batch-delete-notes). Covered by test_strip_removes_max_length_on_array_items (which also asserts the structure the model still needs survives), test_strip_removes_min_bounds, and test_strip_preserves_property_named_max_length — a property named maxLength is data, not a schema keyword, mirroring the existing pattern case.

All three regression tests fail against the pre-fix code and pass after. tests/agent/test_error_classifier.py + tests/tools/test_schema_sanitizer.py: 250 passed.

[rsegrest]

On the sweeper:risk-caching label — no prompt-caching exposure here.

The reactive strip does mutate agent.tools for the remainder of the turn, which in principle could disturb a cached prompt prefix. But it only runs inside the FailoverReason.llama_cpp_grammar_pattern recovery branch, which fires exclusively on llama.cpp / LM Studio's failed to parse grammar (and unable to generate parser) HTTP 400s. Those are local OpenAI-compatible servers with no Anthropic-style prompt caching. Cloud providers accept pattern/format/maxLength/… as prompting hints and never return that error, so the strip is never reached on a cache-bearing path and cache-key stability is untouched.

[mvdbastos]

Hit this exact failure independently on our fork (LM Studio, openai/gpt-oss-20b, ClickUp MCP tools with "pattern": "^\\d{4}-\\d{2}-\\d{2}(...)?$" on date params — same Failed to initialize samplers: failed to parse grammar 400).

Bisection we ran, model-independent:

Test	Result
All 36 built-in tools	200
Approvals MCP (5 tools, 0 pattern)	200
The 4 ClickUp date-pattern tools	400
Same 4, after manual strip_pattern_and_format	200
Same 4 vs zai-org/glm-4.7-flash / qwen/qwen3.5-9b	400

Confirms this is engine-level (llama.cpp GBNF), not model-specific — consistent with what you found with the maxLength-on-array-items trigger.

This PR looks like the right fix (covers our exact phrasing plus the streaming status_code=None gap we hadn't caught). It's showing CONFLICTING against current main — happy to help rebase/resolve conflicts and re-verify against our LM Studio setup if that's useful, since we're pulling it into our fork regardless.

[mvdbastos]

Two follow-ups on my comment above.

Correction. I reported our bisection pointing at ClickUp's date-pattern tools. Re-testing today against LM Studio (openai/gpt-oss-20b), a synthetic simple pattern (^\d{4}-\d{2}-\d{2}$) went through fine — no 400. Only the maxLength-on-array-items shape reproduced. So the pattern half of that report likely reflects more complex real ClickUp schemas or an older LM Studio build; I can't reproduce it on demand and shouldn't have implied it was freshly confirmed. Your maxLength trigger reproduces every time:

$ curl http://127.0.0.1:1234/v1/chat/completions -d '{...items:{maxLength:2000}, maxItems:500...}'
{"error":"Engine protocol predict request returned 400: {\"error\":{\"code\":400,
 \"message\":\"Failed to initialize samplers: failed to parse grammar\", ...}}"}
HTTP_CODE:400  TIME:0.30s

Doesn't change the fix — it covers both keyword families — but the record should be accurate.

Rebase available. This is showing CONFLICTING against main. The cause is 6b81590c5 ("test: prune low-value tests suite-wide (wave 1)"), which removed all four test_llama_cpp_* methods from tests/agent/test_error_classifier.py and left an empty section header where these commits add tests. Purely mechanical, no logic collision.

I've rebased the three commits onto 206eda50a with authorship preserved and opened it as rsegrest#1 — merging that into fix/lmstudio-grammar-error-phrase should clear the conflict here. Resolution details and the one judgment call (which pruned tests to restore) are in that PR's description.

Also verified the fix end-to-end on a profile with no fallback model configured, so the successful tool call can only come from the strip-and-retry path firing rather than a silent failover.

[mvdbastos]

Retracting my correction above — it was wrong, and the original report was right.

I re-tested pattern in isolation against the same LM Studio instance (openai/gpt-oss-20b), one tool with a single string property, varying only the regex:

pattern	result
^\d{4}-\d{2}-\d{2}$	400
^\d{4}-\d{2}-\d{2}( \d{2}:\d{2})?$	400
^\d{4}-\d{2}-\d{2}( \d{2}:\d{2})$	400
^\d{4}a?$	400
^[0-9]{4}( [0-9]{2})?$	200

So it is specifically the \d escape class. Groups, optional groups and bounded quantifiers are all fine on their own — the [0-9] equivalent of the failing pattern passes. My earlier "simple \d pattern went through fine" result is not reproducible; the identical pattern returns 400 now. I don't have an explanation for the discrepancy beyond a possible LM Studio/engine build difference between the two test runs, so treat the table above as the current state.

Net effect for this PR: pattern and the bounded-repetition keywords are two independent triggers, both reproducible on demand. pattern earns its place in _STRIP_ON_RECOVERY_KEYS on its own merit, not just as collateral alongside maxLength.

Separately, I ran your three commits end-to-end on a deployed container against live LM Studio, using a throwaway stdio MCP server carrying both hostile keyword families and no fallback model configured, so a successful tool call can only come from the strip-and-retry path:

Error classified: reason=llama_cpp_grammar_pattern status=None retryable=True
schema_sanitizer: stripped 10 grammar-hostile keyword(s)
  (pattern/format/maxLength/minLength/maxItems/minItems) — llama.cpp grammar-parse recovery
llama.cpp grammar recovery: stripped 10 grammar-hostile keyword(s) from tool schemas

Note status=None — the error arrived on the streaming path with no status code, so the status_code in (400, None) relaxation is load-bearing. With only the phrase match added and the old status_code == 400 guard, this case would still fail.

One testing gotcha worth recording for anyone else verifying this: my first end-to-end run "passed" without the recovery ever firing, because tool-search deferral was active and the model reached the tool via tool_search / tool_describe / tool_call — the hostile schema never reached the grammar converter. Any end-to-end test of this path needs tools.tool_search.enabled: off, and the log should show an explicit grammar-recovery line rather than merely a successful turn.