feat: per-config thinking switch and stored reasoning for gllm #118

Merged

rcsheets merged 1 commit from feat/thinking-option into main

2026-10-02 22:41:28 +00:00

rcsheets commented

2026-10-02 19:49:35 +00:00

Owner

Problem

Granite 4 and Nemotron 3 reason before they answer unless told not to, and gllm follows the model's default for a request that does not say. pr-reviewer had no way to say either way, and when a model did reason, the reasoning was discarded: a call that spent most of its output budget thinking showed a large token count beside a short response, and one cut off mid-thought showed an empty response and no evidence of why.

Change

A per-config Thinking setting. model_configs gains a nullable thinking column, edited on /admin/configs beside temperature: on, off, or "model default".

The gllm backend sends it as chat_template_kwargs: {"enable_thinking": ...} on all four calls (preflight, discovery, extract, review). Unset sends nothing, so the model's chat template decides; the field must be absent rather than defaulted, since gllm rejects an explicit true for a model with no reasoning mode.
Preflight, discovery and extract take the primary config's setting, since they run on its backend.
Being per config, the same model can run with thinking on as primary and off as a shadow, and the two compared on the comparison page.
The vLLM and Anthropic backends ignore it; the form label says so.

Stored and displayed reasoning. The gllm backend reads reasoning_content and usage.completion_tokens_details.reasoning_tokens, and each call's review_passes row stores them (reasoning, reasoning_tokens). The comparison page shows the reasoning inside each shared pass's "Input and output", and as a collapsible "Model reasoning" in a config's own column for its review call. A call that did not reason adds nothing to the page. Nothing decides on the reasoning; it is there to be read.

Things to know before merging or enabling

Migration order. This adds 000021_model_config_thinking and 000022_review_pass_reasoning. 000020 is taken by #116. golang-migrate only moves forward from the database's current version, so if this deploys before #116, #116's 000020 would be skipped on that database. Merge #116 first, or renumber whichever lands second.
Needs brooktrails/gllm#109. Before it, gllm applies the schema from the first generated token, the model can never close its reasoning block, and every thinking request returns empty content.
Needs brooktrails/slp#19 for the reasoning text. Requests reach gllm through SLP, whose stream reassembly drops reasoning_content; without that fix the page shows a reasoning token count with no text.
Reasoning is spent from max_tokens. The review caps are per config, but preflight and discovery are fixed at 1024 in code and may be tight with thinking on. Not changed here.
Storage. Reasoning text is stored per call, on top of the prompts already kept in review_passes.

docs/gllm-schema-notes.md gains a "Thinking" section covering the above.

Verification

TestGLLMThinkingOnTheWire: the kwarg is present with the right value, or absent, on each of the four calls; TestVLLMIgnoresThinking.
TestGLLMReasoningCaptured, TestGLLMReasoningWithoutAnswer: reasoning and its token count reach each result, including a turn with no answer.
TestConfigThinkingRoundTrips, TestPassReasoningRoundTrips (Postgres): all three thinking states survive a round trip, clearing writes NULL back, and reasoning survives the two-phase pass upsert.
TestConfigsListRendersThinking, TestComparePageShowsReasoning: the edit form preselects the stored choice; reasoning renders where its call is shown.
gofmt, go vet, go build, and go test -race ./... pass, the tracker tests against a local Postgres 16. Not exercised against a live gllm.

🤖 Generated with Claude Code

