Skip to content

feat: model slots and automatic fallback - #47

Draft
BabyKoan wants to merge 11 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-27
Draft

BabyKoan wants to merge 11 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-27

Conversation

@BabyKoan

@BabyKoan BabyKoan commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds named model slots (primary/lightweight/fallback) to Haze settings, automatic fallback retry on transient model errors, and a cheaper default model for subagents. Existing flat provider/model configs keep working unchanged.

Closes #27

Changes

  • Add models.{primary,lightweight,fallback} schema and resolveModelSlot helper.
  • Route modelWithConfig through slots; explicit --model / modelSelector still wins.
  • Retry retriable errors against the fallback slot once when it resolves to a different model.
  • Build the subagent tool with the lightweight slot, falling back to primary.
  • Support /model lightweight and /model fallback interactively.
  • Show lightweight/fallback slots in /settings and startup provider info.
  • Document model slots and fallback behavior in README.

Test plan

  • npm test passes (811 tests).
  • npm run lint and npm run typecheck pass.
  • Added unit tests for slot resolution, client slot routing, streaming fallback, subagent lightweight fallback, and slot-aware /model commands.

Quality Report

Changes: 16 files changed, 401 insertions(+), 21 deletions(-)

Code scan: clean

Tests: passed (0 test)

Branch hygiene: clean

Generated by Kōan

@Koan-Bot

Copy link
Copy Markdown
Contributor

PR Review — feat: model slots and automatic fallback

Solid, well-tested feature with clean backward compatibility — a few display and override-interaction rough edges to fix before merge.

Strengths:

  • resolveModelSlot cleanly centralizes slot inheritance, and the explicit-but-invalid-selector returns missing (rather than silently falling back) so config errors surface.
  • Backward compatibility is genuinely preserved: unset slots resolve to primary, flat provider/model keep working, and /model <selector> still sets primary via the flat fields.
  • Good test coverage for slot routing, streaming fallback, the same-model no-op case, and slot-aware /model commands.

Needs attention:

  • --model override + configured fallback: fallback message is printed but the override still wins, so the fallback model is never actually used and backoff is skipped (streaming.ts).
  • Unset lightweight/fallback slots display as the primary model, making inherited vs explicit indistinguishable; the not set/same as primary branches are dead (settingsSummary.ts, startupInfo.ts).
  • Duplicated slot label (Lightweight slot: Lightweight: ...) in the startup banner (startupInfo.ts).

🟡 Important

1. Duplicated slot label in startup output
src/cli/chat/startupInfo.ts:46-47

slotLine already prefixes its label argument, but these lines also hardcode the label in the template literal, so the rendered startup banner reads with the label twice.

With slotLine = (label, resolution) => "${label}: ..." and a call site of - Lightweight slot: ${slotLine('Lightweight', lightweight)}, the output is:

- Lightweight slot: Lightweight: openrouter:gpt-4o
- Fallback slot: Fallback: openrouter:gpt-4o

Why it matters: this is the provider-configuration banner every user sees at startup, so the doubled label looks broken. Note settingsSummary.ts got this right (it passes the full label 'Lightweight slot' and prints slotLine(...) directly). Either drop the inner label from slotLine here, or pass an empty label and keep the template prefix.

`- Lightweight slot: ${slotLine('Lightweight', lightweight)}`,
2. Unset slots render as the primary model, never "not set"/"same as primary"
src/cli/commands/settingsSummary.ts:29-39

The 'not set' (here) and 'same as primary' (startupInfo) branches are effectively dead, and the display can't distinguish an inherited slot from an explicitly configured one.

resolveModelSlot(settings, 'lightweight'|'fallback') returns resolveModelSlot(settings, 'primary') when the slot is unset — i.e. it returns {status: 'found', provider, model} pointing at the primary model. So the status !== 'found' branch only triggers when the primary itself is unconfigured. For any normally configured user, an unset lightweight/fallback slot prints the primary model verbatim:

Lightweight slot: openrouter:gpt-4o   # actually unset, just inheriting primary

The new test asserts exactly this (baseSettings has no models, yet expects Lightweight slot: openrouter:gpt-4o), so it locks in the misleading display rather than catching it.

