feat(core): tiered model aliases — replace opus/sonnet/haiku with high/medium/low

[ldangelo]

Summary

• Replaces opus/sonnet/haiku model aliases with abstract tier aliases high/medium/low across all 18 command YAMLs, all 28 agent YAMLs, and the schema files
• Introduces <project>/.claude/ensemble-model-config.json as the per-project tier→model-ID mapping (replaces XDG user-level config)
• Adds /ensemble:map-model interactive wizard and /ensemble:migrate-model-config one-shot migration command
• Ships packages/core/lib/known-model-ids.js as single source of truth for valid Claude model IDs
• Adds npm run lint:model-ids CI lint script — scans all YAML for retired/unknown model IDs

Breaking Changes

• opus, sonnet, haiku aliases are no longer valid — rename to high, medium, low
• Config now loaded from <project_root>/.claude/ensemble-model-config.json (not XDG path)
• ENSEMBLE_MODEL_OVERRIDE must be a tier alias (high/medium/low), not a model ID or legacy alias
• commandOverrides block in legacy config is not carried forward (set metadata.model in each command YAML instead)

Test plan

• 207 unit tests passing in packages/core/tests (all new code covered)
• node scripts/lint-model-ids.js exits 0 — 58 YAML files scanned, zero legacy/unknown model values
• grep -rn "model: opus|model: sonnet|model: haiku|model: inherit" packages/*/agents/ packages/*/commands/ → 0 matches
• All 28 agents now have explicit model: high|medium|low field
• All 18 command YAMLs migrated
• schemas/command-yaml-schema.json enum updated to ["high","medium","low"]
• schemas/agent-yaml-schema.json now has optional model field with same enum
• CHANGELOG and README updated
• packages/core/lib/usage-logger.js MODEL_PRICING updated to current (non-retired) model IDs

Implementation phases

Phase	Commits	Scope
1 — Core Foundation	e9f9af7	known-model-ids.js, JSON Schema, config-loader.js, model-resolver.js rewrite
2 — Schema + YAML Migration	3524c2e	2 schema files, 18 command YAMLs, 28 agent YAMLs, generated .md files
3 — Wizard + Commands	5ec858f	map-model-wizard.js, legacy-config-migrator.js, 2 command definitions
4 — CI + Docs + Cleanup	09254bd	lint script, npm script, CHANGELOG, README, usage-logger cleanup

🤖 Generated with Claude Code

[claude]

Code Review — feat(core): tiered model aliases

Overall this is a well-architected, well-tested change. The motivation is clear (abstract away model-name churn), the phased implementation is organized, and the test coverage looks solid. I have a few concerns worth addressing before merge.

---

Unintended Files in the PR

.beads/ runtime files should not be committed here.
The diff includes .beads/.local_version, .beads/dolt-server.pid, .beads/dolt-server.port, and .beads/issues.jsonl — these are local database/process-tracking state from the beads issue tracker and are unrelated to the model-alias migration. These should be added to .gitignore and removed from this PR.

---

Bugs / Correctness Issues

1. extractModelValue regex in lint-model-ids.js is fragile

const match = yamlContent.match(/^  model:\s+(\S+)\s*$/m) || yamlContent.match(/^model:\s+(\S+)\s*$/m);

• Only matches exactly 0 or 2 spaces of indentation. YAML files indented with 4 spaces (or tabs) will be silently skipped, creating a false sense of security in the CI check.
• Only extracts the first model: field in a file. A YAML with multiple model references will only have the first validated.

Consider using a proper YAML parser (js-yaml is already available in the monorepo ecosystem), or at minimum use \s* instead of for the indentation capture.

2. Silent key-dropping in migrateLegacyConfig

// Any other keys are silently dropped

Custom model aliases that don't map to opus/sonnet/haiku/high/medium/low are dropped without any user-facing warning. A user who had, for example, { "claude-3": "claude-3-opus-20240229" } in their legacy config would lose that mapping silently. At minimum, emit a warning to stderr for dropped keys.

3. selectModel no longer accepts an injected config — reduces testability

The old signature selectModel(command, config, options) accepted a config object, making tests straightforward to write without touching the filesystem. The new signature selectModel(commandName, explicitTier, startDir) calls loadConfig(startDir) internally. This makes unit tests harder because they must either mock the fs module or write real config files to temp directories. Consider accepting an optional config parameter as an escape hatch for tests.

---

