Repository navigation
feat(desktop): show design usage totals by day and week - #460
Merged
Merged
Conversation
github-actions
Bot
added
docs
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Findings
-
[Minor] Renderer imports runtime helper code from the main process —
apps/desktop/src/renderer/src/components/Sidebar.tsx:4pullsformatUsageCost/formatUsageTokensfrom../../../main/usage-budget, andapps/desktop/tsconfig.web.json:3had to addsrc/main/usage-budget.tsto the renderer TS project so it typechecks. The module is pure today, so it bundles, but a renderer →src/main/**runtime import crosses the process boundary and makes it easy to later pull Electron-only main code into the renderer bundle.
Suggested fix: move the shared, pure pieces (UsageTotals,summarizeUsageBudget,formatUsageTokens,formatUsageCost) intopackages/shared, import them from both processes, and revert thetsconfig.web.jsoninclude. -
[Nit] Locale-independent number formatting in a multi-locale app —
apps/desktop/src/main/usage-budget.ts:60hardcodesnew Intl.NumberFormat('en-US', …), so the sidebar always shows comma grouping even underes/pt-BR/zh-CN.
Suggested fix: pass the active locale (e.g.getCurrentLocale()from@open-codesign/i18n) or useIntl.NumberFormat(undefined, { maximumFractionDigits: 0 }).
Questions
- None.
Summary
Review mode: initial
- No blockers or majors found. The feature is self-contained and truthful: totals are derived only from settled journal responses (
apps/desktop/src/main/run-journal.ts:206), invalid usage numbers degrade to0for a display-only value (acceptable non-critical path), the IPC handler validatesschemaVersion/designIdand is read-only (apps/desktop/src/main/ipc/generate.ts:513), and failures surface as an error toast with the usage line hidden rather than showing stale or wrong totals (Sidebar.tsx:174). - Tests for the pure summarizer/formatter (
apps/desktop/src/main/usage-budget.test.ts) and the journal reopen path (apps/desktop/src/main/run-journal.test.ts:329) are solid and timezone-independent. The IPC handler and the sidebar rendering are not directly covered (see Testing). - Residual risk: this PR also changes
apps/desktop/vitest.config.ts:5and three*.browser.test.tshelpers for the author's local macOS host. Those are unrelated to the feature; the worker cap is a no-op on Linux CI (process.platform === 'linux'keeps the default), and the browser-test edits only change how text is selected, so no CI regression is expected. The PR notes a conflict with unmerged #458 on those files — coordinate merge order.
Testing
- Not run (automation).
- Suggested: add a Vitest covering the new
codesign:v1:usage-budgethandler (badschemaVersion/designId→CodesignErrorIPC_BAD_INPUT;journal === null→ zeroedschemaVersion: 1payload) and a renderer test asserting the sidebar shows the design/today/week line and hides it when the IPC rejects.
Open-CoDesign Bot
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Usage totals can remain stale after cancelled responses and local day or week rollover.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds recorded usage totals for the current design, using existing run-journal responses without new pricing data or storage.
Changes:
- Displays lifetime, local-day, and Monday-start weekly token and USD totals.
- Adds localized labels, usage aggregation, and journal replay coverage.
- Makes browser input replacement portable and limits non-Linux test workers.
| File | Description |
|---|---|
| packages/i18n/src/locales/zh-CN.json | Adds Chinese usage labels. |
| packages/i18n/src/locales/pt-BR.json | Adds Brazilian Portuguese usage labels. |
| packages/i18n/src/locales/es.json | Adds Spanish usage labels. |
| packages/i18n/src/locales/en.json | Adds English usage labels. |
| apps/desktop/vitest.config.ts | Limits non-Linux test workers. |
| apps/desktop/tsconfig.web.json | Includes usage helpers in renderer typechecking. |
| apps/desktop/src/renderer/src/components/TweakPanel.browser.test.ts | Uses native input selection. |
| apps/desktop/src/renderer/src/components/SourceEditPanel.browser.test.ts | Uses native textarea selection. |
| apps/desktop/src/renderer/src/components/Sidebar.tsx | Fetches and displays usage totals. |
| apps/desktop/src/renderer/src/components/chat/PromptInput.browser.test.ts | Makes text selection portable. |
| apps/desktop/src/preload/index.ts | Exposes the typed usage API. |
| apps/desktop/src/main/usage-budget.ts | Aggregates and formats usage. |
| apps/desktop/src/main/usage-budget.test.ts | Tests usage windows and formatting. |
| apps/desktop/src/main/run-journal.ts | Extracts settled response usage. |
| apps/desktop/src/main/run-journal.test.ts | Tests usage recovery after reopening. |
| apps/desktop/src/main/ipc/generate.ts | Registers the usage totals handler. |
| .changeset/usage-budget.md | Records the feature release. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| config?.hasKey && config.modelPrimary ? config.modelPrimary : t('sidebar.chat.noModel'); | ||
| const lastTokens = lastUsage ? lastUsage.inputTokens + lastUsage.outputTokens : null; | ||
|
|
||
| // lastUsage is the signal that a run finished recording provider usage. |
| return () => { | ||
| active = false; | ||
| }; | ||
| }, [currentDesignId, lastUsage, pushToast, t]); |
This reverts commit 01c8a95.
Contributor
There was a problem hiding this comment.
Findings
- [Nit] PR description no longer matches the current diff for the Vitest worker cap. The PR body says "caps non-Linux Vitest workers at 2", but
apps/desktop/vitest.config.ts:5now readsmaxWorkers: process.platform === 'win32' ? 2 : undefined— it caps Windows only and leaves macOS uncapped. The same paragraph ties that harness change to "this Mac", so the description now misdescribes what is actually shipped.
Suggested fix: update the PR body to state the Windows-only worker cap (and describe the macOS browser-test selection change on its own), or restore the macOS cap if that was the intent.
Questions
- Usage totals are refetched on
lastUsagein the sidebar effect deps (apps/desktop/src/renderer/src/components/Sidebar.tsx:183). The description says cancelled runs that carry a provider response are counted in the journal totals. Does a cancelled run updatelastUsage? If it does not, the totals will be right on disk but the sidebar line will not refresh until the next completed run or a design switch. - The three browser-test helpers now select with
page.keyboard.down('Control')+A(e.g.apps/desktop/src/renderer/src/components/SourceEditPanel.browser.test.ts:397). On macOS, select-all is normallyMeta+A, andControl+A can instead move to line start in Chrome/Cocoa text fields. Can you confirm these still pass on your macOS host, or make the modifier platform-aware?
Summary
Review mode: follow-up after new commits
- No blockers or majors. The feature path is unchanged from the prior revision and remains truthful: totals derive only from settled journal responses (
apps/desktop/src/main/run-journal.ts:197), non-finite/negative numbers degrade to0(display-only, acceptable), the IPC handler validatesschemaVersion/designIdand is read-only (apps/desktop/src/main/ipc/generate.ts:514), and an IPC failure hides the line and raises an error toast rather than showing stale totals. - The new commits since the last review only touch test harness/config plus a merge; nothing in the feature code changed, so the earlier analysis still holds.
- Residual (unchanged from the prior review, not re-filed as findings): (1) the renderer still imports
formatUsageCost/formatUsageTokensfrom../../../main/usage-budget(apps/desktop/src/renderer/src/components/Sidebar.tsx:4) andapps/desktop/tsconfig.web.jsonstill addssrc/main/usage-budget.tsto the web project — moving the pure helpers topackages/sharedwould keep the process boundary clean; (2)formatUsageTokensstill hardcodesIntl.NumberFormat('en-US', …)(apps/desktop/src/main/usage-budget.ts:60), so grouping stays en-US underes/pt-BR/zh-CN. - Residual risk: the browser-test edits and
apps/desktop/vitest.config.tschange are unrelated to the feature; the PR notes a conflict with unmerged #458 on those files — coordinate merge order.
Testing
- Not run (automation).
- Suggested: add a Vitest for the new
codesign:v1:usage-budgethandler (badschemaVersion/designId→CodesignErrorIPC_BAD_INPUT;journal === null→ zeroedschemaVersion: 1payload) and a renderer test asserting the design/today/week line renders and hides when the IPC rejects.
Open-CoDesign Bot
Contributor
There was a problem hiding this comment.
Findings
- [Nit] The PR description is out of date relative to the current revision and omits the screenshot the template requires. The body says "Head is now
e2822c9", but the branch head is094eb98(theuseDesignUsageBudgetextraction landed after that), and the UI section still says screenshots are "Not attached" even though.github/PULL_REQUEST_TEMPLATE.mdmarks screenshots as required for any visual change.apps/desktop/src/renderer/src/components/Sidebar.tsx:322adds visible sidebar text.
Suggested fix: refresh the description (or drop the hard-coded commit pins) and attach a screenshot of the new usage line; no code change needed.
Summary
Review mode: follow-up after new commits
- No blockers or majors. The prior review's open items are resolved: the description no longer claims a non-Linux Vitest worker cap, and the "a cancelled run may not change
lastUsage" gap is now covered by subscribing tosettledGenerationIds(apps/desktop/src/renderer/src/components/usage-budget-refresh.ts:15), with a test that pins that behavior (apps/desktop/src/renderer/src/components/usage-budget-refresh.test.tsx). - The extraction into
useDesignUsageBudgetkeeps the truthful-state contract: a failed read clears the line and raises one error toast per distinct message (reportedErrordedupe), a stale in-flight response is dropped via therequestcounter andactiveflag, and a successful refresh never blanks the line. Midnight re-arming uses local time vianextLocalMidnight, so DST-shifted boundaries are handled. nextLocalMidnightis unit-tested alongside the window math (apps/desktop/src/main/usage-budget.test.ts).- Residual (unchanged from the prior review, not re-filed as findings): (1) the renderer imports pure helpers from
../../../main/usage-budgetin bothSidebar.tsx:4andusage-budget-refresh.ts:2, andapps/desktop/tsconfig.web.json:3addssrc/main/usage-budget.tsto the web project — moving the pure helpers topackages/sharedwould keep the process boundary clean; (2)formatUsageTokenshardcodesIntl.NumberFormat('en-US', …)(apps/desktop/src/main/usage-budget.ts:65), so digit grouping stays en-US underes/pt-BR/zh-CN. - Residual risk: the cancel-refresh path depends on the store actually adding an entry to
settledGenerationIdswhen a cancelled run settles; this PR's test models that store behavior but cannot verify the store itself. Worth a quick manual check if not already confirmed.
Testing
- Not run (automation).
- Covered by new tests:
usage-budget.test.ts(summary windows, formatting,nextLocalMidnight),run-journal.test.ts(usage recorded across reopen),usage-budget-refresh.test.tsx(cancel refresh, midnight/week boundary, focus/visibility, stale-response drop). - Still uncovered: the
codesign:v1:usage-budgethandler itself (apps/desktop/src/main/ipc/generate.ts:514) — invalidschemaVersion/designId→CodesignError('IPC_BAD_INPUT'), andjournal === null→ zeroedschemaVersion: 1payload. A small handler-level test would close that gap.
Open-CoDesign Bot
Contributor
There was a problem hiding this comment.
Findings
No new findings. The diff since the last review only relocates the pure usage helpers into packages/shared and updates the changeset; no correctness, security, or project-constraint issue is introduced by that move.
Summary
Review mode: follow-up after new commits
- The prior review's remaining residual observation is resolved: the shared usage math now lives in
packages/shared/src/usage-budget.tsand is re-exported frompackages/shared/src/index.ts:629, so the renderer no longer crosses the main/renderer process boundary (apps/desktop/src/renderer/src/components/Sidebar.tsx:1,apps/desktop/src/renderer/src/components/usage-budget-refresh.ts:1), andapps/desktop/tsconfig.web.jsonno longer includessrc/main/usage-budget.ts. .changeset/usage-budget.md:3now bumps@open-codesign/sharedalongsidedesktopandi18n, matching the new public exports.- The moved code stays pure — no new dependencies, no direct provider SDK imports — and the new IPC handler validates its input (
apps/desktop/src/main/ipc/generate.ts:514). - Residual (unchanged, not re-filed):
formatUsageTokensstill hardcodesIntl.NumberFormat('en-US', …)inpackages/shared/src/usage-budget.ts, so thousands grouping stays en-US underes/pt-BR/zh-CN; and thecodesign:v1:usage-budgethandler still has no handler-level test (invalidschemaVersion/designId→IPC_BAD_INPUT,journal === null→ zeroedschemaVersion: 1payload). - Residual risk: the cancel-refresh path depends on the store adding an entry to
settledGenerationIdswhen a cancelled run settles (apps/desktop/src/renderer/src/components/usage-budget-refresh.ts:15); this PR's test models that store behavior but cannot verify the store itself. A quick manual check is worth it. - Non-blocking, previously noted and not re-filed: the PR description still omits the screenshot the template requires for a visual change and references
094eb98for the local pre-push run while the branch head isbe0c0b72.
Testing
- Not run (automation).
- Covered by new/moved tests:
packages/shared/src/usage-budget.test.ts(window math, formatting,nextLocalMidnight),apps/desktop/src/main/run-journal.test.ts(usage recorded across a journal reopen),apps/desktop/src/renderer/src/components/usage-budget-refresh.test.tsx(cancel refresh, midnight/week boundary, focus/visibility, stale-response drop). - Still uncovered: the
codesign:v1:usage-budgetIPC handler itself (see residual above).
Open-CoDesign Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The sidebar now shows recorded token and USD totals for the current design: the design lifetime, local today, and the Monday-start local week. This is the weekly slice of the README Later item "Cost transparency". There is no separate issue. Totals come from usage already stored on settled run-journal responses. This does not add a price table, a pre-generation estimate, a budget cap, or a new database.
Cancelled runs that include a provider response are counted. Failed runs without a response are not. Invalid usage numbers are treated as zero.
The sidebar refetches those journal totals when a run settles. A cancellation that records a provider response updates the totals even though
lastUsagedoes not change. The same design also refetches at the next local midnight, so Today and the Monday-start week move while it stays open, and again when the window is focused or shown. A response that arrives after cleanup, or after a newer read, is ignored.The pure totals, formatting, and local-midnight helper live in
@open-codesign/shared. Desktop main and the renderer both import that package. The renderer no longer importssrc/main, and the web TypeScript project no longer includes a main-process file.An earlier commit on this branch changed the macOS browser test harness. That change is reverted in
e2822c9and is not part of this PR. The same harness fix remains local on this machine and on unmerged #458.Type of change
Linked issue
No linked issue. README Later item: Cost transparency, weekly budget slice only.
Checklist
pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleDependency additions (if any)
None.
Screenshots / recordings (UI changes)
Not attached. The new text sits under the existing per-run token line in the sidebar.
Checks
be0c0b7passed local pre-push: typecheck, Biome (674 files), and the full test suite. Desktop: 175 files, 2583 tests passed. Shared: 20 files, 313 tests passed. The net diff against main remains the usage change only.