feat(dashboard): add local usage statistics dashboard with session detail#948
feat(dashboard): add local usage statistics dashboard with session detail#948myk1yt wants to merge 32 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds local usage-statistics schemas, append-only event recording, aggregation, export and clearing services, extension message handling, the ChangesUsage contracts and entry points
Statistics engine
Extension integration
Statistics webview
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StatsView
participant WebviewMessageHandler
participant UsageStatsService
participant UsageEventStore
User->>StatsView: Open /stats
StatsView->>WebviewMessageHandler: getUsageStats
WebviewMessageHandler->>UsageStatsService: queryStats
UsageStatsService->>UsageEventStore: readAll
UsageStatsService-->>WebviewMessageHandler: StatsSnapshot
WebviewMessageHandler-->>StatsView: getUsageStatsResponse
StatsView-->>User: Render summary and buckets
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
packages/types/src/usage-stats.ts (1)
20-24: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider stricter validation for date strings and numeric values.
occurredAt/generatedAt/from/toare plainz.string()rather thanz.string().datetime(), andSourcedNumber.value/StatsBucketnumeric fields allow negative or non-finite values. Since this schema is the contract for corruption-quarantining of append-only NDJSON data, tightening these constraints would let the schema itself reject malformed events earlier instead of relying entirely on downstream logic.♻️ Example tightening
export const SourcedNumber = z.object({ - value: z.number(), + value: z.number().finite().nonnegative(), source: UsageValueSource, })- occurredAt: z.string(), // ISO 8601 UTC + occurredAt: z.string().datetime(), // ISO 8601 UTCSince zod's
.datetime()strictness (e.g. offset support) varies by version, please confirm the behavior for zod 3.25.76 before applying broadly.Also applies to: 69-76, 102-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/types/src/usage-stats.ts` around lines 20 - 24, Tighten the usage-stats schema by validating occurredAt, generatedAt, from, and to with the Zod 3.25.76-compatible datetime constraint, confirming required offset behavior before applying it. Update SourcedNumber.value and the numeric fields in StatsBucket to reject negative and non-finite values while preserving valid event data and the existing schema contract.src/services/stats/UsageAggregator.ts (1)
456-498: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the identical inclusion-semantics branches or implement the intended behavior.
All three
if/else if/elseblocks forcacheReadInInput,cacheWriteInInput, andreasoningInOutputperform the exact samebucket.X += tokens, yet the surrounding comments (466-469) state that for"included"the value is already ininputTokens/outputTokensand should not be added separately "to avoid double counting". The code never implements that distinction. SincetotalTokensis read directly from the event this isn't a live double-count bug, but the dead branches plus contradictory comments are a trap for future edits. Either flatten to a single unconditional add or implement the documented semantics explicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageAggregator.ts` around lines 456 - 498, Resolve the contradiction in the token accumulation logic by either collapsing the identical branches in the cacheReadInInput, cacheWriteInInput, and reasoningInOutput handling into unconditional bucket.X additions, or implementing the documented included/excluded semantics explicitly. Keep the chosen behavior consistent with the surrounding comments and avoid retaining dead conditional branches.src/services/stats/UsageStatsService.ts (1)
259-376: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the shared preset/time-range/timezone resolution into one helper.
UsageStatsService.filterEventsByQuery/resolvePresetRange/toTimezoneStartOfDay/getTimezoneOffsetMinutesreimplement the same day/preset/offset math asUsageAggregator.resolveTimeRange/toTimezoneDate/startOfDay/getTimezoneOffsetMinutes. Two copies of subtle DST/offset arithmetic can silently diverge, causingexportStatsto include a different event set thanqueryStatsfor the same query.
src/services/stats/UsageStatsService.ts#L259-L376: replace the local preset/timezone helpers with the shared implementation used by the aggregator (or delegate range resolution to it).src/services/stats/UsageAggregator.ts#L143-L271: moveresolveTimeRange/toTimezoneDate/startOfDay/getTimezoneOffsetMinutesinto a shared module both files import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageStatsService.ts` around lines 259 - 376, Extract the shared preset, time-range, and timezone calculations from UsageStatsService methods filterEventsByQuery, resolvePresetRange, toTimezoneStartOfDay, and getTimezoneOffsetMinutes into a shared module, then update UsageStatsService.ts lines 259-376 to use it. Move UsageAggregator’s resolveTimeRange, toTimezoneDate, startOfDay, and getTimezoneOffsetMinutes from UsageAggregator.ts lines 143-271 into the same module and update the aggregator imports/callers, preserving identical range and DST behavior for both queryStats and exportStats.webview-ui/src/components/stats/StatsSummary.tsx (1)
46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded English strings bypass i18n in stats UI. These stats components use raw English words in user-facing output instead of
t(...)keys, unlike the rest of the panel. Route them through the translation layer (add keys tostats.json).
webview-ui/src/components/stats/StatsSummary.tsx#L46-L50: replace the literalunknownin({unknownCount} unknown)with a translated string.webview-ui/src/components/stats/UsageHeatmap.tsx#L173-L177: replace the literaltokens/eventsin the tooltip content with translated strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/stats/StatsSummary.tsx` around lines 46 - 50, The stats UI contains hardcoded English strings that must use the translation layer. In webview-ui/src/components/stats/StatsSummary.tsx lines 46-50, update StatsSummary to translate “unknown”; in webview-ui/src/components/stats/UsageHeatmap.tsx lines 173-177, translate “tokens” and “events” in the tooltip. Add the corresponding keys and translations to stats.json while preserving the existing count and tooltip formatting.src/services/stats/UsageEventStore.ts (1)
588-621: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
rebuildIdempotencySetreads every segment fully on eachinitialize().Combined with a new
UsageEventStorebeing constructed perTask(seesrc/core/task/Task.tsL533), the first append of every task re-reads and re-parses the entire current generation (up to 100 MiB) into memory to rebuild the in-memory dedupe set. This is fire-and-forget so it won't block the task, but it is repeated I/O + parse + string allocation that grows with stored history. Consider sharing a single store instance (e.g. the provider'sUsageStatsServicestore) across tasks so the scan happens once per process.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageEventStore.ts` around lines 588 - 621, The UsageEventStore is recreated per task, causing rebuildIdempotencySet to rescan and parse the full segment history on every initialize(). Reuse a single UsageEventStore instance, preferably the provider-owned store exposed through UsageStatsService, across tasks so initialization and idempotency rebuilding occur once per process; preserve existing append and deduplication behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/types/src/__tests__/usage-stats.spec.ts`:
- Around line 133-138: Rename the test case around UsageEventV1.parse to
describe that a zero attempt value is accepted, matching its current input and
assertion. Keep the existing test body unchanged and make the name reflect the
absence of a minimum constraint.
In `@src/package.nls.ca.json`:
- Line 1: Remove the leading UTF-8 BOM from src/package.nls.ca.json so the file
begins directly with the JSON object and remains compatible with strict
JSON.parse consumers.
In `@src/package.nls.de.json`:
- Line 1: Remove the leading BOM character from src/package.nls.de.json,
src/package.nls.es.json, src/package.nls.fr.json, src/package.nls.zh-CN.json,
and src/package.nls.zh-TW.json at line 1, leaving each file as valid JSON
beginning with its opening brace.
In `@src/package.nls.hi.json`:
- Line 1: Remove the leading UTF-8 BOM before the opening JSON object in
src/package.nls.hi.json (line 1), src/package.nls.id.json (line 1), and
src/package.nls.it.json (line 1); re-save all three localization files as UTF-8
without BOM while preserving their JSON content.
In `@src/package.nls.ja.json`:
- Line 1: Remove the leading BOM character from src/package.nls.ja.json#L1-L1,
src/package.nls.ko.json#L1-L1, src/package.nls.nl.json#L1-L1,
src/package.nls.pl.json#L1-L1, and src/package.nls.pt-BR.json#L1-L1, leaving
each file as valid JSON beginning directly with its opening brace.
In `@src/services/stats/__tests__/UsageAggregator.spec.ts`:
- Around line 436-451: Update the events in the “should sort category buckets by
totalTokens descending then name ascending” test to populate usage.totalTokens
with values matching the intended order: anthropic 3000, google 2000, and openai
1000. Keep the existing assertions and inputTokens fixtures unchanged.
In `@webview-ui/src/components/stats/StatsView.tsx`:
- Around line 176-180: Update the usageStatsChanged handling in the
message-listener effect to store the debounce timer in a ref instead of
returning cleanup from handleMessage. Clear any existing timer before scheduling
a new fetchStats call, and clear the ref-held timer in the effect cleanup so
pending refetches cannot run after unmount.
---
Nitpick comments:
In `@packages/types/src/usage-stats.ts`:
- Around line 20-24: Tighten the usage-stats schema by validating occurredAt,
generatedAt, from, and to with the Zod 3.25.76-compatible datetime constraint,
confirming required offset behavior before applying it. Update
SourcedNumber.value and the numeric fields in StatsBucket to reject negative and
non-finite values while preserving valid event data and the existing schema
contract.
In `@src/services/stats/UsageAggregator.ts`:
- Around line 456-498: Resolve the contradiction in the token accumulation logic
by either collapsing the identical branches in the cacheReadInInput,
cacheWriteInInput, and reasoningInOutput handling into unconditional bucket.X
additions, or implementing the documented included/excluded semantics
explicitly. Keep the chosen behavior consistent with the surrounding comments
and avoid retaining dead conditional branches.
In `@src/services/stats/UsageEventStore.ts`:
- Around line 588-621: The UsageEventStore is recreated per task, causing
rebuildIdempotencySet to rescan and parse the full segment history on every
initialize(). Reuse a single UsageEventStore instance, preferably the
provider-owned store exposed through UsageStatsService, across tasks so
initialization and idempotency rebuilding occur once per process; preserve
existing append and deduplication behavior.
In `@src/services/stats/UsageStatsService.ts`:
- Around line 259-376: Extract the shared preset, time-range, and timezone
calculations from UsageStatsService methods filterEventsByQuery,
resolvePresetRange, toTimezoneStartOfDay, and getTimezoneOffsetMinutes into a
shared module, then update UsageStatsService.ts lines 259-376 to use it. Move
UsageAggregator’s resolveTimeRange, toTimezoneDate, startOfDay, and
getTimezoneOffsetMinutes from UsageAggregator.ts lines 143-271 into the same
module and update the aggregator imports/callers, preserving identical range and
DST behavior for both queryStats and exportStats.
In `@webview-ui/src/components/stats/StatsSummary.tsx`:
- Around line 46-50: The stats UI contains hardcoded English strings that must
use the translation layer. In webview-ui/src/components/stats/StatsSummary.tsx
lines 46-50, update StatsSummary to translate “unknown”; in
webview-ui/src/components/stats/UsageHeatmap.tsx lines 173-177, translate
“tokens” and “events” in the tooltip. Add the corresponding keys and
translations to stats.json while preserving the existing count and tooltip
formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f0ac10b1-4bba-4501-ba94-ef6ebdff01df
📒 Files selected for processing (48)
packages/types/src/__tests__/usage-stats.spec.tspackages/types/src/index.tspackages/types/src/usage-stats.tspackages/types/src/vscode-extension-host.tspackages/types/src/vscode.tssrc/activate/registerCommands.tssrc/core/task/Task.tssrc/core/task/__tests__/Task.usage-stats.spec.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/usageStatsMessageHandler.spec.tssrc/core/webview/usageStatsMessageHandler.tssrc/core/webview/webviewMessageHandler.tssrc/package.jsonsrc/package.nls.ca.jsonsrc/package.nls.de.jsonsrc/package.nls.es.jsonsrc/package.nls.fr.jsonsrc/package.nls.hi.jsonsrc/package.nls.id.jsonsrc/package.nls.it.jsonsrc/package.nls.ja.jsonsrc/package.nls.jsonsrc/package.nls.ko.jsonsrc/package.nls.nl.jsonsrc/package.nls.pl.jsonsrc/package.nls.pt-BR.jsonsrc/package.nls.ru.jsonsrc/package.nls.tr.jsonsrc/package.nls.vi.jsonsrc/package.nls.zh-CN.jsonsrc/package.nls.zh-TW.jsonsrc/services/command/__tests__/built-in-commands.spec.tssrc/services/command/built-in-commands.tssrc/services/stats/UsageAggregator.tssrc/services/stats/UsageEventStore.tssrc/services/stats/UsageRecorder.tssrc/services/stats/UsageStatsService.tssrc/services/stats/__tests__/UsageAggregator.spec.tssrc/services/stats/__tests__/UsageEventStore.spec.tssrc/services/stats/index.tswebview-ui/src/App.tsxwebview-ui/src/components/chat/ChatView.tsxwebview-ui/src/components/chat/__tests__/ChatView.stats-command.spec.tsxwebview-ui/src/components/stats/StatsSummary.tsxwebview-ui/src/components/stats/StatsView.tsxwebview-ui/src/components/stats/UsageHeatmap.tsxwebview-ui/src/components/stats/__tests__/StatsView.spec.tsxwebview-ui/src/i18n/locales/en/stats.json
| it("should reject negative attempt", () => { | ||
| // z.number() accepts negatives, but attempt should be >= 0 logically | ||
| // This test confirms the schema accepts any number (no min constraint in V1) | ||
| const result = UsageEventV1.parse({ ...validEvent, attempt: 0 }) | ||
| expect(result.attempt).toBe(0) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test name contradicts its body.
The test is titled "should reject negative attempt" but it parses attempt: 0 and asserts it's accepted, not rejected. No negative value is ever passed. The name should reflect what's actually verified (that any number, including 0, is accepted — no min constraint).
💚 Proposed fix
- it("should reject negative attempt", () => {
- // z.number() accepts negatives, but attempt should be >= 0 logically
- // This test confirms the schema accepts any number (no min constraint in V1)
- const result = UsageEventV1.parse({ ...validEvent, attempt: 0 })
- expect(result.attempt).toBe(0)
- })
+ it("should accept attempt of 0 (no min constraint in V1)", () => {
+ // z.number() has no lower bound in V1; this documents current behavior.
+ const result = UsageEventV1.parse({ ...validEvent, attempt: 0 })
+ expect(result.attempt).toBe(0)
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should reject negative attempt", () => { | |
| // z.number() accepts negatives, but attempt should be >= 0 logically | |
| // This test confirms the schema accepts any number (no min constraint in V1) | |
| const result = UsageEventV1.parse({ ...validEvent, attempt: 0 }) | |
| expect(result.attempt).toBe(0) | |
| }) | |
| it("should accept attempt of 0 (no min constraint in V1)", () => { | |
| // z.number() has no lower bound in V1; this documents current behavior. | |
| const result = UsageEventV1.parse({ ...validEvent, attempt: 0 }) | |
| expect(result.attempt).toBe(0) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/types/src/__tests__/usage-stats.spec.ts` around lines 133 - 138,
Rename the test case around UsageEventV1.parse to describe that a zero attempt
value is accepted, matching its current input and assertion. Keep the existing
test body unchanged and make the name reflect the absence of a minimum
constraint.
| @@ -1,4 +1,4 @@ | |||
| { | |||
| { | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for f in src/package.nls.de.json src/package.nls.es.json src/package.nls.fr.json src/package.nls.zh-CN.json src/package.nls.zh-TW.json src/package.nls.json; do
printf '%s: ' "$f"
head -c3 "$f" | xxd -p
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 538
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
files = [
"src/package.nls.de.json",
"src/package.nls.es.json",
"src/package.nls.fr.json",
"src/package.nls.zh-CN.json",
"src/package.nls.zh-TW.json",
"src/package.nls.json",
]
for f in files:
data = Path(f).read_bytes()
print(f"{f}: {data[:3].hex()} BOM={data.startswith(b'\\xef\\xbb\\xbf')}")
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 408
🏁 Script executed:
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' 'package\.nls|JSON\.parse|BOM|utf-8' .Repository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🏁 Script executed:
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' 'package\.nls|JSON\.parse|readFile.*json|utf-8-bom|strip.*bom|BOM' .Repository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🌐 Web query:
VS Code package.nls JSON BOM strip localization bundle loader JSON.parse
💡 Result:
In Visual Studio Code development, issues involving JSON.parse errors with package.nls.json or localization bundle files are frequently caused by the presence of a UTF-8 Byte Order Mark (BOM) [1]. Standard Node.js JSON.parse does not automatically strip a leading BOM, which results in a syntax error when the file content is read into a string [2][3]. To resolve this, developers must ensure that files are read and processed without the BOM, or explicitly strip it before parsing [1]. The following technical details apply to handling these localization files: 1. BOM Stripping: If you are programmatically reading these files in a VS Code extension or build tool, use the following approach to ensure compatibility: if (content.charCodeAt(0) === 0xFEFF) { content = content.slice(1); } const json = JSON.parse(content); 2. Localization Infrastructure: VS Code currently uses the @vscode/l10n and @vscode/l10n-dev libraries for handling localization [4][5][6]. Older extensions may still use the deprecated vscode-nls and vscode-nls-dev packages [7][8]. When using these older tools, ensure that your build pipeline (e.g., Gulp tasks) handles file encoding consistently [7]. 3. Common Pitfalls: - Encoding Mismatches: Always ensure files are saved as UTF-8 without BOM if possible, as tools like standard JSON.parse are sensitive to the leading bytes [2][3][1]. - Invalid JSON Structure: Beyond BOM issues, ensure that your package.nls.json follows the expected schema. If you include "comment" fields for localization purposes, ensure the environment (VS Code version) supports the schema extension, otherwise, it may throw type errors or fail to load [9]. - Hidden Characters: Tools sometimes inject zero-width spaces (\u200b) or other non-printable characters for specific localization scenarios (e.g., URL formatting); ensure your JSON parser or validation logic accounts for these if you are manipulating the JSON object manually [10]. If you are encountering a JSON.parse error during extension loading, check the specific file content for the UTF-8 BOM byte sequence (0xEF, 0xBB, 0xBF) at the start of the file [3][1]. Stripping this prefix is the standard solution for making JSON files BOM-tolerant in the VS Code ecosystem [1].
Citations:
- 1: microsoft/vscode@0bc0832
- 2: https://github.com/microsoft/vscode/blob/main/build/lib/i18n.ts
- 3: https://github.com/microsoft/vscode/blob/main/build/next/index.ts
- 4: https://github.com/microsoft/vscode-extension-samples/tree/main/l10n-sample
- 5: https://registry.npmjs.org/@vscode/l10n-dev
- 6: https://p.rst.im/q/github.com/microsoft/vscode-l10n
- 7: https://github.com/microsoft/vscode-nls-dev
- 8: https://github.com/microsoft/vscode-nls/blob/main/README.md
- 9: Support changes in vscode-nls-dev that allow hint comments in package.nls.json microsoft/vscode#126408
- 10: dotnet/vscode-csharp@918872f
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for f in [
"src/package.nls.de.json",
"src/package.nls.es.json",
"src/package.nls.fr.json",
"src/package.nls.zh-CN.json",
"src/package.nls.zh-TW.json",
"src/package.nls.json",
]:
data = Path(f).read_bytes()
print(f"{f}: {data[:3].hex()} BOM={data.startswith(b'\xef\xbb\xbf')}")
PY
printf '\n--- esbuild ---\n'
sed -n '90,120p' apps/vscode-nightly/esbuild.mjs
printf '\n--- i18n setup ---\n'
sed -n '1,120p' src/i18n/setup.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 3807
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
files = [
"src/package.nls.de.json",
"src/package.nls.es.json",
"src/package.nls.fr.json",
"src/package.nls.zh-CN.json",
"src/package.nls.zh-TW.json",
"src/package.nls.json",
]
for f in files:
data = Path(f).read_bytes()
bom = data.startswith(b"\xef\xbb\xbf")
print(f"{f}: {data[:3].hex()} BOM={bom}")
PY
rg -n "strip-bom|package\.nls" apps/vscode-nightly src scripts .githubRepository: Zoo-Code-Org/Zoo-Code
Length of output: 3409
🌐 Web query:
VS Code package.nls.*.json loader BOM strip source code localization JSON.parse
💡 Result:
In the context of VS Code localization and file loading, the Byte Order Mark (BOM) — specifically the UTF-8 BOM (U+FEFF) — is frequently encountered in package.nls.json files and other localization resources. Because standard JSON.parse() in Node.js and modern JavaScript engines can fail or behave unexpectedly if a leading BOM is present, VS Code infrastructure often includes explicit logic to strip it before parsing [1][2][3][4]. General approaches observed in VS Code source code for handling the BOM include: 1. Buffer Inspection and Conversion: Often, files are read into a Buffer. Before converting the buffer to a UTF-8 string for parsing, or immediately after string conversion, code checks for the BOM. For example, some loaders convert the Buffer to a string using 'utf8' and then perform a check: if (contents.charCodeAt(0) === 0xFEFF) { contents = contents.slice(1); } [1][2] 2. Automated Tools and Build Pipelines: The VS Code build pipeline (e.g., vscode-nls-dev or the core build tools) ensures that when localization files are generated or processed, they are handled in a way that is compatible with downstream consumers [5][6]. When files are read, the logic typically identifies package.nls.json files and uses JSON.parse(buffer.toString('utf8')), sometimes relying on the fact that the platform-specific filesystem or previous build steps have sanitized the content [5][3]. 3. BOM-Tolerant Parsing: In cases where internal models (like TextModel) represent file content, the constructor often contains a defensive check to strip the BOM [1][2]. This allows callers to process the file content without needing to special-case the first character of the first line [1]. In summary, while JSON.parse is the standard tool for reading package.nls.json files [5][3][6], VS Code's technical ecosystem maintains robust defensive programming practices to detect and remove the UTF-8 BOM, ensuring it does not interfere with string matching, parsing, or identity construction [1][2][7][4].
Citations:
- 1: https://github.com/microsoft/vscode/blob/main/build/lib/i18n.ts
- 2: microsoft/vscode@0bc0832
- 3: https://github.com/microsoft/vscode/blob/main/build/next/index.ts
- 4: Strip UTF8 BOM from device id microsoft/vscode#315679
- 5: https://github.com/Microsoft/vscode-nls-dev/blob/master/src/main.ts
- 6: https://github.com/microsoft/vscode-l10n/blob/main/l10n-dev/src/cli.ts
- 7: https://github.com/microsoft/vscode/pull/66189/files
🌐 Web query:
site:github.com/microsoft/vscode build/lib/i18n.ts BOM JSON.parse package.nls
💡 Result:
In the context of the VS Code build system, particularly within build/lib/i18n.ts and related localization scripts, the handling of UTF-8 Byte Order Marks (BOM) is a deliberate technical choice to ensure compatibility and correct file encoding across different platforms [1][2]. When parsing localization files such as package.nls.json, the build scripts often read the file contents as UTF-8 strings [1][3][4]. Because JSON.parse can sometimes fail or behave unexpectedly if a file contains an invisible leading UTF-8 BOM (U+FEFF), developers and build utilities in the VS Code codebase implement specific patterns to handle this: 1. Encoding and BOM Handling: Build scripts that generate localization output (such as.isl files) explicitly include a UTF-8 BOM [1][2]. This is done by creating a Buffer with the BOM sequence [0xEF, 0xBB, 0xBF] and concatenating it with the file content [1][2]. 2. Parsing Logic: When reading configuration files like package.nls.json, the build system typically reads the file content as 'utf8' [1][3][4]. Recent updates and patterns in the codebase demonstrate that if code encounters a leading BOM during processing (for example, in text models), it may explicitly strip the character to prevent it from interfering with subsequent logic or JSON parsing [2]. 3. Reliability: This ensures that files are correctly recognized as UTF-8 by various tools (including Windows-specific utilities) while avoiding common runtime issues where the BOM character might cause a JSON.parse syntax error [2]. In summary, the interaction between these components is designed to strictly manage file encoding by explicitly writing a BOM where required for external compatibility and stripping it or ensuring standard UTF-8 parsing when reading files into the build pipeline to maintain JSON structural integrity [1][2].
Citations:
- 1: https://github.com/microsoft/vscode/blob/main/build/lib/i18n.ts
- 2: microsoft/vscode@0bc0832
- 3: https://github.com/microsoft/vscode/blob/main/build/next/index.ts
- 4: https://github.com/microsoft/vscode/blob/4c055a03/build/lib/extensions.ts
🏁 Script executed:
sed -n '1,120p' .github/workflows/code-qa.ymlRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5375
🏁 Script executed:
sed -n '1,140p' scripts/find-missing-translations.js
printf '\n---\n'
sed -n '270,360p' scripts/find-missing-translations.jsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 7104
Strip the BOM from these locale JSON files. src/package.nls.de.json, src/package.nls.es.json, src/package.nls.fr.json, src/package.nls.zh-CN.json, and src/package.nls.zh-TW.json all start with \uFEFF; scripts/find-missing-translations.js reads them with plain JSON.parse, so the package-nls check will fail on these files.
📍 Affects 5 files
src/package.nls.de.json#L1-L1(this comment)src/package.nls.es.json#L1-L1src/package.nls.fr.json#L1-L1src/package.nls.zh-CN.json#L1-L1src/package.nls.zh-TW.json#L1-L1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/package.nls.de.json` at line 1, Remove the leading BOM character from
src/package.nls.de.json, src/package.nls.es.json, src/package.nls.fr.json,
src/package.nls.zh-CN.json, and src/package.nls.zh-TW.json at line 1, leaving
each file as valid JSON beginning with its opening brace.
| @@ -1,4 +1,4 @@ | |||
| { | |||
| { | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the accidental UTF-8 BOM from the localization JSON files. These three package.nls.*.json files gained a leading \uFEFF byte-order mark before the opening {. It adds diff noise and can break strict JSON.parse-based tooling; re-save each file as UTF-8 without BOM.
src/package.nls.hi.json#L1-L1: strip the leading BOM before{.src/package.nls.id.json#L1-L1: strip the leading BOM before{.src/package.nls.it.json#L1-L1: strip the leading BOM before{.
📍 Affects 3 files
src/package.nls.hi.json#L1-L1(this comment)src/package.nls.id.json#L1-L1src/package.nls.it.json#L1-L1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/package.nls.hi.json` at line 1, Remove the leading UTF-8 BOM before the
opening JSON object in src/package.nls.hi.json (line 1), src/package.nls.id.json
(line 1), and src/package.nls.it.json (line 1); re-save all three localization
files as UTF-8 without BOM while preserving their JSON content.
| @@ -1,4 +1,4 @@ | |||
| { | |||
| { | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
files = [
"src/package.nls.ja.json",
"src/package.nls.ko.json",
"src/package.nls.nl.json",
"src/package.nls.pl.json",
"src/package.nls.pt-BR.json",
]
for f in files:
data = Path(f).read_bytes()
print(f, data[:3], "BOM" if data.startswith(b'\xef\xbb\xbf') else "no-bom")
PY
printf '\n--- NLS-related files ---\n'
git ls-files 'src/*' | rg 'package\.nls|nls|i18n|localization|package\.json' || true
printf '\n--- Search for BOM handling / JSON parsing ---\n'
rg -n "BOM|utf-8-sig|strip.*bom|read.*json|JSON\.parse|nls" src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' | sed -n '1,200p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 28894
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
files = [
"src/package.nls.ja.json",
"src/package.nls.ko.json",
"src/package.nls.nl.json",
"src/package.nls.pl.json",
"src/package.nls.pt-BR.json",
]
for f in files:
data = Path(f).read_bytes()
print(f"{f}: startswith_bom={data.startswith(b'\\xef\\xbb\\xbf')} first_bytes={data[:10]!r}")
PY
printf '\n--- matching files ---\n'
git ls-files 'src/*' | rg 'package\.nls|nls|i18n|localization|package\.json' || true
printf '\n--- BOM / JSON parsing references ---\n'
rg -n "BOM|utf-8-sig|strip.*bom|read.*json|JSON\.parse|nls" src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' | sed -n '1,220p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 30974
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
for f in [
"src/package.nls.ja.json",
"src/package.nls.ko.json",
"src/package.nls.nl.json",
"src/package.nls.pl.json",
"src/package.nls.pt-BR.json",
]:
b = Path(f).read_bytes()
print(f, "BOM" if b.startswith(b"\xef\xbb\xbf") else "no-BOM", b[:6])
print("\n--- likely NLS loader files ---")
for p in Path("src").rglob("*"):
if p.is_file() and any(k in p.name.lower() for k in ["nls", "localization", "package"]):
if p.suffix in [".ts", ".js", ".mjs", ".cjs", ".json", ".jsonc"]:
print(p)
PY
printf '\n--- search for JSON loading in src ---\n'
rg -n "readFileSync\\(|JSON\\.parse\\(|stripBOM|vscode-nls|nls" src -g '!**/node_modules/**' | sed -n '1,220p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 25110
🏁 Script executed:
ast-grep outline src/shared/package.ts --view expanded
printf '\n--- src/i18n/setup.ts ---\n'
sed -n '1,140p' src/i18n/setup.ts
printf '\n--- src/package.json scripts/keywords around localization ---\n'
sed -n '1,220p' src/package.json | sed -n '1,220p'
printf '\n--- search package.nls references ---\n'
rg -n "package\.nls|nls\.json|vscode-nls|localization" src scripts packages webview-ui apps -g '!**/node_modules/**' | sed -n '1,240p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 10065
🏁 Script executed:
sed -n '260,380p' scripts/find-missing-translations.js
printf '\n---\n'
sed -n '90,135p' apps/vscode-nightly/esbuild.mjs
printf '\n--- JSON.parse with BOM probe ---\n'
node - <<'JS'
const samples = ['\ufeff{}', '\ufeff{\n "a": 1\n}', '{}'];
for (const s of samples) {
try {
const v = JSON.parse(s);
console.log('ok', JSON.stringify(s.slice(0,3)), v);
} catch (e) {
console.log('err', JSON.stringify(s.slice(0,3)), e.name + ': ' + e.message);
}
}
JSRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5341
Strip the BOM from these locale JSON files. JSON.parse rejects a leading \uFEFF, so these package.nls.*.json files can break consumers that read them as plain JSON.
src/package.nls.ja.json#L1-L1src/package.nls.ko.json#L1-L1src/package.nls.nl.json#L1-L1src/package.nls.pl.json#L1-L1src/package.nls.pt-BR.json#L1-L1
📍 Affects 5 files
src/package.nls.ja.json#L1-L1(this comment)src/package.nls.ko.json#L1-L1src/package.nls.nl.json#L1-L1src/package.nls.pl.json#L1-L1src/package.nls.pt-BR.json#L1-L1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/package.nls.ja.json` at line 1, Remove the leading BOM character from
src/package.nls.ja.json#L1-L1, src/package.nls.ko.json#L1-L1,
src/package.nls.nl.json#L1-L1, src/package.nls.pl.json#L1-L1, and
src/package.nls.pt-BR.json#L1-L1, leaving each file as valid JSON beginning
directly with its opening brace.
| it("should sort category buckets by totalTokens descending then name ascending", () => { | ||
| const events = [ | ||
| makeEvent({ eventId: "evt-1", idempotencyKey: "idem-1", provider: "openai", usage: { inputTokens: { value: 1000, source: "provider" } } }), | ||
| makeEvent({ eventId: "evt-2", idempotencyKey: "idem-2", provider: "anthropic", usage: { inputTokens: { value: 3000, source: "provider" } } }), | ||
| makeEvent({ eventId: "evt-3", idempotencyKey: "idem-3", provider: "google", usage: { inputTokens: { value: 2000, source: "provider" } } }), | ||
| ] | ||
| const query = makeQuery({ groupBy: ["provider"] }) | ||
|
|
||
| const result = aggregator.query(events, query) | ||
|
|
||
| expect(result.buckets).toHaveLength(3) | ||
| // totalTokens 내림차순: anthropic(3000) > google(2000) > openai(1000) | ||
| expect(result.buckets[0].key.provider).toBe("anthropic") | ||
| expect(result.buckets[1].key.provider).toBe("google") | ||
| expect(result.buckets[2].key.provider).toBe("openai") | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --pattern 'extractValue($_) {
$$$
}' --lang typescript src/services/stats/UsageAggregator.ts
rg -n -A5 'private extractValue' src/services/stats/UsageAggregator.ts
rg -n -A15 'private sortBuckets' src/services/stats/UsageAggregator.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1315
Populate usage.totalTokens in this sort test
The fixture only sets usage.inputTokens, so each bucket ends up with totalTokens === 0 and the order comes from the alphabetical tie-break instead of the intended totalTokens sort. Set usage.totalTokens on these events so the test covers the named behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/stats/__tests__/UsageAggregator.spec.ts` around lines 436 - 451,
Update the events in the “should sort category buckets by totalTokens descending
then name ascending” test to populate usage.totalTokens with values matching the
intended order: anthropic 3000, google 2000, and openai 1000. Keep the existing
assertions and inputTokens fixtures unchanged.
Source: Coding guidelines
| if (message.type === "usageStatsChanged") { | ||
| // Data changed externally — refetch with debounce | ||
| const timer = setTimeout(() => fetchStats(preset, groupBy), 300) | ||
| return () => clearTimeout(timer) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Debounce cleanup is dead code; timer never cleared.
The return () => clearTimeout(timer) is returned from the handleMessage event-listener callback, whose return value is discarded — so the cleanup never runs. Rapid usageStatsChanged messages each schedule an independent refetch (no coalescing), and a pending timer can fire after unmount. Track the timer in a ref and clear it on the effect's cleanup.
🔧 Ref-based debounce
+ const refetchTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
+
useEffect(() => {
const handleMessage = (e: MessageEvent) => {
const message: ExtensionMessage = e.data
@@
if (message.type === "usageStatsChanged") {
- // Data changed externally — refetch with debounce
- const timer = setTimeout(() => fetchStats(preset, groupBy), 300)
- return () => clearTimeout(timer)
+ // Data changed externally — refetch with debounce
+ if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current)
+ refetchTimerRef.current = setTimeout(() => fetchStats(preset, groupBy), 300)
}
@@
window.addEventListener("message", handleMessage)
- return () => window.removeEventListener("message", handleMessage)
+ return () => {
+ window.removeEventListener("message", handleMessage)
+ if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current)
+ }
}, [t, preset, groupBy, fetchStats])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (message.type === "usageStatsChanged") { | |
| // Data changed externally — refetch with debounce | |
| const timer = setTimeout(() => fetchStats(preset, groupBy), 300) | |
| return () => clearTimeout(timer) | |
| } | |
| const refetchTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) | |
| useEffect(() => { | |
| const handleMessage = (e: MessageEvent) => { | |
| const message: ExtensionMessage = e.data | |
| if (message.type === "usageStatsChanged") { | |
| // Data changed externally — refetch with debounce | |
| if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current) | |
| refetchTimerRef.current = setTimeout(() => fetchStats(preset, groupBy), 300) | |
| } | |
| } | |
| window.addEventListener("message", handleMessage) | |
| return () => { | |
| window.removeEventListener("message", handleMessage) | |
| if (refetchTimerRef.current) clearTimeout(refetchTimerRef.current) | |
| } | |
| }, [t, preset, groupBy, fetchStats]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/stats/StatsView.tsx` around lines 176 - 180, Update
the usageStatsChanged handling in the message-listener effect to store the
debounce timer in a ref instead of returning cleanup from handleMessage. Clear
any existing timer before scheduling a new fetchStats call, and clear the
ref-held timer in the effect cleanup so pending refetches cannot run after
unmount.
…StatsView, UsageAggregator
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/stats/__tests__/UsageAggregator.spec.ts (1)
514-518: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWeek-grouping test under-asserts its documented behavior.
The comment states three events fall into weeks 28, 29, 29 (i.e. 2 buckets), but the assertions only check
>= 1bucket and key format — a regression collapsing everything into one bucket or producing wrong keys would still pass. Consider asserting the exact bucket count/keys to actually cover the grouping behavior.♻️ Suggested strengthening
- expect(result.buckets.length).toBeGreaterThanOrEqual(1) - const weekKeys = result.buckets.map((b) => b.key.week) + expect(result.buckets).toHaveLength(2) + const weekKeys = result.buckets.map((b) => b.key.week) weekKeys.forEach((key) => { expect(key).toMatch(/^\d{4}-W\d{2}$/) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/__tests__/UsageAggregator.spec.ts` around lines 514 - 518, Strengthen the week-grouping test assertions around result.buckets by requiring exactly two buckets and verifying their keys are the documented weeks 28 and 29 in the expected year. Retain the key-format validation only if useful, but replace the insufficient greater-than-or-equal count check so collapsed or incorrect grouping fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/stats/__tests__/UsageStatsService.spec.ts`:
- Line 178: Replace the tautological assertion in the relevant UsageStatsService
test with a type check that verifies result is not a CSV string, matching the
existing assertions used near the other JsonExport checks at lines 260 and 308.
---
Nitpick comments:
In `@src/services/stats/__tests__/UsageAggregator.spec.ts`:
- Around line 514-518: Strengthen the week-grouping test assertions around
result.buckets by requiring exactly two buckets and verifying their keys are the
documented weeks 28 and 29 in the expected year. Retain the key-format
validation only if useful, but replace the insufficient greater-than-or-equal
count check so collapsed or incorrect grouping fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0aee3c05-f62b-4c50-abc4-d086171b1c8a
📒 Files selected for processing (4)
src/services/stats/__tests__/UsageAggregator.spec.tssrc/services/stats/__tests__/UsageStatsService.spec.tswebview-ui/src/components/stats/__tests__/StatsView.spec.tsxwebview-ui/src/components/stats/__tests__/UsageHeatmap.spec.tsx
| const query = makeQuery({ preset: "all" }) | ||
| const result = await service.exportStats(query, "json") | ||
|
|
||
| expect(result).not.toBe("string") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Tautological assertion — provides no coverage.
result is a JsonExport object, so expect(result).not.toBe("string") compares an object against the literal string "string" and always passes. The intent (verify it isn't a CSV string) requires checking the type, as done correctly at Lines 260 and 308.
💚 Proposed fix
- expect(result).not.toBe("string")
+ expect(typeof result).not.toBe("string")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(result).not.toBe("string") | |
| expect(typeof result).not.toBe("string") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/stats/__tests__/UsageStatsService.spec.ts` at line 178, Replace
the tautological assertion in the relevant UsageStatsService test with a type
check that verifies result is not a CSV string, matching the existing assertions
used near the other JsonExport checks at lines 260 and 308.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webview-ui/src/i18n/locales/de/stats.json`:
- Around line 33-34: Update the cacheReadTokens and cacheWriteTokens entries in
the German stats translations to use complete cache operation labels, replacing
the truncated wording with “Cache-Lesevorgänge” and “Cache-Schreibvorgänge” or
the established equivalent terminology.
In `@webview-ui/src/i18n/locales/fr/stats.json`:
- Around line 33-34: Update the cacheReadTokens and cacheWriteTokens
translations in the French stats locale to use the complete labels “Lecture du
cache” and “Écriture du cache”, or the project’s established equivalent
terminology.
In `@webview-ui/src/i18n/locales/vi/stats.json`:
- Around line 51-52: Update the Vietnamese coverage labels in the stats locale,
including the related label at the additional referenced entry: replace “Ghi từ”
with “Bắt đầu ghi từ”, replace “Sự kiện bổ sung ngược” with “Sự kiện được bổ
sung hồi tố”, and use “Được bổ sung hồi tố” for the corresponding related value.
In `@webview-ui/src/i18n/locales/zh-TW/stats.json`:
- Around line 11-14: Update the totalTokens, inputTokens, outputTokens, and
cacheTokens labels in the Traditional Chinese stats locale to use appropriate
terminology for “token,” replacing “權杖” with “詞元” consistently across all four
labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 033af2f3-8f50-47dc-80d9-712d6e878d7c
📒 Files selected for processing (17)
webview-ui/src/i18n/locales/ca/stats.jsonwebview-ui/src/i18n/locales/de/stats.jsonwebview-ui/src/i18n/locales/es/stats.jsonwebview-ui/src/i18n/locales/fr/stats.jsonwebview-ui/src/i18n/locales/hi/stats.jsonwebview-ui/src/i18n/locales/id/stats.jsonwebview-ui/src/i18n/locales/it/stats.jsonwebview-ui/src/i18n/locales/ja/stats.jsonwebview-ui/src/i18n/locales/ko/stats.jsonwebview-ui/src/i18n/locales/nl/stats.jsonwebview-ui/src/i18n/locales/pl/stats.jsonwebview-ui/src/i18n/locales/pt-BR/stats.jsonwebview-ui/src/i18n/locales/ru/stats.jsonwebview-ui/src/i18n/locales/tr/stats.jsonwebview-ui/src/i18n/locales/vi/stats.jsonwebview-ui/src/i18n/locales/zh-CN/stats.jsonwebview-ui/src/i18n/locales/zh-TW/stats.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/package.nls.ca.json (1)
19-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate
command.openUsageStats.titlein each locale file.The new command title is English in all six locale overrides, so users of those locales will see untranslated UI text.
src/package.nls.ca.json#L19-L19: add the Catalan translation.src/package.nls.fr.json#L19-L19: add the French translation.src/package.nls.hi.json#L19-L19: add the Hindi translation.src/package.nls.id.json#L28-L28: add the Indonesian translation.src/package.nls.zh-CN.json#L19-L19: add the Simplified Chinese translation.src/package.nls.zh-TW.json#L19-L19: add the Traditional Chinese translation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/package.nls.ca.json` at line 19, Translate the command.openUsageStats.title value in each affected locale: src/package.nls.ca.json#19-19, src/package.nls.fr.json#19-19, src/package.nls.hi.json#19-19, src/package.nls.id.json#28-28, src/package.nls.zh-CN.json#19-19, and src/package.nls.zh-TW.json#19-19. Replace the English title with the appropriate Catalan, French, Hindi, Indonesian, Simplified Chinese, and Traditional Chinese translations respectively.src/package.nls.it.json (1)
19-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize
command.openUsageStats.titlein each locale.The new key is present, so these English values are used directly instead of falling back to English only when a locale file is missing.
src/package.nls.it.json#L19-L19: add the reviewed Italian translation.src/package.nls.ja.json#L28-L28: add the reviewed Japanese translation.src/package.nls.ko.json#L19-L19: add the reviewed Korean translation.src/package.nls.nl.json#L28-L28: add the reviewed Dutch translation.src/package.nls.pl.json#L19-L19: add the reviewed Polish translation.src/package.nls.pt-BR.json#L19-L19: add the reviewed Brazilian Portuguese translation.src/package.nls.ru.json#L28-L28: add the reviewed Russian translation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/package.nls.it.json` at line 19, Localize the command.openUsageStats.title entry in each affected locale file, replacing the English value with the reviewed translation: src/package.nls.it.json#L19-L19, src/package.nls.ja.json#L28-L28, src/package.nls.ko.json#L19-L19, src/package.nls.nl.json#L28-L28, src/package.nls.pl.json#L19-L19, src/package.nls.pt-BR.json#L19-L19, and src/package.nls.ru.json#L28-L28.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/package.nls.ca.json`:
- Line 19: Translate the command.openUsageStats.title value in each affected
locale: src/package.nls.ca.json#19-19, src/package.nls.fr.json#19-19,
src/package.nls.hi.json#19-19, src/package.nls.id.json#28-28,
src/package.nls.zh-CN.json#19-19, and src/package.nls.zh-TW.json#19-19. Replace
the English title with the appropriate Catalan, French, Hindi, Indonesian,
Simplified Chinese, and Traditional Chinese translations respectively.
In `@src/package.nls.it.json`:
- Line 19: Localize the command.openUsageStats.title entry in each affected
locale file, replacing the English value with the reviewed translation:
src/package.nls.it.json#L19-L19, src/package.nls.ja.json#L28-L28,
src/package.nls.ko.json#L19-L19, src/package.nls.nl.json#L28-L28,
src/package.nls.pl.json#L19-L19, src/package.nls.pt-BR.json#L19-L19, and
src/package.nls.ru.json#L28-L28.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c053aebe-829f-4e24-98ea-544a11684a91
📒 Files selected for processing (21)
src/package.nls.ca.jsonsrc/package.nls.de.jsonsrc/package.nls.es.jsonsrc/package.nls.fr.jsonsrc/package.nls.hi.jsonsrc/package.nls.id.jsonsrc/package.nls.it.jsonsrc/package.nls.ja.jsonsrc/package.nls.ko.jsonsrc/package.nls.nl.jsonsrc/package.nls.pl.jsonsrc/package.nls.pt-BR.jsonsrc/package.nls.ru.jsonsrc/package.nls.tr.jsonsrc/package.nls.vi.jsonsrc/package.nls.zh-CN.jsonsrc/package.nls.zh-TW.jsonwebview-ui/src/i18n/locales/de/stats.jsonwebview-ui/src/i18n/locales/fr/stats.jsonwebview-ui/src/i18n/locales/vi/stats.jsonwebview-ui/src/i18n/locales/zh-TW/stats.json
🚧 Files skipped from review as they are similar to previous changes (7)
- webview-ui/src/i18n/locales/zh-TW/stats.json
- src/package.nls.tr.json
- src/package.nls.es.json
- src/package.nls.de.json
- webview-ui/src/i18n/locales/de/stats.json
- src/package.nls.vi.json
- webview-ui/src/i18n/locales/fr/stats.json
github-actions
Bot
added
the
awaiting-review
| /** | ||
| * Usage 이벤트 기록기. API attempt의 terminal finalize에서만 호출된다. | ||
| * store 초기화 실패 시 null이며, 이 경우 기록을 조용히 건너뛴다. | ||
| * (아키텍처 보고서 섹션 5.5-5.8, rollback: writer를 optional service로 주입) |
There was a problem hiding this comment.
Please use English for comments.
There was a problem hiding this comment.
Please wait!! I am adding some new feature here, and i will translate everything into English and upload it again. It's my first time making a PR, so I'm still quite inexperienced!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/services/stats/UsageEventStore.ts (4)
388-394: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
getManifest()can write the manifest without holding the cross-process lock.
getManifest()callsloadOrCreateManifest()(Line 393) directly, butloadOrCreateManifest()performs a write viawriteManifestAtomic()(Lines 514, 520-521) whenever the file is missing or fails basic validation. Every other write path (appendInternal,clear) acquiresacquireManifestLock()first, but this one doesn't. If the manifest happens to be missing/corrupt at the exact momentgetManifest()runs concurrently with a lockedappend()/clear()writer, this unguarded write can race with and clobber the lock holder's in-flight update.🔒 Proposed fix
async getManifest(): Promise<UsageStatsManifest> { await this.ensureInitialized() - return this.loadOrCreateManifest() + let releaseLock: (() => Promise<void>) = async () => {} + try { + releaseLock = await this.acquireManifestLock() + return await this.loadOrCreateManifest() + } finally { + try { + await releaseLock() + } catch { + // ignore + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageEventStore.ts` around lines 388 - 394, Update getManifest() to acquire and release the same cross-process manifest lock used by appendInternal and clear before calling loadOrCreateManifest(), ensuring the lock is released in all outcomes while preserving the existing initialization and return behavior.
273-293: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSchema-invalid (but syntactically complete) lines on the last line are silently dropped without quarantine.
For the last line, when
JSON.parsesucceeds butUsageEventV1Schema.safeParsefails, the entry is pushed toquarantineEntriesand then immediately popped (Lines 280-284), meaning the corrupt record is neither retained as an event nor recorded to quarantine. The "crash tail" heuristic should only apply to lines that failJSON.parse(truly truncated writes); a line that parses as valid JSON but fails schema validation is a complete, non-truncated record and represents real corruption that the PR objectives explicitly require to be quarantined.🐛 Proposed fix
if (result.success) { events.push(result.data) } else { - // zod validation failed: corrupt line - quarantineEntries.push(this.makeQuarantineEntry(segmentFile, lineNum, line)) - // Validation failure of the last line may be a crash tail, so exclude from quarantine - if (isLastLine) { - quarantineEntries.pop() - } + // zod validation failed: the line parsed as JSON but doesn't match the schema, + // so it is not a crash tail (a truncated write would fail JSON.parse instead) — + // always quarantine it. + quarantineEntries.push(this.makeQuarantineEntry(segmentFile, lineNum, line)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageEventStore.ts` around lines 273 - 293, Update the schema-validation failure branch in the UsageEventStore parsing loop so a last line that passes JSON.parse but fails UsageEventV1Schema.safeParse remains in quarantineEntries. Remove the isLastLine-based quarantineEntries.pop() behavior from that branch, while preserving the existing crash-tail exclusion only in the JSON.parse catch block.
336-357: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPartial
clear()failures can mix old (should-be-cleared) events into the new generation, and backups are never cleaned up.
getSegmentPath()(Lines 701-704) builds segment filenames from only the segment number, not the generation. When a rename in this loop fails (Line 351-356), the error is logged and swallowed, execution continues, and the new manifest with the incremented generation is still written (Line 360). The old, un-movedevents-000001.ndjsonfile remains at the same path the new generation will resolve to viagetSegmentPath(1), so subsequent appends land in — andreadAll()/checkTotalSize()still count — the file that "clear" was supposed to remove. Separately, even on full success, moved files underold-generation-{N}/are never deleted; eachclear()call permanently grows disk usage outside theTOTAL_MAX_BYTESaccounting, undermining both the hard-cap design and the user's expectation that "clear" actually deletes data.Simplify by deleting segment files directly instead of moving them to a backup directory — this removes the collision risk and the unbounded backup growth in one change:
♻️ Proposed fix
- // Move existing segment files to a new generation directory (backup) - // Or simply replace with a new manifest and ignore existing files - // Design: "Replace existing segments with a new empty generation" - // Implementation: Move existing segment files under old-generation-{N} - const oldGenDir = path.join(this.statsDir, `old-generation-${manifest.generation}`) - await fs.mkdir(oldGenDir, { recursive: true }) - - const allFiles = await fs.readdir(this.statsDir) + const allFiles = await fs.readdir(this.statsDir) const segmentFiles = allFiles.filter( (f) => f.startsWith(SEGMENT_PREFIX) && f.endsWith(SEGMENT_EXT), ) for (const file of segmentFiles) { - const oldPath = path.join(this.statsDir, file) - const newPath = path.join(oldGenDir, file) + const oldPath = path.join(this.statsDir, file) try { - await fs.rename(oldPath, newPath) + await fs.unlink(oldPath) } catch (err) { - // Log move failures and continue - console.warn(`[UsageEventStore] failed to move old segment ${file}:`, err) + // Delete failure: fail the clear so the caller/UI can retry rather than + // silently mixing stale data with the new generation. + throw err } }If preserving old-generation events indefinitely for audit purposes is intentional, then at minimum incorporate the generation number into
getSegmentPath()so collisions can't occur, and add cleanup/retention forold-generation-*directories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageEventStore.ts` around lines 336 - 357, Update the clear() implementation to delete matching segment files directly from statsDir instead of creating old-generation-{N} backup directories and renaming files. Remove the backup-directory creation and move loop, and ensure deletion failures are handled consistently so clear does not leave old segments at paths resolved by getSegmentPath().
577-581: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid rethrowing from
onCompromised. A compromised lock here can bubble out as an unhandled exception and take down the extension host. Log it and return instead of throwing so this optional stats path stays non-fatal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/stats/UsageEventStore.ts` around lines 577 - 581, Update the onCompromised callback in UsageEventStore so it logs the compromised-lock error and returns without rethrowing it. Keep the existing diagnostic logging, ensuring this optional stats path cannot propagate the exception to the extension host.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/services/stats/UsageEventStore.ts`:
- Around line 388-394: Update getManifest() to acquire and release the same
cross-process manifest lock used by appendInternal and clear before calling
loadOrCreateManifest(), ensuring the lock is released in all outcomes while
preserving the existing initialization and return behavior.
- Around line 273-293: Update the schema-validation failure branch in the
UsageEventStore parsing loop so a last line that passes JSON.parse but fails
UsageEventV1Schema.safeParse remains in quarantineEntries. Remove the
isLastLine-based quarantineEntries.pop() behavior from that branch, while
preserving the existing crash-tail exclusion only in the JSON.parse catch block.
- Around line 336-357: Update the clear() implementation to delete matching
segment files directly from statsDir instead of creating old-generation-{N}
backup directories and renaming files. Remove the backup-directory creation and
move loop, and ensure deletion failures are handled consistently so clear does
not leave old segments at paths resolved by getSegmentPath().
- Around line 577-581: Update the onCompromised callback in UsageEventStore so
it logs the compromised-lock error and returns without rethrowing it. Keep the
existing diagnostic logging, ensuring this optional stats path cannot propagate
the exception to the extension host.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e949947b-7e79-46a2-a92d-8e27b57db58d
📒 Files selected for processing (13)
packages/types/src/usage-stats.tssrc/core/task/Task.tssrc/core/task/__tests__/Task.usage-stats.spec.tssrc/services/stats/UsageAggregator.tssrc/services/stats/UsageEventStore.tssrc/services/stats/UsageRecorder.tssrc/services/stats/UsageStatsService.tssrc/services/stats/__tests__/UsageAggregator.spec.tssrc/services/stats/__tests__/UsageEventStore.spec.tssrc/services/stats/__tests__/UsageStatsService.spec.tssrc/services/stats/index.tswebview-ui/src/components/stats/__tests__/StatsView.spec.tsxwebview-ui/src/components/stats/__tests__/UsageHeatmap.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- src/services/stats/index.ts
- src/services/stats/tests/UsageEventStore.spec.ts
- packages/types/src/usage-stats.ts
- src/core/task/tests/Task.usage-stats.spec.ts
- webview-ui/src/components/stats/tests/UsageHeatmap.spec.tsx
- src/services/stats/UsageRecorder.ts
- src/services/stats/tests/UsageStatsService.spec.ts
- src/services/stats/UsageStatsService.ts
- src/core/task/Task.ts
- webview-ui/src/components/stats/tests/StatsView.spec.tsx
- src/services/stats/UsageAggregator.ts
…xtract format utils
|
4 issues outstanding (1 high, 4 medium, 5 low total). Code review —
|
github-actions
Bot
removed
the
awaiting-review
myk1yt
changed the title
/stats local usage statistics command…gregation, i18n, and CI fixes
852be5d to
b3e90bf
Compare
|
@myk1yt Thanks for the PR. |
…es, and 221 tests
github-actions
Bot
added
the
awaiting-review
4 participants
Closes #947
Summary
Adds a local usage statistics dashboard accessible from the sidebar. Tracks LLM API call token usage and costs per session, with daily heatmap, session list/detail views, model/mode filtering, and export.
Features
Dashboard (Sidebar)
Usage Recording
Task.tsterminal finalize — no streaming duplicatesglobalStorage/usage-stats/— no cloud sync, no telemetryCost Calculation
totalCostcomputation to OpenAI, Anthropic Vertex, DeepSeek, xAI, Mistral, Kenari, Poe, Qwen-Code, Bedrock, Moonshot, and OpenAI-compatible providerscostUsdare recalculated at query time usingcostRecalculation.ts— no NDJSON mutationInternationalization
Architecture
New Files
packages/types/src/usage-stats.tssrc/services/stats/UsageEventStore.tssrc/services/stats/UsageAggregator.tssrc/services/stats/UsageRecorder.tssrc/services/stats/UsageStatsService.tssrc/services/stats/costRecalculation.tssrc/core/webview/usageStatsMessageHandler.tswebview-ui/src/components/dashboard/DashboardView.tsxwebview-ui/src/components/dashboard/DashboardSummary.tsxwebview-ui/src/components/dashboard/SessionList.tsxwebview-ui/src/components/dashboard/SessionDetail.tsxwebview-ui/src/components/stats/UsageHeatmap.tsxwebview-ui/src/utils/formatNumber.tswebview-ui/src/i18n/locales/*/dashboard.jsonModified Files
src/api/providers/openai.tssrc/api/providers/openai-compatible.tssrc/api/providers/moonshot.tssrc/api/providers/deepseek.tssrc/api/providers/xai.tssrc/api/providers/mistral.tssrc/api/providers/kenari.tssrc/api/providers/poe.tssrc/api/providers/qwen-code.tssrc/api/providers/bedrock.tssrc/api/providers/anthropic-vertex.tspackages/types/src/vscode-extension-host.tspackages/types/src/vscode.tssrc/package.jsonsrc/package.nls.json+ 16 locale variantssrc/activate/registerCommands.tssrc/core/webview/webviewMessageHandler.tswebview-ui/src/App.tsxKey Design Decisions
parentTaskIdchainTask.tsterminal finalize prevents streaming duplicatesTesting
pnpm check-types— 11/11 packages passpnpm lint— 0 warningspnpm --dir src test— 6946 tests passpnpm --dir webview-ui test— 1479 tests passcheck-translations— 17 non-English locales verified, 0 missing translationspnpm build— clean buildpnpm vsix— VSIX package generatedCI Compatibility Fixes
package.nls.*.jsonfiles (17 files) for Linux JSON parsertotalCostfield (3 spec files)Summary by CodeRabbit
New Features
Improvements
Tests