Skip to content

fix: positively gate reasoning-effort branch, extend gpt-5 alias fallback - #824

Open
WSxDemise wants to merge 6 commits into
mainfrom
feature/openai-model-effort
Open

fix: positively gate reasoning-effort branch, extend gpt-5 alias fallback#824
WSxDemise wants to merge 6 commits into
mainfrom
feature/openai-model-effort

Conversation

@WSxDemise

Copy link
Copy Markdown
Collaborator

What

Fixes the /model_settings reasoning-effort picker so it exposes the full,
real per-model reasoning_effort scale that OpenAI documents — including
xhigh/max where a model actually supports them — and makes
o1/o3/o4-mini/codex-mini-latest support reasoning_effort at all
(they had none before this change).

Why

  • The picker's xhigh/max options were gated behind flags that no code
    path ever set, so no model — not even ones that genuinely support
    max — could ever show them.
  • o1/o3/o4-mini/codex-mini-latest had zero reasoning_effort support
    in the UI, the catalog builder, or the API request itself, even though
    OpenAI's docs list low/medium/high for all of them.
  • Once reasoning_effort was wired into the UI, a follow-up gap surfaced:
    model_factory.make_model_settings() never actually forwarded the chosen
    value to the OpenAI API for any non-GPT-5 model — the setting was visible
    and selectable but silently dropped on the wire.

