Skip to content

fix(providers): scope think to Ollama endpoints - #64286

Open
AXEG0 wants to merge 3 commits into
NousResearch:mainfrom
AXEG0:fix/custom-ollama-think-param
Open

fix(providers): scope think to Ollama endpoints#64286
AXEG0 wants to merge 3 commits into
NousResearch:mainfrom
AXEG0:fix/custom-ollama-think-param

Conversation

@AXEG0

@AXEG0 AXEG0 commented Jul 14, 2026

Copy link
Copy Markdown

What does this PR do?

Restricts Ollama-only disabled-reasoning controls to endpoints positively identified as Ollama.

When reasoning is disabled, a verified Ollama endpoint receives both extra_body.think=false and top-level reasoning_effort="none". Generic OpenAI-compatible custom relays receive neither field, while explicit enabled reasoning effort remains available for custom reasoning APIs. Ollama detection uses the existing /api/tags probe instead of URL substrings or port numbers.

Related Issue

Fixes #11237.

Related/overlapping work:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/model-providers/custom/__init__.py: identify Ollama through detect_local_server_type(base_url, api_key); scope both disabled controls to verified Ollama endpoints.
  • agent/chat_completion_helpers.py and agent/transports/chat_completions.py: thread the API key through provider-profile and retry/fallback request construction.
  • agent/auxiliary_client.py: keep auxiliary request construction in parity with the main transport path.
  • Tests under tests/agent/, tests/plugins/model_providers/, and tests/providers/: cover Ollama, misleading URLs/ports, generic relays, API-key propagation, auxiliary requests, and retry/transport parity.

How to Test

  1. Run scripts/run_tests.sh tests/plugins/model_providers/test_custom_profile.py tests/providers/test_transport_parity.py tests/agent/transports/test_chat_completions.py tests/agent/test_auxiliary_client.py tests/agent/test_probe_cache_followups.py -q.
  2. Verify a positively identified Ollama endpoint with reasoning disabled receives think=false and reasoning_effort="none".
  3. Verify generic relays—including URLs containing ollama or using port 11434 without a valid /api/tags response—receive neither disabled field.

Focused result: 240 passed.

Additional checks: ruff check ., git diff --check, and scripts/check-windows-footguns.py --all pass.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (focused affected suites pass; the full GitHub Actions matrix is awaiting maintainer approval)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; no user-facing interface or behavior documentation changed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A; no configuration keys changed
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A; no architecture or workflow changed
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — platform-independent Python change; Windows footgun check passes
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A; no tool schema changed

Screenshots / Logs

N/A — regression coverage and command results are listed above.

AI Assistance

OpenAI — GPT-5 via Codex CLI (implementation assistance, test execution, and PR-description drafting).

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jul 14, 2026

@tonydwb tonydwb 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 Summary

Verdict: Approved

This PR scopes the think feature to Ollama endpoints.

Looks Good

  • Targeted fix ensuring think behavior is Ollama-specific.
  • Well-scoped: 6 files, minimal additions.

No Issues Found


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for targeting a real custom-provider compatibility issue. Current main still sends disabled-reasoning controls to every custom endpoint (plugins/model-providers/custom/__init__.py:57-65).

Problems

  • Mainline commit 8662254ab2fe added top-level reasoning_effort="none" beside think=False. This PR predates it and only gates think (plugins/model-providers/custom/__init__.py:65-67 on d9446975), so a salvage must scope both fields or generic relays can still receive an unsupported disabled-reasoning control.
  • "ollama" in base_url and ":11434" in base_url are not positive endpoint identification (plugins/model-providers/custom/__init__.py:56-60). Current detect_local_server_type() verifies Ollama through /api/tags (agent/model_metadata.py:683-732).

Suggested changes

  • Preserve the current Ollama /v1 fix while gating both disabled-case fields, with a transport-level generic-relay test asserting neither is sent.
  • Use an endpoint test that does not classify an arbitrary matching relay URL as Ollama.

Automated hermes-sweeper review.

Comment thread plugins/model-providers/custom/__init__.py Outdated
Comment thread plugins/model-providers/custom/__init__.py Outdated
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@AXEG0
AXEG0 force-pushed the fix/custom-ollama-think-param branch from d944697 to 4e1a167 Compare July 16, 2026 06:27
@aadamowski

Copy link
Copy Markdown

Confirming that this breaks Groq endpoints (https://console.groq.com/home):

⚠️   API call failed (attempt 1/3): BadRequestError [HTTP 400]
   🔌 Provider: custom  Model: llama-3.3-70b-versatile
   🌐 Endpoint: https://api.groq.com/openai/v1
   📝 Error: HTTP 400: property 'think' is unsupported
   📋 Details: {'message': "property 'think' is unsupported", 'type': 'invalid_request_error'}

I manually applied the fix from commit 4e1a167701427ef173565f7bd44c0b367def6554 onto my local Hermes install and it resolved the issue.

Please merge the contributed fix!

@AXEG0
AXEG0 force-pushed the fix/custom-ollama-think-param branch 3 times, most recently from 80557c5 to 62a0253 Compare July 24, 2026 06:36
@AXEG0

AXEG0 commented Jul 24, 2026

Copy link
Copy Markdown
Author

@teknium1 Thanks for the review. Both points are addressed, and the branch is rebased on current main (62a0253):

  • Ollama detection is now a positive identification via detect_local_server_type() instead of the URL substring match. A non-Ollama endpoint whose URL merely contains "ollama", or a non-Ollama service on :11434, is no longer classified as Ollama.
  • The disabled-branch reasoning_effort="none" is scoped under the same Ollama predicate as think, so non-Ollama custom endpoints (Groq included) receive neither field.
  • The rebase also fixes an undefined api_key reference in _build_call_kwargs() that a broad exception handler had been masking; the provider profile now builds correctly on the retry/fallback paths.

Targeted suites pass locally (test_chat_completions, test_custom_profile, test_transport_parity). @aadamowski thanks for confirming the fix on Groq. Ready for another look.

@AXEG0
AXEG0 force-pushed the fix/custom-ollama-think-param branch from 62a0253 to 92c4f9d Compare August 2, 2026 17:34
@AXEG0
AXEG0 force-pushed the fix/custom-ollama-think-param branch from 92c4f9d to 62de055 Compare August 2, 2026 17:39
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Thirty-seven PRs address or reference this broad Ollama/provider-reasoning complex. Their diffs cover Ollama Cloud registration/catalog handling, surrogate and truncation recovery, Windows-adjacent work, reasoning propagation and capability scoping, and—most directly for the target—preventing Ollama-only disable controls from reaching heterogeneous custom endpoints.

Related pull requests

Duplicates

#3197 was absorbed by #10782 and later completed by #64608; #6038 and #10740 were also absorbed by #10782; #11296, #11362, #11395, #11520, #12488, and #12914 are old-path variants of the #11237 fix now best represented by #64286; #16192 was salvaged as #19881 and #16196 was superseded by it; #25866 and #29820 overlap, with #29820 salvaged as #64608; #55280, #55428, and #57601 overlap with merged #58156; #52613 and #59819 share suffix-key deduplication, but #59819 adds the endpoint-specific local-proxy behavior required by its recorded best-fix verdict; #65234 overlaps the disabled-field scope solved more robustly by #64286.

Suggested consolidation

Keep #64286 open with a salvage path: preserve its positive /api/tags Ollama identification, shared scoping of both think=false and reasoning_effort="none", API-key propagation, and generic-relay final-wire tests. This follows the visible keep-open review rather than deviating from it; close #11296, #11362, #11520, and other still-open old-path #11237 variants as duplicates of #64286, while keeping #63315, #59819, #72656, and #75234 separate for their distinct capability, endpoint-shaped catalog, enabled-effort, and Responses issues.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I11237(["issue #11237 (open)"])
    I65233(["issue #65233 (open)"])
    I72649(["issue #72649 (open)"])
    subgraph Dup11296 ["PRs duplicating each other"]
        P11296["PR #11296 (open)"]
        P11362["PR #11362 (open)"]
        P11395["PR #11395 (closed)"]
        P11520["PR #11520 (open)"]
        P12488["PR #12488 (closed)"]
        P12914["PR #12914 (closed)"]
        P64286["PR #64286 (open)"]
        P65234["PR #65234 (closed)"]
        P72656["PR #72656 (open)"]
    end
    P64286 -->|best fix| I11237
    P64286 -->|best fix| I65233
    P64286 -->|best fix| I72649
    class I11237 open
    class I65233 open
    class I72649 open
    class P11296 open
    class P11362 open
    class P11395 closed
    class P11520 open
    class P12488 closed
    class P12914 closed
    class P64286 open
    class P65234 closed
    class P72656 open
    class P64286 best
    class P64286 best
    class P64286 best
    class P72656 best
    class P64286 target
    click I11237 "https://github.com/NousResearch/hermes-agent/issues/11237"
    click I65233 "https://github.com/NousResearch/hermes-agent/issues/65233"
    click I72649 "https://github.com/NousResearch/hermes-agent/issues/72649"
    click P11296 "https://github.com/NousResearch/hermes-agent/pull/11296"
    click P11362 "https://github.com/NousResearch/hermes-agent/pull/11362"
    click P11395 "https://github.com/NousResearch/hermes-agent/pull/11395"
    click P11520 "https://github.com/NousResearch/hermes-agent/pull/11520"
    click P12488 "https://github.com/NousResearch/hermes-agent/pull/12488"
    click P12914 "https://github.com/NousResearch/hermes-agent/pull/12914"
    click P64286 "https://github.com/NousResearch/hermes-agent/pull/64286"
    click P65234 "https://github.com/NousResearch/hermes-agent/pull/65234"
    click P72656 "https://github.com/NousResearch/hermes-agent/pull/72656"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 37 pull requests and 16 issues in this complex. Each diff was read against this issue; Assessment working set: 331 kB of PR diffs, 187 kB of issue/PR text, 71 kB of discussion (93 comments), 74 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists provider/ollama Ollama / local models sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: think=False incorrectly sent to all provider=custom endpoints, not just Ollama

6 participants