Fix Claude vendor pricing after #3002 - #3012
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Bare DeepSeek IDs now invalidate Pi pricing caches and trigger models.dev refresh on the same vendor list used for lookup. Co-authored-by: Cursor <cursoragent@cursor.com>
The pricing-file change left CodexParserHash.generated.swift stale and failed lint. Co-authored-by: Cursor <cursoragent@cursor.com>
…back. Google is now a Claude first-party pricing vendor, so the unrelated-catalog case uses Groq instead. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cde7312168
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private static func claudeModelsDevModelIDs(for rawModel: String) -> [String] { | ||
| let normalized = self.normalizeClaudeModel(rawModel) | ||
| return normalized == rawModel ? [rawModel] : [rawModel, normalized] |
There was a problem hiding this comment.
When a Claude transcript contains the recognizable bare OpenAI alias gpt-5.6, this helper applies only normalizeClaudeModel, so it searches the OpenAI catalog for gpt-5.6 but not the canonical gpt-5.6-sol. A catalog containing only the canonical entry is already a supported case in CostUsagePricingTests, and this new Claude routing path consequently leaves that usage unpriced even though the equivalent Codex lookup resolves the alias. Use the owning vendor's normalization when constructing targets for recognized non-Claude families.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs changes before merge. Reviewed August 17, 2026, 11:50 AM ET / 15:50 UTC. ClawSweeper reviewWhat this changesThe PR routes bare Claude-session model IDs to first-party models.dev catalogs, preserves explicit provider routes, refreshes matching pricing caches, and scopes custom Codex session roots to their own trace database. Merge readinessKeep this owner-authored PR open. The Claude vendor-routing work is still needed, but the new OpenAI route omits existing alias normalization and can leave valid Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local Claude and Codex transcripts, maps model IDs to models.dev vendor catalogs, then displays estimated spend. This PR changes the Claude mapping and cache-refresh inputs while keeping routed models from borrowing another vendor’s price. flowchart LR
A[Local Claude transcript] --> B[Model ID routing]
B --> C[First-party vendor catalog]
C --> D[models.dev price lookup]
D --> E[Estimated spend]
A --> F[Unknown-price refresh]
F --> C
G[Scoped Codex session root] --> H[Matching trace database]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Reuse the OpenAI-specific normalization when creating lookup targets for recognized OpenAI Claude routes, and add a regression test using a canonical-only catalog while preserving explicit-route isolation. Do we have a high-confidence way to reproduce the issue? Yes, by using the established canonical-only OpenAI catalog fixture with a Claude-session Is this the best way to solve the issue? No; the branch should apply the owning OpenAI vendor’s existing normalization before lookup, which is narrower and maintains the established alias contract. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2fe9de487f8a. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
2 participants
Summary
This supersedes #3002 while preserving all contributor commits and credit to @Yuxin-Qiao.
mainRoot causes
The original CI failures came from generated and line-sensitive metadata drifting as the pricing paths changed: the parser fingerprint was stale, gatekeeper anchors shifted, and the Pi unrelated-provider fixture used a catalog that became relevant to Claude pricing. The contributor fixed those on the original head, but merging with #2946 changed the same pricing file again and re-staled the parser fingerprint.
The clean textual merge also hid a semantic conflict: #2946 requires explicit routes and unknown prefixes to stay isolated, while a fixed bare-ID vendor fallback could choose the wrong first-party catalog when names collide. This branch reconciles those rules with deterministic family ownership and fail-closed ambiguity handling.
The full-suite run additionally exposed a pre-existing test isolation bug: custom Codex session roots still scanned the ambient
state_5.sqlite, causing the macOS shard to time out on a large local database. The scoped trace-database fix reduced that suite from 93 seconds to 3.5 seconds.Proof
swiftlint --strict --quietmake checkswift test --filter 'CostUsagePricing|CostUsageFetcher|ProviderArchitectureGatekeeper'swift test --filter PiSessionCostScannerTestsswift test --filter CodexLocalProjectUsageTestsmake test: 883 selections in 74 groups; all passed on the first attemptdeepseek-v4-flashpriced 110 tokens at USD 0.0000168;unknown-route/deepseek-v4-flashremained present and unpricedThis PR does not merge or close #3002.