Repository navigation
Add opt-in OpenCode local history for the ambient Codex ledger - #4574
Danielweng0902 wants to merge 9 commits into
Conversation
Add a default-off Codex source that merges machine-local OpenCode usage for openai-provider assistant records into the ambient Codex menu/CLI history (steipete#3273): - OpenCodeLocalUsageReader reads usage metadata only, read-only, from the database OpenCode itself resolves (OPENCODE_DB, channel db, default), across the v1 message/part tables with step-finish precedence and v2 session_message rows deduplicated by stable message ID. Recorded cost is surfaced as-is; $0 subscription cost is never reinterpreted as spend. auth.json and message content are never read. - The source is permanently unscoped: it merges only into the ambient Codex ledger (foreground scan and cached catch-up publication) and is excluded from scoped CODEX_HOME scans, managed accounts, and the Spend Dashboard, mirroring the ambient Pi merge precedent. - Settings toggle 'Include OpenCode local history (OpenAI)' gates the merge and participates in the snapshot scope signature; codexbar cost and the serve cost endpoint honor the same setting. Reader and gating tests cover mixed-provider selection, step-finish precedence, v1/v2 dedup, idle/active WAL reads, path resolution, scoped-home exclusion, and cached-publication gating.
|
🦞👀 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. |
clawsweeper
Bot
added
clawsweeper:intake:feature
|
Codex review: needs real behavior proof before merge. What this changesAdds a default-off setting that merges labelled OpenCode OpenAI usage into ambient Codex history. Example: OpenCode records 1,545 OpenAI tokens today with a recorded cost of $0.
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 3 items remain Keep open: this PR adds useful missing coverage, but the menu activation repair is incomplete and invalid legacy records can hide valid migrated usage. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe local-history reader extracts usage metadata from OpenCode SQLite and feeds Codex cost aggregation, which publishes menu and CLI history while excluding scoped accounts and the Spend Dashboard. flowchart TD
A[OpenCode SQLite] --> B[Read only usage reader]
C[Default off setting] --> D[Ambient scope gate]
B --> D
D --> E[Codex history aggregation]
F[Native Codex history] --> E
E --> G[Menu and CLI]
Technical reviewBest possible solution: Complete ambient menu visibility and deduplicate against usable legacy usage while preserving recorded costs and account isolation. Do we have a high-confidence way to reproduce the issue? Source inspection establishes both failures: OpenCode-only activation leaves both menu display predicates false, and a negative-cost legacy row suppresses a valid same-ID v2 row; no target code was executed. Is this the best way to solve the issue? The opt-in ambient source follows the established cross-client boundary, but activation must reach the actual menu presentation predicates and deduplication must preserve valid replacement records. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 0ff1cf55c0a2. Provenance checked
TestingProof path: shipped entry point. 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 HistoryReview history (4 earlier review cycles)
Reviewed October 16, 2026, 12:03 PM ET / 16:03 UTC (Revision 5). |
- Normalize the reader's lower bound to the reporting day's start so earlier-today turns survive the now-anchored foreground window. - Key day buckets with the supplied reporting calendar instead of the device calendar. - Resolve the database root through XDG_DATA_HOME (absolute values only, matching xdg-basedir) before falling back to ~/.local/share. - Exclude the merge whenever CODEX_HOME selects a Codex home, mirroring the codexHomePath argument at both merge points. - Label merged models "(OpenCode local)" so the source stays visible in UI breakdowns and CLI JSON. - Drop the unused scope fingerprint machinery and its low-value test; cover the new boundaries with behavior tests instead.
|
@clawsweeper Thanks for the thorough review — all four boundary findings were real, and the provenance concern was the right call. Pushed in 8065b8c ("Harden OpenCode local history merge per review"), point by point: 1. Time boundary normalization — 2. Reporting-calendar day keys — day keys now come from 3. 4. 5. Visible provenance — adopted your recommendation: merged model names carry an 6. Scope-fingerprint test — removed. On reflection the whole scope/fingerprint plumbing was write-only output (nothing consumed it), so the machinery is gone too; Real-behavior proofBuilt Default (setting off): Setting on, ambient home: Under Setting on, Zero rows — empty native home and the machine-wide OpenCode merge excluded, exactly the promised scoped-home behavior. Setting on, Tests: reader (16) + merge (8) suites pass, plus |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:feature
Review follow-up for the recorded-cost contract: - Grouped accounting: OpenCode rows join the unattributed project bucket (the Pi treatment) in both the foreground scan and the cached publication; session text views stay native with an explicit warning, matching the Pi session-view precedent. - Recorded-cost provenance: merged rows mark their entries, snapshots report sourceRecorded or mixed instead of a pure list-price claim, and the CLI title/hint follow. Spend Dashboard text maps the new case conservatively; it never receives these rows. - Database selection: OPENCODE_DB selects exactly one database (:memory: reads as no history, never a substitute) and the release database is the default without channel fallbacks, matching upstream's contract. OPENCODE_UPDATE_CHANNEL is no longer consulted. - Reader hygiene: close the SQLite handle when sqlite3_open_v2 fails. - Tests: pinned-calendar regression for the earlier-today bound, selection-contract coverage, mixed-history provenance and grouped accounting, cached-publication parity, and legacy-entry decoding. - Docs: codex.md and opencode.md now describe the opt-in toggle and its ambient-only boundary instead of denying the capability.
|
@clawsweeper Thanks for the deeper pass — all eight items are addressed in a4294d9 ("Harden OpenCode local merge boundaries and provenance"), point by point: 1. Grouped accounting boundary (P2) — both merge sites now apply the established Pi treatment: 2. Recorded-cost provenance (P2) — merged entries now carry a 3. Database selection contract (P2) — rewrote 4. Pinned calendar in the earlier-today test (P3) — you were right, the fixed timestamp was 2026-03-06 12:00 UTC, not the local afternoon the comment claimed (it only passed in UTC+8 hosts). The test now pins a UTC calendar and constructs both instants from 5. SQLite handle on open failure (P3) — the failure branch closes the handle before throwing, as the Go reader does. 6. Contradictory provider guides (P3) — 7. Data-model compatibility proof — 8. Persisted-setting fresh-install/upgrade proof — terminal verification below: toggling the key on/off flips the merge both directions while an unrelated preference in the same domain survives. Real-behavior proof (branch-built CLI, synthetic DB with 1,545 + 800 token
|
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:feature
Third review round: - Capture the SQLite failure (which reads errcode/errmsg through the handle) before closing it on open failures, avoiding use-after-free. - Encode costIsSourceRecorded so recorded entries round-trip through caches; the legacy-decoding test now also round-trips a true entry. - Pass provenance through the default CLI cost footer and the menu usage hints, so recorded OpenCode rows are never described as computed list-price estimates in any view.
|
@clawsweeper Third round addressed in 3655259 ("Finish provenance disclosure and harden failure paths") — thanks again, all three P2s were real: 1. Capture the SQLite error before closing the connection (P2) — correct catch: the failure now materializes first ( 2. Encode the recorded-cost marker (P2) — 3. Provenance-aware disclosure in the default views (P2) — the default Upgrade proof (latest-stable → branch, existing settings and history)The branch-built CLI running against this machine's real installed-app state — existing The shared Tests: reader (17) + merge (10), gatekeeper, CLI cost, fetcher/scanner/PiNative/calendar (105), menu/dashboard (80), and catch-up/settings suites all pass locally. |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:feature
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
![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: 869f746844
ℹ️ 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".
| tokenCostUsageEnabled: self.settings.isHistoryLaneEffectivelyEnabled(for: provider), | ||
| tokenCostIsRefreshing: !isSettings && self.tokenCostRefreshIsActive(for: provider), | ||
| codexLocalSessionCostLedgerEnabled: self.settings.codexLocalSessionCostLedgerEnabled, | ||
| // Settings exposes available costs regardless of the menu's display style. | ||
| costSummaryInlineEnabled: isSettings || self.settings.costSummaryShowsInline(for: provider), |
There was a problem hiding this comment.
When both older cost toggles are off and a user has retained the inlineSummary display style (for example, by selecting Inline and later turning cost tracking Off), enabling OpenCode makes tokenCostUsageEnabled true here, but costSummaryShowsInline still returns false because it checks only those older toggles, while .inlineSummary also disables the submenu. The still-unwired inline path is fresh evidence beyond the earlier lane-activation fix: the scan runs but neither cost presentation renders, so the opt-in appears to do nothing; the inline gate should use the new history-lane effective state too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified real and fixed in 7d573f9 ("Complete the inline lane and ground mixed provenance on real cost"):
- Inline lane:
costSummaryShowsInlinenow honors the OpenCode opt-in in the same codex branch as the ledger, so the inline summary — the only presentation under.inlineSummary, which disables the submenu — renders from the opt-in alone. New testinline summary style renders from the opencode opt-in alonepins the exact scenario (style retained, both older toggles off): inline enabled, submenu correctly off per the style,menuCardInput(.codex, .menu).costSummaryInlineEnabledtrue, Claude untouched. - Mixed provenance: both predicates (scan + cached path at 1572) now require a native row with an actual
costUSDrather than merely any native row, matching the existingcursorCostProvenanceidiom. New testunpriced native history keeps provenance source recordedpins it: unknown-model native session (tokens only) + recorded OpenCode cost → tokens still merge but provenance stays.sourceRecorded; the existing priced-native.mixedtest passes unchanged.
Gatekeeper and related suites pass locally (the two parallel-load-flaky dashboard timing tests fail identically on the base commit and pass in isolation).
| calendar: options.scanOptions.calendar) | ||
| } | ||
| var openCodeLocalReport: CostUsageDailyReport? | ||
| let listPriceHistoryPresent = !daily.data.isEmpty |
There was a problem hiding this comment.
When native Codex/Pi history is present but fully unpriced (for example, an unknown model leaves every native costUSD nil) and OpenCode contributes the only recorded cost, this nonempty-data check still selects .mixed. The menu and CLI then claim an API-price estimate is included even though no list-price amount exists; the cached path repeats the same predicate at line 1572. Both paths should test for an actual native numeric cost rather than merely any native row.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7d573f9 — the provenance predicate on this line (and its cached-path twin at 1572) now tests for a native row with an actual costUSD instead of any native row, mirroring the existing cursorCostProvenance idiom. unpriced native history keeps provenance source recorded covers the exact scenario: unknown-model native tokens + recorded OpenCode cost stays .sourceRecorded; priced native history still yields .mixed.
Both follow the Codex review of 869f746: costSummaryShowsInline still consulted only the older toggles, so a user retaining the inlineSummary style (which disables the submenu) saw no cost presentation at all from the OpenCode opt-in. The codex branch now honors the opt-in like the ledger, so the inline summary renders on its own. The mixed-provenance predicates tested for any native row, so fully unpriced native history (unknown model) plus recorded OpenCode cost claimed a list-price estimate that was not included. Both the scan and cached paths now require a native row with an actual cost, matching the existing cursorCostProvenance idiom. Adds the inline-style and unpriced-native behavior tests.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d573f97ff
ℹ️ 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".
| self.isCostUsageEffectivelyEnabled(for: provider) || | ||
| (provider == .codex && self.codexIncludeOpenCodeLocalHistory) |
There was a problem hiding this comment.
When a managed-account or profile-home Codex source is active while both older cost toggles are off, this unconditional setting check enables history refreshes, but UsageStore.tokenCostScope still returns that scoped home and shouldIncludeOpenCodeLocalHistoryInTokenSnapshot consequently returns false. The fresh scoped-source path was not addressed by the earlier ambient-lane fix: enabling “Include OpenCode…” scans the scoped native ledger and never publishes the OpenCode-only account-agnostic section, so the opt-in appears ineffective; make this opt-in select the codex:ambient scope (or keep the lane disabled unless that scope is used).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified real and fixed in ad31248 ("Select the ambient scope for the OpenCode lane and split window cost kinds"):
The opt-in now selects the codex:ambient scope in tokenCostScope, exactly mirroring the line four rows above where codexLocalSessionCostLedgerEnabled has always forced ambient — machine-local history reads the machine-local home. With a managed source active, shouldIncludeOpenCodeLocalHistoryInTokenSnapshot now opens (ambient home → nil path → setting) instead of silently scanning a scoped ledger the merge can never touch. New test opencode opt-in selects the ambient scope over the managed account home pins: default-off keeps the managed home and closed gate; opt-in alone switches to codex:ambient and opens the merge.
| .unknown | ||
| } | ||
| if includesMetered { return hasWindowCosts ? .mixed : .vendorMetered } | ||
| if includesSourceRecorded { return hasWindowCosts ? .mixed : .sourceRecorded } |
There was a problem hiding this comment.
When a mixed 30-day snapshot has list-priced native cost only on an older day and today's window has only an OpenCode row, hasWindowCosts is true because of the OpenCode cost itself and includesSourceRecorded is also true, so this branch returns .mixed. The fresh narrow-window path remains after the snapshot-level predicate fix: summary(forLastDays: 1) and CLI JSON claim a list-price estimate is present when the selected window contains only source-recorded cost; track an independent list-priced-cost bit for the selected entries before returning .mixed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified real and fixed in ad31248 — window summaries now carry an independent list-priced bit:
forWindowgainedhasListPricedCosts(optional, legacy callers keep the old approximation) and the.mixed/sourceRecordedbranch uses it instead of the any-cost bit.summary(forLastDays:)passesentries.contains { $0.containsListPricedCost || ($0.costUSD != nil && !$0.costIsSourceRecorded) }.- Because a merged day folds recorded and priced rows into one entry whose recorded marker only says some cost is recorded,
CostUsageDailyReport.Entrynow persistscontainsListPricedCost— set only for genuinely mixed days so single-report merges stay identity-preserving (CostUsageDailyReportMergeTestsunchanged) and older caches decode as false.
New test narrow window of a mixed snapshot stays source recorded pins the exact scenario: priced native row two days older + OpenCode row today → snapshot .mixed, 7-day window .mixed, 1-day window .sourceRecorded.
…kinds Both follow the Codex review of 7d573f9: The OpenCode opt-in enabled the history lane but left a managed-account or profile-home source selecting its scoped home, so the merge gate stayed closed and the toggle appeared ineffective. The opt-in now selects the codex:ambient scope exactly like the local ledger always has — machine-local history reads the machine-local home. forWindow treated any window cost as a list-price cost, so a mixed snapshot's narrow window that held only an OpenCode row still claimed a mixed estimate. Window summaries now carry a list-priced bit, and merged days persist a containsListPricedCost flag for the one case the recorded marker alone mislabels — a day mixing recorded and priced cost. Pure days keep the flag off so single-report merges stay identity-preserving. Adds the ambient-scope and narrow-window behavior tests.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad31248d95
ℹ️ 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".
| // ambient history, so either selects the ambient home like the ledger always has; | ||
| // without this the opt-in would scan a managed home whose scoped rows can never | ||
| // include OpenCode history, leaving the toggle ineffective (#3273 contract). | ||
| if self.settings.codexLocalSessionCostLedgerEnabled || self.settings.codexIncludeOpenCodeLocalHistory { |
There was a problem hiding this comment.
When the OpenCode opt-in is on, this branch makes the cost scope account-agnostic, but accountScopedTokenSnapshot still bypasses provider-config matching only for codexLocalSessionCostLedgerEnabled. Switching the active managed/profile account increments the Codex config revision while this scope stays codex:ambient, so the existing valid publication is rejected and the This Mac cost section and submenu disappear until a potentially long rescan completes. Accept an ambient publication when its scope signature is still current, without accepting a pre-toggle scoped snapshot.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified real and fixed in 29eeef6 ("Keep ambient history across account switches and label it everywhere"):
accountScopedTokenSnapshot now has an explicit opt-in branch: an ambient publication is accepted across provider-config churn (account switches) as long as its scope signature still matches the current one — ambient history is account-agnostic, so rejecting it only made the This Mac section vanish behind a rescan. A pre-toggle scoped publication is rejected by that same signature check, so account rows never leak into the machine-local lane. The ledger branch keeps its existing behavior untouched.
New test ambient opencode publication survives codex account switches pins all three: ambient publication survives a config-revision bump; a publication installed under a managed home returns nil once the opt-in selects ambient.
| "Merges machine-local OpenCode usage for OpenAI-provider assistant records from ~/.local/share/opencode/opencode.db into this Mac's ambient Codex history.", | ||
| "Reads usage metadata only (read-only SQLite: model, tokens, recorded cost); prompts, responses and auth state are never read.", | ||
| "OpenCode records $0 cost for subscription usage; recorded dollars are shown as-is and never treated as billing.", | ||
| "Rows are attributed to the model only, never to an account, and are excluded from managed accounts, scoped CODEX_HOME scans and the Spend Dashboard.", |
There was a problem hiding this comment.
When this opt-in is the only local toggle and a Codex account overview is present, the selected cost scope is codex:ambient, yet PreferencesProviderDetailView chooses its heading and footer only from codexLocalSessionCostLedgerEnabled; it therefore labels these machine-wide rows as “Local usage comes from the current Codex profile.” That contradicts this never-account-attributed contract and can make recorded costs look owned by the selected managed/profile account, so the label should use the effective ambient scope.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 29eeef6 — PreferencesProviderDetailView now picks the "This Mac" heading and "shared across accounts" footer from codexLocalSessionCostLedgerEnabled || codexIncludeOpenCodeLocalHistory, so machine-wide rows are never labeled as the selected profile's while the opt-in selects the ambient scope.
| /// Window mixes list-price rows with vendor-metered rows. | ||
| /// Cost recorded by the source client (for example OpenCode's local history) and | ||
| /// surfaced as-is; no list-price estimation was applied to these rows. | ||
| case sourceRecorded |
There was a problem hiding this comment.
When a .sourceRecorded snapshot is opened through the Cost submenu, CostHistoryChartMenuView receives the daily amounts but not snapshot.costProvenance, so it still renders an “Est. total” and the static Codex “Estimated from token usage” disclaimer. This presents OpenCode's recorded dollars as list-price estimates despite the new provenance case; pass provenance through appendCostHistoryChartItem and choose the chart label and disclaimer from it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 29eeef6 — appendCostHistoryChartItem passes tokenSnapshot.costProvenance into CostHistoryChartMenuView:
.sourceRecordedsnapshots render "Recorded total/subtotal (%@): %@" instead of "Est. …" (new keys localized across all 23 catalogs).- The disclaimer reuses the menu-card provenance strings: "Source-recorded cost from OpenCode local history · not a subscription bill" for recorded-only, "Estimated from token usage plus source-recorded OpenCode cost · …" for mixed; every other case keeps the existing estimate disclaimer.
Gatekeeper (marker-justified), localization catalog parity, and the related suites all pass locally.
All three follow the Codex review of ad31248: accountScopedTokenSnapshot bypassed config matching only for the local ledger, so with the OpenCode opt-in active an account switch rejected the still-valid ambient publication and the This Mac section vanished behind a rescan. The opt-in now accepts an ambient publication across config-revision churn while a pre-toggle scoped publication is still rejected by scope signature. PreferencesProviderDetailView picked its This Mac heading and footer only from the ledger toggle, mislabeling machine-wide rows as the current Codex profile's; it now follows the effective ambient scope. CostHistoryChartMenuView ignored snapshot provenance, so a source-recorded snapshot rendered an Est. total and the static estimate disclaimer. The chart now receives costProvenance and picks the Recorded total label and the source-recorded disclaimer, with the new keys localized across all 23 catalogs. Adds the ambient-publication survival test.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Thanks @Danielweng0902 for the opt-in OpenCode history work and the account-scope coverage, and for iterating on it so many times. We're going to close this implementation, though, for a structural reason rather than size: the separate reader loads all matching history before windowing, and cached Codex publication runs that scan again, which breaks the memory and I/O budgets we recently tightened for the cost ledger. We tried a compact version on top of the shared reader, but two histories sharing Go's 4 MiB cache evict each other and force rescans of unchanged data. The right shape is the shared incremental history path (bounded memory, cache-only publication) with a single off-by-default setting on top. If you'd like to take that on, we'd welcome it; #3273 remains the tracking issue, and labelled list-price estimates are still a valid direction there. |
2 participants
Add opt-in OpenCode local history for the ambient Codex ledger
Closes #3273 (reopened per maintainer invitation: "A PR adding an OpenCode SQLite reader … would be welcome").
What this adds
An opt-in, default-off Codex setting — "Include OpenCode local history (OpenAI)" — that merges machine-local OpenCode usage for assistant records whose provider is exactly
openaiinto this Mac's ambient Codex menu/CLI history, so Codex usage generated through OpenCode finally appears in CodexBar's Codex totals (the original reporter's workflow).It follows the bounded contract worked out in #3273:
SQLITE_OPEN_READONLYwith the same WAL/immutable fallback the OpenCode Go reader uses. Prompts, responses, tool output, andauth.jsonare never read; rows are never attributed to an account or workspace.providerID == 'openai'assistant records (v1message$.providerID; v2session_message$.model.providerID). Other providers in the same database are never selected.step-finishparts win; message-level totals contribute only when a message has no valid parts — mirroring the established OpenCode Go reader behavior.messagerows intosession_messageunder the same IDs (on my database: 4,064 v1 / 4,552 v2 rows, 3,927 shared IDs). v2 rows only contribute IDs the legacy table does not already hold (NOT EXISTS). No fuzzy timestamp/token matching across sources.$0for subscription usage, and that$0is never reinterpreted as spend or a billing receipt. No pricing is computed for these rows.(OpenCode local)suffix (e.g.gpt-5.1-codex (OpenCode local)), entries carry a recorded-cost marker, and snapshots reportsourceRecorded/mixedprovenance instead of a pure list-price claim — CLI titles, hints, and JSON follow, so recorded costs never hide under an estimate label.CODEX_HOMEscans (whether selected through thecodexHomePathargument or theCODEX_HOMEenvironment variable), managed accounts, profile homes, and the Spend Dashboard always exclude it. Historical rows can never be assigned to a Codex account from today's credentials.OPENCODE_DBselects exactly one database —:memory:reads as no on-disk history, never a substitute — and without an override the release database under the XDG data root is the selection (absoluteXDG_DATA_HOMEhonored, relative ignored per the XDG spec), matching upstreamdatabase.path()'s single-path contract.OPENCODE_UPDATE_CHANNELis not consulted; upstream derives the channel from a build constant.Implementation
OpenCodeLocalUsageReader(Sources/CodexBarCore/Providers/OpenCode/): self-contained reader modeled onOpenCodeGoLocalUsageReader. Resolves the data root the way OpenCode'sxdg-basedirdependency does, then the single database per upstream's selection contract, reads both table generations, and emits a mergedCostUsageDailyReportwhose entries are markedcostIsSourceRecorded. Day buckets are keyed with the supplied reporting calendar, and the window's lower bound is normalized to that calendar's start-of-day so earlier-today turns survive thenow-anchored foreground window. A missing database or one with no matching rows reads asnil, not an error; SQLite handles are closed even whensqlite3_open_v2fails.CostUsageFetcher): the Pi-precedent ambient merge, in both the foreground local scan and the cached catch-up publication. Both sites gate oncodexHistoryIsAmbient(nocodexHomePathargument, noCODEX_HOMEin the environment the native scanner honors), append the unattributed project breakdown for grouped accounting, clear session breakdowns, and compute the snapshot's cost provenance (mixedwhen estimated rows coexist,sourceRecordedotherwise).CostProvenance): a newsourceRecordedcase with window narrowing that keeps mixed windows mixed.CostUsageDailyReport.Entrygains acostIsSourceRecordedmarker that persists through caches (older caches decode it asfalse) and ORs across same-day merges.--group-by sessionruns a native-only load with an explicit omission warning (the Pi session-view precedent), while--group-by projectand JSON keep the merged rows.shouldIncludeOpenCodeLocalHistoryInTokenSnapshot(Codex + ambient home + setting), threaded through refresh, hydration, catch-up,codexbar cost, and the serve cost endpoint. The Spend Dashboard path passesfalseexplicitly. The setting participates in the token snapshot scope signature and menu-refresh readiness key, so toggling it triggers a refresh.codexIncludeOpenCodeLocalHistory(UserDefaults, default false) with a Codex settings toggle whose subtitle states the privacy and estimate contract.Tests
OpenCodeLocalUsageReaderTests(17 tests): exactopenaiselection among mixed providers, step-finish precedence without double counting, v2 session messages adding new turns without double counting migrated IDs, v2-only databases, recorded-zero-cost token history, malformed/legacy row skipping, negative/overflowing token counts, window filtering, clock-time lower bound normalizing to the reporting day's start (pinned UTC calendar), day keys following the supplied reporting calendar,XDG_DATA_HOMErelocation (absolute honored, relative ignored), the upstream single-database selection contract (:memory:unavailable, no substitution after an explicit selection, no channel fallbacks), missing/empty database → nil, idle WAL without sidecars, active WAL frames, andOPENCODE_DBpath resolution.OpenCodeLocalHistoryMergeTests(10 tests): foreground ambient merge on/off with visible provenance labels, mixed-history grouped accounting (native project + unattributed bucket + empty sessions +mixedwindow provenance), scopedcodexHomePathexclusion,CODEX_HOME-environment exclusion (foreground and cached publication), Claude scans never merging, cached publication merge on/off with provenance parity, and legacy-entry decoding (pre-feature caches keep loading,costIsSourceRecordeddecodes asfalse) plus a recorded-entry encode/decode round trip.Verified against a real OpenCode v2.0.29 database (both table generations present with real migration duplicates): the composed SQL selects exactly the expected rows and deduplicates shared IDs correctly.
Real-behavior verification (CLI)
Built
CodexBarCLIfrom this branch and ran it against a synthetic OpenCode database (twoopenaiturns: 1,545 tokens today 10:00, 800 tokens yesterday 08:00, both recorded$0, plus aclaude-opusrow that must never appear) on a machine with real native Codex usage:OPENCODE_DB=<synthetic>OPENCODE_DB=<synthetic>gpt-5.1-codex (OpenCode local)breakdown rows;--group-by projectshows the unattributed bucket (2,345 tokens); JSON reportsprovenance: mixed; title reads "API-equivalent estimate + source-recorded cost"OPENCODE_DB=<synthetic> CODEX_HOME=<empty dir>OPENCODE_DB=<synthetic> --group-by sessionXDG_DATA_HOME=<dir holding the db>OPENCODE_DB); a relativeXDG_DATA_HOMEis ignored~/.codexbarcaches, and native historyThe earlier-today turn (10:00) appears under a
--days 1window anchored atnowbecause the reader normalizes the bound to the reporting day's start; the yesterday-morning turn lands on its own calendar day under--days 2. Legacy-entry decoding and cached-publication parity are covered by tests. Full terminal transcripts are in the review-response comments.Notes / open questions for review
auth.json(the Go reader uses it for not-detected detection). Presence of the database itself is the availability signal.session_messagefix for the OpenCode Go reader are SQL-shape only; that PR fixes the Go provider's reader, this one adds a new ambient Codex source. Happy to rebase if that lands first.Checklist
ProviderArchitectureGatekeeperTestspassing (new clusters carry// Provider-specific by design:reasons)docs/providers.md,docs/codex.md,docs/opencode.md,CHANGELOG.md)