Design / Architecture Concerns

4. Schema inconsistency: additionalProperties: true at root

In schemas/ensemble-model-config-schema.json:

"additionalProperties": true   ← root level

But tiers has "additionalProperties": false. The root-level permissiveness means unknown fields (e.g., a typo like "tier" instead of "tiers") will be accepted by schema validators without error. Recommend changing to false at the root, or at minimum adding a note explaining why it's intentionally open.

5. extraKnownModelIds has no format validation

The schema accepts any string in extraKnownModelIds. A typo like "claude-opus-47" (missing dash) would pass validation, silently failing to allowlist the intended model. Adding a regex pattern (e.g., ^claude-[a-z0-9-]+-[0-9]+) would catch obvious mistakes at config-write time.

6. loadConfig has side effects (hint/warning emission) on every call

loadConfig() calls checkLegacyXdgFile() and emitFirstRunHint() on every invocation. For a function named "load config" this is surprising — callers composing multiple config reads or running in test environments will trigger filesystem I/O and stderr output unexpectedly. Consider separating the loading from the one-time notification logic (e.g., call hints/warnings only from a dedicated initialization path or CLI entrypoint).

7. modelAliases backward-compat mutation

parsed.modelAliases = parsed.tiers;  // same reference

This mutation of the loaded config object is subtle and could cause bugs if any caller modifies modelAliases expecting it to be independent from tiers. Since the PR explicitly removes the old API, it's worth auditing whether any code still reads .modelAliases and removing the alias entirely.

8. _getSeenWarningsPath / _getSeenHintsPath exported with underscore prefix

Prefixing with _ conventionally signals "internal, do not use." Exporting them from module.exports and re-exporting from lib/index.js contradicts that signal. If they must be exported for testing, consider using a separate test-only export mechanism or removing the underscore prefix.

---

Minor Observations

• KNOWN_MODEL_IDS hardcodes 3 model IDs — this is intentional per the design, but worth documenting that the package will need a version bump each time Anthropic releases a new generation. The extraKnownModelIds escape hatch is a good mitigation.
• BYPASS_COMMANDS only contains ['map-model', 'migrate-model-config'] — consider whether discover-standards or other setup-phase commands also need bypass.
• The emitFirstRunHint / seen-hints sentinel approach is pragmatic but adds two hidden files to ~/.config/ensemble/. These should be documented so users know what those files are.

---

What's Working Well

• Atomic writes (writeConfigAtomic via tmp-file + rename) — correct and prevents partial-write corruption.
• Object.prototype.hasOwnProperty.call() used consistently — avoids prototype-chain bugs.
• Object.freeze() on DEFAULT_CONFIG — prevents accidental mutation of the fallback.
• BYPASS_COMMANDS design — elegant solution to the circular-dependency problem where migration commands can't require a valid config to run.
• CI lint script (npm run lint:model-ids) with integration test via execSync — exactly the right place to catch regressions.
• 207 unit tests is strong coverage for this scope.

---

Priority Summary

Severity	Item
Must fix	.beads/ files in PR — unrelated runtime state
Should fix	extractModelValue regex — only 2-space indent, first-match-only
Should fix	Silent key-dropping in migrator
Consider	Schema additionalProperties: true at root
Consider	loadConfig side-effect separation
Consider	selectModel testability / injected config

🤖 Reviewed with Claude Code

[claude]

Code Review — feat(core): tiered model aliases

Overview

This PR replaces opaque model aliases (opus/sonnet/haiku) with semantic tier labels (high/medium/low) and moves config from a global XDG user-level file to a per-project .claude/ensemble-model-config.json. The architectural direction is sound — decoupling semantic intent from model ID strings makes the config durable across model generations. The implementation is well-structured with clear separation of concerns across known-model-ids.js, config-loader.js, model-resolver.js, and the wizard/migrator.

---

Strengths

• Single source of truth — known-model-ids.js exports KNOWN_MODEL_IDS as a frozen array, and every module (config-loader, model-resolver, map-model-wizard, lint-model-ids.js) imports from it. No duplicated lists.
• Atomic writes — writeConfigAtomic (write to .tmp, then fs.renameSync) prevents corrupt config files on interrupted writes.
• Preflight validation — preflightValidate blocks command execution when the config references an unknown model ID, making config rot a loud failure rather than a silent wrong-model run.
• First-run UX — emitFirstRunHint fires once per project root (using a seen-hints sentinel) and checkLegacyXdgFile fires once globally. Both degrade gracefully on write failures.
• Test coverage — 207 tests across real tmpdir isolation (not mock-fs) is solid; using jest.resetModules() in beforeEach to prevent module-cache pollution is correct.
• BYPASS_COMMANDS — exempting map-model and migrate-model-config from preflight prevents the chicken-and-egg lockout.

