Repository navigation
feat(pi): retain token history for unsupported backends - #4628
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 8467a1a720
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (fallback?.isUnsupportedBackend == true ? fallback?.providerRawValue : nil), | ||
| self.mappedProvider(fromPiProvider: backend) == nil | ||
| { | ||
| let model = explicitModelText ?? (fallback?.providerRawValue == backend ? fallback?.modelName : nil) |
There was a problem hiding this comment.
When an unsupported backend is repeated with different casing between a model_change record and an assistant message (for example, OpenRouter followed by openrouter) and the message omits its model, this exact comparison refuses to reuse the otherwise matching model context. The row is then treated as incomplete and its valid tokens are discarded, even though provider classification elsewhere is case-insensitive; normalize both backend IDs before comparing them.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
clawsweeper:intake:feature
|
Codex review: blocked before merge. What this changesRetain valid unsupported-backend Pi/OMP usage as unpriced standalone Pi history. Example: A timestamped OpenRouter assistant row records 30 input tokens using gpt-5.4.
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 2 items remain Keep this owner-authored PR open: the feature is useful and approved in direction, but backend-context matching still drops valid tokens when casing differs. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe Pi scanner reads local Pi/OMP JSONL transcripts, resolves assistant identity and usage, persists a shared scan cache, and produces standalone Pi and native-provider report partitions. flowchart TD
A[Pi and OMP transcripts] --> B[Assistant identity and usage parser]
B --> C[Pi scan cache]
C --> D[Standalone Pi report]
C --> E[Native Claude and Codex partitions]
D --> F[CLI dashboards and widgets]
E --> F
Technical reviewBest possible solution: Use consistent backend identity matching within the existing resolver while preserving standalone-only unpriced accounting and native-provider isolation. Do we have a high-confidence way to reproduce the issue? Current-main source and its existing reliability fixture establish the omission; source inspection also confirms the remaining mixed-case context failure, without executing target code. Is this the best way to solve the issue? The standalone-only fallback is a focused extension of existing unpriced accounting, with no competing scanner or new setting. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 7e31e74ea662. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 19, 2026, 12:15 AM ET / 04:15 UTC. |
# Conflicts: # CHANGELOG.md
1 participant
Summary
Retain valid timestamped assistant usage from unsupported Pi/OMP backends as unpriced tokens in standalone Pi history. Keep backend/model labels and incremental model context in the existing cache. These rows never enter native Claude/Codex history or borrow another provider's pricing.
Malformed or undated usage still leaves standalone history incomplete without poisoning native partitions. Bump the pricing-semantics revision once so previously ignored rows are rescanned; no new storage or settings.
Verification
The main baseline failed the unsupported-backend totals and cached model-context regressions. Both pass in the combined candidate, together with malformed/undated usage and native-partition isolation checks.
Local proof used a combined lane candidate; this PR is the independently split change, and exact-head PR CI is the broad gate. All data was synthetic. No running app, real account, Keychain, or browser-cookie probe was used.
swift test --build-system native --jobs 2 -Xswiftc -gnone --filter 'CostUsageClaude(ArtifactWriter|CacheIsolation|Fragment|KimiAlias|MetadataParsing|Overflow|PriceRange|PricingRace|Rebuild|ReconciliationMemory|ReportContext|ReportWire|Resolver|RowCapacity|RowStorage|VertexClassifier|VertexPrecheck|WriteAmplification)Tests|CostUsageScanner(Claude.*)?Tests|PiSession.*Tests|PiBedrockPricingTests|PiNativeProjectionTests|PiInclusiveRefreshTests|PiSharedRootMergeTests|PiProviderTests|PiProjectOverflowTests|UsageStorePiHistoryScopeTests|OpenCodeGo.*Tests|CLICost.*Tests|CLICommonOptionsTests|CLIProviderSelectionTests|CostReportingPeriodTests|ProviderArchitectureGatekeeperTests|ProviderSettingsDescriptorTests|BrowserCookiePromptContractTests|SettingsStoreTokenCostSourceTests|SettingsStoreCoverageTests|SettingsStoreAdditionalTests|SpendDashboardModelTests', under the sharedserial.shlock withScripts/test_environment.sh.--filter 'CLICost.*Tests|ProviderArchitectureGatekeeperTests|CLIProviderSelectionTests|CLICommonOptionsTests|OpenCodeGoLocalUsageReaderTests': ✔ Test run with 144 tests in 9 suites passed after 21.085 seconds..swift test --build-system native --jobs 2 -Xswiftc -gnone --filter 'CostUsageScannerClaudeDesktopTests|CostUsageClaudeReportContextTests|CostUsageScannerClaudeMemoTests|CostUsageScannerClaudeRegressionTests|CostUsageClaudeCacheIsolationTests|PiSession.*Tests|PiBedrockPricingTests|PiNativeProjectionTests|PiInclusiveRefreshTests|PiSharedRootMergeTests|PiProviderTests|PiProjectOverflowTests|UsageStorePiHistoryScopeTests|ProviderArchitectureGatekeeperTests|CLICost.*Tests|CLIProviderSelectionTests|CLICommonOptionsTests|OpenCodeGo.*Tests|ProviderSettingsDescriptorTests|BrowserCookiePromptContractTests|CostUsageClaudeFragmentTests': ✔ Test run with 573 tests in 53 suites passed after 43.853 seconds..make check: passed.python3 /Users/steipete/Projects/CodexBar-triage-evidence/20260921/precheck-catalogs.py .: no findings before push.2 files changed, 64 insertions(+), 62 deletions(-)(net +2); Tests:2 files changed, 64 insertions(+), 8 deletions(-).Fixes #4605