fix(copilot): resolve reasoning effort from the live catalog for all families

[allenliang2022]

What does this PR do?

hermes_cli.models.github_model_reasoning_efforts() is the single chokepoint that decides both whether a Copilot model offers a reasoning control and which effort is actually sent. Callers reach it two ways:

• with an explicit catalog — the setup flow, which already holds one
• with neither catalog nor api_key — every other caller, including AIAgent._supports_reasoning_extra_body() and AIAgent._github_models_reasoning_extra_body()

On that second path the live /models catalog was never fetched, because the catalog branch only ran when an api_key had been passed in. Those callers therefore always fell through to the static table in _github_reasoning_efforts_for_model_id(), which only knew GPT-5 and the o-series:

if raw.startswith(("openai/o1", ..., "o4")): return O_SERIES
if normalized.startswith("gpt-5"):           return GPT5
return []          # every other family

So every non-OpenAI Copilot family resolved to [] — the gate reports "unsupported" and a configured reasoning_effort is silently dropped, even though the catalog advertises capabilities.supports.reasoning_effort for them.

Changes

1. Fetch the catalog on the no-argument path.

When the caller supplied neither a catalog nor a key, resolve a Copilot token (via the same copilot_auth helpers the rest of the stack uses), fetch the catalog once, and thread it through both the id normalisation and the capability lookup — previously each fetched separately.

An explicit catalog still wins and skips auth entirely, so callers that already hold one are unaffected and no existing test needs a network stub it did not need before. Token resolution is best-effort and never raises: a missing or unusable credential degrades to the static fallback rather than breaking model selection.

2. Widen the static fallback so an offline process degrades to something usable.

Rather than "no reasoning at all", the fallback now covers Claude 4.6+, Gemini 3+/2.5+, Grok 4.5+ and MAI-Code with a conservative low/medium/high, version-gated so families without an effort dial still return [] — Claude pre-4.6 (including haiku-4.5), Gemini 1.5/2.0, and grok-4 / grok-4-fast.

The catalog stays authoritative throughout: it can advertise levels the fallback does not list (xhigh, max) and can deny a family the fallback would have allowed. The fallback only decides what happens when there is no catalog answer to defer to.

Relationship to #51953

#51953 fixes the same silent-drop symptom for Claude by rewiring the two AIAgent gates to a new catalog-backed helper. This PR instead fixes the shared entry point, so callers that are not those two gates are covered too, and pairs it with a fallback that is no longer GPT-only.

These are complementary rather than conflicting. If #51953 lands first, its helper would simply be calling an entry point that already resolves correctly.

Tests

tests/hermes_cli/test_model_validation.py::TestGithubReasoningEfforts:

• an explicit catalog is authoritative for non-GPT families, including a family it does not advertise staying []
• the no-argument path resolves a token and consults the live catalog — asserting the fetch happens exactly once, which pins the shared-catalog threading
• an explicit catalog skips token resolution entirely (asserted via the resolver never being called)
• the offline fallback covers the widened families while a non-advertising family and the GPT-5 ladder stay exactly as before

Both directions are asserted, so widening the gate too far fails as loudly as not widening it at all.

Verification on this branch, rebuilt on current main:

tests/hermes_cli/test_model_validation.py
tests/hermes_cli/test_copilot_context.py
tests/plugins/model_providers/test_copilot_profile.py
46 passed

I also confirmed the tests fail first: reverting the auto-resolve makes the catalog test fail with ['low','medium','high'] != ['low','medium','high','xhigh','max'] — i.e. it falls back to the static ladder instead of the catalog's.

Manual verification

Confirmed against a running instance, not tests alone: the reasoning gates now resolve non-empty ladders for the Claude / Gemini / Grok / MAI-Code models the catalog advertises, and a family the catalog does not advertise still resolves empty.

Not verified

I did not capture an outbound request body to confirm the emitted reasoning_effort field end-to-end; the verification above is at the resolver and gate level. The consuming code paths are unchanged by this PR — only the value they receive is.

Checklist

