test: add integration-level test coverage for ModelOption.THINKING on OllamaModelBackend by planetf1 · Pull Request #1613 · generative-computing/mellea

planetf1 · 2026-09-03T10:42:04Z

Pull Request

Issue

Description

ModelOption.THINKING was only tested against a mocked backend — nothing
exercised it against a live model. That's a real gap: structured-output
examples using @generative stubs can intermittently fail because the parser
receives combined think+answer text, and only THINKING: False suppression
prevents it. Nothing was guarding that suppression from silently regressing.

Adds three live-model tests to test/backends/test_ollama.py:

Test	What it proves
`test_thinking_suppressed_per_call`	per-call `THINKING: False` overrides a `THINKING: True` construction-time default
`test_thinking_enabled_mot_field_nonempty`	per-call `THINKING: True` overrides a `THINKING: False` construction-time default (`slow` — generates a real think block)
`test_construction_time_thinking_default_suppresses`	construction-time `THINKING: False` holds with no per-call override

Each override test flips the construction-time default in the opposite
direction from what it sets per-call, so it's actually falsifiable rather than
just agreeing with the default.

Known gap, documented in-line: pyproject.toml's default addopts
excludes slow tests. Verified: PR CI (quality.yml) never overrides
-m, so it never runs the positive-direction test. Unverified: the
nightly script committed in this repo also has no override by default, but
it's driven by an external nightly.py not present here — I can't confirm
whether that driver passes -m slow, so nightly coverage is unknown rather
than ruled out. Worth a maintainer confirming.

A regression in think-block capture itself would only be caught by that one
test; a regression in the override plumbing is still caught by the other two
on every CI run. I ran the positive test manually for this PR (see Testing) —
that's its only verification unless nightly does invoke -m slow.

Also included: a one-line fix for test/README.md's stale CI model list
(said granite4:micro; quality.yml actually pulls granite4.2:3b +
granite4:micro-h + the vision GGUF) — found while verifying the gap above.

Testing

uv run pytest test/backends/test_ollama.py -v          # 12 pass, 1 xpass, 1 deselected
uv run pytest test/backends/test_ollama.py -m slow -v  # the deselected test, passes

Both run live against a warm granite4.2:3b on Ollama 0.33.2.
ruff format --check, ruff check, and mypy all clean.

Tests added to the respective file if code was changed
New code has 100% coverage if code was added
Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

Component
Requirement
Sampling Strategy
Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

Covers per-call suppression, per-call enablement, and construction-time default suppression against a real Ollama model, closing the gap where only mocked unit tests existed for this behaviour. Fixes generative-computing#1553 Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

Code-review NIT/SUGGESTION follow-up: note why test_thinking_enabled_mot_field_nonempty is slow while its THINKING=False siblings aren't. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

Second review round (qwen) follow-up: - Extract _start_ollama_session() so session and thinking_on_session share construction instead of duplicating it (was a verbatim clone with one flag flipped, risking silent divergence). - Move thinking_on_session next to session so the module's fixtures stay grouped, per test/README.md fixture-discipline guidance. - Split test_thinking_enabled_mot_field_nonempty's docstring into a single behavioural claim (test/README.md: one claim per test). - Include the resolved model id in the assertion message so a failure distinguishes plumbing regressions from a non-thinking model/runtime. - Document in-code why the slow-marked positive test is invisible to PR CI and the nightly script (both inherit pyproject.toml's default `-m "not slow"`), so the gap is visible without relying on a PR body. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

quality.yml pulls granite4.2:3b, granite4:micro-h, and the granite-vision-4.1-4b GGUF — the doc still said granite4:micro and omitted the vision model. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

The prior comment stated the nightly script never runs the slow test as fact. That's only verified for PR CI (quality.yml has no -m override) — the nightly script committed here has the same default, but is invoked by an external nightly.py not in this repo, so whether it passes -m slow is unverified, not ruled out. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

psschwei · 2026-09-08T17:33:50Z

+# think-block capture itself (the THINKING=False tests would pass vacuously in
+# that case). A regression in the merge/override plumbing is still caught by
+# the other two.
+@pytest.mark.slow  # generates a full think block, unlike its THINKING=False siblings


fwiw, Claude did not like marking this as a "slow" test, saying that even though it took twice as long as the other tests, it still finished in under a second (at least when claude ran it). Good chance that Claude is being overly literal about the slow, but passing that feedback along.

Confirmed — measured 0.83s. Dropped the marker, thanks.

markstur

LGTM, see inline for one readability nit

markstur · 2026-09-09T04:15:30Z



+@pytest.fixture(scope="function")
+def thinking_on_session():


I think this would read better if s/thinking_on_session/thinking_session/

but author's choice

Done, thanks.

…dability test_thinking_enabled_mot_field_nonempty runs in 0.83s (measured against a live granite4.2:3b Ollama call) — nowhere near the >1 minute threshold test/README.md documents for `slow`. It now runs in default/PR CI like its siblings, and the CI-blind-spot comment justifying the marker is removed along with it. Also renames the thinking_on_session fixture to thinking_session per review nit (redundant "on"). Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

planetf1 added 4 commits September 3, 2026 10:44

test(ollama): document why the THINKING-enabled test is marked slow

a53573e

Code-review NIT/SUGGESTION follow-up: note why test_thinking_enabled_mot_field_nonempty is slow while its THINKING=False siblings aren't. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

docs(test): fix stale PR CI model list in test/README.md

a3d25d1

quality.yml pulls granite4.2:3b, granite4:micro-h, and the granite-vision-4.1-4b GGUF — the doc still said granite4:micro and omitted the vision model. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

github-actions Bot added the testing label Sep 3, 2026

planetf1 marked this pull request as ready for review September 3, 2026 11:18

planetf1 requested a review from a team as a code owner September 3, 2026 11:18

planetf1 requested review from ajbozarth, jakelorocco and markstur September 3, 2026 11:18

psschwei reviewed Sep 8, 2026

View reviewed changes

markstur approved these changes Sep 9, 2026

View reviewed changes

planetf1 enabled auto-merge September 9, 2026 07:12

planetf1 added this pull request to the merge queue Sep 9, 2026

Merged via the queue into generative-computing:main with commit e6861c4 Sep 9, 2026
9 checks passed

planetf1 deleted the issue-1553 branch September 9, 2026 08:09

Navigation Menu

test: add integration-level test coverage for ModelOption.THINKING on OllamaModelBackend - #1613

test: add integration-level test coverage for ModelOption.THINKING on OllamaModelBackend#1613
planetf1 merged 6 commits into
generative-computing:mainfrom
planetf1:issue-1553

planetf1 commented Sep 3, 2026 •

edited

Loading

Uh oh!

psschwei Sep 8, 2026

Uh oh!

planetf1 Sep 9, 2026

Uh oh!

markstur left a comment

Uh oh!

markstur Sep 9, 2026

Uh oh!

planetf1 Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants



		@pytest.fixture(scope="function")
		def thinking_on_session():

Conversation

planetf1 commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Issue

Description

Testing

Attribution

Adding a new component, requirement, sampling strategy, or tool?

Uh oh!

psschwei Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

planetf1 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

markstur left a comment

Choose a reason for hiding this comment

Uh oh!

markstur Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

planetf1 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

planetf1 commented Sep 3, 2026 •

edited

Loading