Skip to content

Read grammar vocabulary from llm_config for Nemotron Omni - #953

Merged
ashhart merged 2 commits into
ashhart:mainfrom
swbiggart:fix/grammar-llm-config-vocab-upstream-20261016
Oct 18, 2026
Merged

ashhart merged 2 commits into
ashhart:mainfrom
swbiggart:fix/grammar-llm-config-vocab-upstream-20261016

Conversation

@swbiggart

Copy link
Copy Markdown
Contributor

Fixes #951.

Nemotron Omni checkpoints store the text vocabulary in llm_config. The grammar compiler only checked text_config and the root, causing native 1.0.12 to reject named/required tools and strict JSON schemas for Super VL. Read the vocabulary in text_config → llm_config → root order and reject invalid values before casting to u32.

The missing lookup was originally proposed by BobClawblaw in #897, specifically commit 5f094543. This PR extracts that fix from the broader Bolt contribution and adds focused regression coverage. The commit includes BobClawblaw's co-author trailer.

The patch uses the existing grammar module, camelCase helper naming, inline arena-backed Zig tests and normal server test registration. It adds no dependencies and preserves existing nested-text priority and root fallback.

Validation on Zig 0.17.0, macOS arm64:

  • Reduced Omni and conflicting-root regressions fail with the old lookup and pass with the fix; existing-layout and integer-boundary cases pass.
  • Full server suite: 370/370; JSON: 5/5; engine seams: 168/168.
  • Host/golden targets: 208/208 build steps succeed; goldens have zero unexpected differences (309 equal, 10 known differences).
  • Zig formatting and whitespace checks pass.

A patched native binary has not been qualified on CUDA. The released-binary config workaround described in #951 supports the diagnosis; it is separate from source-patch runtime qualification.

Backport the llm_config lookup proposed by BobClawblaw in
ashhart#897, commit 5f09454.
Nemotron Omni stores its text vocabulary there, so native 1.0.12 refuses
forced tools and structured output despite serving ordinary requests.

Isolate the lookup in the existing grammar module, retain text_config
priority and root fallback, and check the u32 range before casting.
Add the reduced Omni regression and existing-layout precedence cases to
the server's normal test harness.

Co-authored-by: BobClawblaw <BobClawblaw@users.noreply.github.com>

@cwschroeder cwschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source and CPU review of 80559ad2 on main 46a08c18. The text_config → llm_config → root order matches the documented layouts, Value.get safely skips non-objects, and the integer check excludes bool/string/float values before the cast. The helper is used by grammarCompiler and registered in the standard server test root.

With Zig 0.17.0 on macOS arm64, I independently ran the full CPU server wrapper: 370 server tests, 5 JSON tests and 168 engine-seam tests pass. The focused vocabulary tests also pass in Fast. As a negative control, I copied the server module into a temporary tree and replaced only the helper body with the unchanged main lookup: both new regression tests fail in Debug and Fast. No tracked source was changed. Whole-tree format and patch whitespace pass; lean is 517 on both base and head, zero new. Source review found no new material regression.

One nonblocking ABI-range note is below; the downstream narrowing predates this patch and does not affect the Nemotron vocabulary of 131072. The actual current-head No GPU run 38909568688 is action_required with zero jobs, so remote CI has not executed. These checks do not qualify a patched GPU binary or constrained model output.

Comment thread zig/src/server/grammar.zig Outdated
@easthambuilds

Copy link
Copy Markdown
Contributor

Tested on hardware: one DGX Spark (GB10), driver 580.178.04. nvidia/NVIDIA-Nemotron-3.5-Super-VL-120B-A12B-NVFP4 at 82d93699acd3be9e4617c53360fe5b3f5dbeb1b3, --precision checkpoint (default), 16,384-token window. I compared the 1.0.12 release binary with this branch at 022344e9, built with zig build native -Dsm=121 (Zig 0.17.0, nvcc 13.0.88) and run with the release archive's lib/libtfgrammar.so. Requests were greedy, max_tokens 128, thinking off, using the strict schema and the lookup_symbol tool from #951.

request 1.0.12 this PR
response_format json_schema, strict 400 structured output needs the checkpoint's config.json vocab_size 200, {"ok": true}, matches the schema
tool_choice: "required" 400, same message 200, lookup_symbol({"symbol":"WidgetFactory"}), token_sha 8f20367416dd
tool_choice naming lookup_symbol 400, same message 200, the same call, 8f20367416dd
tools, tool_choice auto 200, 8f20367416dd 200, 8f20367416dd
plain chat, 64 tokens (control) 200, 8f6ae430c55e 200, 8f6ae430c55e

The forced-tool replies are token-identical to the model's own unconstrained tool choice, and the unconstrained control matches 1.0.12, so the grammar only constrains where it is asked to.

@cwschroeder cwschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked 022344e9, specifically the signed-C-range follow-up. With Zig 0.17.0 on macOS arm64, test-server-cpu passes 370 server tests and 5 JSON tests; both focused vocabulary tests also pass in Fast. In a temporary copy, reverting only maxInt(c_int) to maxInt(u32) makes the boundary regression fail on 2147483648 while the Omni lookup control still passes. Whole-tree format and patch whitespace pass.

Astra found no material regression in this adoption delta. This closes my lookup narrowing note, including fallback from an oversized nested value. These tests exercise vocabulary selection, not the C grammar compiler at extreme sizes. Current-head remote checks are absent.

The separate GB10 receipt is pinned to the same head and adds strict-schema/forced-tool evidence with an unchanged plain-chat control. That is tovereast's hardware run; I have not run or deployed a GPU binary here.

@ashhart
ashhart merged commit 93d1116 into ashhart:main Oct 18, 2026
@ashhart

ashhart commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Thank you, @swbiggart, for nested vocabulary discovery and the signed C-ABI boundary check.

We merged both commits for 1.1.0 in 93d1116, retaining @BobClawblaw's co-author credit. The server checks text_config, then llm_config, then the root, accepting a positive vocabulary size only when it fits c_int.

The regressions cover nesting, precedence, invalid values and signed boundaries. This addresses #951's Omni grammar vocabulary mismatch.

Posted by Ash's AI Assistant Synthena

outcastofmusic added a commit to outcastofmusic/TensorFold that referenced this pull request Oct 18, 2026
Bolt's config.json (model_type nemotron_h_omni) keeps vocab_size under
llm_config. grammar.vocab_size read text_config and the top level only,
found nothing, and every response_format or guided request got a 400.
It now reads text_config, llm_config, then the top level, and a value
that is a bool, not positive, or past 2^31 - 1 falls through to the next
place. Before, a JSON true passed isinstance(.., int) as a width of 1.

Ports upstream TensorFold 80559ad (llm_config lookup, from
BobClawblaw's ashhart#897) and 022344e (signed C int bound); ashhart#951, ashhart#953.

Tests: tests/test_grammar_vocab.py, with Bolt's real config.json (skipped
when absent) and upstream's synthetic cases: 11 of 22 failed before,
22 of 22 pass after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qj4LeSvZn6iJnKY4dqjKJ8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native 1.0.12 grammar vocabulary lookup misses llm_config for Nemotron Super VL

4 participants