How

  • Added model_utils.get_openai_reasoning_effort_choices() as the single
    source of truth for which reasoning_effort values each OpenAI model
    family actually supports (per OpenAI's published docs/OpenAPI spec),
    matched via the existing boundary-aware _matches_model_tag() helper so
    short family tokens (o1, o3) can't false-positive on unrelated
    aliases (e.g. a custom model called zoo1-claude) while still matching
    legitimate embedded patterns like codex-gpt-5.
  • Wired that helper into:
    • config.model_supports_setting() — so /model_settings recognizes the
      setting dynamically, even for catalog entries that never declared it
      explicitly (checking both the catalog key and the underlying
      model_config["name"] alias).
    • command_line/model_settings_menu.py — so the picker offers exactly the
      choices that model supports.
    • command_line/add_model_menu.py — so newly added models get the right
      catalog defaults.
    • model_factory.make_model_settings() — a new branch that actually
      forwards the chosen reasoning_effort to openai_reasoning_effort for
      non-GPT-5 OpenAI reasoning models, normalizing legacy aliases
      (minimalnone, ultramax). Gated on a positive
      OpenAI-compatible-type whitelist (mirroring the existing
      _is_anthropic_model pattern) rather than a "not Anthropic" negative,
      so an aliased non-OpenAI catalog entry can't be misrouted into this
      branch either.
    • Extended the pre-existing GPT-5 branch's own condition with the same
      catalog-name alias fallback, so an aliased gpt-5* custom endpoint
      still gets full verbosity/summary/Responses-API handling instead of
      falling into the plainer generic branch.

Testing

  • Full test suite: 7572 passed, 28 skipped, 1 xpassed.
  • ruff check . and ruff format --check .: clean.
  • New/updated targeted regression coverage in tests/test_config_full_coverage.py,
    tests/command_line/test_model_settings_menu_coverage.py, and
    tests/test_custom_model_settings.py covering:
    • Per-model choice correctness (plain gpt-5 excludes xhigh/max,
      gpt-5.6 includes max, o-series gets low/medium/high,
      fixed-effort models like o1-mini/gpt-5-pro get no choices).
    • Dynamic model_supports_setting() detection without a catalog entry.
    • Actual forwarding of reasoning_effortopenai_reasoning_effort on
      the wire for o-series/codex-mini, with legacy alias normalization.
    • Alias fallback (catalog key vs. model_config["name"]) in both the
      settings-menu and make_model_settings() code paths.
    • Boundary-safe matching: codex-gpt-5 still matches the gpt-5 bucket;
      an unrelated alias like zoo1-claude or a gemini-typed alias like
      team-o1-eval does not get hijacked into OpenAI-specific handling.

Scope

  • code_puppy/model_utils.py
  • code_puppy/config.py
  • code_puppy/model_factory.py
  • code_puppy/command_line/model_settings_menu.py
  • code_puppy/command_line/add_model_menu.py
  • Corresponding test files under tests/

No dependency, lockfile, or CI/tooling config changes.

Wes Blakemore and others added 6 commits August 21, 2026 00:16
The /model_settings reasoning_effort picker always hid xhigh/max behind
supports_xhigh_reasoning/supports_max_reasoning flags that no code path
ever set to True, so no OpenAI model -- including GPT-5.6, which
actually documents "max" -- could ever offer it. o1/o3/o4-mini/
codex-mini never got reasoning_effort at all.

- model_utils.get_openai_reasoning_effort_choices(): single source of
  truth mapping real OpenAI model ids to the none/low/medium/high/
  xhigh/max values OpenAI documents for each (verified against the
  live OpenAI OpenAPI spec + platform docs).
- model_settings_menu._get_setting_choices(): use it to gate the
  reasoning_effort picker instead of always-False boolean flags;
  legacy flags still work as an explicit widen-only override.
- config.model_supports_setting(): recognize reasoning_effort
  dynamically for any recognized OpenAI model, regardless of whether
  its catalog entry declares supported_settings (extra_models.json /
  OAuth-sourced entries included).
- add_model_menu._build_model_config(): use the same helper when
  adding a model from models.dev, so o-series/codex-mini models get
  reasoning_effort in their stored supported_settings too (previously
  only gpt-5* did), and list_available_models() reports it correctly.

Recognized-but-not-configurable models (o1-mini, o1-preview,
gpt-5-pro, *-chat-latest) correctly report no choices/no support
rather than a broken empty picker.
Pre-PR review caught that exposing reasoning_effort in the /model_settings
menu and catalog for these models wasn't enough: make_model_settings()
only ever translated the generic reasoning_effort value into the wire
field OpenAI's client actually reads (openai_reasoning_effort) inside
the gpt-5-specific branch. For every other recognized OpenAI reasoning
model the value sat inert in the settings dict and was silently dropped.

Add a dedicated elif branch, gated on the same
get_openai_reasoning_effort_choices() detection already used for the UI,
that mirrors the GPT-5 branch's effort normalization/forwarding for
these Chat-Completions-only models (no verbosity/summary, which are
GPT-5-Responses-API-specific). Fixed-effort models (o1-mini, o1-preview,
gpt-5-pro) correctly fall through untouched since the helper returns an
empty list for them.
Second pre-PR review round found two real gaps in
get_openai_reasoning_effort_choices() and its callers:

1. Plain substring matching let short family tokens ("o1", "o3") false-
   positive on unrelated custom-model aliases (e.g. "zoo1-claude"), which
   could hijack a non-OpenAI model into the wrong make_model_settings()
   branch and skip its real (e.g. Anthropic) handling entirely.
2. config.model_supports_setting() and model_factory.make_model_settings()
   only checked the catalog key (model_name), not the underlying real
   model id in model_config["name"] -- so an aliased extra_models.json
   entry (a common, documented pattern) never got reasoning_effort
   support at all, reintroducing this same PR's original bug for that
   shape of config.

Fixes:
- get_openai_reasoning_effort_choices() now reuses the existing
  boundary-aware _matches_model_tag() helper (DRY) instead of raw
  substring/startswith matching, so "codex-gpt-5" still matches "gpt-5"
  at a real delimiter boundary while "zoo1-claude" no longer falsely
  matches "o1".
- config.model_supports_setting() and model_factory.make_model_settings()
  both now fall back to model_config["name"] when model_name itself
  isn't recognized, mirroring the fallback already used in
  model_settings_menu._get_setting_choices().
- make_model_settings()'s new reasoning-effort branch is also gated on
  the model NOT being Anthropic-typed, as defense in depth against any
  future short-token collision.

New regression tests cover both the alias fallback and the collision
guard in config, model_factory, and the model_utils helper itself.
…back

Third pre-PR review round found two remaining gaps from the round-2 fix:

1. The new elif in make_model_settings() only excluded Anthropic-typed
   models (via _is_anthropic_model), which is a negative exclusion --
   an aliased gemini/zai/cerebras/openrouter/etc. catalog entry whose key
   or underlying model_config["name"] happens to contain a delimited
   OpenAI family token (e.g. a custom eval alias like "team-o1-eval")
   would still be hijacked into this branch and get the wrong wire
   format. Replaced with a positive whitelist,
   _OPENAI_COMPATIBLE_MODEL_TYPES (openai/chatgpt_oauth/
   azure_foundry_openai/azure_openai/custom_openai*), mirroring how
   _is_anthropic_model positively identifies its own family. Every real
   catalog entry has an explicit "type" (ModelFactory.get_model() raises
   otherwise), so this is a safe positive check, not a functional
   regression.

2. This round's own alias-fallback work was inconsistent: the pre-
   existing "elif 'gpt-5' in model_name:" branch condition was left
   checking only the catalog key, so an aliased gpt-5 custom-endpoint
   entry (key without "gpt-5", real id in model_config["name"]) fell
   through into the new generic branch instead -- silently losing
   verbosity/summary/Responses-API/GPT-5.6 handling that
   config.model_supports_setting() (alias-aware since round 3) reports
   as supported. Extended that branch's own condition with the same
   model_config["name"] fallback.

New regression tests cover both: a gemini-aliased "o1"-containing name
proving the positive-type guard, and an aliased gpt-5 custom entry
proving it still takes the full-featured GPT-5 branch (verbosity lands
in extra_body) rather than the generic one.
Trim the essay-length inline commentary around OpenAI
reasoning_effort detection down to the essentials. No behavior
change (204 tests still pass).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants