fix: map Opus 4.6 to @default for Vertex AI

[jeremyeder]

Summary

• Add claude-opus-4-6 → claude-opus-4-6@default mapping to VERTEX_MODEL_MAP — without this, Vertex AI returns 404 for Opus 4.6 sessions
• Fix 2 pre-existing test failures: test_none_input_handling and test_numeric_input_handling expected TypeError but dict.get() handles any hashable key without error
• Remove unused pytest import

Context

PR #581 added Opus 4.6 support but mapped it to itself (no version suffix). We verified via live Vertex AI API calls that claude-opus-4-6@default is the correct model identifier — the @YYYYMMDD date convention doesn't apply to this model.

Test plan

• All 20 model mapping tests pass (previously 18/20)
• Verified claude-opus-4-6@default returns HTTP 200 from Vertex AI (us-east5, project my-project)
• Manual e2e: create session with Opus 4.6 on Vertex AI deployment

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

Claude Code Review

Summary

This PR adds Opus 4.6 support for Vertex AI by mapping claude-opus-4-6 to claude-opus-4-6@default, removes deprecated Opus 4.1 from the frontend dropdown, and fixes two pre-existing test failures related to invalid input handling. The changes are well-structured and include comprehensive test coverage.

Issues by Severity

🚫 Blocker Issues

None identified.

🔴 Critical Issues

None identified.

🟡 Major Issues

1. Formatting changes mixed with functional changes (auth.py)

The auth.py file includes significant auto-formatting changes (line wrapping, string formatting) that obscure the actual functional change (the VERTEX_MODEL_MAP update). This makes code review harder and increases merge conflict risk.

Impact: Makes it difficult to identify the actual business logic change vs. formatting noise.

Recommendation: In future PRs, separate auto-formatting commits from functional changes, or run formatters (black, isort) before starting work to avoid mixing concerns.

---

2. Missing validation for map_to_vertex_model() with non-string inputs

The function signature is map_to_vertex_model(model: str) -> str but the tests now verify it handles None and numeric inputs gracefully by returning them unchanged. This is inconsistent with the type signature.

def test_none_input_handling(self):
    """Test that None input passes through unchanged (dict.get handles it)"""
    result = map_to_vertex_model(None)  # type: ignore[arg-type]
    assert result is None

def test_numeric_input_handling(self):
    """Test that numeric input passes through unchanged (dict.get handles it)"""
    result = map_to_vertex_model(123)  # type: ignore[arg-type]
    assert result == 123

Issues:

• The function relies on dict.get()'s permissive behavior rather than explicit validation
• Type hints are being bypassed with # type: ignore[arg-type]
• Callers might pass invalid types and receive silent failures
• This violates Python's "explicit is better than implicit" principle

Recommendation:
Either:

1. Add explicit type validation and raise TypeError for invalid inputs (stronger typing)
2. Or update the type signature to def map_to_vertex_model(model: str | None) -> str | None and handle None explicitly

Example of explicit validation:

def map_to_vertex_model(model: str) -> str:
    """Map Anthropic API model names to Vertex AI model names."""
    if not isinstance(model, str):
        raise TypeError(f"model must be a string, got {type(model).__name__}")
    return VERTEX_MODEL_MAP.get(model, model)

This aligns with the error handling patterns in .claude/patterns/error-handling.md which emphasize explicit error handling over silent failures.

🔵 Minor Issues

1. Inconsistent test coverage for version suffix types

The PR introduces two different version suffix patterns:

• @YYYYMMDD for most models (e.g., claude-opus-4-5@20251101)
• @default for Opus 4.6 (e.g., claude-opus-4-6@default)

While test_mapping_includes_version_suffix() was updated to handle both patterns, the test logic is somewhat convoluted:

# Version suffix is either "default" or YYYYMMDD date
assert version_suffix == "default" or (
    len(version_suffix) == 8 and version_suffix.isdigit()
), f"Version suffix {version_suffix} should be 'default' or 8-digit date"

