feat: show remaining usage for selected model - #16
Conversation
github-actions
Bot
added
vouch:trusted
|
Review cycle 1 — HIGH: model bucket matching is not exact
Require normalized equality. If aliases are needed, use an explicit tested alias map. Add prefix/suffix collision regression tests. |
|
Review cycle 1 — HIGH: unsupported Codex auth is not gated
Gate this server-authoritatively on ChatGPT subscription auth (and optionally hide immediately in the UI), return |
|
Review cycle 1 — MEDIUM: fallback cache refreshes the apparent observation time
Cache the authoritative observed/read timestamp and source with the payload, preserve them during fallback, and add a clock-controlled transient-failure/staleness test. |
|
Review cycle 1 — MEDIUM: missing legacy limitId is synthesized as shared Codex
Evaluate supplied identities conservatively: only treat an explicitly identified generic Codex bucket as shared; otherwise require an exact selected-model identity or return unavailable. Add a partial/malformed payload regression test. |
|
Review cycle 2 — HIGH: cache can survive an account transition
Associate cache ownership with eligible account identity/generation, clear it on authoritative non-ChatGPT/account transitions, and merge notifications only into an eligible generation. Add ChatGPT seed → API-key transition → later account-read failure coverage. |
|
Review cycle 2 — MEDIUM: sparse window updates discard reset/duration metadata
Deep-merge primary/secondary window fields with defined null semantics for top-level and keyed buckets. Assert duration/reset survive a sparse update. |
|
Review cycle 2 — MEDIUM: older reads can overwrite newer notifications
Serialize mutations or guard full-read replacement with a cache generation captured before the request. Add controlled notification-during-read and inverse-order race tests. |
Final implementation verification
All dev processes and disposable state were torn down. |
![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: b54c58de9f
ℹ️ 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".
| }), | ||
| [WS_METHODS.serverGetCodexUsage]: (input) => | ||
| observeRpcEffect( | ||
| WS_METHODS.serverGetCodexUsage, |
There was a problem hiding this comment.
observeRpcEffect immediately calls requiredScopeForMethod, but RPC_REQUIRED_SCOPE has no serverGetCodexUsage entry. Every indicator poll therefore throws before providerService.getCodexUsage can run, so the new UI can never receive usage data; add the appropriate read scope and corresponding authorization error to the RPC contract.
AGENTS.md reference: AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
| const exact = entries.filter(([key, bucket]) => matchesModel(model, key, bucket)); | ||
| let selected: readonly [string, CodexUsageRawBucket] | null = | ||
| exact.length === 1 ? exact[0]! : null; |
There was a problem hiding this comment.
The protocol defines rateLimitsByLimitId as keyed by metered limit IDs such as codex (packages/effect-codex-app-server/src/_generated/schema.gen.ts:37871-37875), not necessarily by model slugs. For a normal multi-bucket response such as codex plus another metered bucket, no entry exactly matches gpt-5.3-codex, and the generic fallback is explicitly disabled by the competing bucket, causing this function to return null; map the selected model to its metered limit ID before selecting the bucket.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
| const windows = [selected[1].primary, selected[1].secondary] | ||
| .filter((window): window is CodexUsageRawWindow => window !== null && window !== undefined) | ||
| .flatMap((window, index) => { |
There was a problem hiding this comment.
Filtering before flatMap renumbers the surviving windows. When the optional primary window is absent but the secondary window is present without windowDurationMins, the secondary window receives index 0 and is displayed as Session rather than Weekly; retain the original primary/secondary position while filtering invalid windows.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
| }).pipe( | ||
| Effect.provideService(Scope.Scope, usageScope), | ||
| Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, childProcessSpawner), |
There was a problem hiding this comment.
When no Codex session is active—the normal flow before the first turn—this path uses the live makeCodexSessionRuntime, which requires Crypto.Crypto (CodexSessionRuntime.ts:719-725), but only Scope and ChildProcessSpawner are provided here. Runtime construction therefore fails with a missing-service defect before account/read, so usage cannot be fetched until an existing session is available; provide the already captured crypto service as the session-start path does.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
| const limitId = update?.limitId?.trim(); | ||
| if (update && limitId) codexUsageGeneration += 1; | ||
| if (cachedCodexUsage && update && limitId) { |
There was a problem hiding this comment.
The generated protocol permits limitId to be absent or null in account/rateLimits/updated and explicitly describes the notification as a sparse update that must be merged or followed by a refetch (packages/effect-codex-app-server/src/_generated/schema.gen.ts:36398-36406). This guard silently discards every such update, leaving the displayed percentage stale until the next periodic read; when the cached bucket cannot be identified unambiguously, trigger a full account/rateLimits/read instead of ignoring the notification.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
| providerInstanceId: props.providerInstanceId, | ||
| model: props.model, |
There was a problem hiding this comment.
The query identity contains only environment, provider instance, and model, so reauthenticating the same instance from one ChatGPT account to another continues exposing the previous account's successful snapshot. The server invalidates its own cache on account/updated, but nothing invalidates this mounted client atom, and the returned snapshot echoes the same provider/model identifiers, so it remains visible until the 60-second refresh; include an account/provider revision in the query identity or explicitly clear the atom when provider authentication changes.
AGENTS.md reference: AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
Plan: https://html-publisher-production-a236.up.railway.app/p/wFjg3Cn1GOu_JHpcs7V4UH22YDa2qeU1
Local plan:
~/.plans/selected-model-usage-bf14ed2c-e54d-4491-98b7-63c5f8c2ab64.htmlDone criteria
Approach
Reuse targeted, proven pieces from upstream PR pingdotgg#2484, informed by pingdotgg#2155 and pingdotgg#4326. Do not cherry-pick wholesale. Adapt model/bucket resolution so the displayed quota applies only to the currently selected model.