Why it matters: a user reading /settings cannot tell whether they configured a separate lightweight model or are silently inheriting primary — which is the whole point of showing the slot. For display, check settings.models?.[slot] directly and show not set (inherits primary) when absent, reserving the resolved value for explicitly-set slots.

const slotLine = (label, resolution) =>
  `${label}: ${resolution.status === 'found' ? `${resolution.provider.name}:${resolution.model}` : 'not set'}`;
3. Fallback fires (and misreports) even when an explicit --model override is active
src/cli/commands/streaming.ts:389-404

The fallback decision is computed purely from the primary vs fallback slots and ignores modelOverride, but modelWithConfig lets modelOverride win over any slot. The two disagree when --model is in play.

modelOverride is the --model flag forwarded from haze run (cli/index.ts:67 → runCommand → runAgentTurn). When it's set and the turn hits a retriable error with a distinct fallback slot configured:

  • hasDistinctFallback is true (it never consults modelOverride).
  • It emits the fallback event and prints Primary model unavailable; falling back to <fallback>.
  • It recurses with slot: 'fallback' but modelOverride still set. In modelWithConfig, override takes precedence over slot, so the same overridden model runs again — the fallback model is never actually used.

Net effect: the message lies (claims a fallback that doesn't happen), and the normal backoff retry is skipped for that first error. An explicit --model should win here just as it does in slot resolution.

Fix: gate the fallback on the absence of an override, e.g. add !modelOverride to the hasDistinctFallback condition (or resolve the fallback against the override). There's also no test covering the override+fallback combination.

const hasDistinctFallback =
  !fallbackUsed &&
  primaryResolution.status === 'found' &&
  fallbackResolution.status === 'found' &&
  !sameModelResolution(primaryResolution, fallbackResolution);

Checklist

  • Logic correctness of fallback/override interaction — warning #3
  • User-facing output is accurate and unambiguous — warning #1, warning #2
  • Backward compatibility preserved for existing configs
  • New behavior has test coverage — warning #3
  • Diff matches PR description
  • No hardcoded secrets / injection / unsafe ops

Automated review by Kōan (Claude · model opus) HEAD=6026be7 3 min 46s

@Koan-Bot Koan-Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • Duplicated slot label in startup output
  • Unset slots render as the primary model, never "not set"/"same as primary"
  • Fallback fires (and misreports) even when an explicit --model override is active

BabyKoan added a commit to BabyKoan/haze that referenced this pull request Jun 27, 2026
@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-27 was rebased onto main and review feedback was applied.

Changes applied

  • of printing the primary model.
  • Explicitly configured but invalid selectors show <selector> (not found).
  • src/cli/commands/settingsSummary.ts
  • Same slotLine logic as startup info: unset slots report not set (inherits primary), configured valid slots report provider:model.
  • tests/cli/commands/settingsSummary.test.ts
  • Updated expectations to assert not set (inherits primary) for unset slots.
  • Added case confirming configured slots render as provider:model.
  • src/cli/commands/streaming.ts
  • Added !modelOverride to hasDistinctFallback so fallback does not fire (and misreport) when an explicit --model override is active, matching modelWithConfig precedence.
  • tests/cli/commands/streaming.test.ts
  • Added test verifying no fallback message and no slot change when modelOverride is set, while normal retry still happens.
  • Fixed modelWithConfigCalls type from nested arrays to flat array in existing fallback tests.
  • Typecheck and lint pass.

Stats

16 files changed, 478 insertions(+), 21 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=No commit in the provided recent main history mentions model slots, automatic fallback, or subagent )
  • Rebased koan/implement-27 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-27 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@atoomic

atoomic commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan review

@BabyKoan

BabyKoan commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Previous review — superseded by a newer review below.

@BabyKoan

Copy link
Copy Markdown
Contributor Author

PR Review — feat: model slots and automatic fallback

Solid feature with clean slot resolution and backward compatibility, but user-facing displays disagree with the new primary-slot semantics.

Strengths:

  • resolveModelSlot centralizes slot inheritance and surfaces invalid selectors instead of silently falling back.
  • Prior blocking review items are addressed: !modelOverride prevents misleading fallback when --model is active, and unset slots render as "not set (inherits primary)".
  • Subagent lightweight routing and streaming fallback logic are well-scoped and tested.

