Skip to content

test(agents): align Claude static model naming#140

Open
caseyjkey wants to merge 1 commit intohappier-dev:devfrom
keycasey:dev
Open

test(agents): align Claude static model naming#140
caseyjkey wants to merge 1 commit intohappier-dev:devfrom
keycasey:dev

Conversation

@caseyjkey
Copy link

@caseyjkey caseyjkey commented Mar 23, 2026

Summary\n- align the static-model test for the Claude catalog with the refreshed 'Opus 4.6' display name that is now returned by getAgentModelConfig\n\n## Testing\n- yarn test (fails once it reaches @happier-dev/cli-common because that workspace does not declare a test script; all prior packages pass)\n- node ../../node_modules/vitest/vitest.mjs run --config ./vitest.config.ts (packages/cli-common)\n- yarn workspace @happier-dev/connection-supervisor test

Summary by CodeRabbit

  • Updates
    • Updated the display name for the Claude Opus 4.6 model to "Opus 4.6".

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

Walkthrough

Updated test expectations in models.test.ts to reflect a change in the static Claude Opus 4.6 model's display name from 'Claude Opus 4.6' to 'Opus 4.6' across two test assertions.

Changes

Cohort / File(s) Summary
Test Updates
packages/agents/src/models.test.ts
Updated expected name field for Claude Opus 4.6 model in static models lookup and getAgentStaticModels array validation from 'Claude Opus 4.6' to 'Opus 4.6'.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating test expectations for Claude static model naming to align with refreshed display names.
Description check ✅ Passed The description covers the summary and testing approach, but is missing recommended sections like Why (issue/discussion link) and the PR checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 23, 2026

Greptile Summary

This PR updates two test assertions in packages/agents/src/models.test.ts to align with the display name 'Opus 4.6' (dropping the 'Claude ' prefix) already present in the CLAUDE_STATIC_MODELS catalog in models.ts. The test was previously failing because it expected 'Claude Opus 4.6' while the source of truth had already been updated to 'Opus 4.6'.

  • The change is purely in the test file — no production code is modified.
  • Both occurrences of the stale string (toMatchObject spot check on line 12 and the toEqual full-object assertion on line 25) are correctly updated.
  • The naming convention is consistent with all other Claude static models ('Sonnet 4.6', 'Haiku 4.5', etc.) which omit the 'Claude ' prefix, mirroring how Gemini models use 'Gemini X.Y'.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, self-contained test fix with no production-code impact.
  • The only change is bringing two stale string literals in a test file in line with the already-correct source in models.ts. There is no logic change, no risk of regression, and the fix is verified against the live catalog definition.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/agents/src/models.test.ts Updates two test assertions to expect 'Opus 4.6' instead of 'Claude Opus 4.6' for the claude-opus-4-6 model, correctly aligning with the naming convention already used in models.ts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["models.test.ts\ngetAgentModelConfig('claude')"] --> B["models.ts\nAGENT_MODEL_CONFIG.claude"]
    B --> C["CLAUDE_STATIC_MODELS"]
    C --> D["claude-opus-4-6\nname: 'Opus 4.6'"]
    A --> E["Test assertion\n.toMatchObject({ name: 'Opus 4.6' })"]
    D -- "matches" --> E
    A2["models.test.ts\ngetAgentStaticModels('claude')"] --> F["claudeModels[0]"]
    F --> G["Test assertion\n.toEqual({ name: 'Opus 4.6', ... })"]
    D -- "matches" --> G
Loading

Reviews (1): Last reviewed commit: "test(agents): align Claude static model ..." | Re-trigger Greptile

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/agents/src/models.test.ts (1)

14-14: Reduce brittle copy-pinning in model-name assertions

On Line 14 and Line 26, asserting exact label text ('Opus 4.6') makes this test fragile to non-functional naming refreshes. Prefer asserting stable behavior (id contract + parity between returned structures) and that name is present.

Proposed refactor
-    expect(claude.staticModels?.find((model) => model.id === 'claude-opus-4-6')).toMatchObject({
-      id: 'claude-opus-4-6',
-      name: 'Opus 4.6',
-      description: expect.any(String),
-    });
+    const claudeOpusFromConfig = claude.staticModels?.find((model) => model.id === 'claude-opus-4-6');
+    expect(claudeOpusFromConfig).toMatchObject({
+      id: 'claude-opus-4-6',
+      name: expect.any(String),
+      description: expect.any(String),
+    });
@@
-    expect(claudeModels[0]).toEqual({
-      id: 'claude-opus-4-6',
-      name: 'Opus 4.6',
-      description: expect.any(String),
-    });
+    expect(claudeModels[0]).toEqual({
+      id: 'claude-opus-4-6',
+      name: expect.any(String),
+      description: expect.any(String),
+    });
+    expect(claudeModels[0]?.name).toBe(claudeOpusFromConfig?.name);

Based on learnings: “Content-only changes … do not require new tests. If existing tests fail because they pin copy/formatting, loosen the assertions to check stable behavior instead of exact text.”

Also applies to: 26-26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/agents/src/models.test.ts` at line 14, The test currently pins exact
model label text ("name" value 'Opus 4.6') making it brittle; update the
assertions in models.test.ts to stop checking exact name text and instead assert
that each model object has a non-empty name property, that the model "id"
matches the expected id contract, and that the returned structures (e.g.,
arrays/objects under test) are consistent/parity-equal where applicable (compare
ids and other stable fields rather than exact name strings). Locate the
assertions referencing the "name" field (the entries with name: 'Opus 4.6') and
replace exact-equality checks with presence/non-empty checks and id-based
equality/parity checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/agents/src/models.test.ts`:
- Line 14: The test currently pins exact model label text ("name" value 'Opus
4.6') making it brittle; update the assertions in models.test.ts to stop
checking exact name text and instead assert that each model object has a
non-empty name property, that the model "id" matches the expected id contract,
and that the returned structures (e.g., arrays/objects under test) are
consistent/parity-equal where applicable (compare ids and other stable fields
rather than exact name strings). Locate the assertions referencing the "name"
field (the entries with name: 'Opus 4.6') and replace exact-equality checks with
presence/non-empty checks and id-based equality/parity checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 85ad88eb-da1c-4b85-aad8-1b8f2cc55e7c

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba2449 and 0c9bc6b.

📒 Files selected for processing (1)
  • packages/agents/src/models.test.ts

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