refactor(parsers): align the unified parser surface with peer traits, and let vendors supply their own

[keivenchang]

Overview:

This PR aligns the unified parser's ordered-event and incremental-parse surface with the peer traits, and lets a third party supply their own unified parser — including replacing one this crate ships — without forking.

Two peer capabilities are deliberately NOT in this PR, so "aligned" should not be read as "drop-in adoptable": the peer's create takes a tokenizer as well as the tools, while the vendor factory here takes only tools, so a peer parser that resolves its markers from a tokenizer cannot be constructed through this registry yet; and structural_tag_model, which constrains generation for strict tool calling, has no type or consumer in this crate. Both need a design, and neither is closed by the follow-up request-mode PR — that one selects between native markup and guided JSON in already-generated bytes, which is a different question. It also fixes hover popups in the conformance report, which had stopped opening entirely.

Split out of DIS-2544 / #174, which bundled several unrelated concerns into ~7k lines across 56 files and stalled in review. This half does not depend on qwen3 request modes, so it lands on its own; the request modes follow stacked on top in #174.

No corpus change. The unified golden feed is byte-identical to main.

Where this sits

flowchart TB
    M["main  ·  99 golden cases"]
    A["#178  ·  THE SURFACE  ·  UnifiedParser trait + output type + vendor registry"]
    B["#174  ·  REQUEST MODES  ·  StartingState + ToolOutputMode  ·  corpus 99 → 237"]
    C["#166  ·  SECOND FAMILY  ·  gemma4 on the surface, graded by #174's corpus"]
    M --> A --> B --> C
    classDef here fill:#fff4ce,stroke:#b06000,stroke-width:3px,color:#000
    classDef other fill:#f6f8fa,stroke:#8c959f,color:#000
    classDef base fill:#eef1f4,stroke:#8c959f,color:#000
    class M base
    class A here
    class B,C other

Loading

The full graph — every trait, type and registry entry each layer adds, and which layer consumes it — is in DIS-2644, kept there because it is too wide to read inline on GitHub.

Review aid: the lifecycle this PR pins down

stateDiagram-v2
    direction LR
    [*] --> Fresh
    Fresh --> Streaming : parse_into
    Streaming --> Streaming : parse_into
    Streaming --> Errored : Err
    Errored --> Fresh : reset()
    Streaming --> Ended : finish()
    Ended --> [*]

Loading

What each edge guarantees, and where to check it in the diff:

edge	guarantee	where
parse_into	committed events land in the caller's output, so a later Err in the same advance cannot retract them	ScannerUnified::parse_into writes straight through EventSink
Err	the failing invoke was never consumed — buffer intact, next_index not advanced	both invoke sites call the emitter before buffer.drain(..)
reset()	returns the unconsumed text and clears every field carrying stream position: in_block, in_reasoning, resume_reasoning, suppress_normal_text, next_index	reset_stream in scan.rs
finish()	flushes into a fresh output; open reasoning is promoted rather than leaked	ScannerUnified::finish

push() is the one spelling that CANNOT honour the first row — it owns its buffer and returns Result<Vec<_>>, which has nowhere to carry partial output. A parser that may commit and then fail must be driven through parse_into.

Review aid: what goes in, what comes out

Every line below is marked with where it comes from. Verified against the vLLM Rust parser crate at rust/src/parser/src/unified/mod.rs (v0.26.0).

// ---- IN ----
// (FROM vLLM — identical signature)
fn parse_into(&mut self, delta: &str, output: &mut UnifiedParserOutput) -> Result<()>;
// (FROM vLLM — identical signature)
fn initialize(&mut self, prompt_token_ids: &[u32]) -> Result<()>;

// ---- OUT, streaming ----
// (FROM vLLM — identical struct, identical public field)
pub struct UnifiedParserOutput { pub events: Vec<UnifiedParserEvent> }

// (FROM vLLM — identical variants, identical tuple shapes)
pub enum UnifiedParserEvent {
    Text(String),
    Reasoning(String),
    ToolCall(ToolCallDelta),
}

// (FROM vLLM — identical fields)
pub struct ToolCallDelta { pub tool_index: usize, pub name: Option<String>, pub arguments: String }

