fix(backend/copilot): normalise SDK analytics fallback model to CLI form - #13018
fix(backend/copilot): normalise SDK analytics fallback model to CLI form#13018majdyz wants to merge 3 commits into
Conversation
Removed COLLABORATOR checks for comments and reviews in the workflow.
The standard-tier subscription path returns sdk_model=None from _resolve_sdk_model_for_request whenever LD's served value matches the config default — which is the common production hit after the LD-flag consolidation (#12917) moved the four copilot-*-model flags into one JSON copilot-model-routing flag. The finally-block fallback effective_model = sdk_model or config.thinking_standard_model then leaked the raw anthropic/claude-sonnet-4-6 config slug into PlatformCostLog.model, breaking historical admin filters keyed on the unprefixed CLI form (model=claude-sonnet-4-6). Route the fallback through _normalize_model_name so the recorded value matches the actual transport — unprefixed under subscription / direct-Anthropic, prefix preserved under OpenRouter — matching what sdk_model already produces and what the cost dashboard / admin filters expect. Soft-fails to the raw value on vendor-rejection ValueError so the row still records, mirroring the resolver's own fallback semantics.
|
Caution Review failedFailed to post review comments WalkthroughThis PR tightens GitHub Actions access controls by removing COLLABORATOR associations from Claude code action workflows, and adds fallback model name normalization logic to the copilot SDK for consistent analytics and cost logging across different transport configurations. ChangesWorkflow Access Control Tightening
Analytics Fallback Model Normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 0 conflict(s), 0 medium risk, 5 low risk (out of 5 PRs with file overlap) Auto-generated on push. Ignores: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #13018 +/- ##
==========================================
+ Coverage 69.56% 69.86% +0.30%
==========================================
Files 2114 2134 +20
Lines 157370 160309 +2939
Branches 16217 16524 +307
==========================================
+ Hits 109467 112002 +2535
- Misses 44688 45011 +323
- Partials 3215 3296 +81
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Why
Admin cost-dashboard filter
model=claude-sonnet-4-6&block_name=copilot:SDKstopped matching new rows around April 28-30 — extending the time range still surfaces the historical entries, so the rows weren't deleted, the column value just changed.Two recent changes combined to flip the recorded model name on standard-tier subscription turns:
copilot-*-modelLD flags into one JSONcopilot-model-routingflag. If the LD JSON now serves a value equal toconfig.thinking_standard_model(the default"anthropic/claude-sonnet-4-6") — which is the typical state right after the migration —_resolve_sdk_model_for_requesthits the subscription early-return and yieldssdk_model=None(service.py:1085-1094).That raw value got written to
PlatformCostLog.model, even though the SDK CLI subprocess on subscription mode talks toapi.anthropic.comvia OAuth and never sees theanthropic/prefix. Historical admin filters keyed on the unprefixed CLI form (model=claude-sonnet-4-6) silently stopped matching.The LD-override branch already routed
resolvedthrough_normalize_model_name(and #12932 made that subscription-aware), so only theNonefallback was missing the same treatment.What
Route the finally-block fallback through
_normalize_model_nameso the cost-logmodelcolumn carries the CLI-form name regardless of which branch producedeffective_model:claude-sonnet-4-6(matches the historical filter and what's actually on the wire)anthropic/claude-sonnet-4-6(unchanged — that's the slug that goes to OpenRouter)Vendor-rejection
ValueError(e.g.moonshotai/...on a non-OpenRouter transport) soft-fails to the raw value so the row still records, mirroring the resolver's own fallback semantics.How
Single-spot fix in
service.pyfinally block (line 4434):Regression coverage in
service_helpers_test.py: three transport scenarios assert the fallback string the cost log would record (subscription strips,direct-Anthropic strips,OpenRouter preserves).Test plan
poetry run pytest backend/copilot/sdk/service_helpers_test.py::TestAnalyticsFallbackModelNormalisation -v— 3 passedpoetry run ruff format+ruff checkclean on touched filesChecklist
Generated by Claude Code