Needs attention:

  • startupInfo.ts and settingsSummary.ts still derive the main provider/model lines from flat provider/model settings, so the displayed primary can differ from models.primary.
  • /model lightweight (or /model fallback) without a selector is misinterpreted as a primary model named after the slot.
  • The slotLine formatter is duplicated across the two display modules.

🟡 Important

1. Main provider/model display ignores models.primary
src/cli/chat/startupInfo.ts:7-52

startupProviderInfo still derives the main Provider and Model lines from activeModel(settings), which only reads the flat provider/model settings. When models.primary is explicitly set, resolveModelSlot makes it the effective primary, so the displayed primary can disagree with the model actually used for requests.

Impact: A user with models.primary configured sees startup output that claims the primary is the flat-config model, while lightweight/fallback slots silently inherit a different models.primary value.

Fix: Resolve the displayed primary with resolveModelSlot(settings, 'primary') instead of activeModel(settings). Apply the same change in settingsSummary.ts.

const selection = activeModel(settings);
// ...
`- Provider: ${provider}`,
`- Model: ${model} (${modelSource})`,
2. Main provider/model display ignores models.primary
src/cli/commands/settingsSummary.ts:12-39

Same inconsistency as startupInfo.ts: the Provider and Model lines use resolveActiveProvider(settings) and settings.model, while slot resolution treats models.primary as the authoritative primary when set. The summary can show "Model: not set" or the wrong provider even though a primary slot is configured and active.

Fix: Use resolveModelSlot(settings, 'primary') to populate the main provider/model lines, falling back to the current flat behavior when the slot is unset.

const activeProvider = resolveActiveProvider(settings);
// ...
`Provider: ${activeProvider?.name ?? 'not configured'}`,
`Model: ${settings.model ?? 'not set'}`,
3. /model with missing selector misinterpreted
src/cli/commands/modelCommand.ts:6-12

/model lightweight or /model fallback without a following selector falls through to the primary path and treats the slot name itself as a model selector. The user gets a misleading confirmation and a bogus model is added to the active provider.

Impact: A simple incomplete command silently corrupts the provider model list.

Fix: When parts.length === 1 and the part is a slot name, return an error such as "Provide a model selector, e.g. /model lightweight openai:gpt-4o-mini" instead of treating it as a primary model.

function parseModelCommandArgs(value: string): {slot: ModelSlotName; selector: string} {
  const args = value.slice('/model '.length).trim();
  const parts = args.split(/\s+/);
  if (parts.length >= 2 && SLOT_NAMES.includes(parts[0] as ModelSlotName)) {
    return {slot: parts[0] as ModelSlotName, selector: parts.slice(1).join(' ')};
  }
  return {slot: 'primary', selector: args};
}

🟢 Suggestions

1. slotLine helper duplicated in settingsSummary.ts
src/cli/chat/startupInfo.ts:19-26

The slotLine formatter is identical in startupInfo.ts and settingsSummary.ts. Extracting it to a shared helper in src/config/providers.ts or a shared formatting module would prevent the two displays from drifting when slot formatting changes.

const slotLine = (label: string, slot: ModelSlotName) => {
  const configured = settings.models?.[slot]?.trim();
  if (!configured) return `${label}: not set (inherits primary)`;
  const resolved = resolveModelSlot(settings, slot);
  return resolved.status === 'found'
    ? `${label}: ${resolved.provider.name}:${resolved.model}`
    : `${label}: ${configured} (not found)`;
};

Checklist

  • No hardcoded secrets / credentials
  • Input validation at boundaries — warning #3
  • Error handling for missing/invalid slots
  • Backward compatibility preserved
  • User-facing output accurate and consistent — warning #1, warning #2, warning #3
  • Avoid code duplication — suggestion #1

To rebase specific severity levels, mention me: @BabyKoan rebase critical (fixes 🔴 only), @BabyKoan rebase important (fixes 🔴 + 🟡), or just @BabyKoan rebase for all.


Automated review by Kōan (Claude) HEAD=cee5ecd 6 min 22s

@atoomic

atoomic commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase

@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-27 was rebased onto main and review feedback was applied.

