feat(hy3): tool parser + reasoning parser + reasoning_effort default override (PR 2 of 3)#1070
Merged
Conversation
added 4 commits
July 9, 2026 14:32
Wire the Tencent Hunyuan 3 (Hy3) tool-call parser as PR 2 of 3 for the vendor initiative kicked off in #1069. Format handled — canonical Hy3 emission is ``<tool_call:opensource>NAME<tool_sep:opensource>{json} <end_of_tool_call:opensource>`` with the XML-pair ``<arg_key>K</arg_key><arg_value>V</arg_value>`` variant when the model transcribes each argument separately. Every tag is matched with ``(?::[\\w-]+)?`` so future model revisions can drop or swap the ``:opensource`` label without a code change. Defensive close-tag strip — on the 4-bit preview checkpoint the model occasionally emits ``<tool_call:opensource>NAME</arg_value:opensource>``, skipping ``<tool_sep>`` + the arguments block. Empirically observed on 4 of 32 real prompts against ``mlx-community/Hy3-preview-4bit`` in the 2026-07-09 spike (agent ``ac2851864dbd17b07``). The recovery: extract the name, return ``arguments="{}"``, mark tools_called=True so the user doesn't see an empty ``tool_calls`` array and think the model refused. Registered under both ``hy_v3`` and ``hy3`` (CLI convenience). Declares the new ``hy3_native`` wire format label in ``WIRE_FORMAT_LABELS`` so the structural audit stays honest. Adds streaming parity fixture and audit exemption entries covering the alias pair. 17 unit tests cover canonical / suffix-less / malformed-close / XML-pair / multi-key / multiple-calls / think-prefix / no-tool / streaming buffer-until-close paths. Fixes half of the upstream PR #1211 review feedback we surfaced in comment 4927710973.
…regex) Add ``Hy3ReasoningParser`` — a suffix-tolerant subclass of ``Qwen3ReasoningParser`` that normalizes ``<think:opensource>`` / ``</think:opensource>`` back to the plain ``<think>`` / ``</think>`` shape before delegating. This keeps the entire qwen3 state machine (Case 1/2/3/4, streaming multi-block, SSE-boundary withhold, tool-call promotion, D-STOP-THINK finalize suppression) in one place — no 1500-line duplication. Registered under both ``hy_v3`` and ``hy3`` in the reasoning parser registry. Matches the closed set of think-tag suffixes with ``(?::[\\w-]+)?`` so a future model revision that swaps ``:opensource`` for ``:v1`` / ``:internal`` continues to work. 13 unit tests cover suffixed / plain / mixed / implicit-close / no-tag extract paths + tag-atomic streaming + finalize-on-truncation delegation + ``is_open_in_think`` recognition of suffixed openers. Fixes half of the upstream PR #1211 review feedback we surfaced in comment 4927711484.
…or hy3
The Hy3 chat_template.jinja defaults ``reasoning_effort=no_think`` which
empirically returns "France" instead of "Paris" on factual-recall
questions (upstream PR #1211 comment 4927711484). Override the default
to ``low`` at the ``apply_chat_template`` boundary so out-of-the-box
requests produce correct answers without the client having to learn the
template kwarg.
Injection is gated on:
* ``model_name`` matches ``hy3|hy-v3|hunyuan.?3`` (case-insensitive)
* ``enable_thinking`` is not ``False`` — an explicit client opt-out
for no-think is respected
Non-Hy3 models never see the kwarg, so other tokenizers don't
TypeError. The existing retry-on-TypeError chain also drops
``reasoning_effort`` if a future / older Hy3 checkpoint rejects it.
Chosen NOT to modify:
* ``chat_template.jinja`` itself (that's upstream / vendor-owned)
* ``tokenizer_config.json`` (would bake a fork into the cached
tokenizer and drift from upstream)
16 unit tests cover the Hy3 detection predicate (positive + negative
matches) plus the injection semantics (fires when enabled, held when
disabled, absent on non-Hy3, orthogonal to enable_thinking=True,
recovers on TypeError).
…ew-4bit Update the ``hy3-preview-4bit`` alias to use the new suffix-tolerant ``hy_v3`` tool + reasoning parsers wired in the previous three commits. Add a regex fallback in ``model_auto_config._MODEL_PATTERNS`` so a direct-serve of ``mlx-community/Hy3-preview-4bit`` (or any future community re-quant that matches ``hy3|hy-v3|hunyuan.?3``) inherits the same routing without requiring an alias-profile lookup. Closed-key schema respected — the alias uses only ``hf_path`` / ``tool_call_parser`` / ``reasoning_parser`` / ``is_hybrid`` / ``is_moe`` / ``supports_spec_decode`` / ``min_memory_gb``, mirroring the field set of nearest neighbors. ``test_aliases_contract.py -k hy3`` — 14 tests PASS.
Contributor
PR #1070 validation scorecardTitle: feat(hy3): tool parser + reasoning parser + reasoning_effort default override (PR 2 of 3) Verdict: MERGE-SAFE
|
…hain Four fixes from the pr_validate codex adversarial review scorecard. BLOCKING #1 (hy_v3_tool_parser streaming gate). The earlier gate treated ``_TOOL_CALL_OPEN.search(current_text)`` as "in a tool call", so ANY completed ``<tool_call>...<end_of_tool_call>`` earlier in the stream suppressed every subsequent plain-content delta forever. Fixed by scoping to the LAST unclosed opener (``_last_unclosed_tool_call_position``) — a completed pair no longer blocks post-call passthrough. Regression pinned in ``test_streaming_content_after_completed_tool_call_is_not_dropped``. BLOCKING #2 (hy3_parser SSE-boundary partial-tag straddle). When ``<think:opensource>`` split across SSE chunks, the base qwen3 partial-tag withhold didn't cover the ``:[label]`` region, so ``current_norm`` collapsed the completed tag on delta 2 while ``previous_norm`` still ended with ``<think:opens`` — invariant broken, tag fragments leaked into the wrong channel. Added ``_hy3_straddle_suffix_len`` withhold covering the ``:[\\w-]*`` suffix region so ``previous_norm + delta_norm == current_norm`` holds by construction on every tick. Regression pinned in ``test_streaming_suffix_tag_split_across_boundary_preserves_invariant``. BLOCKING #3 (hy_v3_tool_parser SSE-split close detection). The earlier close-tag check searched only ``delta_text`` — a close tag split across two SSE chunks (e.g. ``<end_of_tool_c`` then ``all>``) would never fire and the parser hung indefinitely. Replaced with a state-transition detector: parse when ``previous_text`` had an unclosed opener AND ``current_text`` no longer does. Regression pinned in ``test_streaming_close_split_across_sse_boundary_still_emits``. NIT #4 (chat_template TypeError retry drops reasoning_effort). The single-stage retry dropped both ``enable_thinking`` and ``reasoning_effort`` so a Hy3 checkpoint that supported the effort override but rejected ``enable_thinking`` lost the ``low`` value. Split into two-stage retry: first drop ``enable_thinking``, only drop ``reasoning_effort`` on the second TypeError. Regression pinned in ``test_hy3_default_dropped_only_after_second_type_error`` + ``test_hy3_default_dropped_when_reasoning_effort_alone_is_rejected``. Also: ruff format autoclean on 3 files + F401 unused-import fixes on 2 tests. All 50 HY3-specific tests pass. Full targeted parser/reasoning/aliases suite (2857 tests) still green.
…-call turn purity Codex round-2 review of PR #1070 flagged two BLOCKING findings: 1. **XML-pair first-arg </arg_value> flushed the call early.** The round-1 fix restricted the streaming pending-close check to the canonical <end_of_tool_call> tag only, but the malformed-close salvage case (<tool_call>NAME</arg_value> with no <tool_sep>) then hung until timeout. This commit makes the close-tag definition MODE-DEPENDENT: - **XML-pair mode** (a <tool_sep> is in flight after the opener): ONLY <end_of_tool_call> counts as close. Each argument value legitimately ends with </arg_value> and must not flush. - **Malformed / no-sep mode** (no <tool_sep> yet): both <end_of_tool_call> AND </arg_value> count, preserving the salvage path for 4-bit numerical noise cases. New regression test `test_streaming_xml_pair_first_arg_close_does_not_emit_early` locks the XML-pair contract; the existing malformed-close test still covers the salvage path. 2. **Post-tool-call plain content violated the OpenAI spec.** Round-1 kept post-close content deltas flowing to preserve any trailing prose, but round-2 pointed out that OpenAI-compatible clients treat tool_calls and content as MUTUALLY EXCLUSIVE for a single assistant turn. Mixed tool/content streams break tool-call dispatch. This commit adopts the `Glm47ToolParser` policy: once ANY <tool_call> opener has appeared in `current_text`, suppress every content delta after that (the close-transition delta still emits the tool_calls array). `test_streaming_content_after_completed_tool_call_is_not_dropped` (round-1 assertion enforcing the wrong behavior) is renamed to `_is_suppressed` and now locks the round-2 contract. Refs PR #1070. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… boundary regex Round-3 codex flagged 3 BLOCKING + 2 NIT findings; this commit addresses all five and adds regression tests for the load-bearing three. BLOCKING #1 — vllm_mlx/tool_parsers/hy_v3_tool_parser.py `_last_unclosed_tool_call_position` gated XML-pair-mode detection on `<tool_sep>` only. Sep-less XML-pair bodies (some 4-bit checkpoints emit `<tool_call>NAME<arg_key>...` directly, or the sep arrives in a later SSE delta than the first arg pair) fell to salvage-mode and flushed the call with truncated `arguments={}` on the first `</arg_value>`. Extend the XML-pair signal to include `<arg_key>` / `<arg_value>` openers so the sep-less stream still waits for `<end_of_tool_call>`. Also extended `_parse_hy3_body` (extract-path) to detect the same sep-less shape: if no `<tool_sep>` but an arg-key/value opener is present in the body, treat that opener as the implicit separator so the name and args parse correctly rather than the whole XML content becoming the "name". Regression test: `test_streaming_xml_pair_without_sep_does_not_flush_early`. BLOCKING #2 — vllm_mlx/tool_parsers/hy_v3_tool_parser.py When `valid_names` is set and every parsed call is rejected, `residual_parts` omitted the raw span and returned `tools_called=False` with empty content — silently erasing the model output. Fix: preserve the raw XML span in `residual_parts` on rejection so the caller can diagnose the hallucination. The exclusive-turn contract still applies when SOME calls are valid (`tools_called=True` + `content=None`). Regression tests: `test_valid_names_filter_preserves_rejected_span_in_content` + `test_valid_names_filter_preserves_mixed_valid_and_rejected`. BLOCKING #3 — vllm_mlx/utils/chat_template.py `template_kwargs["reasoning_effort"] = "low"` unconditionally overwrote any caller-provided value. Fix: use `setdefault` so a future request-side plumb-through (or an intermediary that pre-populates the dict) can still supply explicit graded effort (`medium` / `high`) and see it survive to the tokenizer. NIT #4 — vllm_mlx/utils/chat_template.py `_HY3_MODEL_NAME_RE` used unanchored `hunyuan.?3` which matched substrings inside unrelated names (`not-hunyuanx3-test`). Tighten to family separators `[/_.-]` plus start / end of string. Added negative parametrize cases for the boundary behaviour. NIT #5 — tests/test_hy3_chat_template_default.py `assert ... enable_thinking is not None` was too loose (accepted any truthy/invalid value). Tightened to `is True` — the expected default when the caller doesn't override and the model isn't a coder alias. All 57 HY3-family tests pass. ruff check + format clean.
…SON-body salvage Codex round-4 flagged 2 BLOCKING findings on PR #1070. **BLOCKING #1** — `vllm_mlx/model_auto_config.py` The Hy3 auto-config regex was still unanchored (``hy3|hy-v3|hunyuan.?3``), matching substrings inside unrelated HF paths and auto-wiring them to the Hy3 tool/reasoning parsers. Codex round-3 fixed the same pattern in ``chat_template.py`` but missed the sibling copy in ``model_auto_config.py`` (two independent detection sites, one shared policy). Tighten to the same family-boundary form: start-of-string OR path/name separator (``/`` ``_`` ``.`` ``-``) precedes the family root, symmetric boundary follows. Regression test ``test_auto_config_regex_boundary_rejects_incidental_substring`` directly probes the ``_MODEL_PATTERNS`` entry rather than only the sibling ``_HY3_MODEL_NAME_RE``, so future divergence between the two detectors trips the test. **BLOCKING #2** — `vllm_mlx/tool_parsers/hy_v3_tool_parser.py` The round-3 fix used a broad XML-pair mode discriminator (any of ``<tool_sep>`` / ``<arg_key>`` open / ``<arg_value>`` open in the body). But ``<tool_sep>`` is present in BOTH JSON-body streams (``NAME<tool_sep>{...}``) and XML-pair streams — so a JSON body with a stray malformed ``</arg_value>`` in the tail (4-bit noise corrupting the JSON) fell to canonical-only mode and hung forever waiting for ``<end_of_tool_call>``. Refactor to a per-``</arg_value>`` body-prefix inspection helper ``_closed_after_opener``: a ``</arg_value>`` fires the salvage-close ONLY when its body-prefix contains no ``<arg_key>`` opener AND no ``<arg_value>`` opener. This handles both shapes correctly: - JSON body with tail ``</arg_value>`` — no arg openers in prefix, salvage fires, parser emits rather than hangs. - Real XML-pair body — first ``<arg_value>`` opener presence blocks the immediately-following ``</arg_value>`` from firing salvage, so canonical close remains the only trigger. Regression test ``test_streaming_json_body_with_corrupted_arg_value_tail_salvages``. Refs PR #1070. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…+ JSON literal salvage Codex round-5 flagged 3 BLOCKING SSE-boundary edge cases on PR #1070. **BLOCKING #1** — `vllm_mlx/tool_parsers/hy_v3_tool_parser.py` A ``<tool_call:opensource>`` opener split across SSE deltas (e.g. delta 1 ``"Sure, <tool_ca"``, delta 2 ``"ll:opensource>..."``) leaked the partial-opener bytes as ``content`` — an OpenAI-compatible client sees tool markup rendered as prose seconds before the tool-call turn engages. Add ``_tool_call_open_straddle_suffix_len(text)`` which returns the byte length of a trailing strict-prefix of ``<tool_call>`` / ``<tool_call:LABEL>``, and slice those bytes off the delta on the "no opener seen yet" branch. The withhold is scoped: a straddle larger than the current delta means the reserve happened on a prior tick, so the delta itself is unrelated and we emit ``None`` instead of over-reaching. Regression test: ``test_streaming_partial_opener_does_not_leak_as_content``. **BLOCKING #2** — `vllm_mlx/reasoning/hy3_parser.py` The round-1 straddle regex required a colon to be present: ``<think:[\w-]*$``. A boundary split ``"<think"`` then ``":opensource>Hello"`` released ``<think`` from the qwen3 base partial-tag hold (qwen3 releases when the next char isn't ``>``); the following ``:opensource>`` then leaked as content. Widen the regex to ``<think(?::[\w-]*)?$`` (and the close-tag mirror ``</think(?::[\w-]*)?$``) so BOTH the bare ``<think`` prefix AND the ``<think:LABEL`` labelled prefix straddle uniformly. Qwen3's base hold still handles the ``:``-less normalised form after our withhold completes. Regression tests: ``test_streaming_pre_colon_prefix_still_withholds`` + ``test_streaming_pre_colon_close_prefix_still_withholds``. **BLOCKING #3** — `vllm_mlx/tool_parsers/hy_v3_tool_parser.py` ``_parse_hy3_body`` truncated the JSON tail at the first ``</arg_value>`` before ``json.loads``. A valid JSON payload whose STRING VALUE contains the literal substring ``</arg_value>`` (``{"snippet": "see </arg_value> here"}``) was corrupted into empty or truncated args. Switch to ``json.JSONDecoder.raw_decode`` which consumes only a well-formed JSON prefix of the tail; any structural residue after that (the malformed close, whitespace, extra tags) is discarded harmlessly. Adds a module-level ``_JSON_DECODER`` singleton to avoid per-call allocation. Regression test: ``test_json_body_containing_literal_arg_value_close_parses_correctly``. All 70 HY3-family tests pass. ruff check + format clean. Refs PR #1070. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ions Round-5 codex flagged 2 BLOCKING + 1 NIT on PR #1070. The straddle-based tool_call-opener-withhold and JSON body raw_decode fixes landed in an earlier operator patch on this branch; this commit adds the third BLOCKING fix (streaming think-tag straddle) and regression tests for both the fixed bug and the misdiagnosed one. **BLOCKING #1** — `vllm_mlx/tool_parsers/hy_v3_tool_parser.py` `extract_tool_calls_streaming` mid-content branch stripped inline `<think>...</think>` pairs from `delta_text` only. When the opener landed in `previous_text` and the closer arrived in `delta_text`, the `re.sub` pattern (which requires opener-in-same-string) did not match, leaving `</think:opensource>` plus the tail reasoning text to leak into emitted content — the user saw `about this</think:opensource>reasoning done` in their content stream. Refactor to a straddle-tolerant diff: strip think spans from both `previous_text` and `current_text` end-to-end (treating any unclosed opener in prev as a boundary that JUST closed in this delta), then emit only the difference between the two clean strings. One code path handles every straddle pattern — opener-in-prev + closer-in-delta, opener-and-closer-in-delta, multiple spans, etc. Regression test `test_streaming_think_close_split_across_deltas_does_not_leak_close_tag` locks the split-delta scenario codex flagged. **BLOCKING #2** — codex false positive Codex claimed the sep-less XML-pair stream `<tool_call>fn<arg_key>x</arg_key>` → `<arg_value>1</arg_value>` → `<end_of_tool_call>` would never emit at the final canonical close because the "prior chunk is considered already closed". Direct execution proves this is wrong: `_closed_after_opener` correctly skips the mid-body `</arg_value>` because the body-prefix has an `<arg_value>` opener, so the pending count survives to delta 3 and the emit fires cleanly on `<end_of_tool_call>`. Regression test `test_streaming_sep_less_xml_pair_across_three_deltas_emits_on_end_of_tool_call` locks the working behaviour so a future refactor can't accidentally regress on codex's mistaken scenario. **NIT #3** — already addressed by the earlier operator patch on this branch (switch `_parse_hy3_body` to `json.JSONDecoder().raw_decode` so a legitimate JSON string argument containing the literal `</arg_value>` substring survives). Verified via the existing `test_json_body_containing_literal_arg_value_close_parses_correctly`. All 65 HY3-family tests pass. ruff check + format clean.
…wlist + JSON literal salvage Codex round-6 flagged 3 BLOCKING + 1 NIT on PR #1070. **BLOCKING #1 (round-6)** — streaming allowlist bypass The transition branch already passed ``request`` to ``extract_tool_calls`` (round-5 commit `4b7eed06` line 562), so this finding is a stale-line-number false positive. Adding a regression test ``test_streaming_respects_request_tool_allowlist`` to lock in the invariant so a future refactor that drops the argument trips CI. **BLOCKING #2 (round-6)** — JSON literal ``</arg_value>`` salvage misfire Round-5 fixed the non-streaming path via ``json.raw_decode``, but ``_closed_after_opener`` still fired the salvage close on a ``</arg_value>`` sitting inside a JSON string value. Add a JSON-body guard: if a ``<tool_sep>`` is in the prefix AND the tail-after-sep starts with ``{``, try ``raw_decode`` on the tail. If the decode succeeds and the ``</arg_value>`` lands INSIDE the decoded prefix, skip it — it's a literal in a JSON string. If the decode fails, the JSON isn't complete yet, so also skip. Only if the ``</arg_value>`` lands AFTER a well-formed JSON prefix is it treated as salvage residue. Regression test: ``test_streaming_json_body_with_literal_arg_value_close_does_not_flush_early``. **BLOCKING #3 (round-6)** — pre-straddle prose dropped on falsification Round-5's straddle branch emitted the pre-straddle prose as content BEFORE withholding the partial-opener bytes. Codex round-6 (BLOCKING #1/#2) said that violates the exclusive tool-call turn contract if the opener completes, AND on falsification the pre-straddle prose was permanently lost. Full redesign — buffer-until-resolved with a per-turn watermark: - Add ``self._streamed_bytes: int`` — cumulative byte offset of ``current_text`` emitted as content. Reset to 0 on ``reset()``. - Move the straddle check to run BEFORE the ``</think>`` emit path so a straddle in the same delta as a ``</think>`` still short-circuits (return None). - On non-straddle deltas, emit ``current_text[self._streamed_bytes:len(current_text)]`` as content and update the watermark. This handles all three cases: (a) normal delta emit — same as ``delta_text``; (b) opener completes — control has already flowed to the transition branch; the pre-straddle prose was withheld and never emitted, correctly dropped per exclusive-turn; (c) straddle falsifies — ``_streamed_bytes`` still points at the LAST emit boundary, so the release captures ``pre_straddle_prose + falsifying_bytes`` as one content delta. Regression tests: - ``test_streaming_partial_opener_withholds_entire_delta`` (round-5 test renamed + tightened — asserts None on partial-opener delta, round-6 exclusive-turn contract). - ``test_streaming_partial_opener_falsified_releases_buffered_prose`` (new — locks in the falsification-release semantics). **NIT (round-6)** — prefix-check alphabet drift ``_is_strict_prefix_of_tool_call_opener`` used ``str.isalnum()`` which accepts a subtly different alphabet than the compiled opener regex ``[\w-]+``. Delegate to a shared ``_LABEL_CHAR_RE`` regex so the two definitions stay in lockstep. Regression test: ``test_prefix_check_rejects_unicode_label_matching_isalnum_only``. All 76 HY3-family tests pass. ruff check + format clean. Refs PR #1070. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…token-ID gate + 2-phase FSM), drop bespoke full-text-regex design The pre-pivot Hy3 tool parser reinvented the streaming algorithm: it re-parsed the full accumulated text on every SSE delta, matched every tag with a suffix-alternation regex `(?::[\w-]+)?`, and carried ~85 LOC of partial-opener straddle-guard code. Seven codex rounds (R1-R7) all chased symptoms of that architecture self-ambushing on SSE boundaries. This ports vLLM main's `HYV3ToolParser` + SGLang's `resolve_hunyuan_tokens` design (both already handle the exact Hy3 wire format) into rapid-mlx's own `ToolParser` idiom — matching the vLLM-ported `deepseek_v3` / `qwen3coder` siblings. vLLM cannot be imported on Apple Silicon / MLX, so the algorithm is ported, not depended on (no new deps: stdlib `re` + `json`, no `regex`, no `partial_json_parser`). Architecture: - Resolve the wire suffix ONCE at __init__ by scanning tokenizer.get_vocab() for `<tool_call(:LABEL)?>`; pin every tag as a FIXED string. No regex alternation on the hot path. - Streaming entry gated on the opener (token-ID when the tokenizer exposes it, else the pinned fixed string). Special tokens are atomic on the tokenizer boundary, so the opener cannot straddle an SSE chunk — this single gate deletes the R5/R6 straddle-guard family (_tool_call_open_straddle_suffix_len, _is_strict_prefix_of_tool_call_opener, _closed_after_opener, _last_unclosed_tool_call_position, _streamed_bytes). - Two-phase FSM: SEEKING_NAME (find <tool_sep> -> emit function name) -> STREAMING_ARGS (stream the args body as a JSON diff, withholding the trailing `}` until <end_of_tool_call>). Buffer keyed on str.find of the pinned strings from the last opener — never a full-history re-parse. - Watermark on args (streamed_args_for_tool), not content. - A minimal single-string partial-opener prefix hold (the established hermes `_safe_content_prefix` idiom, not the deleted 85-LOC machinery) covers char-split delivery, since rapid-mlx's postprocessor does not pass token IDs to the parser. <think> handling is removed entirely from the tool parser — it lives in the separate `Hy3ReasoningParser` (--reasoning-parser hy_v3), already registered. The two parsers see disjoint token streams, exactly as vLLM's do. Malformed-close salvage (`<tool_call>NAME</arg_value>` — real 4-bit numerical noise on pipenetwork/Hy3-REAP50/75-MLX-4bit, 10/10 BFCL simple_python prompts) is kept as rapid-mlx's value-add but runs ONLY on the non-streaming extract_tool_calls path (streaming clients re-parse on completion). Tests rewritten to the new architecture. Malformed-close, XML-pair, JSON- literal-</arg_value>, multi-tool, allowlist, and real-wire scenarios are preserved (they encode real model behavior); the streaming-boundary tests are now trivially green via the opener gate + fixed-string finds. Verified green against the 2026-07-09 quality-spike golden fixtures in both streaming and non-streaming modes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onotonic
The initial port re-serialized args via json.dumps(parsed) on the STREAMING
close while emitting the raw wire prefix while open. These diverge for
`\uXXXX` unicode escapes (raw `é` vs decoded `é`) and whitespace, so the
closed snapshot was not a superstring of the open prefixes — the monotonic
diff broke and reassembled args were corrupted (`{"content": "café`
then `{"content": "café"}` appended = invalid JSON).
Fix: stream the RAW wire JSON verbatim throughout — open prefixes emit
`text[:cut]` up to a safe escape boundary, close emits `text[:end]` (the
raw well-formed prefix incl. `}`). `_partial_json_string` now emits the
already-JSON-escaped wire body unchanged (no re-escape), holding back only a
dangling `\` or partial `\uXX`. `_decode_json_partial` returns a raw-faithful
slice, not a json.dumps reconstruction. Every streamed prefix is now
byte-aligned with the closed document.
Adds 2 regression tests: char-by-char streaming of a JSON body with `\n`,
`\"`, `\\`, and a `\uXXXX` unicode escape; and nested/mixed-type args.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…back (codex R1) Two BLOCKING findings from codex round 1 on the ported parser: 1. Multi-tool streaming: `_name_sent` was set on the first streamed call and never reset, so a turn with multiple `<tool_call>` blocks folded every later call's name+args into index 0. Fix: the streaming FSM now walks opener positions index-by-index; on each `<end_of_tool_call>` it transitions back to SEEKING_NAME and bumps the tool index, so the next opener starts a fresh indexed call with its own id/name/args. Off-list (suppressed) calls still claim their index so the FSM advances past them. 2. Text-format fallback was unreachable: `extract_tool_calls` returned plain content on missing native opener BEFORE the `[Calling tool="X" k="v"]` degradation fallback could run. Fix: route the no-opener early return through a shared `_text_format_or_content` helper used at both exits. Adds regression tests: two-call char-by-char streaming (each gets its own index/name/args) and text-format fallback with no native opener. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dex R2)
Codex R2 flagged three blocking issues in the streaming path, all rooted in
the partial-JSON-streaming machinery:
1. single-delta call dropped its arguments (phase 1 emitted only the header
and returned before args processing);
2. `_decode_json_partial` accepted an incomplete number (`{"n": 12`) as
complete, corrupting streamed args;
3. `_partial_json_string` emitted an unterminated string prefix (`"abc`),
violating the OpenAI streaming contract that concatenated
`function.arguments` deltas form valid final JSON.
Resolved all three by switching to an emit-at-close model (glm47 sibling
pattern): arguments are emitted exactly once, as a single COMPLETE-JSON delta,
the moment `<end_of_tool_call>` arrives. Every `function.arguments` delta is
therefore a valid-JSON piece whose concatenation is the final document. This
deletes the partial-JSON machinery entirely (`_args_snapshot`,
`_partial_json_prefix`, `_decode_json_partial`, `_partial_json_string`).
The header (name) still ships as soon as `<tool_sep>` delimits it; a call that
arrives whole in one delta emits BOTH header and args in that delta. Multi-delta
streams emit the header first, then the args once the close lands — fixed a bug
where the args-emit path was gated on the header being present in the SAME delta
(`header is not None`), which dropped args for every real multi-delta stream.
Args-emit is now gated on non-suppression (`_suppressed_tools`) + not-already-
emitted, independent of whether the header shipped this tick.
Also (codex R2 NIT): made `_resolve_suffix` deterministic — it now collects all
`<tool_call(:LABEL)?>` suffixes and ranks them (complete token set first, then
`:opensource`, then other labels, then bare) instead of taking the first vocab
dict-iteration hit; and made `_find_call_close` JSON-aware so a literal
`<end_of_tool_call>` inside a JSON string value is not treated as the close.
Tests: 37 hy3 parser tests + 259 across the tool-parser suites pass; 9-fixture
real-wire pilot green (multi-tool, SSE-split close, literal-end-token-in-JSON).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `extract_tool_calls_streaming` docstring still described the deleted
partial-JSON model ("stream the args body incrementally, emitting a JSON diff
and withholding the trailing }"). Update it to match the emit-at-close FSM:
args are buffered until `<end_of_tool_call>` and emitted once as a complete
valid-JSON delta. Doc-only; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ex R3) Codex R3 flagged two blocking streaming bugs and one stale comment: BLOCKING #1 (_stream_tool_call): only one opener was processed per streaming invocation, so if the model emitted TWO complete tool calls in the same delta, the second was never emitted unless another delta arrived (which may never happen on the final chunk). Fix: `_stream_tool_call` now DRAINS — it loops over `_process_one_call`, accumulating deltas for every call that closes this tick, and stops at the first call that is still open or when no further opener has arrived. `_process_one_call` (extracted from the old body) returns `(deltas, closed)`; `_emit_args_and_advance` likewise returns `(deltas, closed)` so the drain loop knows when the FSM advanced. BLOCKING #2 (streaming entry): once any opener existed anywhere in `current_text`, the tool-call branch suppressed all plain text, so content that preceded the FIRST opener in the SAME delta was silently dropped. Fix: added a `_content_emitted` high-water mark (advanced by `_emit_safe_content`) and `_flush_pre_opener_content`, which emits the un-sent content between the mark and the first opener exactly once before any tool-call delta. The postprocessor treats each result as EITHER content OR tool_calls (never both in one delta), so the pending content ships on the delta that first reveals the opener and the tool-call deltas flow on the next; char-by-char delivery already emitted the leading content incrementally, so both paths agree (no drop, no double-emit). NIT #3 (chat_template.py): the `_looks_like_hy3` gate comment still said "case-insensitive substring match" after `_HY3_MODEL_NAME_RE` was tightened to separator-bounded matching. Updated the comment to match the implementation. Tests: +3 regressions (two-complete-calls-in-one-delta both emit; content-before-opener-in-same-delta not dropped, streaming two-stage flow; content-before-opener char-by-char not dropped). 40 hy3 parser tests + 262 across the tool-parser suites pass; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t suppression (codex R4) codex R4 flagged three blocking issues + two nits: BLOCKING #1 (_resolve_suffix): the resolver could pin a suffix that carries only `<tool_call>` but not its matching `<tool_sep>` / `<end_of_tool_call>`, making every downstream fixed-string `find` look for a non-existent separator/close and silently drop valid calls. Fix: consider ONLY suffixes whose complete parsing-critical trio is present; fall back to `:opensource` when no complete candidate exists. (Tests that constructed an incomplete `{"<tool_call>": 1}` vocab to exercise the suffix-less path now supply the complete bare trio, as a real suffix-less checkpoint would.) BLOCKING #2 (_next_block): an opener with NEITHER a canonical nor a malformed close (truncated / streaming-incomplete `<tool_call:opensource>get_weather`) was fabricated into a parsed call with `{}` args. Fix: `_next_block` returns `None` when no close of any kind exists, so the caller keeps the raw tail as residual content (pending/plain text) instead of a phantom empty call. A completed call followed by a dangling opener now returns only the completed call. BLOCKING #3 (off-list suppression): hardened + proven. A hallucinated off-list name must emit NEITHER a header NOR argument deltas. The per-index `_suppressed_tools` flag (added in R3) already gates the args emission via `idx = self.current_tool_id`, which matches the index recorded at suppression time in BOTH same-delta and multi-delta flows; added regressions that fail if any argument-only delta leaks for a suppressed call (whole-call- in-one-delta and chunked). NIT #4: strengthened `test_streaming_respects_request_tool_allowlist` to assert `tool_acc == {}` + no `tool_calls` in any raw delta (a name-only assertion would pass even if args leaked). NIT #5: documented `_normalize_hy3_tags` as accepted parser policy — the `<think(:LABEL)?>` namespace is reserved for reasoning delimiters on Hy3, same as every plain-tag reasoning parser treats `<think>`; a transition-only state machine would duplicate the ~1500-line qwen3 streaming machine for a payload no real checkpoint emits. Tests: 45 hy3 parser tests + 267 across the tool-parser suites pass; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l (codex R5)
codex R5 BLOCKING: when leading content and a complete tool call arrived in the
SAME final streaming delta, the parser returned only `{"content": pending}` and
deferred the tool-call deltas to a later invocation that may never happen (the
FINAL delta has no successor). The R3 fix relied on `finalize()` re-parsing as a
safety net, but the streaming path itself should be correct.
Fix: emit BOTH halves in one return using the postprocessor's mixed-content
contract. `_detect_tool_calls` preserves a `content` key alongside `tool_calls`
(postprocessor.py:3819 returns the dict as-is), and the caller (2688-2710)
splits it into a leading content StreamEvent then the tool events — the same
path the llama parser uses. `extract_tool_calls_streaming` now runs both
`_flush_pre_opener_content` and `_stream_tool_call`, and when both produce
output folds them into `{"content": ..., **tool_result}`. Nothing is deferred.
Updated `test_streaming_content_before_opener_in_same_delta_not_dropped` to
assert the single-tick mixed emission (content + tool_calls in one result)
rather than the old two-stage deferral.
Tests: 45 hy3 parser tests + 267 across the tool-parser suites pass; 9-fixture
real-wire pilot green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… not a phantom call (codex R6) codex R6 flagged two blocking issues, both the same root cause: a JSON string argument value may legitimately contain the literal ``<tool_call:opensource>`` opener text, and the pre-fix boundary logic treated that interior substring as a real call boundary. BLOCKING #1 (_next_block, non-streaming): the block was bounded at the next raw opener BEFORE the JSON-aware close scan, so a literal opener inside the body truncated the segment and dropped the real close. Fix: run the JSON-aware close scan (`_find_call_close_in_body`, whose `raw_decode` consumes any literal opener/end-token inside a string value) over the WHOLE remainder first; only fall back to a next-opener-bounded segment for the `</arg_value>` malformed salvage (no JSON body to protect there). BLOCKING #2 (_opener_positions, streaming): a plain substring scan split a JSON arg containing the opener literal into phantom calls, corrupting the argument stream. Fix: `_opener_positions` now walks call spans forward — each genuine opener, then its JSON-aware close, then advance past the close — so an opener substring inside a parsed body is opaque. The last (in-progress) call stops the walk, so its incomplete body's interior can never become a phantom boundary. Tests: +2 regressions (literal opener inside a JSON arg parses as exactly ONE call, non-streaming and char-by-char streaming). 47 hy3 parser tests + 269 across the tool-parser suites pass; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… reasoning straddle (codex R7) codex R7 flagged three blocking issues: BLOCKING #1 (_text_format_or_content): the no-native-opener degradation fallback did not apply the request `tools` allowlist, so a low-quant `[Calling tool="bogus"]` bypassed the filtering that native Hy3 calls enforce. Fix: thread `request` into `_text_format_or_content`; off-list names are dropped and preserved as content, mirroring the native path. BLOCKING #2 (_next_block malformed salvage): the `</arg_value>` salvage accepted ANY body ending in that close, so a truncated XML-pair call merely missing its `<end_of_tool_call>` (but carrying `<tool_sep>` / `<arg_key>` / `<arg_value>`) was promoted to a completed executable call. Fix: restrict salvage to the documented bare `NAME</arg_value>` 4-bit-noise shape — reject bodies containing any structural token before the malformed close; such truncated output is content, not a call. BLOCKING #3 (reasoning straddle): the `<think` prefix hold matched only the full `<think` root, so the withheld span grew NON-monotonically (`see <thin` held nothing, then `see <think` held 6 bytes) — the visible span retreated and the qwen3 base machine, having already emitted the earlier bytes, re-emitted them as garbage (`see ` → `k>see`) when the prefix falsified into content (`<thinking`). Fix: widen the straddle matcher to EVERY tag prefix (`<`, `<t`, … `<think`, `<think:label`) so the hold is monotonic — once a `<` that could begin a think tag appears it stays held until the tag completes or falsifies, and the held bytes are delivered on the tick they resolve. Also reworked the streaming delta derivation to operate on the raw held-visible spans (monotonic) instead of a `startswith`-recompute that could double-emit. Tests: +6 regressions (text-format allowlist drop + admit; truncated XML-pair not salvaged; falsified-think-prefix in content + in reasoning not dropped). 50 hy3 tool tests + 18 hy3 reasoning tests pass; 455 across the tool + reasoning suites; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ols fallback, release held reasoning suffix (codex R8) codex R8 flagged three blocking issues: BLOCKING #1 (streaming text-format parity): the non-streaming path recovers `[Calling tool="X"]` degradation but the streaming path emitted those bytes as content. Documented the deliberate contract (parity note): the degraded form has no native token boundaries to drive an incremental FSM, so during streaming it flows as content and the postprocessor's `finalize()` re-runs the (allowlist- aware) `extract_tool_calls` over the full text — which fires on the `[Calling` marker — to promote it exactly once. Added parity coverage. BLOCKING #2 (chat_template.py tools fallback): after the second `TypeError`, `reasoning_effort` was popped UNCONDITIONALLY, so a Hy3 request on a template that rejects `tools` (not `reasoning_effort`) lost the load-bearing `reasoning_effort="low"` override before the prompt-injection tools fallback. Fix: only drop `reasoning_effort` when the error names it; otherwise keep it so the tools fallback preserves it. Added a regression with a tools-rejecting tokenizer asserting the injected call still carries `reasoning_effort=low`. BLOCKING #3 (reasoning finalize drop): our widened straddle hold (R7) reserves even a lone trailing `<`, but `finalize_streaming` delegated the full buffer to qwen3 — which tracks its own emit position and never re-surfaced the withheld tail — so content ending in `<` or `<think` was dropped at stream end. Fix: `finalize_streaming` now releases the held non-tag suffix on the correct channel (reasoning if still inside an open think span, else content); a held run that IS a complete tag stays opaque markup. Added regressions for content ending in `<` and `<think`. Tests: +5 regressions across the three files; 50 hy3 tool + 22 hy3 reasoning + 21 hy3 chat-template tests pass; 546 across the tool + reasoning + chat-template suites; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ze double-emit, tighten kwarg match (codex R9) codex R9 flagged two blocking + two nits: BLOCKING #1 (_find_call_close): a COMPLETED call (has `<end_of_tool_call>`) whose JSON body is malformed junk (`{bad}`) was treated as no-call because `raw_decode` failed → -1. Fix: on `raw_decode` failure, still search for the close token — if present the call IS closed and `_final_args_json` degrades the args to `{}`; if absent the JSON is still truncated (return -1, keep streaming). Both the streaming and non-streaming paths share `_find_call_close`, so this recovers malformed-but-closed calls everywhere while still treating a truncated-no-close body as incomplete content. BLOCKING #2 (reasoning finalize double-emit): the R8 held-suffix release appended the tail even when `super().finalize_streaming` had already surfaced it, risking a `<think<think` duplication for content ending in `<think`. Fix: compare the normalised held tail against the target channel and only append when it is not already present. NIT #3 (_resolve_suffix): documented (with a test) that the JSON-only trio (call/sep/end) is the INTENTIONAL completeness signal — `arg_key`/`arg_value` are an optional XML-pair variant and are minted under the same suffix, so requiring them would wrongly reject a valid JSON-only checkpoint. NIT #4 (chat_template): replaced the loose `"reasoning_effort" in str(e2)` substring test with a match on Python's actual unexpected-kwarg error text (`unexpected keyword argument 'reasoning_effort'`), so a tools failure whose message merely mentions the kwarg no longer drops the override. Tests: +8 regressions (malformed-JSON completed call degrades to {} both paths; truncated-no-close stays content; JSON-only suffix resolution; finalize no double-emit + idempotent; reasoning_effort survives misleading error). 58 hy3 tool + 24 hy3 reasoning + 23 hy3 chat-template tests pass; 553 across the tool + reasoning + chat-template suites; 9-fixture real-wire pilot green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rmat from pending predicate (codex R10) codex R10 BLOCKING #1 — a SEP-LESS first call (XML-pair or bare-name body, no <tool_sep>) followed by a second call in the same delta swallowed the second opener. _find_call_close_in_body located the FIRST <tool_sep> in the whole post-opener segment, which for a sep-less first call is the NEXT call's separator, then searched past the next call's JSON for the close — advancing the span cursor over everything, so _opener_positions returned only [0]. The streaming FSM also stalled because the sep-less block never delimited a name. Fix: bound the sep to before this call's own <end_of_tool_call> (a later sep belongs to the next call), and drain a sep-less CLOSED call via the shared _parse_body in _emit_sepless_closed_call (header + complete args in one tick, allowlist-aware), then advance the FSM so the following opener drains too. codex R10 BLOCKING #2 — has_pending_tool_call returned True for a complete [Calling tool="X"] text-format message. That form is self-delimited with no trailing close delimiter to wait for and is finalized via the non-streaming recovery path (gated on the [Calling marker, not this predicate), so reporting it pending made streaming shutdown treat a finished message as perpetually in-flight. Fix: pending iff the LAST native <tool_call> opener has no <end_of_tool_call> after it; text-format is not pending. Recovery via extract_tool_calls is unaffected. Regressions: sep-less-first multi-call (single-delta + char-by-char), bare-name sep-less call, off-list sep-less suppression, text-format not-pending predicate, text-format still recovered by non-streaming extract. Evidence: hy3 tool 61 + reasoning 22 + chat-template 22 pass; postprocessor + tool-tag-leak + tool-call-normalization 132 pass; pilot + R10 real-wire sep-less multi-call proof green; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…me-scoped auto-detect (codex R11) codex R11 BLOCKING #1 — _find_call_close accepted the FIRST <end_of_tool_call> substring on the raw_decode-failed ({-body) path, so a still-streaming JSON string value containing that literal ({"m": "contains <end_of_tool_call> inside) closed the call early and emitted {}. Fix: _end_token_outside_string walks the {-body tracking JSON string state and accepts only a close token OUTSIDE a string. A completed malformed body ({bad}<end>, codex R9) still closes (its <end> is outside a string); a truncated body whose only occurrence is inside an unterminated string returns -1 and keeps streaming. codex R11 BLOCKING #2 — _find_call_close_in_body scanned for <tool_sep> across the whole remaining segment, so a garbled/sep-less opener before a valid call stole the real call's separator and fabricated a bogus name (gar<tool_call>realtool). Fix: _opener_positions bounds each call's body at the next opener ONLY when that opener precedes this call's own <tool_sep> (a genuine separate call); an opener AFTER the sep lives inside the args and stays JSON-aware-opaque (preserves R6 literal-opener-in-JSON). A close-less garbled residue opener is skipped and scanning resumes at the later opener; the streaming FSM skips the residue index and drains the real call. codex R11 BLOCKING #3 — the Hy3 auto-detect regex trailing class included /, so a non-Hy3 model under an org/parent dir named hy3 (hy3/qwen-model, some/hy3/nested-qwen) was auto-wired to the Hy3 parsers via the PARENT segment. Fix: drop / from the trailing class so the family root must sit in the FINAL path segment; still matches mlx-community/Hy3-preview-4bit, bare hy3, org/hy3, hunyuan-3-preview. Regressions: literal end-token in unterminated JSON string (stream + unit), incomplete-vs-malformed-complete discrimination, garbled-opener-before-real -call (stream + _opener_positions unit), Hy3 basename detected / parent-segment + substring rejected (TestHy3AutoDetectBoundary, 11 cases). Evidence: hy3 tool 65 + reasoning 22 + chat-template 22 + auto-config 217 pass; postprocessor + tool-tag-leak + tool-call-normalization 132 pass; pilot green; ruff clean; no spec_decode/speculative/pyproject/forbidden-dep touched. (Unrelated pre-existing red on origin/main: test_vision_extra_install README assertion, from README-trim PR #1054 — not in this diff, not caused here.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…bled opener, correct effort comment (codex R12) codex R12 BLOCKING #1 — the raw_decode-failed close scan accepted a close token merely OUTSIDE a JSON string, so an incomplete-but-open object ({"a": <end> — value not yet arrived) closed early and emitted {}. Fix: _end_token_at_object_close tracks brace depth alongside string state and accepts a close only when NOT in a string AND depth has balanced back to 0 (object closed). {bad}<end> still closes (braces balance before the token); {"a": <end> and {"a": {"b": <end> keep streaming (depth > 0). codex R12 BLOCKING #2 — the NON-STREAMING _next_block scanned the whole remaining text for the close, so a garbled/sep-less opener before a valid call merged into one bogus block (gar<tool_call>realtool) — the same class fixed in _opener_positions for streaming, but the non-stream path was missed. Fix: bound the body at a later <tool_call> that precedes this call's own <tool_sep> (a genuine separate opener), leaving an opener AFTER the sep JSON-aware-opaque (preserves R6 literal-opener-in-JSON); when the current opener is close-less garbled residue, resume _next_block at the later opener so the real call (and any calls after it) is recovered. codex R12 NIT — the chat_template reasoning_effort comment implied a request-side graded-effort (medium/high) API surface that does not exist. Corrected to state plainly that there is no plumb-through today and setdefault is future-proofing (identical to assignment until a plumb-through pre-populates the key); no behavior change. Regressions: incomplete-open-object still-streaming (unit + stream, incl. nested depth), non-streaming garbled opener recovers real call (+ two real calls after residue), non-streaming literal-opener-in-JSON stays one call (R6 guard on the R12-bounded path). Evidence: hy3 tool + reasoning + chat-template + auto-config boundary 125 pass; postprocessor + tool-tag-leak + tool-call-normalization 132 pass; pilot green; ruff clean; no spec_decode/speculative/pyproject/forbidden-dep touched. (Unrelated pre-existing red on origin/main: test_vision_extra _install README assertion from README-trim PR #1054 — not in this diff.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ps effort injection (codex R13) codex R13 BLOCKING — _HY3_MODEL_NAME_RE in chat_template.py still allowed ``/`` as a trailing boundary (the sibling of the model_auto_config.py R11 fix, missed at the time). So a non-Hy3 repo under an HF org / local parent directory named ``hy3`` (``hy3/qwen-model``, ``some/hy3/nested-qwen``) had ``reasoning_effort="low"`` injected via the PARENT segment — mis-conditioning the request or triggering the TypeError fallback chain on a non-Hy3 template. Fix: drop ``/`` from the trailing class so the family root must sit in the FINAL path segment; still matches ``mlx-community/Hy3-preview-4bit``, bare ``hy3``, ``org/hy3``, ``Hunyuan-3-Preview``. Both Hy3 boundary regexes (this one + model_auto_config.py) now share the identical basename-scoped form; grep confirms no third sibling. Regressions: _looks_like_hy3 parent-segment negatives (hy3/qwen-model, some/hy3/nested-qwen) + org/hy3 basename positive; end-to-end apply_chat_template asserts no effort kwarg injected for hy3/qwen-model. Evidence: hy3 chat-template + tool + reasoning + auto-config boundary 129 pass; ruff clean; no spec_decode/speculative/pyproject/forbidden-dep touched. (Unrelated pre-existing red on origin/main: test_vision_extra_install README assertion from README-trim PR #1054 — not in this diff.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ruff 0.15.21 (matches CI `pip install ruff` latest) collapses two multi-line expressions that fit on a single line. Pure formatting, no semantic change. Fixes the `lint` pr_validate step (ruff format --check). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…recovery (codex R14)
BUG 1 — garbled-opener skip corrupted the client-visible tool_calls index.
The emitted `index` used the PHYSICAL opener-position index, which advances
for skipped residue openers and suppressed off-allowlist calls too, so a
first real call after a skipped residue got index 1 and left a null hole at
0 in OpenAI-SDK reconstruction. Added a `_client_index_of` map +
`_next_client_index` counter that only advances for calls actually emitted;
every emitted `{"index": ...}` (header, args, sep-less path) now uses the
client-visible value while internal bookkeeping stays physical.
BUG 2 — poisoned brace-depth in `_end_token_at_object_close` never recovered.
A mid-stream noise `<end_of_tool_call>` outside a string while an object was
still open (`{"a": <end>{"a": 42}<end>`) left the never-closed leading `{`
poisoning depth forever, so the real close was rejected and the call hung
pending. Now the scan RESYNCHRONIZES: a noise close at depth>0 is skipped and
depth reset to 0, and `_final_args_json` strips the noise prefix via
`_resync_args_body` so the real trailing object serializes correctly.
BUG 3 — strengthened the false-green streaming test to assert the real args
(`json.loads(...) == {"a": 42}`) instead of only the name.
Also added an index-0 assertion for the garbled-opener case, a dense
two-call index test, and a close-finder resync unit test. 344 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…token (codex R15) An <arg_value> payload can legitimately carry the literal <end_of_tool_call> string as free-form text. The non-JSON (XML-pair) close search used a plain str.find, truncating the call early and dropping the argument. Add _end_token_outside_arg_value which skips complete <arg_value>…</arg_value> spans; wire it into both the streaming (_find_call_close) and non-streaming (_find_call_close_in_body sep-less branch) paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_emit_sepless_closed_call used a plain str.find for the close, so a sep-less XML-pair <arg_value> containing the literal <end_of_tool_call> string was truncated — the third and last occurrence of the class R15 fixed for the sep-full streaming and non-streaming paths. Reuse _end_token_outside_arg_value here too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reasoning (codex R17) finalize_streaming released a held straddle suffix (e.g. `</think` from a stream truncated mid-close-tag while inside an open think span) as raw reasoning/content text. A partial delimiter prefix is incomplete markup, not user-visible text — drop it. The complete-tag fullmatch guard only caught finished tags; extend it to committed partial-tag prefixes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
raullenchai
pushed a commit
that referenced
this pull request
Jul 10, 2026
…-only docs (PR 3 of 3) Adds Hy3 (Tencent Hunyuan 3, 295B/21B-active MoE) as the 5th Tier-1 family across the agent + framework integration matrices, plus the Ultra-only launch docs. Stays strictly in the test/docs layer — no parser or engine code touched. A/B — matrix family + strict-xfail cells (conftest.py): * Register `hy3-preview-4bit` as the 5th `_FAMILY_ALIASES` entry with a reason documenting Ultra-only / 166 GB / weekly-Golden-Path. * Every Hy3 cell (11 agents + 3 frameworks = 14) is `xfail(strict=True)` via `pytest_collection_modifyitems`, mirroring the DeepSeek V4-Flash single-node-infeasible precedent (166 GB + G11 100 GB floor > 256 GB M3 Ultra). G8: strict-xfail, not plain skip. XPASSes force a revisit if a smaller quant / bigger Mac ever makes always-on CI feasible. * Family map recognises `hy3` / `hunyuan-3` served ids. C — offline parser-level integration test (test_hy3_offline.py): * The always-on-CI value-add. Drives captured Hy3 wire strings (from the same 2026-07-09 REAP50/75 spike that seeded PR-2's unit tests) through the `hy_v3` tool + reasoning parsers WITHOUT booting the 166 GB model, asserting the OpenAI-API-shape contract: tool_calls array well-formed, `<think>` reasoning routed to its own channel, no content leak, plus a composed reasoning-then-tool-call turn. 8 tests, sub-second, runs in the normal `pytest tests/` sweep. D — Ultra-only docs: * docs/reference/models.md: new "Ultra-only: Hunyuan 3 (Hy3)" section with the min_memory_gb=192 pre-download warning example + LM family row. * tests/integrations/README.md: Hy3 column in both status matrices, family-alias table row, counts (56→70 cells), and the offline-test note. * Verified the `min_memory_gb: 192` warning (PR-1 wiring, cli.py `_check_alias_min_memory`) fires for hy3-preview-4bit on a sub-192 GB Mac — no gap. Stacked on #1070 (PR-2 parsers) — rebase after #1070 merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
raullenchai
pushed a commit
that referenced
this pull request
Jul 10, 2026
…dex converge) Address codex adversarial-review findings on the gpt-5.6-sol sign-off round: - test_hy3_offline.py: the `_tool_parser` / `_reasoning_parser` factories wrapped their imports in `try/except pytest.skip`, a vestigial guard from when this file predated the now-merged PR-2 parser (#1070). Post-merge the `hy_v3` tool + reasoning parsers are permanent, so the imports are now HARD: a parser-import regression or accidental deletion FAILS the offline test instead of silently skipping it green — the exact failure mode this always-on-CI test exists to catch. Dropped the now-unused `pytest` import. - conftest.py: corrected the "11 Hy3 cells" comment to "14 (11 agents + 3 frameworks)" and documented that the matrix parametrizes by a single `family` argument so every Hy3 nodeid ends in exactly `[hy3]` (refutes a gpt-5.5-round false positive about hypothetical combined `[agent-hy3]` ids). - Updated the file docstring: fixtures re-verified green against the merged parser at rebase time; no assertion change was needed. Offline suite still 8/8 green; ruff 0.15.21 clean.
raullenchai
added a commit
that referenced
this pull request
Jul 10, 2026
…ocs (PR 3 of 3) (#1072) * test(hy3): add Hy3 as Tier-1 5th family in integration matrix + Ultra-only docs (PR 3 of 3) Adds Hy3 (Tencent Hunyuan 3, 295B/21B-active MoE) as the 5th Tier-1 family across the agent + framework integration matrices, plus the Ultra-only launch docs. Stays strictly in the test/docs layer — no parser or engine code touched. A/B — matrix family + strict-xfail cells (conftest.py): * Register `hy3-preview-4bit` as the 5th `_FAMILY_ALIASES` entry with a reason documenting Ultra-only / 166 GB / weekly-Golden-Path. * Every Hy3 cell (11 agents + 3 frameworks = 14) is `xfail(strict=True)` via `pytest_collection_modifyitems`, mirroring the DeepSeek V4-Flash single-node-infeasible precedent (166 GB + G11 100 GB floor > 256 GB M3 Ultra). G8: strict-xfail, not plain skip. XPASSes force a revisit if a smaller quant / bigger Mac ever makes always-on CI feasible. * Family map recognises `hy3` / `hunyuan-3` served ids. C — offline parser-level integration test (test_hy3_offline.py): * The always-on-CI value-add. Drives captured Hy3 wire strings (from the same 2026-07-09 REAP50/75 spike that seeded PR-2's unit tests) through the `hy_v3` tool + reasoning parsers WITHOUT booting the 166 GB model, asserting the OpenAI-API-shape contract: tool_calls array well-formed, `<think>` reasoning routed to its own channel, no content leak, plus a composed reasoning-then-tool-call turn. 8 tests, sub-second, runs in the normal `pytest tests/` sweep. D — Ultra-only docs: * docs/reference/models.md: new "Ultra-only: Hunyuan 3 (Hy3)" section with the min_memory_gb=192 pre-download warning example + LM family row. * tests/integrations/README.md: Hy3 column in both status matrices, family-alias table row, counts (56→70 cells), and the offline-test note. * Verified the `min_memory_gb: 192` warning (PR-1 wiring, cli.py `_check_alias_min_memory`) fires for hy3-preview-4bit on a sub-192 GB Mac — no gap. Stacked on #1070 (PR-2 parsers) — rebase after #1070 merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(hy3): harden offline parser imports + fix cell-count comment (codex converge) Address codex adversarial-review findings on the gpt-5.6-sol sign-off round: - test_hy3_offline.py: the `_tool_parser` / `_reasoning_parser` factories wrapped their imports in `try/except pytest.skip`, a vestigial guard from when this file predated the now-merged PR-2 parser (#1070). Post-merge the `hy_v3` tool + reasoning parsers are permanent, so the imports are now HARD: a parser-import regression or accidental deletion FAILS the offline test instead of silently skipping it green — the exact failure mode this always-on-CI test exists to catch. Dropped the now-unused `pytest` import. - conftest.py: corrected the "11 Hy3 cells" comment to "14 (11 agents + 3 frameworks)" and documented that the matrix parametrizes by a single `family` argument so every Hy3 nodeid ends in exactly `[hy3]` (refutes a gpt-5.5-round false positive about hypothetical combined `[agent-hy3]` ids). - Updated the file docstring: fixtures re-verified green against the merged parser at rebase time; no assertion change was needed. Offline suite still 8/8 green; ruff 0.15.21 clean. * test(hy3): resolve parsers via alias config + assert reasoning-channel cleanliness (codex converge r2) Address the genuine findings from the gpt-5.6-sol adversarial round: - test_hy3_offline.py: resolve the tool + reasoning parser NAMES from the production `hy3-preview-4bit` alias profile (`resolve_profile(...)`) instead of hard-coding `"hy_v3"`. The test now also guards the alias -> parser wiring: if the alias stops declaring `tool_call_parser` / `reasoning_parser` (or repoints them), the test fails instead of passing on a stale literal. - test_hy3_offline.py: the reasoning tests now assert the extracted `reasoning` field is itself tag-clean (exact expected payload + no `<think>` tag + no `:opensource` suffix), not only that `content` is clean. A parser that left the raw think tags in `reasoning` would now fail. Verified against the merged parser: `reasoning` is returned stripped (asserting correct behavior, not downgrading — G8). - conftest.py: rewrote the Hy3 infeasibility rationale to evaluate the RAM constraint (~156 GB peak vs 256 GB unified memory, 192 GB floor) and the disk constraint (166 GB weights + G11 100 GB free-disk floor) SEPARATELY rather than summing disk+RAM figures, and to state plainly that a supported M3 Ultra CAN boot it (that is the weekly Golden Path job) — only the always-on per-PR CI runners cannot. Refuted (arguing against merged, intentional design; documented in PR comment): the `xfail(strict=True)` XPASS-on-Golden-Path point (identical to the merged DeepSeek/gpt-oss strict-xfail tripwire pattern) and the "exercise production serialization" scope point (this file's declared scope is the parser-level API-shape contract, deliberately server-free). Offline suite 8/8 green; ruff 0.15.21 clean. * docs(hy3): clarify 192 GB enforced floor vs 256 GB Ultra recommendation + date the cell-status matrix (codex NIT) Two doc-precision NITs from the gpt-5.6-sol round: - docs/reference/models.md: the Ultra-only callout said "requires M3 Ultra 256 GB" but the runtime only enforces a 192 GB `min_memory_gb` floor with no chip-generation check. Reworded to state 192 GB as the enforced threshold and 256 GB M3 Ultra as the validated recommendation, matching actual runtime behavior. - tests/integrations/README.md: the "Current cell status" heading was still dated "2026-07-06 · 0.10.2" while the section now spans the 0.11.0 Hy3 column. Reworded so the heading states the matrix spans through 0.11.0 while attributing the PASS/XFAIL data to the 2026-07-06 0.10.2 pilot run (the data genuinely is from that pilot; the Hy3 column is strict-xfail). No code change; offline suite still 8/8 green. --------- Co-authored-by: Raullen <raullenstudio@Raullens-Mac-Studio.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR 2 of 3 for the Tencent Hunyuan 3 (Hy3) vendor initiative kicked off in #1069 (which vendored the model +
hy3-preview-4bitalias at SHA122dd685). This PR wires the tool + reasoning parsers, updates the tokenizer auto-detect, and overrides a factual-recall-breaking chat-template default.Fixes 2 bugs we surfaced in the upstream mlx-lm PR #1211 review:
Comment 4927710973 — malformed close boundary on the 4-bit checkpoint: model emits
<tool_call:opensource>NAME</arg_value:opensource>instead of the canonical<tool_call:opensource>NAME<tool_sep:opensource>{...}<end_of_tool_call:opensource>. Empirically observed on 4/32 real prompts onpipenetwork/Hy3-REAP50/75-MLX-4bitin the 2026-07-09 boot spike (agentac2851864dbd17b07). The malformed-close salvage inhy_v3_tool_parser.pyrecovers the tool name so users don't see an emptytool_calls[]and think the model refused.Comment 4927711484 —
chat_template.jinjadefaultsreasoning_effort=no_thinkwhich produces "France" instead of "Paris" on factual-recall questions. Theapply_chat_templateboundary invllm_mlx/utils/chat_template.pynow injectsreasoning_effort='low'as the default whenmodel_namematches Hy3 and the client didn't setenable_thinking=False.The tool parser was rewritten after 7 codex rounds (R1–R7) all chased symptoms of a structurally-wrong design. The original bespoke implementation re-parsed the full accumulated text on every streaming delta, matched every tag with a suffix-alternation regex
(?::[\w-]+)?, and carried ~85 LOC of partial-opener straddle-guard code — a design that self-ambushed on every SSE boundary.It now ports vLLM main's
HYV3ToolParser+ SGLang'sresolve_hunyuan_tokensarchitecture (both already handle the exact Hy3 wire format), adapted into rapid-mlx's ownToolParseridiom — matching the vLLM-porteddeepseek_v3/qwen3codersiblings. Credit: vLLMvllm/tool_parsers/hy_v3_tool_parser.pyand SGLanghunyuan_detector.py. vLLM cannot be imported on Apple Silicon / MLX (zero-vLLM-dep is a hard constraint), so the algorithm is ported, not depended on — no new deps (stdlibre+json; noregex, nopartial_json_parser).New architecture:
__init__by scanningtokenizer.get_vocab()for<tool_call(:LABEL)?>; pin every tag as a fixed string. No regex alternation on the hot path._tool_call_open_straddle_suffix_len,_is_strict_prefix_of_tool_call_opener,_closed_after_opener,_last_unclosed_tool_call_position,_streamed_bytes).SEEKING_NAME(find<tool_sep>→ emit function name) →STREAMING_ARGS(stream args as a JSON diff, withholding the trailing}until<end_of_tool_call>). Buffer keyed onstr.findof the pinned strings from the last opener — never a full-history re-parse.streamed_args_for_tool), not content._safe_content_prefixidiom, ~15 LOC — not the deleted 85-LOC machinery) covers char-split delivery, since rapid-mlx's postprocessor does not pass token IDs to the parser.<think>handling removed entirely from the tool parser — it lives in the separateHy3ReasoningParser(--reasoning-parser hy_v3), already registered. The two parsers see disjoint token streams, exactly as vLLM's do.rapid-mlx value-add kept: the malformed-close salvage (
<tool_call>NAME</arg_value>— real 4-bit numerical noise) runs only on the non-streaming path (4-bit noise is rare; streaming clients re-parse on completion).Real-wire pilot (before codex)
The new parser was verified against the 2026-07-09 quality-spike golden fixtures (
pipenetwork/Hy3-REAP50/75-MLX-4bit, documented in the mlx-lm #1211 issue draft) in both streaming and non-streaming modes: well-formed JSON body, malformed close, JSON-body-then-stray-<arg_value>, XML-pair args,<think>routing to reasoning (not tool_calls), pure-content passthrough, multi-tool, SSE-boundary close split, and JSON string containing literal</arg_value>. All green.Reasoning parser + chat-template override (unchanged from original PR)
Reasoning parser.
Hy3ReasoningParserextendsQwen3ReasoningParserby normalizing<think:xxx>/</think:xxx>→<think>/</think>at every public entry point, keeping the entire qwen3 state machine in one place — no ~1500 LOC duplication. Registered as--reasoning-parser hy_v3/hy3.Chat-template override. Injection at
apply_chat_template()gated on Hy3 name-match (hy3|hy-v3|hunyuan.?3, case-insensitive) ANDenable_thinking is not False. Non-Hy3 tokenizers never see the kwarg.Not in scope
reasoning_effortinto HY3's kwarg — only the DEFAULT is overridden todayTest plan
tests/test_hy_v3_tool_parser.py— rewritten to the new architecture. Preserved (real model behavior): canonical JSON body with/without suffix, malformed close, JSON-body-then-stray-<arg_value>, XML-pair + type coercion, sep-less XML pairs, multiple calls, JSON-literal-</arg_value>, no-tool passthrough, request allowlist filtering. New: suffix-resolution-from-vocab (labelled + suffix-less + id), streaming char-by-char no-leak, partial-opener hold/release, flush-held-content.tests/test_hy3_reasoning_parser.py— unchanged, still green (parser untouched; orthogonal).tests/test_hy3_chat_template_default.py— unchanged, still green.test_tool_parser_wire_formats.py,test_tool_call_streaming_parity.py,test_tool_parser_coverage.py,test_tool_parsers.py,test_model_auto_config.py,test_prefix_boundary_path_parity.py— all green (371 + 30 + 69 checked locally).Post-merge follow-up: add
hy3-preview-4bittoscripts/pr_validate/golden_models.yamlonce a 192 GB+ M3 Ultra CI slot is secured. PR-3 exercises the full parser + template override on a live serve.🤖 Generated with Claude Code