fix: make complex connectors compatible with llama.cpp grammar

[xDenside]

Summary

• project MCP tool schemas into a grammar-safe form for llamacpp and llamacpp-upstream chat requests
• remove only validation keywords that expand into large bounded or regex GBNF rules; preserve tool names, descriptions, properties, types, required fields, and enums
• retain the original MCP schema in app state so the MCP server remains the final argument validator
• keep cloud and MLX schemas unchanged
• include provider identity in the tool cache and measure tool cost from the schema actually sent

Why

Firecrawl connects successfully, but its 25 tools expand into a 168 KB, 596-rule grammar containing bounds such as char{0,10000} and arrays up to 100 items. The shipped llama.cpp parser rejects the grammar before generation when repeated-rule complexity reaches its safety threshold, producing Failed to initialize samplers: failed to parse grammar. The same class of failure appears with other large connector catalogs and under both llama.cpp providers.

This projection avoids the grammar compiler limit without silently dropping tools. A model can produce an argument outside a removed validation constraint, but the MCP server still validates its original contract and returns the normal tool error.

Verification

• focused helper and transport tests: 29 passed
• web TypeScript build: passed
• affected source and test lint: passed
• full web lint: passed with 13 existing warnings
• full web suite: 2,605 passed, 11 skipped; one unrelated existing provider-picker assertion remains failing (DropdownModelProvider.connected.test.tsx)

Upstream guard

The b10431 llama.cpp backend uses MAX_REPETITION_THRESHOLD = 2000 and rejects a bounded repetition when previous_rule_complexity * repetition_count reaches that limit: https://github.com/ggml-org/llama.cpp/blob/b10431/src/llama-grammar.cpp#L485-L496

[Vect0rM]

Thanks for this, @xDenside — the diagnosis is exact and the fix is narrower than it had to be, which is the right instinct. Projecting the schema rather than dropping tools means a user with a large connector catalogue keeps every tool and only loses constraints the MCP server re-checks anyway; linking the specific MAX_REPETITION_THRESHOLD guard in the b10431 source is the kind of citation that makes a fix reviewable years later.

Verified on your branch merged onto current main (it fast-forwards from 8e4c0a0 — no conflicts):

• tsc -b — exit 0.
• yarn lint — 0 errors, 13 react-refresh/exhaustive-deps warnings, byte-identical to the main baseline.
• Full vitest run — 283 files, 2773 passed, 16 skipped. Baseline on main is 2771, so your 2 land clean with nothing else disturbed.

I also checked the projection independently rather than reading the diff, because a recursive schema walker is easy to get subtly wrong. I built a schema that hides expansion keywords at every nesting construct the walker knows about — $defs behind a $ref, prefixItems, anyOf, if/then/else, additionalProperties, propertyNames, not, contains/maxContains, unevaluatedProperties, allOf, and array-form items — and no expansion keyword survives at any depth, while $ref, required, enum and type information come through intact and the input object is not mutated. The property-name-versus-keyword distinction holds too: a property literally named maxLength is preserved, which your own test already pins.

One thing I want to record because it's the question a reviewer would ask next: this is the only path that needs it. buildToolsRecord is the sole jsonSchema( call site for MCP tools in the web app, and the Rust agent never reaches llama.cpp's JSON-schema→GBNF compiler at all — core/agent/grammar.rs writes GBNF by hand from tool names, and core/agent/prompt.rs:1048 puts input_schema into the prompt as text. So the fix covers the affected surface and there's no second place quietly still broken.

Including providerId in the tool cache key is a good catch that wasn't strictly asked for by the bug — without it, switching provider mid-thread would reuse whichever projection was built first.

Two questions and two nits. Nothing here blocks.

1. What are the numbers after?

You give the before precisely — 168 KB, 596 rules, 25 tools. The description never says what it becomes. That's the measurement that shows the fix clears the threshold rather than merely moving toward it, and it's the number the next person will want when a bigger catalogue fails.

Related: integer minimum / maximum survive the projection deliberately (your test pins maximum: 10000 as preserved), and llama.cpp expands integer bounds into digit-range rules as well. That's not the repetition path you cite, so I'd expect it to be well under the guard — but if a catalogue still fails after this, that's where I'd look next. Worth a sentence in the ADR so the next person doesn't have to rediscover it.

2. format removal is slightly broader than "bounded repetition"

format is on the strip list alongside the genuinely repetition-driven keywords. Its expansions (date-time, uuid) are small fixed repetitions, so removing it costs little — but it's the one entry in LLAMA_GRAMMAR_EXPANSION_KEYS that isn't really justified by the constant's own name. Either keep it and say why in the comment, or drop it from the set. A reader auditing that list later will stumble on exactly this.

Nits

• The projection runs twice per tool on every rebuild — once inside buildToolsRecord, then again to build measuredTools for the cost report. Projecting once and passing the result to both would be cheaper and, more usefully, would make it structurally impossible for the measured schema to drift from the sent one.
• Third copy of the predicate. providerId === 'llamacpp' || providerId === 'llamacpp-upstream' now appears twice in custom-chat-transport.ts (yours, plus :648). A one-line isLlamaCppProvider() next to the existing isLocalProviderName import would collapse them.

Two housekeeping notes

• Your description mentions DropdownModelProvider.connected.test.tsx failing as a pre-existing issue — it doesn't reproduce here. The full suite is green on current main and on your branch. Something local, or it was fixed on main after you ran it; worth a re-check so the claim doesn't outlive the problem.
• No CI has run on this branch, so my local run is the only signal so far. Also: fix: stop estimating ChatGPT subscription token speed #270 and fix(linux): isolate host processes from AppImage libraries #273 both touch docs/decisions/INDEX.md in the same place, so whichever of the three lands first will leave the other two with a trivial conflict on the index.

Answer the numbers question and this is good to go — I found no defect in the change itself 🔧

---

Generated by Claude Code

[Vect0rM]

Thanks for chasing this down — the diagnosis here (Firecrawl's 25 tools expanding into a 168 KB / 596-rule grammar, and llama.cpp's MAX_REPETITION_THRESHOLD rejecting it before generation) is exactly right, and the upstream link to llama-grammar.cpp#L485-L496 is the thing that made it easy to confirm.

Closing this as superseded: the same fix has since landed on main in #276 as withGrammarSafeToolSchemas in web-app/src/lib/custom-chat-transport-helpers.ts. It arrived at the same place from the same analysis, and covers a bit more ground:

• it also applies to llamacpp-server (self-hosted llama.cpp behind an OpenAI-compatible URL builds the same grammar), not just llamacpp / llamacpp-upstream;
• it keeps pattern when the pattern is grammar-safe, dropping it only when a quantifier exceeds MAX_PATTERN_REPETITION (256), rather than removing every pattern and format;
• it is applied at the request boundary against effectiveProviderName, so the tool cache key does not have to carry the provider.

Net effect is identical for the Firecrawl case, so there is nothing left to port from this branch. Sorry for the duplicated effort — that is on our side for not flagging the overlap sooner.

We truly appreciate your help in making Atomic Chat better!