// ---- OUT, assembled ----
// (NOT vLLM — ours. vLLM has no assembled-event type and no assemble().)
pub enum UnifiedEvent {
    Reasoning { text: String },
    Text      { text: String },
    ToolCall  { name: String, arguments: serde_json::Value },
}

Provenance of every trait method:

method	provenance
parse_into · initialize · reset · preserve_special_tokens · tool_call_id	FROM vLLM — identical signatures
finish	FROM vLLM, one deliberate difference: vLLM gives it a default, we make it REQUIRED. A family that forgets to flush silently drops the tail of every stream, which is not a failure worth inheriting for symmetry
push(chunk) -> Result<Vec<UnifiedParserEvent>>	NOT vLLM — ours, additive convenience. The conformance corpus asserts against this spelling
parse_complete(output) -> Result<Vec<UnifiedEvent>>	NOT vLLM — ours. vLLM has no batch entry point; routing batch through parse_into/finish makes stream/batch parity structural
register_unified_parser / unregister / aliases	NOT vLLM — ours. vLLM has no vendor registry for unified parsers

Accumulation helpers push_text · push_reasoning · push_call · append are FROM vLLM by name and semantics, including that append coalesces across the seam. One difference: vLLM's append(other: Self) consumes; ours takes &mut Self.

In vLLM, deliberately NOT here: create(tools, tokenizer) — their construction takes a tokenizer, ours takes only &[Tool], so a peer parser that resolves markers from a tokenizer cannot be built through this registry yet. structural_tag_model() — constrains generation for strict tool calling; no type or consumer in this crate. append_tool_output() — bridges their separate tool-only output type.

Why a flat Vec and not {reasoning_text, content, tool_calls}. A bundle of parallel fields physically cannot say whether text came before or after a call — that is why the split path today hoists every thought to the front and merges them. Ordering is not a field you can add; it has to be the shape. This is the part vLLM already got right, and the reason aligning to it was worth doing.

Two distinctions that are easy to misread in the diff:

• ToolCallDelta.arguments is a String fragment that concatenates across deltas; UnifiedEvent::ToolCall.arguments is a parsed serde_json::Value. assemble() is the single fold between them — and both the fold and the assembled type are ours, not vLLM's.
• UnifiedParserEvent uses tuple variants (vLLM's shape); UnifiedEvent uses struct variants with a serde tag, because it serializes to the golden-corpus schema.

Result is anyhow::Result throughout.

Review aid: what an emitter failure costs, after this PR

The ordering below is the fix. Before it, the buffer was drained before the fallible call and the drain loop collected into a local vector that ? dropped — so this same input lost prefix, the ok call that had already succeeded, and both invoke bodies, leaving reset() with only </tool_call>suffix.

sequenceDiagram
    autonumber
    participant C as Caller (owns output)
    participant S as Scanner drain loop
    participant E as Emitter (fallible)

    C->>S: parse_into("prefix<tool_call>ok…boom…</tool_call>suffix")
    S-->>C: push_text("prefix") — COMMITTED
    S->>E: parse_invoke(ok)
    E-->>S: Ok(ToolCall)
    S-->>C: push_call(ok) — COMMITTED
    S->>S: drain the ok bytes (only after success)
    S->>E: parse_invoke(boom)
    E-->>S: Err
    Note over S: boom bytes were NOT drained
    S-->>C: Err
    C->>S: reset()
    S-->>C: "<function=boom></function></tool_call>suffix"

Loading

Example (before → after):

Hovering a cell in the conformance report opened nothing on a normal desktop:

before:  (hover: hover) reports false -> pointerenter listener never registered
         browser delivers real hover, cell matches CSS :hover, popup never opens
         keyboard focus also dead, because focusin sat behind the same gate
after:   listeners always registered; only tap-to-pin consults PointerEvent.pointerType
         real mouse, media queries false -> popup with 1334 chars, opacity 1, on top

Details:

• Peer alignment — UnifiedParserEvent in peer variant order and payload shapes, parse_into as the required method, finish -> UnifiedParserOutput, initialize(&[u32]), plus reset / preserve_special_tokens / tool_call_id and the peer accumulation helpers. parse_complete and the assembled UnifiedEvent view stay additive, so a peer-shaped caller never sees them.
• Vendor registry — register_unified_parser / unregister_unified_parser, keyed on the canonical family name so registering qwen3 also shadows its qwen3_coder alias. Shadowing is non-destructive; unregistering restores the built-in exactly. builtin_unified_families() excludes vendor entries, because the conformance suite iterates it and a vendor family has no cases here. CUSTOM_PARSERS.md documents the contract, and its worked example exists as a compiled integration test (tests/vendor_parser_example.rs) using only the public API, with a guard asserting the guide's impl UnifiedParser methods match that file's. The guard is scoped to those method bodies — it does not compile the Markdown block, so a doc-only change to imports or struct fields would still slip past it.
• GUI + tooling — hover listeners no longer gated on a media query, capture-result semantics, ORDER/MERGE sequence display, render fixes.

Verification:

cargo test --workspace --all-targets --locked   1386 passed, 33 suites, 0 failed
python3 -m pytest conformance/utils/tests/      125 passed
cargo clippy --workspace --all-targets          0 warnings
piece-1 coupling gate                           PASS
golden.tar.gz / inputs.tar.gz                   43db2838… / 70fda09d… unchanged

conformance/utils/piece1_coupling_gate.sh is the falsifiable proof that no request-mode work leaked in: it rejects every request-mode symbol from the diff, proves conformance/fixtures and the manifest are byte-identical to main, pins both LFS pointers, and confirms the generator still emits main's 33 scenarios / 99 cases. It was negative-controlled against the combined branch, where it fails for eight independent reasons.

Where should the reviewer start?

parsers/v2/src/unified/mod.rs (the trait and registry), then parsers/v2/CUSTOM_PARSERS.md.

/coderabbit profile chill

Summary by CodeRabbit

• New Features

  • Added support for custom unified parsers, including registration, overrides, lifecycle controls, and vendor family management.
  • Improved conformance views with clearer event-order comparisons, compact tables, formatted tooltips, and sequence-divergence indicators.
  • Enhanced tooltip interactions across mouse, keyboard, touch, and pen input.

• Bug Fixes

  • Updated parser output handling for consistent reasoning, text, and tool-call events.

• Documentation

  • Added guidance for implementing, registering, testing, and integrating custom unified parsers.

• Tests

  • Expanded browser, parser, registry, and layout coverage for the updated behavior.

[github-actions]

📊 Conformance matrix rendered — view in CI summary

[keivenchang]

@coderabbitai review

@devin review

Context for both: this is the peer-trait + vendor-registry half of a two-way split of #174. It is deliberately behaviour-neutral — main's unified golden feed and this branch's are the same sha256 (169880fb…), so the review surface is API shape and the new process-global registry, not parser output.

Worth the most scrutiny:

• parsers/v2/src/unified/mod.rs — the registry is process-global mutable state consulted on every parser construction. Registration is documented as startup-only and is NOT linearizable against concurrent construction; I want that assumption challenged.
• Vendor registrations canonicalise family aliases, so registering qwen3 also shadows qwen3_coder. An earlier version keyed on the caller's spelling and silently ran two different parsers for one family depending on routing.
• parsers/v2/tests/vendor_parser_example.rs is the compiled form of the CUSTOM_PARSERS.md example. If those two drift, the docs are wrong and nothing else catches it.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Changed files ==='
git diff --name-status origin/main...HEAD || git diff --name-status main...HEAD

echo
echo '=== Unified module outline ==='
ast-grep outline parsers/v2/src/unified/mod.rs --items all --view expanded

echo
echo '=== Unified module diff ==='
git diff --unified=80 origin/main...HEAD -- parsers/v2/src/unified/mod.rs \
  || git diff --unified=80 main...HEAD -- parsers/v2/src/unified/mod.rs

echo
echo '=== Documentation and compiled example diffs ==='
git diff --unified=60 origin/main...HEAD -- parsers/v2/CUSTOM_PARSERS.md parsers/v2/tests/vendor_parser_example.rs \
  || git diff --unified=60 main...HEAD -- parsers/v2/CUSTOM_PARSERS.md parsers/v2/tests/vendor_parser_example.rs

echo
echo '=== Registry API definitions, calls, and tests ==='
rg -n -C 5 -P '\b(register_unified_parser|unregister_unified_parser|builtin_unified_families|UnifiedParserEvent|parse_into|tool_call_id|preserve_special_tokens)\b' \
  parsers/v2 --glob '*.rs' --glob '*.md'

Length of output: 127129

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Public exports ==='
cat -n parsers/v2/src/lib.rs | sed -n '1,55p'

echo
echo '=== Registry integration tests ==='
cat -n parsers/v2/tests/vendor_registry.rs | sed -n '1,245p'

echo
echo '=== Documentation example and compiled example ==='
cat -n parsers/v2/CUSTOM_PARSERS.md | sed -n '1,80p'
cat -n parsers/v2/tests/vendor_parser_example.rs | sed -n '1,150p'

echo
echo '=== Checks that link CUSTOM_PARSERS.md to vendor_parser_example.rs ==='
rg -n -C 4 -P 'CUSTOM_PARSERS|vendor_parser_example|include_str!|extract.*markdown|markdown.*extract|mdbook' \
  . --glob '!target/**' --glob '!**/.git/**'

echo
echo '=== All unified parser construction call sites ==='
rg -n -C 4 -P '\bcreate_unified_parser_for_family\s*\(' \
  . --glob '*.rs' --glob '!target/**'

Length of output: 44929

---

@keivenchang I found one documentation correctness issue.

P1 — Keep the published parser example identical to the compiled example.
parsers/v2/CUSTOM_PARSERS.md Line 24 appends each delta to buffered and then emits the same delta. Line 35 emits the complete buffered value again from finish. A vendor who copies this example receives duplicated output at end of stream.

For example, push("hello"), then finish() emits hellohello.

parsers/v2/tests/vendor_parser_example.rs Line 24 uses different buffering logic and does not have this defect. No check compares the Markdown code block with the integration-test source. This contradicts the claim at parsers/v2/CUSTOM_PARSERS.md Line 41 that the shown example is compiled and run.

Use the same implementation in both locations. Prefer a single source of truth or a test that extracts and compiles the Markdown block.

I found no correctness issue in the registry under its documented startup-only contract. The alias canonicalization and process-isolated registry tests cover the requested behavior.

_{You are interacting with an AI system.}

[keivenchang]

@coderabbitai full review

The head you analysed (8c785de7) no longer exists — this branch was rebased onto main after #140 landed, and force-pushed. Current head is a086bd3d. Your earlier pass posted an analysis chain but no findings, so please re-review from scratch rather than treating prior commits as reviewed.

What changed in the rebase, since it is material:

• Three conflicts in build_stream_fixtures.py, fixtures.py and resolve_stream_fixtures.py, all the same class: main renamed a case-schema key to exception, this branch still used errors. Resolved by taking main's side — main's version is a superset (normalisation on both mappings, mutual exclusivity documented) and is used consistently across five files.
• Seven conformance-tooling files reverted to main entirely, because chore(conformance): vLLM 0.26.0 + vLLM Rust 0.24.0/0.26.0 peer captures — NO PARSER CHANGE (DIS-2408) #140 landed a better version of the same work and this PR has no hover or parser content in them. The PR shrank 24 files -> 14.
• A CSS assertion still expecting 44px after the cell narrowing to 26px.

Worth the most scrutiny on this head:

• parsers/v2/src/unified/mod.rs — the vendor registry is process-global mutable state consulted on every parser construction. Registration is documented as startup-only and is NOT linearizable against concurrent construction. Challenge that assumption.
• Alias canonicalisation: registering qwen3 must also shadow qwen3_coder. An earlier version keyed on the caller's spelling and silently ran two different parsers for one family depending on routing.
• parsers/v2/tests/vendor_parser_example.rs is the compiled form of the CUSTOM_PARSERS.md example. If those drift, the docs are wrong and nothing else catches it.

This PR is deliberately behaviour-neutral: main's unified golden feed and this branch's are the same sha256, so parser output is not the review surface — API shape and the registry are.

[coderabbitai]

@keivenchang Understood. I will review the current head a086bd3d from scratch. I will not rely on the previous review state. I will verify the registry concurrency contract, alias canonicalization, and the documentation-example consistency.

✅ Action performed

Full review finished.

[coderabbitai]

Walkthrough

The unified parser now uses UnifiedParserEvent and UnifiedParserOutput, adds streaming lifecycle methods and vendor parser registration, and updates conformance tooling for compact layouts, cross-device tooltips, and event-order diagnostics.

Changes

Unified parser API

Layer / File(s)	Summary
Event and streaming contracts parsers/v2/src/unified/mod.rs	The parser uses tuple-style UnifiedParserEvent values and UnifiedParserOutput for ordered streaming updates, assembly, projection, and tests.
Scanner event emission parsers/v2/src/tool_calling/scan.rs	Scanner output now uses UnifiedParserEvent for text, reasoning, and tool-call events.
Parser registry and public surface parsers/v2/src/unified/mod.rs, parsers/v2/src/lib.rs, parsers/v2/CUSTOM_PARSERS.md, parsers/v2/tests/*, conformance/utils/piece1_coupling_gate.sh	The public API exposes parser factories and registration controls. Built-in and vendor families support canonical aliases, shadowing, restoration, and diagnostics.
Conformance event adapters conformance/tests/capture_cross_version.rs, conformance/tests/unified_render.rs	Conformance serializers consume UnifiedParserEvent while preserving existing output formats.

Conformance tooltip and layout behavior

Layer / File(s)	Summary
Tooltip interaction rules conformance/utils/src/assets/conformance.js, conformance/utils/tests/test_browser_smoke.py, conformance/utils/tests/test_stream_on_batch.py	Tooltip handlers run without media-query gating. Touch and pen taps pin tooltips; mouse clicks retain navigation behavior.
Sequence diagnostics and tooltip text conformance/utils/src/assets/conformance_view.js, conformance/utils/tests/test_browser_smoke.py	ORDER and MERGE results show expected and observed event sequences. Divergent cells receive a sequence marker, and tooltip literals use formatted spans.
Compact conformance tables conformance/utils/src/assets/conformance.css, conformance/utils/tests/test_stream_on_batch.py	Standard cells use 26px widths. Transposed Details columns use bounded sizing and ellipsized marker text.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: indrajit96

Poem

I’m a rabbit with events in a row,
Parsing streams where the green grasses grow.
Tooltips now glow, taps settle in place,
Tiny cells leave room for each face.
Vendor parsers hop through the gate—
Clean conformance arrives right on time!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main parser API refactor and vendor parser registry changes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch keivenchang/DIS-2644__peer-align-registry

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

parsers/v2/src/unified/mod.rs (2)

583-586: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reuse canonical_unified_family instead of a second generated match.

The vendor branch at Line 560 already resolves the canonical name with canonical_unified_family(family).unwrap_or(family). This second match repeats the alias arms and needs an unreachable!. Using the helper removes the duplication and the unreachable arm.

♻️ Proposed refactor

-            let canonical = match family {
-                $($family $(| $alias)* => $family,)+
-                _ => unreachable!("matched above"),
-            };
+            let canonical = canonical_unified_family(family).unwrap_or(family);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@parsers/v2/src/unified/mod.rs` around lines 583 - 586, Replace the generated
canonical match in the family handling flow with the existing
canonical_unified_family(family).unwrap_or(family) helper used by the vendor
branch. Remove the duplicated alias arms and unreachable! fallback while
preserving the resolved canonical family behavior.

---

555-573: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use one return type for all parser constructors

qwen3_unified returns Box<dyn UnifiedParser>, while UnifiedParserFactory returns Result<Box<dyn UnifiedParser>>. This prevents fallible built-in constructors from using unified_registry! directly. Make built-in constructors return Result<Box<dyn UnifiedParser>>, or document this intentional distinction.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@parsers/v2/src/unified/mod.rs` around lines 555 - 573, The built-in
constructor path in create_unified_parser_for_family must use the same
Result<Box<dyn UnifiedParser>> return type as UnifiedParserFactory. Update
qwen3_unified and any other constructors registered through unified_registry! to
return the fallible result type, propagating construction errors through the
existing match and preserving the vendor factory path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@conformance/utils/src/assets/conformance.js`:
- Around line 694-703: Scope touch/pen provenance to the originating click so
stale pointer state cannot affect later keyboard activation. In
conformance/utils/src/assets/conformance.js at lines 694-703, track and clear
the state on click cancellation or completion; at lines 824-830, consume the
click-scoped state before preventDefault() and pin(). In
conformance/utils/tests/test_browser_smoke.py at lines 553-644, add a
touch-then-keyboard regression test verifying keyboard activation does not enter
pin mode.

In `@parsers/v2/CUSTOM_PARSERS.md`:
- Around line 24-35: The documented parser must emit each input byte only once:
update the parse_into/finish example in parsers/v2/CUSTOM_PARSERS.md lines 24-35
to emit committed content immediately while retaining only uncommitted buffered
content for finish, and ensure finish flushes that remainder once. In
parsers/v2/tests/vendor_parser_example.rs lines 23-45, extract and compile the
Markdown code block or generate the block from the tested implementation so the
documentation and executable example cannot diverge.

In `@parsers/v2/src/unified/mod.rs`:
- Around line 144-152: Implement the documented reset contract by overriding
reset in WrappedBlockScanner/ScannerUnified: return the current buffer via
std::mem::take, clear the scanner state flags such as in_block and in_reasoning,
and reset next_index to its initial value so subsequent parsing starts as a
fresh stream.

---

Nitpick comments:
In `@parsers/v2/src/unified/mod.rs`:
- Around line 583-586: Replace the generated canonical match in the family
handling flow with the existing
canonical_unified_family(family).unwrap_or(family) helper used by the vendor
branch. Remove the duplicated alias arms and unreachable! fallback while
preserving the resolved canonical family behavior.
- Around line 555-573: The built-in constructor path in
create_unified_parser_for_family must use the same Result<Box<dyn
UnifiedParser>> return type as UnifiedParserFactory. Update qwen3_unified and
any other constructors registered through unified_registry! to return the
fallible result type, propagating construction errors through the existing match
and preserving the vendor factory path.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 7251359e-5339-47f9-bcd1-13e40c3faa88

📥 Commits

Reviewing files that changed from the base of the PR and between 27c3901 and a086bd3.

📒 Files selected for processing (14)

• conformance/tests/capture_cross_version.rs
• conformance/tests/unified_render.rs
• conformance/utils/piece1_coupling_gate.sh
• conformance/utils/src/assets/conformance.css
• conformance/utils/src/assets/conformance.js
• conformance/utils/src/assets/conformance_view.js
• conformance/utils/tests/test_browser_smoke.py
• conformance/utils/tests/test_stream_on_batch.py
• parsers/v2/CUSTOM_PARSERS.md
• parsers/v2/src/lib.rs
• parsers/v2/src/tool_calling/scan.rs
• parsers/v2/src/unified/mod.rs
• parsers/v2/tests/vendor_parser_example.rs
• parsers/v2/tests/vendor_registry.rs

[KrishnanPrash]

AI Generated Comment

Six comments, all in parsers/v2. Four are the same shape: the contract this PR publishes and the code implementing it disagree, in ways assemble currently hides, which is why the suite is green. Since handing this surface to people outside the crate is the whole point, those seem worth settling before the API is out. The last two are small.

Verified against head 2fd0eba locally: cargo test --workspace --all-targets --locked gives 1372 passed, cargo clippy --workspace --all-targets gives 0 warnings, and the conformance/fixtures tree plus both LFS pointers are byte-identical to main. The evidence in the description holds.

[indrajit96]

Some higher level commetns

1. The parser factory receives too little information

Today the factory looks like:

fn create_parser(tools: &[Tool]) -> Box

It receives only the available tools.

That works for Qwen because its parser mostly needs the tool schemas. Future parsers may also need:

• Which tokenizer/model variant is being used?
• Did the prompt already open the reasoning channel?
• Is the model producing native markup or guided JSON?
• Must special tokens be preserved?
• Does this deployment have vendor-specific configuration?

For example, Kimi K3 might start in reasoning mode:

Prompt already opened:
<|open|>think<|sep|>

Generated output starts directly with:
I need to call the weather tool...

The parser must know that those first generated bytes are reasoning. tools alone cannot tell it that.

A richer factory could receive:

  ParserCreateContext {
      tools,
      model_config,
      tokenizer_info,
      starting_channel: Reasoning,
      output_mode: NativeMarkup,
  }

2. The scanner structure may be too specific to Qwen-like formats

The current scanner appears to expect a mostly flat structure:

  BLOCK
    INVOKE
    INVOKE
  BLOCK_END

Kimi K3 may have nested channels or sections. A simplified example is:

  reasoning
  └── tool section
      └── invoke

A flat scanner may incorrectly close the outer block when it encounters the inner section, or emit events in the wrong order.

Before adding more model families, could we clarify whether ScannerUnified is intended to support nested formats? If it is, the scanner likely needs an explicit stack or model-specific state-machine extension point rather than only
configurable delimiters.

3. Kimi K3 (OR any NEW Model) needs a real incremental parser, not only a registry entry

Adding Kimi K3 support ideally will involve more than?

  register_unified_parser("kimi_k3", create_kimi_parser);

The parser must handle arbitrary streaming boundaries. For example, these chunks must behave exactly like one complete string:

  Chunk 1: "<|open|>to"
  Chunk 2: "ol<|sep|>{\"name\":"
  Chunk 3: "\"weather\"}..."

We should avoid implementing this logic separately in the v2 tool parser and unified parser. Otherwise, fixes for partial tokens, nesting, or malformed output must be made twice.

I suggest:

  One incremental Kimi K3 state machine
  ├── tool-only adapter
  └── unified-parser adapter

Both adapters would then share identical buffering, token recognition, validation, and error handling.

Is that the design for new models?

[keivenchang]

Yes — that is the design boundary. #178 only aligns the trait and adds startup registration for parsers that already exist; it intentionally does not claim the tools-only factory can construct tokenizer/model-dependent parsers or that ScannerUnified supports nested grammars. For a new family with distinct nesting, I would implement one model-specific incremental state machine and expose tool-only and unified adapters over it; request state/output mode belongs to per-request initialization, while tokenizer/model/vendor construction context needs a separate factory-context design before such a parser can register. I added both limitations to the Overview so this API is not presented as drop-in adoptable.

[keivenchang]

@indrajit96 — all three are fixed, here's where. Heads up that your three threads show as resolved under my account; I didn't resolve them and I'd have left them for you, so please reopen any you're not happy with.

1. reset_stream() losing bytes on emitter error — fixed in 0b84b647.

You were right, and it was worse than the single-invoke case. With prefix<tool_call><function=ok></function><function=boom></function></tool_call>suffix we also lost prefix and the ok call that had already succeeded, because drain() collected into a local vec that ? dropped on the way out.

Two changes: the drain loop now writes through a sink the caller owns, so a committed event can't be retracted; and both invoke sites call the emitter before buffer.drain(..), so a failing invoke stays recoverable. reset() now returns <function=boom></function></tool_call>suffix.

Regressions for first/later x wrapped/bare, driven through the public parse_into with exact output and exact reset bytes: first_wrapped_failure_keeps_committed_text_and_recovers_the_invoke, later_wrapped_failure_keeps_the_call_that_already_succeeded, and the two bare equivalents.

I checked vLLM while fixing this — their tool trait documents exactly this contract and CombinedParser tests it, and I reproduced their parser keeping prefix and returning the failed body from reset(). So the contract was achievable, not aspirational. Thanks, that was the deciding evidence.

2. Contradictory preserve_special_tokens — fixed in 0b84b647.

Agreed, two surfaces over one scanner can't answer differently. The value now lives on the grammar spec and both adapters read it, aggregated as the OR over tool + reasoning components — same rule as vLLM's CombinedParser, which ORs its wrapped parsers. Parity tests for the canonical family, the alias, and the tool-only adapter.

Partly deferred, deliberately: I kept the shipped true rather than flipping it. vLLM's own Qwen parser returns false, and the cached Nemotron tokenizer marks <tool_call> as special:false — so true may itself be wrong. But one tokenizer isn't an inventory, and that flip is an observable behaviour change. Upcoming PR: pin the value with a repo-owned tokenizer inventory across every supported Qwen/Nemotron alias, changing both adapters together.

3. Two vectors per push() — fixed in 0b84b647, though not by overriding push().

Real. parse_into now writes straight into the caller's UnifiedParserOutput using the same sink the recovery fix needed, so the intermediate vec is gone from the serving path without adding a second advance implementation.

I went the other way on your second suggestion on purpose: overriding ScannerUnified::push() would only help the convenience spelling, leave parse_into unchanged, and create a second advance path that can drift. Worth noting vLLM's parse_into signature is identical to ours and their native parsers also write directly, so the extra vec was ours, not inherent to the shape.

Current head is ad88face; the golden feed is byte-identical to main throughout, so none of this changes parsed output.

[indrajit96]

Thanks a lot for this massive lift off!!
@keivenchang
Approved with a couple of nits I see nothing blocking