---

Issues & Suggestions

Bug — logUsage always enabled now (breaking behavior change)

The old logUsage returned early if config.costTracking?.enabled was falsy. The new version logs unconditionally, regardless of config. The config param is now silently ignored (// eslint-disable-line no-unused-vars). If any users had costTracking.enabled: false in their legacy config, their disk will start accumulating usage logs after this migration — and there is no way to opt out.

Suggestion: Either add an env var gate (ENSEMBLE_DISABLE_USAGE_LOG=1) or restore an opt-out mechanism in the new config schema.

Design concern — additionalProperties: true in the JSON Schema

schemas/ensemble-model-config-schema.json sets additionalProperties: true at the root. This means typos like "tier" (instead of "tiers") or "extraKnownModelId" (missing the s) will silently pass schema validation and silently fall back to defaults, which is very hard to debug.

Suggestion: Set additionalProperties: false. The schema only defines three known properties (version, tiers, extraKnownModelIds); unknown keys should be an error.

Fragility — extractModelValue uses regex instead of parsing YAML

scripts/lint-model-ids.js uses a regex (/^ model:\s+(\S+)\s*$/m) to extract model: from YAML files rather than parsing the YAML. This means:

1. A valid YAML file with model: indented at a depth other than 0 or 2 spaces (e.g., 4 spaces, or inside a nested object) will be silently missed.
2. A YAML file with a multi-line value for model: will be silently missed or misread.
3. The test suite for lint-model-ids.js replicates the extractModelValue function rather than importing it from the script. If the regex ever diverges, the tests won't catch it.

Suggestion: Either parse YAML with a real parser (the js-yaml package is already widely used), or export extractModelValue from the script so tests import the real function.

Minor — runOneShotUpdate mutates the loaded config object before rebuilding it

config.tiers[tier] = modelId;   // mutates
const newConfig = { version: 1, tiers: { ...config.tiers }, ... };

This is safe here because loadConfig returns a plain object, but it's an unexpected side effect. The mutation is unnecessary since the final newConfig rebuilds the tiers object anyway.

Suggestion: Build newConfig directly without the mutation:

const newConfig = { version: 1, tiers: { ...config.tiers, [tier]: modelId }, extraKnownModelIds: config.extraKnownModelIds || [] };

Stale .beads/ files in the diff

The diff includes .beads/.local_version, .beads/dolt-server.pid, .beads/dolt-server.port, and .beads/issues.jsonl. These are internal tooling state files unrelated to this feature.

Suggestion: Add .beads/dolt-server.pid, .beads/dolt-server.port, and .beads/.local_version to .gitignore (.beads/issues.jsonl may be intentional; the others look like ephemeral runtime state). At minimum, stage only the relevant files in the PR commits.

Minor — mobile-developer.md included in diff with no apparent changes

packages/development/agents/mobile-developer.md appears in the diff but may just be a generated markdown re-render. Verify it's intentional.

Nit — redundant XDG path construction

_getSeenWarningsPath() and _getSeenHintsPath() each independently compute the same XDG base path. Consider extracting:

function _xdgEnsembleDir() {
  return path.join(process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'), 'ensemble');
}

This pattern also already exists in getLogPath() in usage-logger.js, so extracting it to a shared utility would DRY up four callsites.

---

Security

No significant concerns. The config file is read-only from the project root; there is no shell command injection or dynamic require() based on config values. The atomic-write pattern prevents partial-write races.

---

Summary

This is a well-planned and thoroughly tested migration. The three items worth addressing before merge:

1. logUsage always-enabled (behavior regression for users who had opted out)
2. additionalProperties: true in the new JSON Schema (silent typo swallowing)
3. .beads/ ephemeral files in the diff (clean up .gitignore)

The regex-based YAML extraction in the lint script is a lower-priority concern but worth noting as a future brittleness point.

🤖 Generated with Claude Code

[claude]

Code Review: feat(core) tiered model aliases

Overall this is a clean, well-structured change. The phased implementation, comprehensive test suite, and CI lint script are all solid. A few issues worth addressing before merge:

---

Bugs / Correctness

1. BYPASS_COMMANDS name mismatch (potential bug)

BYPASS_COMMANDS = ['map-model', 'migrate-model-config'] uses short names, but the selectModel callers in integration tests use both 'ensemble:map-model' (namespaced) and 'map-model' (short). If any production caller passes 'ensemble:map-model', the bypass won't trigger, causing a PreflightError even during the setup/repair flow.

// model-resolver.js:38
if (BYPASS_COMMANDS.includes(commandName)) { // only matches 'map-model', not 'ensemble:map-model'

Consider normalising with commandName.replace(/^.*:/, '') before the check, or documenting the required convention explicitly.

2. mobile-developer.md escapes lint validation

packages/development/agents/mobile-developer.md has a model: medium field, but the lint script (lint-model-ids.js) only scans *.yaml files. This agent gets no model-ID validation. Either rename to .yaml for consistency or extend the lint script to also cover .md agent files.

---

Design / API

3. Preflight runs before ENSEMBLE_MODEL_OVERRIDE is checked

In selectModel, preflightValidate(config, commandName) is called before the env override is processed. If a project has an invalid model ID in one tier (e.g. a retired model for low) but the user has set ENSEMBLE_MODEL_OVERRIDE=high, they'll still get a PreflightError even though the broken tier will never be used. This is a surprise when using the override precisely to work around a broken config.

Consider validating only the tier that will actually be selected, or checking the override first.

4. XDG_LEGACY_PATHS captured at module load time

In legacy-config-migrator.js:

const XDG_LEGACY_PATHS = [
  path.join(process.env.XDG_CONFIG_HOME || ..., ...)

This is evaluated once at require() time. Tests reset modules per-test (good), but if any long-lived process changes XDG_CONFIG_HOME after module load, this won't reflect the change. The config-loader.js reads XDG_CONFIG_HOME lazily inside each function — consider aligning the migrator to the same pattern.

---

Schema / Validation

5. ensemble-model-config-schema.json top-level additionalProperties: true

The schema allows unknown top-level keys silently. A typo like "tierss" instead of "tiers" would pass schema validation and produce a confusing "missing tier 'high'" error from the JS validator rather than a clear schema error. Consider "additionalProperties": false at the top level.

6. No format constraint on extraKnownModelIds entries

The schema accepts any non-empty string in extraKnownModelIds. Consider adding a "pattern": "^claude-" constraint to catch accidental non-Claude model strings early.

---

Non-functional / Clean-up

7. Ephemeral .beads/ files in the diff

.beads/dolt-server.pid and .beads/dolt-server.port contain runtime state (process ID, port number) and should not be tracked in git. If .beads/ is intentionally tracked (e.g. issues.jsonl), add *.pid and *.port to .gitignore to avoid future churn.

---

Positives worth calling out

• writeConfigAtomic (tmp + rename) correctly prevents partial writes.
• Test isolation is thorough: tmpDir, isolated XDG_CONFIG_HOME, and jest.resetModules() per test.
• KNOWN_MODEL_IDS as a single source of truth for config-loader, model-resolver, wizard, and lint script is the right design.
• The extraKnownModelIds escape hatch for preview models is a thoughtful addition.
• Integration tests cover the full selection + preflight + env-override flow end-to-end.

---

Summary

The two most important items are #1 (BYPASS_COMMANDS mismatch — a latent bug that will surface if the namespaced form is ever used as commandName) and #2 (mobile-developer.md escaping lint). Everything else is polish. Happy to approve once those are addressed.

[claude]

Code Review — feat(core): tiered model aliases

Overview

This PR replaces legacy opus/sonnet/haiku model aliases with abstract tier aliases (high/medium/low) across all 46 YAML files, rewrites the config/model-resolution stack, and ships two new commands + a CI lint script. The phased implementation is clearly structured, and the test coverage (real tmpdir instead of mock-fs) is solid. Overall this is a well-engineered change; the items below are mostly minor.

---

Issues

1. Ephemeral .beads/ runtime files committed (should fix before merge)

.beads/dolt-server.pid and .beads/dolt-server.port contain the developer's local PID and port number. These are runtime state and will cause meaningless noise in every future diff. They should be added to .gitignore and removed from this PR.

.beads/dolt-server.pid    ← local process ID, changes every run
.beads/dolt-server.port   ← ephemeral port, changes every run

2. logUsage now runs unconditionally — silent behavior change

Old behavior: if (!config.costTracking?.enabled) return; — logging was opt-in.
New behavior: logging always happens, config argument is ignored (renamed to _config via the eslint-disable comment).

Users who had costTracking: { enabled: false } will silently start writing to ~/.config/ensemble/logs/model-usage.jsonl after upgrading. This breaking change isn't listed under Breaking Changes in the PR description. Either document it, or keep an explicit opt-out path.

// packages/core/lib/usage-logger.js
function logUsage(params, config) { // eslint-disable-line no-unused-vars
  const logPath = getLogPath();    // always runs now — no gate

3. KNOWN_MODEL_IDS comment contradicts its own content

The file-level JSDoc says "All IDs must be fully-pinned (no '-latest')" but claude-opus-4-7 and claude-sonnet-4-6 use no date suffix while claude-haiku-4-5-20251001 does. The comment should clarify the actual convention (date suffix required only where Anthropic publishes multiple dated snapshots of the same model generation), otherwise contributors will be confused about what "fully-pinned" means here.

4. lint-model-ids.js uses regex instead of YAML parser — fragile

// scripts/lint-model-ids.js
const match = yamlContent.match(/^  model:\s+(\S+)\s*$/m) || yamlContent.match(/^model:\s+(\S+)\s*$/m);

The script intentionally avoids full YAML parsing, but the 2-space indent assumption will silently skip any model: field at a different indentation depth (e.g., inside a step block). A false negative here means a retired model ID slips through CI. The YAML files already have a consistent structure, so using js-yaml to parse and then inspecting metadata.model would be more reliable and not meaningfully slower.

5. ensemble-model-config-schema.json allows extra top-level properties

"additionalProperties": true

This means a config with legacy fields (commandOverrides, defaults) passes schema validation without error, even though the loader silently ignores them. Setting additionalProperties: false would catch stale migration artifacts and is consistent with the tiers object which already sets additionalProperties: false.

6. writeConfigAtomic will fail on Windows

fs.writeFileSync(tmpPath, ...);
fs.renameSync(tmpPath, configPath);  // atomic on POSIX, throws on Windows if target exists

Windows does not support rename over an existing file. This isn't a blocker if the project is macOS/Linux-only, but it's worth a comment near the renameSync call so future contributors don't silently break Windows users.

7. selectModel can surface TypeError instead of PreflightError for invalid tiers

If a command YAML passes an explicitTier value of 'opus' (a legacy alias, not caught by the env-var path), resolveModel throws a TypeError rather than a PreflightError. Callers that catch only PreflightError would see an unhandled exception. Either funnel the resolveModel throw through PreflightError, or add an explicit legacy-alias check for explicitTier analogous to the one for envOverride.

---

Suggestions (non-blocking)

• emitFirstRunHint uses absolute projectRoot as the hint key. If the project is moved or checked out at a different path, the hint fires again. A hash of the projectRoot basename or the git remote URL would be more stable. Low impact for now.

• getDefaultConfig() creates modelAliases = tiers as a shared reference. Mutating cfg.tiers will also mutate cfg.modelAliases — the docs note this, but it's a footgun. Consider making modelAliases a non-enumerable getter instead to make the alias invisible to JSON.stringify and Object.keys.

• BYPASS_COMMANDS is exported from config-loader.js but conceptually belongs in model-resolver.js. The only consumer is preflightValidate in model-resolver.js. Having the bypass list live in the loader creates a layering dependency (loader has knowledge of resolver-level concerns).

• legacy-config-migrator retains the legacy file. This is conservative and safe, but the migration output message could explicitly tell users rm <legacyPath> once they've verified the new config, rather than leaving them to discover the lingering file triggers the one-time warning.

---

Test Coverage — Positive Notes

• Switching from jest.mock('fs') to real tmpdirs is the right call — the old tests were masking actual filesystem behavior.
• Using jest.resetModules() in beforeEach/afterEach to isolate XDG env changes is clean.
• Integration test makeProject() helper is well-structured.
• 207 tests + lint script covering 58 YAML files is strong for a migration of this scope.

---

Summary

The architecture is sound and the YAML migration is thorough. The two items most worth addressing before merge are #1 (ephemeral .beads/ files polluting git history) and #2 (silent logging behavior change). The rest are polish items.

🤖 Generated with Claude Code