fix(reviewer): match the submission instruction to the backend's mechanism

rcsheets commented

2026-07-26 21:56:49 +00:00

Owner

Every system prompt closed with "Submit ... using the tool.", hard
coded into the constant. That sentence was correct for Anthropic and
vLLM, which force a named tool call, and wrong for gllm, which has no
tool support at all — it drops tools/tool_choice at JSON decode, so all
four gllm passes shipped an instruction naming a mechanism the request
never carried. A model told to call a tool it has not been given cannot
comply, and improvises: prose, a fenced JSON blob, or a hallucinated
tool-call transcript, none of which decode.

Introduce OutputMode (OutputTools, OutputJSONSchema) and add it to the
Backend interface. The five prompt constants now hold body text only;
the closing instruction is appended per mode. Tool mode reproduces the
original sentences byte for byte, so nothing changes for Anthropic or
vLLM. JSON mode instead points the model at the schema by name and
forbids anything outside the object.

The schema name in the instruction is the same string the gllm backend
passes to response_format, so the prompt and the constraint cannot name
different things. The JSON-schema constraint itself was already in place
on all four gllm passes; what was missing was a prompt that agreed with
it.

The runner resolves the prompt for persistence through the same mode as
the backend it dispatches to, so prompt_system on the event row stays
identical to what was sent.

A config's SystemPromptOverride is still sent verbatim in both modes. It
replaces the prompt wholesale by contract, and appending our closing
line could contradict one the admin wrote deliberately.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

Every system prompt closed with "Submit ... using the <name> tool.", hard coded into the constant. That sentence was correct for Anthropic and vLLM, which force a named tool call, and wrong for gllm, which has no tool support at all — it drops tools/tool_choice at JSON decode, so all four gllm passes shipped an instruction naming a mechanism the request never carried. A model told to call a tool it has not been given cannot comply, and improvises: prose, a fenced JSON blob, or a hallucinated tool-call transcript, none of which decode. Introduce OutputMode (OutputTools, OutputJSONSchema) and add it to the Backend interface. The five prompt constants now hold body text only; the closing instruction is appended per mode. Tool mode reproduces the original sentences byte for byte, so nothing changes for Anthropic or vLLM. JSON mode instead points the model at the schema by name and forbids anything outside the object. The schema name in the instruction is the same string the gllm backend passes to response_format, so the prompt and the constraint cannot name different things. The JSON-schema constraint itself was already in place on all four gllm passes; what was missing was a prompt that agreed with it. The runner resolves the prompt for persistence through the same mode as the backend it dispatches to, so prompt_system on the event row stays identical to what was sent. A config's SystemPromptOverride is still sent verbatim in both modes. It replaces the prompt wholesale by contract, and appending our closing line could contradict one the admin wrote deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

rcsheets added 1 commit

2026-07-26 21:56:49 +00:00

fix(reviewer): match the submission instruction to the backend's mechanism

ci / check (pull_request) Has been cancelled

Details

f5aa7fd454

Every system prompt closed with "Submit ... using the <name> tool.", hard
coded into the constant. That sentence was correct for Anthropic and
vLLM, which force a named tool call, and wrong for gllm, which has no
tool support at all — it drops tools/tool_choice at JSON decode, so all
four gllm passes shipped an instruction naming a mechanism the request
never carried. A model told to call a tool it has not been given cannot
comply, and improvises: prose, a fenced JSON blob, or a hallucinated
tool-call transcript, none of which decode.

Introduce OutputMode (OutputTools, OutputJSONSchema) and add it to the
Backend interface. The five prompt constants now hold body text only;
the closing instruction is appended per mode. Tool mode reproduces the
original sentences byte for byte, so nothing changes for Anthropic or
vLLM. JSON mode instead points the model at the schema by name and
forbids anything outside the object.

The schema name in the instruction is the same string the gllm backend
passes to response_format, so the prompt and the constraint cannot name
different things. The JSON-schema constraint itself was already in place
on all four gllm passes; what was missing was a prompt that agreed with
it.

