fix(gemma4): stop <|channel> thought markers leaking into text output (#304)

[pekkah]

Fixes #304.

Problem

Driving Gemma 4 12B (QAT q4_0) through an in-process agentic tool-call loop leaked the model's <|channel>thought<channel|> reasoning-channel header into the user-facing GenerateChunkKind.Text stream. The issue + its comment surface three coupled problems in the same scenario:

1. Channel markers leak into Text (the titled bug) — and a bare <channel|> close can appear with no preceding open.
2. Generation doesn't stop at the tool-call boundary — this QAT line opens a <|tool_response> turn (not in the EOG set) instead of <end_of_turn>, so it runs on and hallucinates the channel tail.
3. StopTokenIds is a replace-not-augment footgun — sp.StopTokenIds ?? EogTokenIds silently drops EOG when a consumer adds a stop.

Root cause of (1)

The reasoning split is keyed on token IDs the engine is told about. <|channel> / <channel|> are single special tokens in the Gemma 4 vocab (verified: ids 100 / 101, type 4 USER_DEFINED), and the server + CLI each resolved them — but InferenceEngine's convenience constructor hardcoded the split off (thinkTokenId: -1), so an in-process host (e.g. Ayu) never got it. The boundary handling also emitted an orphan/bare close as literal text.

Fix

Core

• ITokenizer.ReasoningTokens (default (-1,-1)); GgufTokenizer resolves <think>/</think> then <|channel>/<channel|> — one source of truth for engine, loader, and CLI.
• IToolCallAdapter.ToolBoundaryStopMarkers (default []); Gemma 4 returns <|tool_response>.
• Gemma4ToolCallAdapter.Parse scrubs <|channel>label … <channel|> blocks (incl. bare close / unterminated open) from PlainText, mirroring the GGUF chat template's own history scrubber.

Engine

• Convenience constructor auto-resolves the split from tokenizer.ReasoningTokens (fixes the in-process leak; explicit -1 still forces it off).
• Reasoning boundary tokens are always consumed, never emitted — a bare <channel|> close (which the post-tool generation prompt can prime) is swallowed, not leaked. Mirrored in ContinuousBatchingEngine.
• SamplingParams.AdditionalStopTokenIds — always unioned with the effective stop set (footgun-free way to add a stop); StopTokenIds replace semantics documented.

Server

• Loader resolves the adapter's tool-boundary markers → ids, stashed on ChatTemplateRenderer; the OpenAI + Anthropic chat endpoints add them to the stop set on tool-active turns so generation halts the instant the tool calls finish.

Compatibility

All default paths are byte-identical: no reasoning tokens → (-1,-1); null AdditionalStopTokenIds → base stop set unchanged; non-Gemma adapters → no tool-boundary markers.

Tests

• Core — channel scrub shapes (header-only, with reasoning content, bare close, unterminated open, after a tool call, untouched inside a tool arg) + tool-boundary markers; model-free ResolveReasoningTokens (think/channel/precedence/zero-id/partial-pair).
• Engine — bare-close suppression, convenience-ctor auto-resolve, additive-stop union + EOG retention.
• Server — renderer tool-boundary stash/clear.

Build clean (warnings-as-errors). Model-free suites green (Core 169, Server 125, Cli 41) + mock-based engine tests (50). The heavy GPU/CPU model matrix was not run locally; channel behavior is unit-tested against the real Gemma 4 vocab ids (100/101).

🤖 Generated with Claude Code

https://claude.ai/code/session_01KDR5BLSrhvCk7URYqrp6zA

[gemini-code-assist]

Code Review

This pull request centralizes the resolution of reasoning boundary tokens (such as ChatML and Gemma 4 <|channel>) onto the tokenizer and introduces AdditionalStopTokenIds to safely append stop tokens without overriding the default end-of-generation (EOG) set. It also ensures that orphan or malformed boundary tokens are swallowed rather than leaked as plain text. The review feedback suggests adding defensive null checks for fwd and tokenizer in the InferenceEngine constructor, and simplifying the HashSet initialization in ResolveStopSet to avoid an explicit cast.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

To prevent a potential NullReferenceException when dereferencing tokenizer in the constructor initializer, and to enforce defensive programming, we should validate that fwd and tokenizer are not null using the ?? throw pattern.

    public InferenceEngine(
        IForwardPass fwd,
        ITokenizer tokenizer,
        string modelId,
        params IDisposable[] owned)
        : this(fwd ?? throw new ArgumentNullException(nameof(fwd)),
               tokenizer ?? throw new ArgumentNullException(nameof(tokenizer)),
               modelId,
               tokenizer.ReasoningTokens.Open,
               tokenizer.ReasoningTokens.Close,
               owned)
    {
    }

[gemini-code-assist]

We can simplify the initialization of the HashSet<int> by using the conditional operator directly on the constructor, which avoids the need for the explicit cast to IEnumerable<int>.

    internal ImmutableArray<int> ResolveStopSet(ImmutableArray<int> eog)
    {
        if (AdditionalStopTokenIds is not { Length: > 0 } extra)
            return StopTokenIds is { } userStops ? [.. userStops] : eog;

        var set = StopTokenIds is { } userStops ? new HashSet<int>(userStops) : new HashSet<int>(eog);
        foreach (int id in extra) set.Add(id);
        return [.. set];
    }

[pekkah]

Addressed the review feedback across two cycles (internal review agents + Gemini Code Assist):

Internal review (correctness / silent-failure):

• ContinuousBatchingEngine.AdmitOne routed the first sampled token through the old guarded boundary form — a bare <channel|> first token (or a double-open) still leaked on the batched path. Migrated to the same always-consume contract as the decode loop.
• CLI EmitToken had the same orphan-close gap (a bare close while !inThinking printed the literal marker). Now always-consumed; double-open is idempotent.
• Loader now warns (stderr, mirroring the image-input diagnostic) when an adapter declares tool-boundary markers that don't resolve to a vocab id, instead of silently re-opening the run-on symptom.
• Documented the OpenAI-vs-Anthropic tool-boundary-stop gate difference (each is scoped to that endpoint's own tool-aware rendering condition).

Gemini Code Assist:

• Added ?? throw new ArgumentNullException guards to the InferenceEngine convenience ctor (it now dereferences tokenizer.ReasoningTokens in the base initializer).
• Simplified ResolveStopSet's HashSet seeding to drop the IEnumerable<int> cast.

Build clean (warnings-as-errors); model-free suites green (Core 169, Server 125, Cli 41) + mock-based engine tests. The heavy GPU/CPU model matrix wasn't run locally; channel behavior is unit-tested against the real Gemma 4 vocab ids (<|channel>=100 / <channel|>=101). The ContinuousBatchingEngine + CLI boundary fixes are 1:1 mirrors of the engine logic covered by GenerateChunksAsync_BareCloseWithNoOpen_IsSwallowedNotLeaked.