Changes applied

  • Done.
  • Changes:
  • src/config/providers.ts — added shared formatModelSlot helper so slot formatting lives in one place.
  • src/cli/chat/startupInfo.ts — main Provider/Model lines now use resolveModelSlot(settings, 'primary'), matching actual runtime primary. Removed duplicated slot label so startup no longer prints Lightweight slot: Lightweight: ....
  • src/cli/commands/settingsSummary.ts — same primary-slot resolution and switched to shared formatModelSlot, removing the duplicated local helper.
  • src/cli/commands/modelCommand.ts/model lightweight or /model fallback without a selector now returns an error instead of treating the slot name as a model.
  • tests/cli/commands.test.ts — added test for rejected /model lightweight.
  • tests/cli/commands/settingsSummary.test.ts — updated "no provider" expectation and added models.primary display test.
  • Verification: npm run typecheck and npm run lint pass; relevant unit tests pass.

Stats

16 files changed, 516 insertions(+), 34 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=Main still uses flat provider/model settings and lacks model slots, resolveModelSlot, fallback retry)
  • Rebased koan/implement-27 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-27 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

BabyKoan added a commit to BabyKoan/haze that referenced this pull request Jun 29, 2026
@atoomic

atoomic commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rr

@BabyKoan

Copy link
Copy Markdown
Contributor Author

@atoomic: @BabyKoan rebase

Rebased changes resolved the duplicated slot labels and missing-selector parsing from the earlier review. Two display-consistency issues remain in this revision: src/cli/commands/chat.tsx still builds its status-bar model label from activeModel, and src/cli/commands/runCommand.ts preflights headless runs with activeModel instead of resolveModelSlot('primary'). Both break when only models.primary is configured.

BabyKoan and others added 10 commits June 30, 2026 02:42
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…triable errors

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…t tests

docs: document model slots and automatic fallback

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@BabyKoan
BabyKoan force-pushed the koan/implement-27 branch from bb6d28a to 2ca7270 Compare June 30, 2026 02:51
@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-27 was rebased onto main and review feedback was applied.

Changes applied

  • Implemented requested fixes.
  • src/cli/commands/chat.tsx — status-bar model label now uses resolveModelSlot(settings, 'primary') instead of activeModel(settings). Fixes case where only models.primary was configured and the status bar showed unconfigured.
  • src/cli/commands/runCommand.ts — headless preflight now checks resolveModelSlot(settings, 'primary').status !== 'found' instead of !activeModel(settings). Fixes same models.primary-only configuration gap.
  • Verified existing shared formatModelSlot helper is already used in startupInfo.ts and settingsSummary.ts, so slot labels are not duplicated and unset slots render as not set (inherits primary).
  • Verified fallback logic in src/cli/commands/streaming.ts already skips fallback when modelOverride is active (!modelOverride guard), so explicit --model selectors are not misreported or retried against the fallback slot.
  • Typecheck and lint pass. Tests not run per instructions.

Stats

18 files changed, 521 insertions(+), 39 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=Main branch lacks model slots, fallback retry, and lightweight subagent routing; providers.ts, setti)
  • Resolved merge conflicts (1 round(s))
  • Rebased koan/implement-27 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-27 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@BabyKoan

Copy link
Copy Markdown
Contributor Author

@Koan-Bot:

Duplicated slot labels, unset-slot rendering, and explicit-override fallback misreporting are all resolved in the current revision. chat.tsx, runCommand.ts, and streaming.ts now use resolveModelSlot and the !modelOverride guard as requested.

@BabyKoan

Copy link
Copy Markdown
Contributor Author

PR Review — feat: model slots and automatic fallback

Solid, well-structured feature with good test coverage and clean backward compatibility, but three rough edges need fixing before merge.

Strengths:

  • resolveModelSlot centralizes slot inheritance cleanly and preserves legacy flat provider/model behavior.
  • Fallback gating is careful: it skips explicit --model overrides, skips when fallback equals primary, and allows only one fallback attempt.
  • Subagent lightweight routing through requestContext.ts is simple and falls back to the primary model gracefully.
  • Tests cover slot resolution, client routing, streaming fallback, and the new /model lightweight|fallback commands.

Needs attention:

  • Context-overflow recovery drops fallbackUsed, allowing a second fallback attempt.
  • Bare /model <selector> writes flat provider/model rather than models.primary, so it does not update the active primary model when models.primary already exists.
  • Headless stream-json output maps retry and context_overflow but omits the new fallback event.

