Skip to content

feat(#975): model roles — semantic indirection between phase policy and concrete model strings - #979

Merged
admarble merged 4 commits into
mainfrom
feature/975-feat-config-model-roles-semantic-indirection-betwe
Aug 30, 2026
Merged

feat(#975): model roles — semantic indirection between phase policy and concrete model strings#979
admarble merged 4 commits into
mainfrom
feature/975-feat-config-model-roles-semantic-indirection-betwe

Conversation

@admarble

@admarble admarble commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds run.modelRoles map with shipped defaults (fast=sonnet, strong=opus, frontier=fable) so phase policy, --models, and ladder entries can reference roles with a role:<name> prefix instead of hard-coding model strings
  • resolveRoleToModel is the single choke point — called inside resolvePhasePolicies so both ExecutionConfig producers resolve roles through the same function (no drift, AC-5)
  • agents.model now accepts any free string instead of the closed haiku|sonnet|opus enum (AC-6)
  • Adds requestedModel/resolvedModel fields to per-phase metrics and PhaseMarker for cross-time benchmark comparisons (AC-4)

AC Verification

AC Description Status Evidence
AC-1 run.modelRoles parses; role:<name> dispatches with mapped model config-resolver.ts:resolveRoleToModel, resolvePhasePolicies wiring; 10 tests
AC-2 Missing role fails at config-resolution time naming role + available keys resolveRoleToModel throws before any session; 4 tests
AC-3 Raw model string passes through verbatim No role: prefix → early return; existing tests green + 3 new tests
AC-4 Metrics and phase marker record requestedModel + resolvedModel metrics-schema.ts, state-schema.ts updated; 4 metrics tests
AC-5 Both ExecutionConfig producers use same function (no second impl) buildExecutionConfig + commands/ready.ts both call resolvePhasePolicies; source-inspection drift guard (3 tests)
AC-6 agents.model free string; docs no longer rank capability z.string().default("haiku"); JSDoc updated; 4 tests

Key Decisions

Test Plan

  • npm run build — passes (0 TypeScript errors)
  • npm run lint — passes (0 ESLint errors)
  • 34 new unit tests all green (AC-1 through AC-6)
  • All pre-existing config-resolver.phase-policy.test.ts tests still pass (no regression)
  • Full suite: pre-existing flaky failures unchanged (4 files, infrastructure-level flakes)

Closes #975

🤖 Generated with Claude Code

Orchestrator verification (2026-08-29, commits d229de8 + 8bfb8fe)

Independently verified in the worktree after the AC-4 fix round: all model-roles suites pass (config-resolver 37 incl. AC-5 producer-2 drift guard; run-orchestrator AC-4 9/9; settings/metrics-schema), build + lint clean. Data-flow traced end to end: claude-code driver captures modelUsageAgentPhaseResult.modelUsage → phase-executor extracts the first key as resolvedModel (phase-executor.ts:1381-1386) → enrichPhasePoliciesFromResults merges it per phase → the metrics record's phasePolicies at run-orchestrator.ts:1778. requestedModel is captured at resolution time for role: references.

Added in 8bfb8fe: a source-inspection guard pinning the metrics call site — the first QA pass's defect (fields declared, never populated) could otherwise recur by dropping the call while every helper unit test stays green.

Mutation-verified: AC-4 — replaced the metrics record's enrichPhasePoliciesFromResults(...) call with a plain config.phasePolicies pass-through; test "run-orchestrator's metrics record passes phasePolicies through enrichPhasePoliciesFromResults" failed; restored.

Honest divergence flagged for the owner — AC-4's phase-marker clause

AC-4 says run metrics and the phase marker record requested + resolved model. The metrics half is fully implemented and gated. The marker half is schema-only (PhaseMarkerSchema carries the optional fields) and — as written — architecturally misfit: SEQUANT_PHASE markers are emitted by the phase agents themselves from the skill templates, before the driver's modelUsage exists, so an agent cannot put the resolved concrete ID in its own marker (formatPhaseMarker has no production caller; markers come from skill-templated comments). Options: (a) accept metrics as AC-4's system of record (the #916/#944 consumers read metrics, not markers) and drop the marker clause; (b) a small follow-up threading requestedModel into the marker via the existing SEQUANT_* env contract (feasible) while resolvedModel stays metrics-only. Not silently rationalized — owner's call at review.

…nd model strings

Adds `run.modelRoles` (shipped defaults: fast=sonnet, strong=opus,
frontier=fable) so config expresses roles; a single resolver maps roles
to concrete model strings. Phase policy, --models, and ladder entries
use `role:<name>` to reference an entry; bare strings pass through
verbatim for full backward compat (AC-3).

- `resolveRoleToModel` exported from config-resolver.ts; called inside
  `resolvePhasePolicies` so both ExecutionConfig producers resolve roles
  through the same function with no second implementation (AC-5).
- A missing role fails loudly at config-resolution time before any
  agent session spawns, naming the role and available map keys (AC-2).
- `agents.model` now accepts a free string (not closed enum) — accepts
  dated IDs and new family aliases (AC-6).
- `requestedModel` / `resolvedModel` added to per-phase phasePolicies
  in metrics schema and PhaseMarker — enables cross-time benchmark
  comparisons as the roster moves under aliases (AC-4).
- 34 new tests covering AC-1 through AC-6 including a source-inspection
  drift guard for the two-producer invariant (AC-5).

Entire-Checkpoint: 3c96ca19bf4a
- Adds enrichPhasePoliciesFromResults to run-orchestrator, merging the
  concrete modelUsage key from driver results into phasePolicies before
  metrics write (AC-4 resolvedModel)
- Adds requestedModel capture in resolvePhasePolicies for role: prefixes
  (AC-4 pre-resolution value)
- Adds run-orchestrator.model-roles.test.ts with 8 tests covering AC-4
- Expands config-resolver.model-roles.test.ts with AC-4 requestedModel
  assertions and AC-5 producer-2 drift guard for ready.ts
- Updates settings-schema.test.ts for AC-6: agents.model is now a free
  string (z.string()), accepting any alias or dated ID
- Adds modelUsage field to AgentResult in agent-driver.ts
- Threads resolvedModel from phase-executor through the PhaseResult type

Entire-Checkpoint: c018f9472e8c
The first QA pass found AC-4's fields declared but never populated. The
enrichment helper fixes that, but its unit tests cannot catch the call
being dropped from the metrics record — pin the call site with a
source-inspection guard, same pattern as the AC-5 producer-2 test.

Claude-Session: https://claude.ai/code/session_0178TZyoqDoCYehwfnQUDyNW
…ugh, phase-executor resolvedModel extraction, PhaseMarkerSchema round-trip

Entire-Checkpoint: 99aaa03d41f6
@admarble
admarble merged commit 10bd427 into main Aug 30, 2026
6 checks passed
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.

feat(config): model roles — semantic indirection between phase policy/ladders and concrete model strings

1 participant