Skip to content

fix(ollama): show full thinking levels for discovered reasoning models#93946

Closed
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:fix/ollama-discovered-thinking-menu-v2
Closed

fix(ollama): show full thinking levels for discovered reasoning models#93946
Alix-007 wants to merge 1 commit into
openclaw:mainfrom
Alix-007:fix/ollama-discovered-thinking-menu-v2

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

The /think menu shows only off for live-discovered Ollama reasoning models (e.g. glm-5.2:cloud), while the runtime actually accepts /think medium — the menu-vs-runtime divergence reported in #93835.

Root cause: the menu path (directive-handling → provider-thinking → ollama resolveThinkingProfile) receives reasoning=undefined for live-discovered models, so it returns OLLAMA_NON_REASONING_THINKING_PROFILE (only off). But discovery (provider-models.ts) already computes reasoning=true from /api/show capabilities.includes("thinking") — that flag just never reached the policy surface. (Runtime stream.ts uses model?.reasoning !== false, treating undefined as thinking-capable, hence the divergence.)

Changes

  • resolveThinkingProfile now prefers the catalog/discovered reasoning flag, falling back to a model-name heuristic only when it is absent — so capabilities: thinking live models get the full off/low/medium/high/max profile, matching runtime acceptance.
  • Extracted the lightweight name heuristic into extensions/ollama/src/reasoning-models.ts so the policy surface doesn't pull in discovery/runtime deps from provider-models.ts; discovery reuses the same helper (batch logic unchanged).

Real behavior proof

Behavior addressed: discovered reasoning models (reasoning=true) now expose the full thinking-level menu; non-reasoning (reasoning=false) stays off-only; absent flag falls back to the name heuristic.

Test (real exported function): provider-policy-api.test.ts drives the real resolveThinkingProfile — added cases for reasoning=true → returns off/low/medium/high/max, reasoning=false → off only, and reasoning absent → heuristic fallback.

Scope / context

Linked to #93835 (reported with reproduction). Competing PRs #93844/#93864/#93872/#93882 only fall back to the model-name heuristic, but /r1|reasoning|think|reason/i returns false for the reported glm-5.2:cloud, so they don't fix the reported model. This fix uses the actual capabilities-derived reasoning flag from discovery, correctly covering it.

@openclaw-barnacle openclaw-barnacle Bot added extensions: ollama size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 17, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 17, 2026
Closed
@Alix-007
Alix-007 force-pushed the fix/ollama-discovered-thinking-menu-v2 branch 2 times, most recently from 837891f to b5b4006 Compare June 17, 2026 08:02
@Alix-007
Alix-007 force-pushed the fix/ollama-discovered-thinking-menu-v2 branch from b5b4006 to 6cfebdc Compare June 18, 2026 02:13
This was referenced Jun 18, 2026
Closed
Closed
Closed
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

This PR should close as superseded: it changes the Ollama provider-policy fallback, but the reported menu bug is best owned by the native command menu using runtime catalog metadata, which #94067 already targets across Telegram, Slack, and Discord.

Root-cause cluster
Relationship: superseded
Canonical: #94067
Summary: This PR is one of several candidate fixes for the live-discovered Ollama /think menu bug; the channel-side runtime-catalog PR is the strongest canonical landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this branch and route the remaining fix through #94067, where native /think menus consume the runtime catalog instead of provider-name guessing.

So I’m closing this here and keeping the remaining discussion on #94067.

Review details

Best possible solution:

Close this branch and route the remaining fix through #94067, where native /think menus consume the runtime catalog instead of provider-name guessing.

Do we have a high-confidence way to reproduce the issue?

Yes for source reproduction: the linked user issue gives concrete OpenClaw 2026.6.8 Telegram/Ollama steps, and current source shows native menu choices fall back to configured-only catalog metadata. I did not run a live Telegram/Ollama endpoint in this read-only review.

Is this the best way to solve the issue?

No. This provider-name heuristic is not the best fix because the missing fact is already available in the runtime model catalog, and the canonical channel-side PR passes that catalog into native /think menu resolution.

Security review:

Security review cleared: The diff only changes TypeScript Ollama provider-policy logic and tests, with no dependency, workflow, secret, permission, package, or supply-chain surface changes.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy read: Read the full root AGENTS.md and scoped extensions guide; plugin-boundary, best-fix, duplicate/superseded, and Telegram visible-proof guidance affected this review. (AGENTS.md:1, c4d1f37d3326)
  • Current provider policy collapses missing reasoning to off-only: Current main's Ollama policy returns the full profile only when catalog reasoning is truthy, so absent catalog metadata still produces the off-only profile. (extensions/ollama/provider-policy-api.ts:54, c4d1f37d3326)
  • Runtime catalog already records Ollama thinking capability: Ollama model discovery maps /api/show capabilities containing thinking to reasoning: true, which is the authoritative metadata the menu needs. (extensions/ollama/src/provider-models.ts:257, c4d1f37d3326)
  • Native menu path falls back to configured-only catalog: resolveCommandArgMenu uses a caller-provided catalog or builds a configured-only catalog, and the Telegram native command path currently calls it without a runtime catalog. (src/auto-reply/commands-registry.ts:338, c4d1f37d3326)
  • This PR's fallback misses the reported model: The PR falls back to /r1|reasoning|think|reason/i when reasoning is absent; that does not match the reported glm-5.2:cloud model, so it leaves the central symptom unfixed on the menu path. (extensions/ollama/provider-policy-api.ts:62, 6cfebdc5622d)
  • Canonical replacement is viable and broader: The open canonical PR is mergeable, closes the user bug, and passes the runtime model catalog into Telegram, Slack, and Discord native /think menu/autocomplete resolution. (extensions/telegram/src/bot-native-commands.ts:1138, a2aed9e4ffc4)

Likely related people:

  • openperf: Authored the open canonical runtime-catalog PR that targets the same linked /think menu bug across Telegram, Slack, and Discord native surfaces. (role: likely follow-up owner; confidence: high; commits: a2aed9e4ffc4; files: extensions/telegram/src/bot-native-commands.ts, extensions/slack/src/monitor/slash.ts, extensions/discord/src/monitor/native-command.ts)
  • steipete: GitHub file history shows repeated Ollama provider-model and live catalog metadata work, including the reasoning facts the canonical menu fix consumes. (role: adjacent provider/catalog contributor; confidence: high; commits: af79cd6a9d35, a1b89317a8bb, 7a4574376a79; files: extensions/ollama/src/provider-models.ts, extensions/ollama/provider-policy-api.ts)
  • yfge: GitHub file history shows this account introduced the Ollama provider-policy API artifact that this PR changes. (role: introduced behavior; confidence: medium; commits: 7a9efc138998; files: extensions/ollama/provider-policy-api.ts, extensions/ollama/provider-policy-api.test.ts)

Codex review notes: model internal, reasoning high; reviewed against c4d1f37d3326.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 19, 2026
This was referenced Jun 19, 2026
Closed
Closed
@Alix-007

Alix-007 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #94067 (platinum, ready for maintainer review), which supersedes this change for the same area. Keeping the queue focused — thanks!

@Alix-007 Alix-007 closed this Jun 20, 2026
@pr10mail5 pr10mail5 mentioned this pull request Jul 15, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: ollama mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Alix-007