Skip to content

fix(runtime-fallback): skip equivalent claude aliases#3322

Open
RaviTharuma wants to merge 3 commits intocode-yeongyu:devfrom
RaviTharuma:fix/runtime-fallback-equivalent-skip
Open

fix(runtime-fallback): skip equivalent claude aliases#3322
RaviTharuma wants to merge 3 commits intocode-yeongyu:devfrom
RaviTharuma:fix/runtime-fallback-equivalent-skip

Conversation

@RaviTharuma
Copy link
Copy Markdown
Contributor

@RaviTharuma RaviTharuma commented Apr 10, 2026

Summary

  • skip equivalent Claude-family provider aliases during runtime fallback selection
  • advance runtime fallback to the next genuinely distinct model instead of looping through equivalent aliases
  • update runtime-fallback tests to cover the corrected fallback progression and preserved-agent retry path

Verification

  • bun run typecheck
  • bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts

Summary by cubic

Skip equivalent Claude-family provider aliases during runtime fallback so retries move to a truly different model, not another alias. Variants are treated as distinct, and non-Claude suffixes like -high/-max/-thinking remain meaningful.

  • Bug Fixes
    • Canonicalize provider/model IDs and include parsed variant when computing equivalence; collapse Claude aliases across providers and strip -thinking, -max, -high only for Claude models.
    • Detect and skip equivalent fallbacks; log "Skipping equivalent fallback model" and advance to the next distinct option.
    • Update tests to verify skip-log presence, progression to openai/gpt-5.4, and preserved-agent retry behavior.

Written for commit 669afe2. Summary will update on new commits.

Prevent runtime fallback from cycling through provider aliases that resolve to the same underlying Claude family model. This keeps retry handling moving toward a genuinely distinct fallback model instead of appearing to fallback while staying on the same effective model.

Constraint: Live retry/fallback bug is in /Users/ravi/Code/personal/oh-my-opencode, while oh-my-openagent contribution work remains isolated to /Users/ravi/Code/forks/oh-my-openagent
Rejected: Change fallback chain precedence (category vs agent) first | lower-confidence root cause than equivalent-model retry
Confidence: high
Scope-risk: narrow
Directive: Keep alias-equivalence logic limited to model families that are intentionally interchangeable for runtime failover, and expand with targeted tests before broadening provider-family collapsing
Tested: bun run typecheck
Tested: bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts
Not-tested: Full live end-to-end session repro against external provider outages
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cac1d01947

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: The logic skips equivalent models across different providers (e.g., Bedrock vs Anthropic). This is a regression if a user relies on provider redundancy for the same model during outages.

Limit runtime-fallback equivalence collapsing to Claude-family aliases only so explicit non-Claude suffixes like -high remain meaningful fallback targets.

Constraint: Preserve the live Claude alias skip fix while addressing Codex review feedback on PR code-yeongyu#3322
Rejected: Keep global -high/-max/-thinking stripping | would collapse distinct non-Claude fallback targets
Confidence: high
Scope-risk: narrow
Directive: Only broaden equivalence collapsing when a provider/model family has explicit tests proving the variants are interchangeable for runtime failover
Tested: bun run typecheck
Tested: bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts
Not-tested: Full live outage repro across non-Claude provider suffix variants
@RaviTharuma
Copy link
Copy Markdown
Contributor Author

Re: #3322 (comment)

Good catch — this was valid.

I pushed a follow-up fix in edb2d7f2 that narrows the equivalence collapsing so suffix stripping is now Claude-family only (claude-opus-*, claude-sonnet-*, claude-haiku-*).

That preserves the intended runtime-fallback alias skip for equivalent Claude provider aliases, while keeping explicit non-Claude suffixes like -high, -max, and -thinking distinct fallback targets.

Re-verified after the change with:

  • bun run typecheck
  • bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts

So the original Claude alias loop fix remains, but the broader non-Claude regression is no longer present.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 1 file (changes from recent commits).

Requires human review: The canonicalization logic assumes cross-provider equivalence for Claude models, which might skip valid fallback paths if one provider is down but another is up.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: edb2d7f2b0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Treat parsed variant as part of runtime-fallback model equivalence so variant-only fallback hops remain distinct while preserving the existing Claude-family alias handling.

Constraint: Oracle verification flagged unresolved PR code-yeongyu#3322 review concerns about variant equivalence and remote state
Rejected: Preserve provider identity in equivalence | contradicted the original live-loop fix for equivalent Claude aliases
Confidence: medium
Scope-risk: narrow
Directive: Any future equivalence broadening must prove both live retry-loop behavior and variant/provider semantics with targeted tests before merging
Tested: bun run typecheck
Tested: bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts
Not-tested: Full live end-to-end repro across all provider redundancy policies
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 1 file (changes from recent commits).

Auto-approved: The PR correctly implements canonicalization logic to skip functionally equivalent Claude model aliases across different providers, and includes comprehensive test updates.

@RaviTharuma
Copy link
Copy Markdown
Contributor Author

Re: #3322 (comment) and #3322 (comment)

I tightened this one further in 669afe22.

Final behavior now is:

  • provider identity remains preserved in runtime-fallback equivalence
  • variant is part of equivalence (foo vs foo(high)/foo high no longer collapse)
  • the earlier follow-up in edb2d7f2 still keeps the non-Claude suffixes (-high, -max, -thinking) distinct

Re-verified after the latest change with:

  • bun run typecheck
  • bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts

So the current PR state now addresses:

  1. non-Claude suffixes staying distinct
  2. variant-only fallback hops staying distinct
    while preserving the already-tested runtime fallback behavior in the focused suite.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 669afe22d2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@RaviTharuma
Copy link
Copy Markdown
Contributor Author

Re: #3322 (comment) and #3322 (comment)

I rechecked the current branch state against the intended runtime behavior.

The final implementation on the PR branch keeps:

  • provider identity preserved in equivalence checks
  • variant as part of equivalence
  • and only the original, narrower non-Claude suffix fix from edb2d7f2

That means the two latest concerns are now addressed in the current branch state:

  1. provider-specific Claude fallbacks are no longer collapsed together
  2. variant-only fallback hops remain distinct

I also re-ran:

  • bun run typecheck
  • bun test src/hooks/runtime-fallback/index.test.ts src/hooks/runtime-fallback/error-classifier.test.ts src/plugin/event.model-fallback.test.ts

with the current local branch state before re-requesting review.

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