• I've read the Contributing Guide
• Commit message follows Conventional Commits
• I searched existing PRs and issues for duplicates (found fix(copilot): resolve Claude reasoning-effort from live catalog #51953 — overlap explained above)
• Contains only changes related to this fix
• Added tests
• Tested on my platform: Windows

@teknium1

[allenliang2022]

Follow-up: I found and fixed a defect in my own first commit before review.

While auditing every call site of the function I touched, I noticed that
_resolve_copilot_catalog_api_key() already existed earlier in
hermes_cli/models.py — it is used by the /model picker's catalog fetch. My
first commit defined a second function with the same name further down the
file. Python keeps the last definition, so mine silently shadowed the original
for every caller in the module, not just the reasoning path.

That is not a cosmetic duplication. The two are not equivalent: the original
falls back to read_credential_pool("copilot") and exchanges each candidate,
which is the only path that resolves a token stored in
credential_pool.copilot[] (populated by hermes auth add copilot or seeded
from .env). Shadowing it would have made the model picker fall back to a
stale hardcoded list for exactly those users — a regression in code unrelated
to this PR's stated scope.

a77f64b3a5 deletes the duplicate and calls the existing helper, which is
strictly more capable. Verified against a live account that the resolved
ladders are unchanged: opus-5 low..max, gemini-3.6-flash minimal..high,
grok-4.6 low..xhigh, haiku-4.5 empty.

I also added a regression test asserting there is exactly one definition of
that helper and that the surviving one is the pool-aware version, since
"a duplicate definition silently wins" is invisible in a diff that only shows
added lines. Mutation-checked both ways: re-introducing the duplicate turns the
test RED, removing it again turns it GREEN.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(copilot): resolve reasoning effort from the live catalog for all families

Making the live catalog authoritative and adding offline fallbacks for Claude/Gemini/Grok/MAI-Code is a solid improvement. Observations:

1. hermes_cli/models.py github_model_reasoning_efforts — this now performs a token resolution + network fetch (fetch_github_model_catalog) on every call where neither catalog nor api_key is passed, whereas before it was purely offline. If fetch_github_model_catalog caches aggressively this is fine, but if it doesn't, hot-path callers (WebUI chip, agent send gates) each pay an HTTP round-trip per turn. Please confirm the catalog fetch is TTL-cached; if not, cache it.
2. _copilot_claude_supports_reasoning_effort — the major_only regex [-.](\d+)(?:[-.]\d{6,})?(?:\b|-) treats claude-5-haiku (any unversioned-minor 5.x variant) as effort-capable, while claude-4 (no minor) falls through to False. That asymmetry is probably intentional (5.x unversioned advertises the ladder, per the docstring), but the claude-4 → False path is worth a second look: a bare claude-4 in a future catalog would be denied even though 4.6+ supports effort. A unit test for the boundary versions (4.5 vs 4.6, bare 4 vs bare 5) would pin the intent.
3. The offline fallback for Gemini matches gemini-(?:3|[4-9]|2\.[5-9]) — note gemini-3.5 (intentionally excluded, since Gemini 3.x includes 3.5) matches [4-9]? No — 3 matches the first alternative. But gemini-3.10 also matches the 3 alternative via the leading 3. Fine. One gap: gemini-2.5 matches 2\.[5-9], but gemini-2.6+ too — presumably intentional per the family rule.
4. tests/hermes_cli/test_model_validation.py test_reuses_the_existing_catalog_token_helper uses inspect.getsource(models_mod) and counts string occurrences to detect a shadowing duplicate definition. That is a source-reading test, which the repo's testing guidelines explicitly ban ("Never read source code in tests") — it fails for correct refactors and can't guard the real risk (an import-time shadow). A behavioral alternative: assert models_mod._resolve_copilot_catalog_api_key.__module__ == "hermes_cli.models" plus a functional test that a credential-pool-only token resolves — which the other tests already cover functionally.
5. Minor: in the offline fallback, a model containing "claude" that matches none of the version patterns gets True (e.g. a bare claude-sonnet). Advertising effort for an unsupported model can produce API-side 400s; the conservative default might be False with version-allowlisting instead of deny-listing.

[allenliang2022]

Thanks — points 1, 4 and 5 were all real and are fixed in 9d1c73a7d0. Point 2 I looked into and reached a different conclusion; reasoning below. Point 3 noted.

1. Per-call token resolution — confirmed, and worse than described

You were right that this needed checking, and the measurement is worse than "an HTTP round trip". fetch_github_model_catalog does have a 5-minute TTL and it works correctly. The problem was the other half: _resolve_copilot_catalog_api_key() has no cache, and it can shell out to gh auth token and exchange credential-pool candidates.

So the catalog was being served from cache while the credential was re-resolved from scratch on every single call:

BEFORE: 20 lookups -> 6.94s   token_resolve=20   ~347 ms per lookup
        (token resolution alone: ~1.1s cold)

AFTER:  50 lookups -> 14.3ms  token_resolve=1    ~0.29 ms per lookup

Added _cached_copilot_reasoning_token() — 5-minute TTL to match the catalog cache, plus a 60-second negative TTL so a missing credential is retried reasonably soon without re-running the subprocess every call. The underlying helper is untouched, so the /model picker keeps its existing behaviour.

Guarded by test_resolved_token_is_cached_across_calls (one resolution across five lookups).

5. Unversioned Claude returning True — confirmed

Reproduced: claude-sonnet and claude-opus both returned True via the bare return True tail. Offline, that is the expensive direction to be wrong in — the control is offered and the provider rejects it.

The gate now fails closed: an id must positively prove a version at or above the adaptive-thinking generation. I also applied the date-stamp guard to the bare-major branch, so claude-opus-4-20250514 reads as major 4 instead of parsing the stamp.

2. claude-5-haiku — I disagree, deliberately

This one resolves to True and I believe that is correct, so I did not "fix" it.

Haiku 4.5 lacking a ladder is a property of the 4.5 generation, not of the name "haiku". A hypothetical 5-series haiku would follow its version like every other model in the family. Keying the gate on a size qualifier would encode a coincidence of the current lineup rather than the rule, and would then be wrong the moment a 5-series haiku ships with a ladder.

The gate is versioned by design, and the live catalog stays authoritative either way — this fallback only decides what happens when the catalog cannot answer.

Your bare-claude-4 observation was right though, and it is now in the boundary table: claude-4 and claude-opus-4 resolve to [] (no minor, and 4 < 5).

4. Source-reading test — you are right, and the repo says so explicitly

I checked and AGENTS.md:1490 bans this outright ("Never read source code in tests"), so this was my violation of a documented rule, not a judgement call. Your specific objections are also correct on the merits: it would fail a pure rename, and it could not catch an import-time shadow.

Replaced with two behavioural tests. The important one asserts the capability that a shadowing regression actually destroys: with stage-1 credential resolution empty and the token present only in credential_pool.copilot[], that pool token must still reach the catalog fetch and the catalog ladder must win over the offline fallback.

Writing it surfaced something the source-reading version could never have caught — I had the pool contract wrong. Pool entries carry access_token, not api_key, and exchange_copilot_token returns a 2-tuple there, not 3. My first attempt at the behavioural test failed for exactly that reason, which is the point: it executes the path instead of pattern-matching it.

Boundary coverage

Added a 21-case parametrised table for the offline Claude gate as you asked — explicit minors, bare majors, the 3.x family, date stamps, and unversioned ids.

Also added an autouse fixture resetting the module-level token cache around each test, so a real credential resolution cannot leak between tests and satisfy the cache assertion for free.

Verification

Mutation-checked each fix against the test meant to guard it:

cache removed              -> RED (1 failed)
permissive tail restored   -> RED (5 failed, 16 passed)
pool-aware path bypassed   -> RED (1 failed)
all restored               -> GREEN

516 tests pass across the copilot/model suites and provider profiles.

On #51953

Still open with no changes. As noted in the description, these remain complementary — that PR rewires the two AIAgent gates to a catalog-backed helper, this one fixes the shared entry point so other callers are covered too. If it lands first, its helper would be calling an entry point that already resolves correctly.