fix: price Claude Kimi context aliases without crossing providers - #3259
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![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: 70a204242d
ℹ️ 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".
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 28, 2026, 10:13 PM ET / August 29, 2026, 02:13 UTC. ClawSweeper reviewWhat this changesThis PR prices Claude transcript Merge readinessKeep open for maintainer review: this owner-authored PR has no remaining actionable correctness finding, but it intentionally invalidates affected derived Pi/OMP pricing caches and the exact-head macOS test shards are still running. Priority: P2 Review scores
Verification
How this fits togetherClaude local-usage scans produce model-token rows, then provider-scoped pricing maps those rows to catalog prices and cached reports. The fetcher can refresh a missing catalog entry, while Pi/OMP session reports retain derived pricing aggregates separately from native Codex caches. flowchart LR
A[Claude and Pi transcripts] --> B[Local usage scanners]
B --> C[Provider-scoped pricing routes]
C --> D[Models.dev catalog]
D --> E[Cost reports]
E --> F[Cached Pi and OMP reports]
B --> G[Unknown-price refresh]
G --> D
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the provider-scoped alias mapping and explicit one-time derived-cache repricing, then merge once the exact-head macOS test shards confirm the process-inventory change. Do we have a high-confidence way to reproduce the issue? Yes: the new focused tests and source path establish the missing Is this the best way to solve the issue? Yes: appending the canonical Kimi candidate only within Claude’s existing Kimi-scoped targets preserves exact-route precedence and avoids a cross-provider fallback. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8a20919a3e0b. LabelsLabel justifications:
EvidenceWhat 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
HistoryReview history (6 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
Landed as 69df3415adf9 after all checks passed on The fix preserves exact provider routes and recorded model names while resolving the documented Kimi alias. It also uses a canonical price immediately when an earlier provider group downloaded it. Maintainer decision: accept the tested one-time Pi/OMP derived-cache recalculation; raw transcripts remain untouched, and unaffected native Codex cache state is preserved. Validation is complete: full exact-head CI passed both macOS shards, Linux x64/arm64, musl, lint, and the aggregate gate; The isolated actual-CLI before/after preserves the same synthetic transcript's 160 tokens and raw model name while changing its unknown price to the expected synthetic The 0.56.1 Unreleased changelog and pricing documentation are updated. Local |
steipete#3259 moved the Codex parser hash to `d9a91f31d0addc15` and removed the Pi cache's reviewed-predecessor adoption entirely, so this branch no longer carries a Pi gate: `PiSessionCostScanner` and `PiSessionCostCompatibilityTests` are byte-identical to main again, which is the conservative option the maintainer offered on the previous head. - Regenerate `CodexParserHash.value` to `ac4862abcdfe21a8`. - Record main's `d9a91f31d0addc15` in `CostUsageStore.compatiblePredecessorParserHashes` and in its exact-set assertion.
steipete#3259 removed the Pi cache's reviewed-predecessor adoption entirely, so this branch no longer carries a Pi gate: `PiSessionCostScanner` and `PiSessionCostCompatibilityTests` are byte-identical to main again, which is the conservative option the maintainer offered on an earlier head. - Regenerate `CodexParserHash.value` to `7757495e4cc975df`. - Record main's `b77d4ec72e14ea63` in `CostUsageStore.compatiblePredecessorParserHashes` and in its exact-set assertion.
1 participant
Summary
Repair an omission in the existing Claude local-cost path: the documented Kimi Code
k3[1m]alias does not reach the canonicalkimi-for-coding/k3price row. Without recognizing its vendor, a bare alias can also match an unrelated vendor's same-named row.Append the canonical candidate only inside Claude's existing Kimi Code routes, after all exact candidates. Keep transcript model names, Codex lookup, other explicit routes, context variants, and paid Moonshot routing unchanged. Known catalog zero remains distinct from missing pricing.
After grouped unknown-model refreshes, recheck the shared catalog so a canonical price downloaded for a preceding group is used immediately. Preserve the existing retry/TTL guards and one rescan limit; actual provider routing still decides the price.
Preserve compatible native Codex SQLite rows, cursors, and retained reports when the generated parser hash changes. Remove the retired Pi scheduler-only compatibility exception: Pi's Claude costs use the changed pricing path and must be recalculated. Tests retain independent parser/formula/custom-price/catalog invalidation checks. Documentation and the 0.56.1 Unreleased changelog are updated.
Maintainer decision: accept the one-time Pi/OMP report recalculation. These are derived pricing caches, and retaining the predecessor key would preserve incorrect or missing estimates after the pricing change. Raw transcripts are not modified; unaffected native Codex cache state has a separate, tested preservation path.
During full validation, repair a test-runner failure in process inventory: use native macOS PID enumeration and Linux
/procinstead of spawningpson every ownership poll. Preserve required-PID inclusion and existing birth/session cleanup checks, reject incomplete inventories, and cover the observed failure plus native error/growth boundaries. This changes test infrastructure only, not app behavior or test assertions.Related to #2374; thanks @joeVenner for the alias lead. This does not close or replace that PR's broader Kimi/Moonshot Pi-provider and CLI feature work, nor touch its contributor branch or the adjacent pricing-series branches.
Proof
k3[1m]row and 160 tokens with an expected synthetic estimate of0.000385USD andlistPriceEstimateprovenance. No force-refresh was needed. These are deliberately synthetic rates, not a claim about current Kimi prices.CodexBarCLI cost --provider claude --provider-native-only --format json --pretty. Both runs used an empty inherited environment, explicit temporary config/transcript/cache roots, disabled Keychain access, Codex credential-file isolation, and a sandbox denying network access and real-home data reads/writes. No accounts, credentials, browser cookies, or live provider APIs were used. The packaged app was not relaunched.make check: passed, zero violations across 2,038 files; generated parser hash verified.swift test --no-parallel --filter 'CostUsageClaudeKimiAliasTests|CostUsagePricingTests|CostUsageScannerClaudeMemoTests|CostUsageFetcherUnknownModelPricingTests|CostUsageStoreTests|PiSessionCostCompatibilityTests|PiSessionCostScannerTests|ProviderArchitectureGatekeeperTests': all 218 tests in eight suites passed (166.6 seconds). The initial parallel run exceeded five SQLite lock-test deadlines and exposed stale architecture line anchors plus a test-only refresh-interval mismatch; the corrected serial run passed every case without changing product lock behavior or test deadlines.44729e8736c830e4a5ae84e5611f4a7e3530c556: all three alias spellings now pass the first-call refresh regression with exactly one catalog request. Follow-upmake check, blocking review, and repeated isolated CLI proof passed. The final focused rebuild passed all 112 tests in five suites (50.6 seconds), including the corrected architecture anchors. The latest source review of this head confirms no actionable findings remain.44729e8736c830e4a5ae84e5611f4a7e3530c556. Fresh full CI including the harness repair passed every job on1740f8aa71d32cfd3e87ed57248a93e7cadf4cf2: both macOS shards, Linux x64/arm64, musl, lint, and the aggregate gate.CODEXBAR_TEST_SUITE_TIMEOUT=600outer watchdog and passed all first 47 groups without failures or retries, then stopped at group 48 when the runner'spssubprocess exceeded its separate two-second timeout.psregressions failed on the old implementation and passed after the repair. All 73 cleanup tests passed (one platform-specific skip), including real-process lifecycle and unrelated-process protection; follow-upmake checkpassed with zero violations, and Codex autoreview plus independent source review found no actionable defect.1740f8a(800.6 seconds, zero failures/retries/timeouts). This is verified checkpoint-plus-continuation coverage, not a claim that the interruptedmake testinvocation completed. No production timeout or test assertion was relaxed; the fresh exact-head CI matrix also passed in full.The official guide establishes the alias contract; which client versions naturally persist that spelling in transcripts was not measured. No new provider support, paid-region inference, authentication change, or release publication is included.