fix(usage): dedup provider-wide notes and add report-level notes field#3312
fix(usage): dedup provider-wide notes and add report-level notes field#3312oldschoola wants to merge 4 commits into
Conversation
Provider-wide disclaimers (e.g. OpenCode Go's "OMP-observed spend only") were duplicated onto every UsageLimit, then repeated N times in the TUI aggregate renderer (once per account × window). With 2 accounts × 3 windows, the same disclaimer appeared 6 times bullet-joined. Structural fix: - Add notes?: string[] to UsageReport (interface + both schema copies: usage.ts and auth-broker/wire-schemas.ts) so the field survives the broker client's "+": "reject" deserialization gate. - Move opencode-go's disclaimer from per-limit notes to provider-level notes. Defensive fix: - Dedup identical per-limit notes in the TUI aggregate renderer (command-controller.ts) via [...new Set(...)]. - Render provider-level notes once above per-account sections in all three rendering paths: TUI (command-controller), CLI (usage-cli), and ACP (usage-report helper). Regression tests: - usage-cli.test.ts: provider-level notes render once, not duplicated per account or limit; positioned above per-account rows. - usage-report-notes-schema.test.ts: wire-schema round-trip proving notes survives usageResponseSchema validation. Fixes can1357#3268
roboomp
added
cli
![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: f5225f54f6
ℹ️ 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".
| // above the per-account sections instead of duplicating onto every limit. | ||
| const providerNotes = [...new Set(providerReports.flatMap(report => report.notes ?? []))]; | ||
| if (providerNotes.length > 0) { | ||
| lines.push(` ${uiTheme.fg("dim", providerNotes.join(" • "))}`.trimEnd()); |
There was a problem hiding this comment.
When a provider returns a report-level note with tabs, embedded newlines, or an overlong string, this new TUI path injects it directly into the transcript. The root AGENTS.md TUI Sanitization section requires raw provider/tool output to be normalized (e.g. replace tabs and truncate) before display; otherwise /usage can create terminal holes, wrapping, or overflow. Please sanitize the joined note text before passing it to uiTheme.fg.
Useful? React with 👍 / 👎.
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P1: tight root fix for provider-wide usage notes; schema and changelog updates are coherent.
Inline should-fix: the TUI aggregate per-limit note dedup is untested; the added CLI test exercises the opposite contract.
Verification: bun test packages/ai/test/usage-report-notes-schema.test.ts passed. bun test packages/coding-agent/test/usage-cli.test.ts could not start because packages/coding-agent/src/export/html/index.ts imports missing ./tool-views.generated.js in this worktree.
Thanks for the focused fix.
| lines.push(` ${uiTheme.fg("dim", resetText)}`.trimEnd()); | ||
| } | ||
| const notes = sortedLimits.flatMap(limit => limit.notes ?? []); | ||
| const notes = [...new Set(sortedLimits.flatMap(limit => limit.notes ?? []))]; |
There was a problem hiding this comment.
should-fix: This is the only changed path that deduplicates per-limit notes across the TUI aggregate groups, but the added test coverage is in formatUsageBreakdown() and explicitly expects two occurrences for the same note. A regression here would still pass. Please add coverage through /usage/handleUsageCommand (or another exported TUI path) asserting one Overage requests: 5 when two reports share the same provider/window group.
Address review feedback: provider notes could contain tabs, embedded newlines, or control characters that break TUI rendering. Both note rendering sites (provider-wide and per-group) now wrap the joined text through sanitizeText → truncateToWidth → replaceTabs per AGENTS.md TUI Sanitization rules.
|
Fixed in the latest push. Both note rendering sites (provider-wide at line 1592 and per-group at line 1666) now sanitize the joined text through |
Apply sanitizeText to provider notes in the shared usage-report renderer and the CLI usage output, matching the sanitization added to command-controller.ts. Prevents tabs/newlines/control characters in provider notes from breaking terminal rendering.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58de02ea05
ℹ️ 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".
| const providerNotes = [...new Set(providerReports.flatMap(report => report.notes ?? []))]; | ||
| if (providerNotes.length > 0) { | ||
| lines.push( | ||
| ` ${uiTheme.fg("dim", replaceTabs(truncateToWidth(sanitizeText(providerNotes.join(" • ")), 110)))}`.trimEnd(), |
There was a problem hiding this comment.
When a provider or broker returns a report-level note containing \n, this still injects that newline into the rendered transcript row because sanitizeText(...) preserves LF; the continuation line is not prefixed or independently width-limited, so a note like "caveat\nvery long text..." can break the /usage TUI layout despite this sanitization path. Fresh evidence in the current patch is that the new line still passes the joined provider notes through sanitizeText directly instead of splitting or replacing newlines before truncation.
Useful? React with 👍 / 👎.
sanitizeText preserves newlines (\n) which break TUI line layout when injected into a single rendered row. All notes rendering sites now replace \r\n sequences with spaces before sanitization: - command-controller.ts: provider-wide notes (line 1591) + per-group notes (line 1666) - usage-report.ts: provider-wide notes (line 58) + per-limit notes (line 90, previously completely unsanitized) - usage-cli.ts: provider-wide notes (line 455)
|
Fixed in the latest push. All notes rendering sites now normalize newlines (
|
2 participants
Problem
Provider-wide disclaimers (e.g. OpenCode Go's "OMP-observed spend only") were duplicated onto every
UsageLimit.notes, then repeated N times in the TUI aggregate renderer — once per account × window. With 2 accounts × 3 windows, the same disclaimer appeared 6 times bullet-joined.Fixes #3268
Changes
Structural fix
notes?: string[]to theUsageReportinterface (the type source of truth)packages/ai/src/usage.tsusageReportSchemapackages/ai/src/auth-broker/wire-schemas.tsarkUsageReportSchema— critical because the broker client validates/v1/usageresponses withusageResponseSchemawhich uses"+": "reject", so an undeclared field would be rejected on deserializationnotesto provider-levelnotesDefensive fix
command-controller.ts) via[...new Set(...)]command-controller.ts), CLI (usage-cli.ts), ACP (usage-report.ts)Tests
usage-report-notes-schema.test.ts(new): wire-schema round-trip provingnotessurvivesusageResponseSchemavalidationusage-cli.test.ts: provider-level notes render once, not duplicated per account or limitbun checkpasses (all 16 packages)