Recommendation: Consider extracting version suffix validation into a helper function for clarity:

def is_valid_vertex_version_suffix(suffix: str) -> bool:
    """Check if version suffix is either 'default' or YYYYMMDD date."""
    return suffix == "default" or (len(suffix) == 8 and suffix.isdigit())

# Then in test:
assert is_valid_vertex_version_suffix(version_suffix), \
    f"Invalid version suffix: {version_suffix}"

---

2. Test naming could be more descriptive

Test test_mapping_includes_version_suffix() now tests two different patterns but the name doesn't reflect this.

Recommendation: Consider renaming to test_mapping_includes_valid_version_suffix() or splitting into two tests:

• test_opus_4_6_uses_default_suffix()
• test_other_models_use_date_suffix()

---

3. Frontend model ordering comment inconsistency

In test_model_ordering_consistency(), the comment states:

# Expected ordering: Sonnet → Opus 4.6 → Opus 4.5 → Haiku (matches frontend dropdown)

But the assertions check:

assert expected_order[0] == "claude-sonnet-4-5"  # Balanced (default)
assert expected_order[1] == "claude-opus-4-6"  # Latest Opus
assert expected_order[2] == "claude-opus-4-5"  # Previous Opus
assert expected_order[3] == "claude-haiku-4-5"  # Fastest

This is correct, but the comment "Latest Opus" for 4.6 might be confusing since 4.6 is numerically higher than 4.5. Consider clarifying that this is the latest released Opus model.

Positive Highlights

✅ Comprehensive test coverage: 20 tests covering edge cases (case sensitivity, whitespace, partial matches, round-trip flows, ordering)

✅ Proper test organization: Separated unit tests (TestMapToVertexModel) from integration tests (TestModelMappingIntegration)

✅ Excellent PR description: Clear summary, context about why the mapping is needed, and explicit test plan

✅ Frontend consistency: Removed Opus 4.1 from both the dropdown and the backend mapping, maintaining consistency across the stack

✅ Version suffix documentation: Tests include inline comments explaining the difference between @default and @YYYYMMDD suffixes

✅ Fix for pre-existing test failures: Corrected invalid test expectations for None and numeric input handling

Recommendations

Priority 1 (Address before merge)

1. Add explicit type validation to map_to_vertex_model() (see Major Issue Epic: RAT Architecture & Design #2 above) OR document in comments why permissive dict.get() behavior is intentional

Priority 2 (Nice to have)

2. Consider separating formatting changes: In future PRs, run black and isort before starting work to avoid mixing formatting with functional changes

3. Extract version suffix validation helper: Improves test readability and reusability

4. Add inline comment explaining @default suffix: In VERTEX_MODEL_MAP, add a comment explaining why Opus 4.6 uses @default instead of a date (e.g., # Opus 4.6 uses @default - no date suffix per Vertex AI docs)

Priority 3 (Future improvements)

5. Consider adding a CI check: Verify that all frontend model values have corresponding backend mappings (catches drift automatically)

6. Document Vertex AI model versioning: Add a brief comment in auth.py explaining the two suffix patterns and when each is used

---

Architecture Alignment

✅ Follows project patterns: Changes align with established Python standards (black formatting, type hints)

✅ Test-driven: Comprehensive test suite validates all mappings and edge cases

✅ Configuration centralization: Model mappings are centralized in VERTEX_MODEL_MAP constant

⚠️ Type safety: The # type: ignore comments in tests indicate a potential type system bypass that should be addressed

---

Overall Assessment: This is a solid PR that correctly implements Opus 4.6 support for Vertex AI. The main concerns are around type safety and the mixing of formatting changes with functional changes. The test coverage is excellent and gives high confidence in the implementation.

Recommendation: ✅ Approve with minor changes requested (address type validation in map_to_vertex_model())

🤖 Generated by Claude Code Review

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.