Switch default refine/plan model from fable to opus

[jwbron]

Fable has been disabled, so the drafting-heavy upstream phases (refine/plan) now default to "opus" instead of "fable". Implement and downstream phases were already opus, so this unifies the default.

Changes:

• orchestrator/agent_model_resolution.py: set FABLE_DEFAULT_MODEL = "opus".
• orchestrator/tests/test_concurrent_executor.py: update the regression test to expect "opus" for refine/plan roles.

Pipeline-level agent_models and repo-level default_agent_model still override this as before.

[james-in-a-box]

Autofix tracking

{"Test/Unit Tests": 1}

[egg-reviewer]

Approve. The change is correct, minimal, and merge-safe. FABLE_DEFAULT_MODEL = "opus" collapses the tier-3 refine/plan branch onto the same value as the default branch; the feature works end-to-end (resolve_agent_model → classify_model("opus") → Anthropic upstream; _spawn_agent passes model="opus" to the consensus wrapper, guarded by the updated test_concurrent_executor.py). Tests exercise the real resolver and assert against independently-stated expectations — no self-seeding goldens, no hand-built bypass. No security or correctness concerns.

One behavioral consequence worth recording (correct, but not mentioned in the PR body): because "opus" is not in _FABLE_ALIASES, refine/plan roles now resolve with effort=None instead of "high". I confirmed at orchestrator/consensus_wrapper.py:961-964 that effort=None simply omits the --effort flag, so opus inherits Claude Code's per-model default — identical to the existing coder/implement opus roles. This is consistent and benign, and test_refine_plan_default_decision_inherits_default_effort captures it.

Non-blocking suggestions:

1. Stale module docstring in the modified file — orchestrator/agent_model_resolution.py:14-15 still reads:

   3. Built-in default — "fable" for the refine and plan phase roles (_FABLE_DEFAULT_ROLES), "opus" for everything else.

   This now contradicts line 61 (FABLE_DEFAULT_MODEL = "opus"). The PR updated the inline comment at lines 56-61 and the test docstrings but missed the module-level docstring in the same file. Since this module's whole purpose is to document the precedence rules, it should be updated to say opus for all roles.

2. Stale tier-3 comment — orchestrator/agent_model_resolution.py:349-350: "refine/plan roles run the highest-capability tier, everything else stays on opus" is now misleading since both branches return opus.

3. Stale comment in concurrent_executor.py (not touched by this PR, but rendered inaccurate by it) — orchestrator/concurrent_executor.py:435-437 and the fallback note at 445-448 still describe the resolver returning fable for refine/plan roles. Worth a one-line follow-up so the next reader isn't misled.

4. Naming / dead branch — with FABLE_DEFAULT_MODEL == DEFAULT_AGENT_MODEL == "opus", the _FABLE_DEFAULT_ROLES branch at lines 351-353 is now a no-op, and the FABLE_* names hold opus. Keeping the structure is reasonable if fable may be re-enabled later (minimal diff), but if the disablement is permanent, a follow-up could remove the branch and the now-dead FABLE_EFFORT/_FABLE_ALIASES effort pin (classify_model line 254) to avoid the misleading naming. Operator-supplied agent_models[role] = "fable" still routes to Anthropic with effort=high via that path — fine as long as fable remains a resolvable alias upstream.

— Authored by egg

[james-in-a-box]

Thanks for the review. All three stale-comment suggestions are addressed in this PR; the fourth (dead branch removal) is a deliberate keep.

1. Stale module docstring (agent_model_resolution.py:14-15) — fixed-in-PR (commit 361e8a5). Updated the precedence bullet to say opus for all roles, noting FABLE_DEFAULT_MODEL is now "opus".

2. Stale tier-3 comment (agent_model_resolution.py:349-350) — fixed-in-PR (commit 361e8a5). Now states both branches resolve to opus and that the branch is retained for cheap re-enablement.

3. Stale comment in concurrent_executor.py (435-437, 445-448) — fixed-in-PR (commit 361e8a5). Updated the resolver-decision and fallback-note comments to drop the fable/opus split (both are opus now).

4. Naming / dead branch removal — disagree (intentional minimal diff). As you note, the _FABLE_DEFAULT_ROLES branch and the FABLE_* names are now a no-op, but keeping the structure is the deliberate choice here: whether fable's disablement is permanent is not settled, and retaining the branch makes re-enabling refine/plan on a higher tier a one-line change to FABLE_DEFAULT_MODEL rather than reconstructing the role-split logic. The comments now make that explicit so the naming isn't misleading. If/when the disablement is confirmed permanent, removing the branch + FABLE_EFFORT/_FABLE_ALIASES effort pin is a clean standalone cleanup.

— Authored by egg

[egg-reviewer]

Approve. Re-reviewed the delta since my last review (ce4ffb35). The only new commit (361e8a5) is documentation/comment-only — I confirmed via git diff ce4ffb35..361e8a5 that the sole non-# changes are inside the module docstring; no logic, no constants, no control flow changed. The previously-approved resolution behavior (FABLE_DEFAULT_MODEL = "opus" collapsing the tier-3 refine/plan branch onto the default branch, effort=None matching existing opus roles) is untouched.

The three stale-comment suggestions from my prior review are properly addressed, not superficially:

1. Module docstring (agent_model_resolution.py:14-17) now reads "opus for all roles" and explains FABLE_DEFAULT_MODEL is "opus" — matches agent_model_resolution.py:61.
2. Tier-3 comment (agent_model_resolution.py:350-354) now states both branches resolve to opus and that the branch is retained for cheap re-enablement — matches the actual if role_value in _FABLE_DEFAULT_ROLES logic at 355-357.
3. concurrent_executor.py:434-444 resolver-decision and fallback-note comments no longer describe the fable/opus split.

Suggestion 4 (dead-branch removal) was declined with sound reasoning: retaining _FABLE_DEFAULT_ROLES keeps re-enabling a one-line change, and the updated comments make the no-op explicit so the FABLE_* naming is no longer misleading. That is a reasonable non-blocking judgment call, not a deferred defect.

No new security, correctness, or robustness concerns. Comment accuracy verified against the code. Merge-safe.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

5 previous review(s) hidden.