Skip to content

fix(model): preserve [1m] tag for the 'best' alias - #1671

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
0xfandom:fix/best-alias-1m-tag
Jun 17, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
0xfandom:fix/best-alias-1m-tag

Conversation

@0xfandom

@0xfandom 0xfandom commented Jun 16, 2026 •

Copy link
Copy Markdown
Contributor

Problem

parseUserSpecifiedModel appends the [1m] (1M-context) tag for the opus, sonnet, and haiku aliases, but not for best:

case 'sonnet':
  return getDefaultSonnetModel() + (has1mTag ? '[1m]' : '')
case 'opus':
  return getDefaultOpusModel() + (has1mTag ? '[1m]' : '')
case 'best':
  return getBestModel()   // <- drops the tag

getBestModel() returns getDefaultOpusModel() — the same model as the opus alias — so the two should behave identically. They don't:

opus[1m]  -> claude-opus-4-7[1m]
best[1m]  -> claude-opus-4-7      # 1M tag silently dropped

A user pinning best[1m] (best model, 1M context) silently loses the larger context window.

Fix

Append the tag for best as well, matching the other aliases:

case 'best':
  return getBestModel() + (has1mTag ? '[1m]' : '')

Test

Added parseUserSpecifiedModel.bestTag.test.ts with relational assertions (no hardcoded model id):

  • best[1m] === best + [1m]
  • best/best[1m] track opus/opus[1m] exactly
  • tag is case-insensitive and not duplicated

Verified it fails before the fix and passes after. tsc --noEmit clean.

Summary by CodeRabbit

  • Bug Fixes

    • The "best" model alias now correctly preserves variant specifications when applied.
  • Tests

    • Added regression test coverage for model alias variant tag handling.

parseUserSpecifiedModel appended the [1m] (1M-context) tag for the opus,
sonnet, and haiku aliases but not for 'best'. Since 'best' resolves to the
same model as 'opus' (getDefaultOpusModel), 'best[1m]' silently dropped the
1M-context request while 'opus[1m]' kept it — so a user pinning 'best[1m]'
lost the larger context window.

Append the tag for 'best' as well, matching the other aliases. Add a
relational regression test (best[1m] tracks opus[1m], tag is case-insensitive
and not duplicated).
@coderabbitai

coderabbitai Bot commented Jun 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a4979b65-0696-417e-abcc-50c5d9651868

📥 Commits

Reviewing files that changed from the base of the PR and between 7be9dce and ba44470.

📒 Files selected for processing (2)
  • src/utils/model/model.ts
  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: typecheck
  • GitHub Check: smoke-and-tests
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Keep comments useful and concise in code

Files:

  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
  • src/utils/model/model.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
  • src/utils/model/model.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
  • src/utils/model/model.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes fix: skip assertMinVersion for third-party providers #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
  • src/utils/model/model.ts
🔇 Additional comments (2)
src/utils/model/model.ts (1)

763-763: LGTM!

src/utils/model/parseUserSpecifiedModel.bestTag.test.ts (1)

8-30: Test suite is comprehensive and well-written.

The three test cases provide good relational coverage:

  1. Direct tag preservation (best[1m] includes [1m])
  2. Behavioral equivalence with opus alias (both bare and [1m] variants)
  3. Case-insensitive parsing and no-duplication guarantee

No test-isolation or async cleanup issues. Tests exercise the changed behavior without mocking provider or auth state, which is appropriate for relational assertions.


📝 Walkthrough

Walkthrough

In parseUserSpecifiedModel, the best alias resolution is updated to append [1m] when has1mTag is present. A new Bun test file adds regression coverage asserting tag preservation, case-insensitive parsing, and equivalence between best/best[1m] and opus/opus[1m].

Changes

best[1m] alias tag preservation

Layer / File(s) Summary
best alias fix and regression tests
src/utils/model/model.ts, src/utils/model/parseUserSpecifiedModel.bestTag.test.ts
parseUserSpecifiedModel appends [1m] to the resolved best model when has1mTag is true; new tests cover best[1m] tag retention, opus[1m] equivalence, and case-insensitive variants without duplication.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed Title directly and concisely summarizes the main change: preserving the [1m] tag for the 'best' alias, matching the actual diff.
Description check ✅ Passed Description covers problem statement, fix, testing approach, and verification. Most template sections are addressed with clear context.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Risk Surface Disclosed ✅ Passed PR does not touch auth, provider routing, permissions, network, background, startup, plugins, CI, or release areas—only model alias parsing.
No Hidden Policy Change ✅ Passed PR contains only model alias tag handling changes in model.ts and associated tests—no policy, telemetry, routing, trust-model, or permission-policy changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution. I do not see any actionable issues from my review.

@kevincodex1 LGTM

@kevincodex1
kevincodex1 merged commit da551e6 into Twigpine:main Jun 17, 2026
4 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.

3 participants