Skip to content

fix(planning): make auto model defaults runtime-aware; surface empty harness completions distinctly#108

Open
AbirAbbas wants to merge 3 commits into
mainfrom
fix/model-cascade-runtime
Open

fix(planning): make auto model defaults runtime-aware; surface empty harness completions distinctly#108
AbirAbbas wants to merge 3 commits into
mainfrom
fix/model-cascade-runtime

Conversation

@AbirAbbas

Copy link
Copy Markdown
Collaborator

Incident

A caller invoked swe-planner.plan with ai_provider="codex" explicitly and no
model overrides, in an environment where only OPENROUTER_API_KEY was set (no
Anthropic/OpenAI keys, no SWE_DEFAULT_* vars). Two independent failures
collapsed into one opaque error:

  1. Cross-runtime model-id leakage. The planning-model default cascade
    (_default_planning_model()) resolved from env keys alone, ignoring the
    caller's pinned runtime. With only an OpenRouter key present it returned the
    OpenRouter auto-default model id — so the codex CLI was spawned with an
    openrouter/…-prefixed model its OpenAI backend cannot resolve. Every
    attempt completed in ~1s with a null/empty message.

  2. Opaque empty completions. The pipeline surfaced only the generic
    "Product manager failed to produce a valid PRD" — indistinguishable from a
    genuine schema-quality failure (a weak model emitting unparseable output).
    One message, two very different root causes.

What changed

Runtime-aware auto default (fix(planning)) — _default_planning_model()
now takes the resolved runtime and delegates to the already-runtime-aware
resolve_runtime_models() cascade for the high-tier pm role. plan() threads
the resolved ai_provider into it. The auto default is gated on runtime:

  • codex → a codex-native model (never an openrouter/…-prefixed id)
  • open_code → the OpenRouter auto default (OpenRouter-only env) or the
    open_code base default otherwise
  • claude_code → the historical sonnet default

Explicitly passed models and deployer env (SWE_DEFAULT_MODEL / AI_MODEL /
HARNESS_MODEL, SWE_MODEL_HIGH) still win verbatim — only the auto
default became runtime-aware. Omitting the runtime arg preserves the prior
env-only behavior, so existing call sites and tests are unaffected.

Distinct empty-completion error (fix(runtime)) — new
EmptyHarnessCompletionError + check_empty_harness_completion(), wired into
the PM, architect, tech-lead and sprint-planner reasoners between the
fatal-error check and the parsed is None check. An empty completion (no parsed
object and no text — the signature of a provider/model mismatch or bad auth)
now raises an error naming the provider and model, e.g.:

PM harness returned an empty completion (provider=codex,
model=openrouter/deepseek/deepseek-v4-flash) — check provider auth/model
compatibility

The generic "failed to produce a valid …" message is kept only for genuinely
non-empty-but-unparseable output, and now also includes provider+model context.

Validation Contract

Contract behavior Test
OpenRouter-only env + runtime codex → auto default is NOT openrouter/-prefixed (codex-native) test_default_planning_model_matrix[openrouter_only-codex], test_codex_default_is_never_openrouter_prefixed
OpenRouter-only env + runtime open_code → OpenRouter auto default preserved test_default_planning_model_matrix[openrouter_only-open_code]
runtime claude_codesonnet default preserved test_default_planning_model_matrix[*-claude_code]
SWE_DEFAULT_MODEL / explicit arg wins in all cases test_default_planning_model_matrix[swe_default_model-*], existing test_plan_explicit_args_override_env, test_plan_swe_default_model_overrides_openrouter_auto
Empty harness completion → error containing provider+model, distinct from schema-invalid message TestCheckEmptyHarnessCompletion::*, test_run_product_manager_empty_completion_surfaces_provider_and_model

Also covered: runtime-alias normalization, the omitted-runtime env fallback, and
that non-empty-but-unparseable output is a no-op for the empty-completion check
(so the generic schema-invalid path still fires).

Test results

  • make check (full pytest suite + compileall, Python 3.12,
    AGENTFIELD_SERVER set as in CI): green.
  • New test module: 22 tests (9-cell env×runtime matrix + empty-completion unit
    and reasoner-level tests) passing.
  • Full pre-existing suite unaffected (1150 passed, 1 skipped).
  • ruff check on all changed/new files: clean.
  • Go CI job untouched — this change is Python-only and cannot affect the Go
    module.

🤖 Generated with Claude Code

AbirAbbas and others added 3 commits July 22, 2026 15:11
The planning pipeline resolved its default model from env keys alone via
_default_planning_model(), ignoring the caller's resolved runtime. A caller
that pinned ai_provider="codex" in an OpenRouter-only environment therefore
had the codex CLI spawned with an "openrouter/..." model its OpenAI backend
cannot resolve — completing in ~1s with a null message.

Thread the resolved runtime into _default_planning_model() and delegate to
the already-runtime-aware resolve_runtime_models() cascade for the high-tier
pm role. The auto default is now gated on runtime:
  - codex       -> a codex-native model (never openrouter/...-prefixed)
  - open_code   -> the OpenRouter auto default (OpenRouter-only env) or the
                   open_code base default otherwise
  - claude_code -> the "sonnet" historical default

Explicit args and deployer env (SWE_DEFAULT_MODEL / AI_MODEL / HARNESS_MODEL,
SWE_MODEL_HIGH) still win verbatim — only the auto default became
runtime-aware. Omitting the runtime arg preserves the prior env-only behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…der+model

An empty harness completion (no parsed object, no text — the signature of a
provider/model mismatch or bad auth) was collapsed into the generic
"failed to produce a valid <artifact>" message, indistinguishable from a
genuine schema-quality failure. Two distinct root causes, one opaque error.

Add EmptyHarnessCompletionError and check_empty_harness_completion(), and wire
them into the PM, architect, tech-lead and sprint-planner reasoners between the
fatal-error check and the parsed-None check. The empty case now raises an error
naming the provider and model; the generic schema-invalid message is kept only
for non-empty-but-unparseable output and also gains provider+model context.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n error

Matrix over (env: openrouter-only / anthropic / SWE_DEFAULT_MODEL) x
(runtime: open_code / codex / claude_code) asserting the resolved planning
default, including that codex never yields an openrouter/-prefixed id. Plus
unit and reasoner-level tests that an empty harness completion raises the
distinct EmptyHarnessCompletionError naming provider and model, separate from
the schema-invalid message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant