fix(hf): set whitespace_flexible=True in grammar_from_json_schema to prevent silent array collapse - #1513
fix(hf): set whitespace_flexible=True in grammar_from_json_schema to prevent silent array collapse#1513planetf1 wants to merge 4 commits into
Conversation
… calls
With whitespace_flexible=False (compact JSON), llguidance constructs a grammar
where the highest-probability token after opening an array '[' is immediately
']', silently collapsing structured output to empty arrays (e.g.
{"result":[]}) under greedy decoding. No exception is raised and the caller
cannot distinguish a correct empty list from a grammar-induced collapse.
This bug was introduced in PR generative-computing#288 as a mechanical outlines->llguidance port.
The llguidance library's own default for whitespace_flexible is True; the
original False was never intentional.
Fix all three grammar_from_json_schema call sites in LocalHFBackend:
- _generate_from_context_with_kv_cache
- _generate_from_context_standard
- _generate_from_raw
Regression tests added for all three call sites (mock-based, no real model).
Fixes generative-computing#1510
Assisted-by: IBM Bob
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
|
@nrfulton @jakelorocco any thoughts on this? See issue for more details - it came up when debugging some unexpected responses with melee |
jakelorocco
left a comment
There was a problem hiding this comment.
This seems like a reasonable fix.
However, we did previously hit a lot of issues with json generation where the model would generate a large amount of whitespace and then run out of tokens.
@planetf1, would you be able to run your test set of 250 tests with a few other parameters to see if other options are able to keep performance?
It looks like llguidance has a far number of parameters that we could potentially tweak to get the performance improvements you point out without risking unlimited whitespace: https://github.com/guidance-ai/llguidance/blob/main/docs/json_schema.md#whitespace-handling.
I think changing the default key / item separators might work. whitespace_pattern seems like a very promising option though where we could basically mimic the whitespace_flexible pattern but with an upper bound on the number of whitespace tokens allowed.
From the docs:
whitespace_flexible: true is equivalent to whitespace_pattern: r"[\x20\x0A\x0D\x09]+"
And we could do:
r"[\x20\x0A\x0D\x09]{1,8}"
I'll approve; but if you don't have time to run additional tests, could you please open an issue about investigating this upper bounding / additional parameters and publish your qualitative tests somewhere?
- Rewrite regression tests to model each method's actual async control flow instead of swallowing exceptions with a silent try/except. - Hoist the llguidance grammar defaults into a module constant typed as JsonCompileOptions (imported under TYPE_CHECKING from the private llguidance._lib submodule, since it isn't re-exported publicly) to keep mypy accurate without resorting to Any. - Remove dead mock setup (allocate_token_bitmask, unused LLMatcher return value) left over from the old test scaffolding. - Document the whitespace_flexible=True token overhead in the HF integration guide. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
The chat_completion_request_to_transformers_inputs() grammar_from_json_schema() call (the OpenAI-compatible /chat/completions path used by m serve) omitted defaults entirely. JsonCompileOptions.whitespace_flexible's docstring claims it "defaults to true" when omitted, but empirically it does not: omitting defaults produces no explicit whitespace_flexible setting at all, and falls through to the underlying JsonCompiler's real default of False -- so this call site was exposed to the same generative-computing#1510 collapse bug as the three already-fixed call sites in LocalHFBackend. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
- overrides= instead of defaults= at the m serve grammar call site, since its schema comes straight off the wire and a caller could otherwise embed x-guidance.whitespace_flexible=false to defeat a defaults= pass - move _LLGUIDANCE_GRAMMAR_DEFAULTS to util.py as the single definition, imported by huggingface.py instead of duplicated - correct util.py comment: llguidance's own default is already True when defaults is omitted, so the m serve call site was never affected by generative-computing#1510 - fix test mock signature regression from the overrides= swap, and a stale test comment - reword docs token-overhead note to match Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Summary
Closes #1510 — fix silent array truncation in
LocalHFBackendstructured output.Why
LocalHFBackendwas callingllguidance.LLMatcher.grammar_from_json_schemawithdefaults={"whitespace_flexible": False}at three call sites. This compiles a compact-JSON grammar (no whitespace between tokens). Under greedy decoding, this puts the model in a state where the highest-probability grammar-compatible token immediately after opening an array[closes it early — either immediately (], producing{"result":[]}) or after the first item.The result is a silent wrong answer: no exception is raised, the output is schema-valid, and the caller cannot detect the failure.
The
Falsewas introduced in PR #288 as a mechanicaloutlines → llguidanceport and was never intentional. llguidance's own_lib.pyistub documents thatwhitespace_flexibledefaults toTruewhendefaultsis omitted — verified empirically (byte-for-byte identical compiled grammar to passingTrueexplicitly). So the bug was scoped to exactly the three call sites that explicitly passedFalse; a fourth call site, inchat_completion_request_to_transformers_inputs(them serveOpenAI-compatible path), already omitteddefaultsentirely and was never affected by #1510.What changed
mellea/backends/huggingface.pywhitespace_flexible: False→Trueat all threegrammar_from_json_schemacall sites (_generate_from_context_with_kv_cache,_generate_from_context_standard,_generate_from_raw); explanatory comment referencing #1510; imports the shared_LLGUIDANCE_GRAMMAR_DEFAULTSconstant fromutil.pyinstead of redefining itmellea/formatters/granite/base/util.py_LLGUIDANCE_GRAMMAR_DEFAULTS(single source of truth). The fourth call site, inchat_completion_request_to_transformers_inputs, was not affected by #1510 (it already got llguidance's implicitTruedefault) — hardened anyway with an explicitoverrides=pass, since this endpoint takes its schema straight off the wire and a caller could otherwise embed their ownx-guidance.whitespace_flexible: falseto defeat adefaults=pass. The threehuggingface.pysites usedefaults=since their schemas come from local Pydantic models, not caller-controlled inputtest/backends/test_huggingface_unit.pywhitespace_flexible=Trueis passed. The fourth test exercisesutil.py's function but lives here deliberately, alongside the other three #1510 regression tests, rather than in a new dedicated test filedocs/docs/integrations/huggingface.mdwhitespace_flexible=Truebehaviour and the measured token overheadBefore / After
Empirical evidence
Tested across three models, three devices, 10 trials, 5 schema variants, n=1–8 items:
wf=FalseOKwf=TrueOKgranite-4.1-3bgranite-4.1-3bgranite-4.1-3bgranite-4.1-8bgranite-4.1-8bgranite-4.1-8bgranite-4.0-microwf=Trueis clean for 3b and 8b on all devices. The 20wf=Truefailures on micro aresingle_str_field n=2producing 1 item instead of 2 on every trial — a model-level instruction-following limitation on that schema, not caused by this bug.Token overhead
wf=Trueproduces spaced JSON, using ~1.5× more tokens for the same content. Callers with a tight explicitModelOption.MAX_NEW_TOKENSbudget should size accordingly. Generation that runs to EOS still completes, but consumes proportionally more tokens getting there.Testing
The four new regression tests each fail on their respective pre-fix code and pass on the fix.