Skip to content

feat(cli): --tools / --tool-grammar in sharpi run; canonical server ToolGrammar option - #382

Merged
pekkah merged 2 commits into
masterfrom
feat/cli-server-tool-grammar-opt-in
Jun 24, 2026
Merged

feat(cli): --tools / --tool-grammar in sharpi run; canonical server ToolGrammar option#382
pekkah merged 2 commits into
masterfrom
feat/cli-server-tool-grammar-opt-in

Conversation

@pekkah

@pekkah pekkah commented Jun 24, 2026

Copy link
Copy Markdown
Owner

What

Exposes schema/grammar-constrained tool-call decoding (issue #374) as first-class CLI/server surface instead of only the SHARPI_TOOL_GRAMMAR environment variable. Branches off master (independent of the #376/#377/#378 follow-ups).

CLI — sharpi run

  • --tools <file.json> — load OpenAI-format tool definitions (a bare array, or a { "tools": [ … ] } wrapper). They're advertised to the model via its chat template (the tools variable, previously hardcoded null). On a single-prompt (-p) run the parsed tool calls are printed after generation.
  • --tool-grammar — constrain the argument bytes to the supplied JSON Schemas via the model family's adapter (Gemma 4 today; Qwen/Llama/DeepSeek arrive with the Tool-arg grammar: Qwen/Llama JSON-syntax constraints (#374 follow-up) #376 JSON constraint). Default off → byte-identical to unconstrained decoding.
  • The shared DecodeLoop resets the constraint once per response, masks logits with Filter() while the grammar is restricting, and Accept()s every emitted token.
  • Tool-boundary stop (issue fix(gemma4): <|channel>thought<channel|> channel header leaks into text output (not recognized as reasoning) #304): the adapter's markers (Gemma 4 <|tool_response>) are resolved against the vocab and added to the stop set, so generation halts right after the call instead of running into a hallucinated trailing turn.
  • The speculative-decode path and the MTP fast path are guarded off when a constraint is active (a multi-token verify can't honor a per-token mask).
  • JSON parsing and argument rendering use manual JsonElement / Utf8JsonWriter walks — no reflection-based serialization, NativeAOT-clean.
sharpi run -m gemma.gguf -g -1 --tools weather.json --tool-grammar \
  -p "What is the weather in Paris? Use the get_weather tool."

Server

ToolGrammarHelper.Enabled now returns opts.ToolGrammar directly. The runnable host already maps SHARPI_TOOL_GRAMMAR onto that option at startup (Program.cs), so the env var still works while the library layer stops re-reading the environment — the option is the single source of truth.

Invariants

  • Default off → byte-identical (no --tools/--tool-grammar: tools template var stays null, sp.Constraint null, capture null, DecodeLoop unchanged).
  • NativeAOT-clean, TreatWarningsAsErrors satisfied (0 warnings), InvariantGlobalization-safe (Ordinal).

Verification

Build clean. Tests.Cli (51) + Tests.Server (125) green.

GPU-verified (gemma-4-12b-it-qat-q4_0, CUDA -g -1):

sharpi run --tools weather.json --tool-grammar -p "weather in Paris?"
→ Loaded 1 tool(s) from weather.json.
→ Tool-call arguments are grammar-constrained (issue #374).
→ <|tool_call>call:get_weather{location:<|"|>Paris<|"|>}<tool_call|>
→ Parsed 1 tool call(s): get_weather({"location":"Paris"})

The model halts cleanly at the tool-boundary (14 tokens, no trailing-turn noise) — vs. running to the 512-token cap emitting <|tool_response> repeats before the stop marker was added.

🤖 Generated with Claude Code

…Grammar option canonical

Exposes schema/grammar-constrained tool-call decoding (issue #374) as first-class
CLI/server surface instead of only the SHARPI_TOOL_GRAMMAR environment variable.

CLI (`sharpi run`):
- `--tools <file.json>` loads OpenAI-format tool definitions (a bare array, or a
  { "tools": [ … ] } wrapper) and advertises them to the model via its chat
  template (the `tools` variable, previously hardcoded null). On a single-prompt
  (-p) run the parsed tool calls are printed after generation.
- `--tool-grammar` constrains the argument bytes to the supplied JSON Schemas via
  the model family's adapter (Gemma 4 today; Qwen/Llama/DeepSeek arrive with the
  #376 JSON constraint). Default off → byte-identical to unconstrained decoding.
- The shared DecodeLoop resets the constraint once per response, masks logits with
  Filter() while the grammar is restricting, and Accept()s every emitted token.
  Tool-boundary stop markers (issue #304, Gemma 4 <|tool_response>) are resolved
  against the vocab and added to the stop set so generation halts after the call
  instead of running into a hallucinated trailing turn.
- The speculative-decode path and the MTP fast path are guarded off when a
  constraint is active (a multi-token verify can't honor a per-token mask).
- JSON parsing and tool-call argument rendering use manual JsonElement /
  Utf8JsonWriter walks (no reflection-based serialization) — NativeAOT-clean.

Server: ToolGrammarHelper.Enabled now returns opts.ToolGrammar directly. The
runnable host already maps SHARPI_TOOL_GRAMMAR onto that option at startup, so the
env var still works while the library layer stops re-reading the environment —
the option is the single source of truth.

GPU-verified (gemma-4-12b-it-qat-q4_0, CUDA -g -1):
  sharpi run --tools weather.json --tool-grammar -p "weather in Paris?"
  → <|tool_call>call:get_weather{location:<|"|>Paris<|"|>}<tool_call|>
  → Parsed 1 tool call(s): get_weather({"location":"Paris"})
  → clean halt at the tool-boundary (14 tokens, no trailing-turn noise)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces tool-calling capabilities to the CLI command, allowing users to specify OpenAI-format tool definitions via --tools and enforce JSON Schema constraints using --tool-grammar. It integrates these features into the decoding loop, manages stop tokens, and disables incompatible features like speculative decoding and the MTP fast path when tool constraints are active. Additionally, the server-side ToolGrammarHelper is simplified to rely solely on options as the single source of truth. The review feedback suggests resetting the static s_tools field at the start of execution to prevent state leakage across runs in the same process, and catching specific exceptions instead of a generic Exception during file I/O and JSON parsing.

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.

Comment on lines +823 to +826
List<ToolSchema>? toolSchemas = null;
ITokenConstraint? toolConstraint = null;
int[] toolBoundaryStops = [];
if (settings.ToolsPath is { Length: > 0 } toolsPath)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The static field s_tools is conditionally populated when --tools is provided, but it is never reset. In scenarios where the CLI command is executed multiple times within the same process (such as in unit/integration test suites), the state from a previous run can leak into subsequent runs that do not specify --tools. Resetting s_tools to null at the start of the execution block prevents this state leakage.

        List<ToolSchema>? toolSchemas = null;
        ITokenConstraint? toolConstraint = null;
        int[] toolBoundaryStops = [];
        s_tools = null;
        if (settings.ToolsPath is { Length: > 0 } toolsPath)

Comment thread src/SharpInference.Cli/RunCommand.cs Outdated
Comment on lines +837 to +841
catch (Exception ex)
{
AnsiConsole.MarkupLine($"[red]Error:[/] could not parse --tools file: {Markup.Escape(ex.Message)}");
return 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When performing file I/O and JSON parsing operations, it is best practice to catch specific exceptions (such as IOException, UnauthorizedAccessException, SecurityException, NotSupportedException, JsonException, and FormatException) instead of a generic Exception. This ensures precise error handling and prevents swallowing unrelated runtime exceptions (like NullReferenceException or OutOfMemoryException).

            catch (Exception ex) when (ex is IOException or UnauthorizedAccessException
                                          or System.Security.SecurityException or NotSupportedException
                                          or JsonException or FormatException)
            {
                AnsiConsole.MarkupLine($"[red]Error:[/] could not parse --tools file: {Markup.Escape(ex.Message)}");
                return 1;
            }
References
  1. When performing file I/O operations, catch specific exceptions (such as IOException, UnauthorizedAccessException, SecurityException, and NotSupportedException) instead of a generic Exception to ensure precise error handling and graceful application exits.
  2. When using Spectre.Console's AnsiConsole.MarkupLine to print file paths or user-controlled strings, use Markup.Escape to prevent bracketed characters from being incorrectly parsed as markup.

…ceptions (Gemini #382)

Address Gemini review on PR #382:
- s_tools is a static field set only when --tools is given, so a second in-process
  run without --tools (e.g. a test harness) could inherit the prior run's tools.
  Reset it to null at the start of the tool-calling block.
- Catch only the file-I/O / JSON exceptions LoadTools can actually raise
  (IOException, UnauthorizedAccessException, SecurityException, NotSupportedException,
  JsonException, FormatException) instead of a bare Exception, so unrelated runtime
  faults aren't swallowed as a "could not parse" message.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LKvjcWw1gH5HyXAwsK3quP
@pekkah

pekkah commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

Addressed both review comments in 6d471e9:

  • s_tools is now reset to null at the start of the tool-calling block so tools never leak across in-process runs.
  • The --tools parse catch is narrowed to IOException/UnauthorizedAccessException/SecurityException/NotSupportedException/JsonException/FormatException so unrelated runtime faults aren't swallowed.

Build clean, Tests.Cli (51) green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant