Skip to content

fix(copilot): clamp reasoning effort to nearest supported level instead of capping xhigh - #51480

Closed
arminanton wants to merge 1 commit into
NousResearch:mainfrom
arminanton:feat/copilot-plugin-effort-nearest-down
Closed

fix(copilot): clamp reasoning effort to nearest supported level instead of capping xhigh#51480
arminanton wants to merge 1 commit into
NousResearch:mainfrom
arminanton:feat/copilot-plugin-effort-nearest-down

Conversation

@arminanton

Copy link
Copy Markdown
Contributor

Problem

The Copilot provider profile (plugins/model-providers/copilot/__init__.py) normalized the reasoning effort by unconditionally mapping xhigh to high before consulting the model's supported-effort set:

if effort == "xhigh":
    effort = "high"
if effort in supported_efforts:
    extra_body["reasoning"] = {"effort": effort}

But the live Copilot /models catalog reports supported efforts per model, and several models (the gpt-5.x family) do support xhigh. Capping xhigh to high unconditionally silently downgraded those models one level below what the user asked for and what the model supports.

Fix

Honor the requested effort when the catalog lists it as supported, and only downgrade when it does not, choosing the nearest weaker supported level:

if effort not in supported_efforts:
    if effort == "xhigh" and "high" in supported_efforts:
        effort = "high"
    elif effort == "minimal" and "low" in supported_efforts:
        effort = "low"
    elif "medium" in supported_efforts:
        effort = "medium"
    else:
        effort = supported_efforts[0]
if effort in supported_efforts:
    extra_body["reasoning"] = {"effort": effort}

This is the same nearest-down clamp behavior used elsewhere for the max effort: a requested level the model genuinely supports is forwarded verbatim; an unsupported level steps down to the nearest weaker supported one instead of being dropped or hard-mapped.

Scope

The Copilot profile's build_api_kwargs_extras effort handling only. No change to the catalog lookup itself or to any other provider.

Tests

Adds tests/plugins/model_providers/test_copilot_profile.py (6 tests, catalog lookup stubbed): a supported xhigh is forwarded verbatim; an unsupported xhigh steps to high; minimal steps to low; an unrecognized effort falls to medium; and when neither the specific rule nor medium applies, the first supported level is chosen. ruff check and the Windows-footgun check pass.

…not xhigh->high

The Copilot provider profile unconditionally mapped ``xhigh`` to ``high`` before
checking the model's catalog, so models that DO support ``xhigh`` (e.g. the
gpt-5.x family per the live /models catalog) were silently capped one level
down.

Honor the requested effort when the catalog lists it as supported, and only
downgrade when it does not, choosing the nearest weaker supported level
(xhigh->high, minimal->low, else medium, else the first supported level). This
matches the nearest-down clamp behavior used elsewhere for the ``max`` effort.

Adds tests/plugins/model_providers/test_copilot_profile.py covering forward,
downgrade, and fallback paths (catalog lookup stubbed).
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins provider/copilot GitHub Copilot (ACP + Chat) P3 Low — cosmetic, nice to have labels Jun 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this fixes the xhigh-clamp in the Copilot provider profile (plugins/model-providers/copilot/__init__.py), a different code path from the core-agent fixes #10391 and #49644 (run_agent.py _github_models_reasoning_extra_body) and the xAI sibling #42330. Same nearest-down-clamp goal, distinct layer — not a duplicate. Maintainer to decide whether the provider-profile fix and the core-path fix should both land.

@jakepresent

Copy link
Copy Markdown
Contributor

Local validation on draft PR head 02cef482d:

python -m pytest tests/plugins/model_providers/test_copilot_profile.py -q -o 'addopts='
# 6 passed, 1 warning
python -m py_compile plugins/model-providers/copilot/__init__.py tests/plugins/model_providers/test_copilot_profile.py
# passed

The clamp behavior is correct when supported_efforts is already populated: supported xhigh is forwarded unchanged, unsupported xhigh downgrades to high, and other unsupported values fall back predictably.

One scope caveat: the profile still gets supported_efforts from github_model_reasoning_efforts(model) without a catalog or API key. That means catalog-only models like Copilot-hosted Claude still return [] in the real profile path. So this pairs well with a catalog-backed profile fix, but by itself it only fixes the “don’t downgrade a supported xhigh” case once the supported-effort set is available.

@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 15, 2026

@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 isolating the supported-effort negotiation and adding focused tests.

Problems

  • This changes only the chat-completions provider profile. GPT-5 Copilot models route to codex_responses (hermes_cli/models.py:3293-3307), where run_agent.py:5454-5455 still unconditionally changes xhigh to high.
  • The profile invokes github_model_reasoning_efforts(model) without catalog/API-key input (plugins/model-providers/copilot/__init__.py:35); the helper consults live capabilities only when supplied one (hermes_cli/models.py:3511-3517). The new stubbed-xhigh case is therefore not reachable through the current production wiring.

Suggested changes

  • Prefer the existing #62028 salvage carrier: it preserves this commit's authorship, covers both request paths, and adds current-main regression coverage. This branch is also currently conflicted with main after 7550c594c changed the same normalization.

Automated hermes-sweeper review.

# the higher level.)
if effort not in supported_efforts:
if effort == "xhigh" and "high" in supported_efforts:
effort = "high"

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.

This fixes only the chat-completions profile. GPT-5 Copilot models use codex_responses (hermes_cli/models.py:3293-3307), whose payload still comes from run_agent.py:_github_models_reasoning_extra_body() and unconditionally clamps xhigh at run_agent.py:5454-5455; update both paths (as carried by #62028).

@teknium1

Copy link
Copy Markdown
Contributor

This landed (in spirit) a while back — the nearest-weaker catalog clamp shipped for the copilot profile, and as of PR #90350 it routes through the canonical clamp_effort() ladder walk in agent/reasoning_effort.py, so ultra/max resolve to the strongest catalog-supported level and the ladder can't invert (#74295 fixed). The test file from this PR's approach lives on in tests/plugins/model_providers/test_copilot_profile.py. Closing as implemented on main — thanks for the direction.

@teknium1 teknium1 closed this Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have provider/copilot GitHub Copilot (ACP + Chat) 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.

4 participants