The runner resolves the prompt for persistence through the same mode as
the backend it dispatches to, so prompt_system on the event row stays
identical to what was sent.

A config's SystemPromptOverride is still sent verbatim in both modes. It
replaces the prompt wholesale by contract, and appending our closing
line could contradict one the admin wrote deliberately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pr-reviewer-bot commented

2026-07-26 21:56:50 +00:00

Collaborator

Automated review by pr-reviewer v0.41.1 | Safety Check | Claude Sonnet 5 | tracking id r-6682a2-33f8db
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

✅ Verdict: LGTM — Clean, well-tested refactor with no behavior change for existing tool-based backends and a targeted fix for the gllm prompt mismatch; nothing alarming found.

Reviewed the diff: this is a well-scoped refactor that adds an OutputMode concept to the Backend interface and threads it through the prompt-building functions so gllm (JSON-schema mode, no tool support) no longer gets an instruction to "use the X tool."

Checks:

Interface change (Backend.OutputMode()) is implemented on all three backends (Anthropic, vLLM, gllm) plus the test doubles (fakeBackend, stubBackend), so nothing should fail to compile.
Tool-mode prompt text is verified byte-for-byte identical via TestToolModePromptsUnchanged, matching the stated goal of no behavior change for Anthropic/vLLM.
JSON-schema mode instructions correctly avoid mentioning tools, and tests assert the schema name in the prompt matches the name passed to response_format in the actual request body — good regression coverage.
SystemPromptOverride is still passed through verbatim in both modes, matching stated intent, and there's a test for it.
Runner's outputModeFor falls back to OutputTools for unregistered providers; that's cosmetically wrong for JSON-schema providers but harmless since runOne errors out for that config anyway before this matters.
No secrets, no incidental large files, no obviously incorrect logic — the change is additive/backwards-compatible for existing (Anthropic/vLLM) behavior and fixes a real bug for gllm.

Nothing here looks unsafe or like it shouldn't have been pushed.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.41.1 | Safety Check | Claude Sonnet 5 | tracking id `r-6682a2-33f8db`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — Clean, well-tested refactor with no behavior change for existing tool-based backends and a targeted fix for the gllm prompt mismatch; nothing alarming found. Reviewed the diff: this is a well-scoped refactor that adds an `OutputMode` concept to the `Backend` interface and threads it through the prompt-building functions so gllm (JSON-schema mode, no tool support) no longer gets an instruction to "use the X tool." Checks: - Interface change (`Backend.OutputMode()`) is implemented on all three backends (Anthropic, vLLM, gllm) plus the test doubles (`fakeBackend`, `stubBackend`), so nothing should fail to compile. - Tool-mode prompt text is verified byte-for-byte identical via `TestToolModePromptsUnchanged`, matching the stated goal of no behavior change for Anthropic/vLLM. - JSON-schema mode instructions correctly avoid mentioning tools, and tests assert the schema name in the prompt matches the name passed to `response_format` in the actual request body — good regression coverage. - `SystemPromptOverride` is still passed through verbatim in both modes, matching stated intent, and there's a test for it. - Runner's `outputModeFor` falls back to `OutputTools` for unregistered providers; that's cosmetically wrong for JSON-schema providers but harmless since `runOne` errors out for that config anyway before this matters. - No secrets, no incidental large files, no obviously incorrect logic — the change is additive/backwards-compatible for existing (Anthropic/vLLM) behavior and fixes a real bug for gllm. Nothing here looks unsafe or like it shouldn't have been pushed.

rcsheets force-pushed fix/gllm-prompt-output-mode from f5aa7fd454

ci / check (pull_request) Has been cancelled

Details

to f85830faa2

ci / check (pull_request) Successful in 41s

Details

2026-07-26 21:56:54 +00:00

Compare

rcsheets merged commit 7c7d782527 into main

2026-07-26 22:09:52 +00:00

rcsheets deleted branch fix/gllm-prompt-output-mode

2026-07-26 22:09:52 +00:00