Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ decision and its measurements live (`docs/maxusai/`). The
| | upstream ollama | this fork | record |
|---|---|---|---|
| **`think` + `format` in one request** | since v0.34.4, one pass: the grammar applies from the first token, after free thinking that ends at the parser's closing strings | the same by default. `OLLAMA_FORMAT_TWO_PASS=1` keeps the fork's two-pass flow: pass one thinks without the grammar and stops at the think-close marker, and pass two answers under it. Production runs it, because on MLX it lets the thinking draft | ADR 0045 (0002/0004/0010) |
| **whitespace in structured output (MLX)** | the JSON grammar allows an unlimited whitespace run between tokens, so a stalled decode can fill `num_predict` with indentation and never close the answer | runs bounded at 32 characters per separator (`max_whitespace_cnt` on the `json_schema` element) | ADR 0035 (proposed) |
| **whitespace in structured output (MLX)** | the JSON grammar allows an unlimited whitespace run between tokens, so a stalled decode can fill `num_predict` with indentation and never close the answer | runs bounded at 22 characters per separator (`max_whitespace_cnt` on the `json_schema` element), the length of llama.cpp's longest run for a JSON schema but not its shape (any 22 whitespace characters); GGUF's `format: "json"` grammar uses llama.cpp's own rule, one per gap | ADR 0035 (proposed; amended 2026-10-12) |
| **drafting under a grammar (MLX)** | always on | on by default to match upstream; `OLLAMA_MLX_DRAFT_UNDER_GRAMMAR=0` restores the gate, and production sets it | ADR 0033 |
| **stop sequences (MLX)** | not honoured by the MLX runner | honoured, with a possible stop prefix held back until it matches or the stream ends | `mlxrunner/stopper.go` |
| **KV cache type** | one global `OLLAMA_KV_CACHE_TYPE` | per model, with K/V pair syntax; production runs f16 on every platform, and a quantized cache is per model or per request | ADR 0005, ADR 0043 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,25 @@ per document, and does not touch the schema it wraps.
documents that were lost is the confirmation, and it has not been done.
- Nothing changes for the GGUF path, which passes the schema to llama-server and
is bounded by llama.cpp's own converter.

## Amendment 2026-10-12: 22, the GGUF run, and both bounds checked by a matcher

**What moved it.** muse-glimmer on MLX in single pass (`OLLAMA_FORMAT_TWO_PASS=0`), answering after its message header, padded the first and last slot of every object to the bound. `format_schema` on `30b-nvfp4` returned `{` + 33 spaces, the value, about 23 spaces and `}` (#463, #464). The bound held: stock 0.40.2 has no bound and filled all 16384 tokens with whitespace (FAIL). The cost of the padding was the bound itself.

**Decision, amended:**
- **MLX:** `max_whitespace_cnt` is **22**. That is the length of the longest run llama.cpp allows for a JSON schema: `SPACE_RULE`, two newlines and 20 spaces or tabs. The runners agree on that length only, not on the shape: `max_whitespace_cnt` counts any whitespace, so MLX accepts 22 spaces in one gap, where `SPACE_RULE` allows at most one space. A newline plus ten levels of two-space indent (21 characters) still fits.
- **GGUF, `format: "json"`:** the GGUF path is not wholly bounded. A schema goes to llama-server's converter, which is bounded. But `format: "json"` uses the fork's own grammar (`grammarJSON`, `llm/llama_server.go`), and its `ws ::= ([ \t\n] ws)?` was unbounded. It is now `ws ::= (" " | "\n"{1,2} [ \t]{0,20})?`: llama.cpp's rule, written as an optional group, because xgrammar's parser rejects a leading empty alternative.
- **Placement (2026-10-13, after review on the PR):** the rule's bound held only per rule, and two rules sat side by side in three places: `"{" ws (...)? ws "}"`, the same for arrays, and a literal's trailing `ws` before `ws "}"` or `ws "]"`. A gap could run to 44 characters, and `{ }` was accepted. The grammar now has the shape of llama.cpp's `grammars/json.gbnf`: whitespace follows `{`, `[`, `:`, `,` and every value, and nothing puts it before `}` or `]`. So each gap is one `ws`, and no run is longer than 22. `{ }` is refused, as json.gbnf refuses it; `{ }` and `{` + newline + `}` are accepted.
- Two differences from json.gbnf were kept from the old grammar. The root object ends at its `}`, with no trailing whitespace, so the grammar completes there. And there is no whitespace between a key and its `:`.
- One placement is new: whitespace after a string, number, object or array value, before a `,`. The old grammar allowed it only after `true`, `false` and `null`; json.gbnf allows it after every value.
- **Not taken: patching xgrammar to llama.cpp's exact rule.** On MLX that would cut glimmer's padding to one space per slot. It costs a native patch on every xgrammar bump, and it moves every MLX format request's mask. xgrammar v0.2.7 hardcodes `any_whitespace=true` for structural-tag `json_schema` elements (`cpp/structural_tag.cc`), so only the count can be set. Revisit if xgrammar adds a whitespace-rule option.

**Verification, amended:**
- `TestRequestGrammarBoundsWhitespaceRunLength` drives `requestGrammar`'s tag through xgrammar one byte at a time, for a plain format, a format after thinking, and `"json"` after a header. It checks that 22 spaces are accepted, 23 refused, and two newlines plus 20 spaces accepted. It proves the run's length, not llama.cpp's shape.
- `TestGrammarJSONBoundsWhitespace` checks `grammarJSON` with a reference GBNF recognizer written in Go (`llm/gbnf_reference_test.go`), so it runs everywhere. The cases include two gaps at the bound side by side in an empty object, in an empty array and after a literal, and `{ }`. `TestGrammarJSONBoundsWhitespaceXGrammar` drives the same cases through xgrammar's grammar parser one byte at a time. llama.cpp's engine has no Go entry point.
- Both fail on the old grammars. The xgrammar tests run wherever the MLX payload is built and skip elsewhere. The "not yet verified end to end" above still holds for MLX documents; the real-model check of this amendment is the GGUF and MLX A/B on the branch's PR.

**Upstream:**
- ollama/ollama has no bound on MLX. ollama/ollama#18567 (a whitespace runaway) was closed as standard behaviour.
- ollama/ollama#18569 (a cap of 32) was closed untested.
- The fork's bound has not been offered upstream.
Loading