## Problem Granite 4 and Nemotron 3 reason before they answer unless told not to, and gllm follows the model's default for a request that does not say. pr-reviewer had no way to say either way, and when a model did reason, the reasoning was discarded: a call that spent most of its output budget thinking showed a large token count beside a short response, and one cut off mid-thought showed an empty response and no evidence of why. ## Change **A per-config Thinking setting.** `model_configs` gains a nullable `thinking` column, edited on `/admin/configs` beside temperature: on, off, or "model default". - The gllm backend sends it as `chat_template_kwargs: {"enable_thinking": ...}` on all four calls (preflight, discovery, extract, review). Unset sends nothing, so the model's chat template decides; the field must be absent rather than defaulted, since gllm rejects an explicit `true` for a model with no reasoning mode. - Preflight, discovery and extract take the primary config's setting, since they run on its backend. - Being per config, the same model can run with thinking on as primary and off as a shadow, and the two compared on the comparison page. - The vLLM and Anthropic backends ignore it; the form label says so. **Stored and displayed reasoning.** The gllm backend reads `reasoning_content` and `usage.completion_tokens_details.reasoning_tokens`, and each call's `review_passes` row stores them (`reasoning`, `reasoning_tokens`). The comparison page shows the reasoning inside each shared pass's "Input and output", and as a collapsible "Model reasoning" in a config's own column for its review call. A call that did not reason adds nothing to the page. Nothing decides on the reasoning; it is there to be read. ## Things to know before merging or enabling - **Migration order.** This adds `000021_model_config_thinking` and `000022_review_pass_reasoning`. `000020` is taken by #116. golang-migrate only moves forward from the database's current version, so if this deploys before #116, #116's `000020` would be skipped on that database. Merge #116 first, or renumber whichever lands second. - **Needs brooktrails/gllm#109.** Before it, gllm applies the schema from the first generated token, the model can never close its reasoning block, and every thinking request returns empty content. - **Needs brooktrails/slp#19 for the reasoning text.** Requests reach gllm through SLP, whose stream reassembly drops `reasoning_content`; without that fix the page shows a reasoning token count with no text. - **Reasoning is spent from `max_tokens`.** The review caps are per config, but preflight and discovery are fixed at 1024 in code and may be tight with thinking on. Not changed here. - **Storage.** Reasoning text is stored per call, on top of the prompts already kept in `review_passes`. `docs/gllm-schema-notes.md` gains a "Thinking" section covering the above. ## Verification - `TestGLLMThinkingOnTheWire`: the kwarg is present with the right value, or absent, on each of the four calls; `TestVLLMIgnoresThinking`. - `TestGLLMReasoningCaptured`, `TestGLLMReasoningWithoutAnswer`: reasoning and its token count reach each result, including a turn with no answer. - `TestConfigThinkingRoundTrips`, `TestPassReasoningRoundTrips` (Postgres): all three thinking states survive a round trip, clearing writes NULL back, and reasoning survives the two-phase pass upsert. - `TestConfigsListRendersThinking`, `TestComparePageShowsReasoning`: the edit form preselects the stored choice; reasoning renders where its call is shown. - `gofmt`, `go vet`, `go build`, and `go test -race ./...` pass, the tracker tests against a local Postgres 16. Not exercised against a live gllm. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

rcsheets added 1 commit

2026-10-02 19:49:45 +00:00

feat: per-config thinking switch and stored reasoning for gllm

ci / check (pull_request) Successful in 54s

Details

ci / integration (pull_request) Successful in 43s

Details

ci / browser (pull_request) Successful in 40s

Details

91a3215a5c

Granite 4 and Nemotron 3 reason before they answer unless told not to,
and gllm follows the model's default for a request that does not say.
pr-reviewer had no way to say, and discarded the reasoning when there
was any.

A model config gains a Thinking setting -- on, off, or unset for the
model's own default -- edited on /admin/configs beside temperature. The
gllm backend sends it as chat_template_kwargs.enable_thinking on all
four calls, and omits the field entirely when unset, since gllm rejects
an explicit true for a model with no reasoning mode. Preflight,
discovery and extract take the primary config's setting, as they run on
its backend. Being per config, the same model can run with thinking on
as primary and off as a shadow and be compared directly. The vLLM and
Anthropic backends ignore it.

The gllm backend now also reads reasoning_content and
usage.completion_tokens_details.reasoning_tokens, and every call's pass
row stores them (review_passes.reasoning, reasoning_tokens). The
comparison page shows the reasoning inside each shared pass's "Input and
output" and as "Model reasoning" in a config's own column. A call cut
off mid-thought has reasoning and no response, which is the case the
stored text explains.

Migrations are numbered 000021 and 000022 because 000020 is taken by the
review-retries change still in review.

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

pr-reviewer-bot commented

2026-10-02 19:50:11 +00:00

Collaborator

Automated review by pr-reviewer v0.52.3 | Safety Check | Granite | tracking id r-c00aef-6a9dc0
This is an AI-generated review and may contain mistakes.

Status: ❌ Failed

Review failed. Tracking id r-c00aef-6a9dc0 — see logs for details.

Comment @pr-reviewer-bot retry to try again.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.52.3 | Safety Check | Granite | tracking id `r-c00aef-6a9dc0`* *This is an AI-generated review and may contain mistakes.* **Status:** ❌ Failed --- Review failed. Tracking id `r-c00aef-6a9dc0` — see logs for details. Comment `@pr-reviewer-bot retry` to try again.

rcsheets merged commit 8d1018f61a into main

2026-10-02 22:41:28 +00:00

rcsheets deleted branch feat/thinking-option

2026-10-02 22:41:29 +00:00

rcsheets referenced this pull request from a commit

2026-10-02 22:41:30 +00:00

Merge pull request 'feat: per-config thinking switch and stored reasoning for gllm' (#118) from feat/thinking-option into main