🟡 Important

1. Context-overflow recovery resets fallbackUsed
src/cli/commands/streaming.ts:398

The context-overflow retry does not forward the fallbackUsed flag, so it defaults back to false. If a fallback had already been consumed before the overflow, the recovered turn gets a fresh fallback attempt, allowing more than one fallback per user turn.

Forward fallbackUsed as the tenth argument so the post-recovery turn preserves the retry budget already spent.

          return await runAgentTurn(value, displayValue, contextFiles, callbacks, retryAttempt, true, true, session, modelOverride);
2. Bare /model does not update the primary slot
src/cli/commands/modelCommand.ts:57-61

For the primary slot, handleModelCommand writes the resolved model to the legacy flat provider/model fields. Lightweight and fallback slots are written to settings.models. Because resolveModelSlot('primary') prefers models.primary when it exists, a bare /model gpt-4o-mini will not change the active primary model if models.primary is already set. This contradicts the README, which says /model sets the primary slot.

Write the selector to models.primary when setting the primary slot (while still updating the flat fields for compatibility), so /model always updates the resolved primary model.

  const selectorString = modelSelector(resolved.provider, resolved.model);
  if (slot === 'primary') {
    await ctx.updateSettings({provider: resolved.provider.name, model: resolved.model});
  } else {
    await ctx.updateSettings({models: {...ctx.settings.models, [slot]: selectorString}});
  }

🟢 Suggestions

1. stream-json output omits the fallback event
src/cli/commands/runCommand.ts:61-83

toHeadlessStreamEvent maps retry and context_overflow but has no case for the new fallback event. With --output stream-json, consumers see the retry before the fallback but not the fallback itself, making the stream harder to interpret.

Add a fallback case and extend HeadlessStreamEvent with {type: 'fallback'; provider: string; model: string; at: string}.

function toHeadlessStreamEvent(event: AgentEvent): HeadlessStreamEvent | undefined {
  switch (event.type) {
    case 'turn_start':
      return {type: 'turn_start', request: event.request, at: event.at};
    ...
    case 'context_overflow':
      return {type: 'context_overflow', recovered: event.recovered, error: event.error, at: event.at};
  }
}

Checklist

  • No hardcoded secrets / credentials
  • Input validation at boundaries
  • Error handling preserves retry/fallback state in recovery paths — warning #1
  • User-facing commands match documented slot semantics — warning #2
  • Public stream-json events reflect new fallback behavior — suggestion #1
  • Tests cover slot resolution and fallback paths

To rebase specific severity levels, mention me: @BabyKoan rebase critical (fixes 🔴 only), @BabyKoan rebase important (fixes 🔴 + 🟡), or just @BabyKoan rebase for all.


Automated review by Kōan (Claude) HEAD=2ca7270 11 min 26s

@atoomic

atoomic commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase

@BabyKoan

BabyKoan commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-27 was rebased onto main and review feedback was applied.

Changes applied

  • y flat provider/model fields, so /model always updates the resolved primary model.
  • src/cli/commands/streaming.ts: forwarded fallbackUsed through the context-overflow recovery retry so the recovered turn does not get a fresh fallback attempt.
  • src/cli/commands/runCommand.ts: added fallback event type to HeadlessStreamEvent and toHeadlessStreamEvent, so --output stream-json consumers see fallback switches.
  • tests/cli/commands.test.ts: updated expectations to assert models.primary is written for primary-slot /model commands.
  • tests/cli/commands/runCommand.test.ts: added test covering fallback event emission in stream-json output.
  • Note: top-level review mentions duplicated slot labels and unset slots rendering as primary. Current startupInfo.ts already uses formatModelSlot, which renders unset slots as not set (inherits primary), and the prior commit 2f23204 already removed the duplicate label issue. No further startup-info changes were required.

Stats

19 files changed, 541 insertions(+), 43 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=No model-slot schema, resolveModelSlot helper, lightweight/fallback routing, or slot-aware /model co)
  • Rebased koan/implement-27 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-27 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

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.

🧩 Multi-slot model config + automatic fallback (Complexity 7/10 · Value 8/10)

3 participants