[Critical] NaN timestamp from malformed chat records propagates to usage_record.jsonl as permanent phantom records

If firstRecord.timestamp is missing, empty, or malformed (which occurs naturally via corruption, legacy records, or interrupted writes), new Date(undefined).getTime() returns NaN. This NaN flows into metricsToUsageRecord as timestamp, startTime, and durationMs. JSON.stringify serializes NaN → null, so the record is persisted as "timestamp":null.

On subsequent reads, the null-timestamp record passes the version === 1 filter and enters aggregateUsage. For the 'all' range, null >= 0 evaluates to true in JavaScript, so the phantom record inflates sessionCount. The projectMap accumulates an entry keyed on undefined (from missing cwd), producing a mystery unnamed project in the dashboard.

This corruption persists until the user manually deletes usage_record.jsonl.

— qwen3.7-max via Qwen Code /review

[Critical] Unhandled promise rejection crashes the TUI

loadStatsData(range, liveRecord).then(...) has no .catch() handler. If loadStatsData throws (e.g., from rebuildFromSessionJsonl encountering an unexpected shape, or aggregateUsage propagating NaN), the rejected promise is unhandled.

There is no React error boundary (componentDidCatch) anywhere in the CLI UI tree — I searched packages/cli/src and found zero matches. An unhandled rejection in Ink/React crashes the entire process. The user types /stats, the TUI dies silently.

Additionally, there's no error state in the UI. If the load fails, the dialog would either show "Loading stats..." forever or render an empty box.

— qwen3.7-max via Qwen Code /review

[Critical] rebuildFromSessionJsonl loads entire chat JSONL files into memory and uses synchronous I/O on the main thread

Two compounding issues:

1. Memory spike: jsonl.read<ChatRecord>(filePath) loads the entire file into memory. Each ChatRecord contains full Content objects (assistant text, tool results with file contents). A single chat JSONL for a session with heavy tool use can exceed 100 MB. The function iterates ALL project directories and ALL chat files.

2. TUI freeze: fs.readdirSync, fs.existsSync, and writeLineSync (which wraps fs.appendFileSync) are all synchronous. For a user with hundreds of sessions, the rebuild blocks the Node.js event loop for 10+ seconds. The only feedback is "Loading stats..." text. A user may kill the process thinking it's hung, potentially interrupting writeLineSync mid-write and leaving usage_record.jsonl partially written.

Consider streaming records and discarding non-telemetry entries immediately, or capping the backfill to the N most recent files by modification time.

— qwen3.7-max via Qwen Code /review

[Suggestion] useEffect has no cancellation/staleness guard + re-reads file on every range change

Two related issues:

1. Race condition: Pressing r rapidly to cycle ranges fires multiple loadStatsData calls concurrently. A slower earlier request can resolve after a faster later one, briefly displaying data for the wrong range.

2. Redundant I/O: Each range change triggers loadUsageHistory() which reads and parses the entire usage_record.jsonl from disk. Cycling through 4 ranges = 4 full file reads, even though the underlying data hasn't changed — only the filter window differs.

For the redundant I/O, consider caching raw records in a useRef and re-aggregating in memory when only the range changes.

— qwen3.7-max via Qwen Code /review

1. 触发条件极其罕见 — 只在第一次使用/stats指令会基于历史session数据构建 usage_record.jsonl ，只有用户手动删除文件才会 rebuild。
2. 实际数据量有限 — 本地 rebuild 跑过mock测试，71-81 个 session，耗时 < 2 秒。100MB 单文件的 session
   是极端边界。
3. 已有 try/catch — 单个文件解析失败不会中断整体 rebuild。
4. 修复成本高 — 流式读取需要改 jsonl.read 工具函数的接口，影响面大。

[Suggestion] Avg Latency delta arrow points the wrong way

The other five KPI deltas render the arrow from the sign of the change (delta >= 0 ? '▲' : '▼'), but Avg Latency uses avgLatency <= 0 ? '▲' : '▼'. Since computeDelta defines avgLatency = current − previous, a negative delta means latency dropped — yet it renders ▲. So a falling latency (good) shows an up-arrow and a rising latency shows a down-arrow, contradicting the number and every other card. The color is already correct; only the glyph is inverted.

Keep the inverted color test (<= 0 ? success : error); just flip the glyph so it tracks direction.

— claude-opus-4-8 via Qwen Code /qreview

[Suggestion] Untrusted model/tool/project names are rendered to the terminal unsanitized

tool.name here (and the model name at line 193, the model name in StatsSessionTab, and the project path in StatsActivityTab) comes verbatim from usage_record.jsonl / the rebuilt chat logs and is rendered via ink <Text>. .slice(0, 14) truncates by code-unit count but does not strip control bytes, so an embedded ESC (\x1b…) or newline in a name survives into the terminal — enabling cursor/ANSI escapes or broken table alignment when /stats is opened on a crafted or corrupted record. Defense-in-depth: strip control chars before rendering (e.g. name.replace(/[\u0000-\u001f\u007f-\u009f]/g, '')) at the point these fields enter the render layer.

— claude-opus-4-8 via Qwen Code /qreview

[Suggestion] fmtSuccessBar can throw RangeError and crash the CLI on malformed data

'░'.repeat(10 - filled) throws when filled > 10 (or '█'.repeat(filled) when filled < 0). Normal telemetry keeps success ≤ count so rate ∈ [0,100] and this is safe today — but successRate ultimately comes from usage_record.jsonl, which loadUsageHistory accepts on a version === 1 check alone (no field validation). A single corrupt/hand-edited line with success > count makes rate > 100, and since the dialog has no React error boundary, the throw during render takes down the whole TUI. Cheap guard:

— claude-opus-4-8 via Qwen Code /qreview

[Suggestion] The rebuild / load / streak paths — where the prior review found several bugs — have no test coverage

This suite exercises metricsToUsageRecord and aggregateUsage only. rebuildFromSessionJsonl + loadUsageHistory (the cold-start path) and the streak / currentSession-merge logic in statsDataService are never invoked, so the recently-added guards can regress silently. Worth covering: rebuild dedup via seenSessionIds, the isNaN(timestamp) skip, the hasEvents === false skip, missing/empty projects/ and chats/ dirs → [], the write-back to usage_record.jsonl, loadUsageHistory falling through to rebuild when the file is absent, calculateStreaks (consecutive / gap / last-activity-stale / single-day), and loadStatsData dedup when a persisted record shares the live sessionId.

— claude-opus-4-8 via Qwen Code /qreview

[Suggestion] .catch() silently swallows all errors — activity/efficiency tabs render a blank content area on failure

The new handler clears loading but sets no error state. The render gate at lines 205 / 211 requires both !loading && data, so when loadStatsData rejects the activity and efficiency tabs render nothing — no error message, no retry affordance. The user sees a seemingly healthy empty box, indistinguishable from "no data in this range." Combined with the silent continues in rebuildFromSessionJsonl and aggregateUsage, a corrupted ~/.qwen/usage_record.jsonl now bricks /stats invisibly.

Then render a fallback when !loading && error && activeTab !== 'session' (e.g., <Text color={theme.text.error}>{t('Failed to load stats. Press r to retry.')}</Text>).

— qwen3.7-max via Qwen Code /review

[Critical] aggregateUsage unconditionally deep-accesses record fields — a single malformed record in usage_record.jsonl crashes /stats permanently

totalCalls += r.tools.totalCalls (line 338), r.tools.totalSuccess, r.tools.totalFail, r.files.linesAdded, Object.entries(r.models), and Object.entries(r.tools.byName) all throw TypeError if any persisted UsageSummaryRecord is missing one of those fields (truncated write, older schema, concurrent-write race, disk full mid-append). The throw propagates through loadStatsData into the .catch() above, leaving the dashboard blank with no indication of which record is poisoned.

The worst failure mode: one corrupted line in ~/.qwen/usage_record.jsonl bricks /stats permanently for that user. The user can still use the rest of the CLI, so they report "the stats command is buggy" with no useful diagnostic.

— qwen3.7-max via Qwen Code /review

[Suggestion] models: {} is a plain object accumulating model stats from JSON-parsed JSONL — prototype pollution via __proto__ key

const models: AggregatedReport['models'] = {}; is iterated by Object.entries(r.models) where r.models comes from usage_record.jsonl. When a user-edited JSONL contains "__proto__" as a model name, Object.entries returns it as an own property (because JSON.parse creates own __proto__), and then const existing = models[name] resolves models["__proto__"] to Object.prototype (truthy). The merge branch executes existing.requests += m.requests, which writes requests: NaN onto Object.prototype — polluting every new {} in the process for the remainder of the CLI lifetime.

The same file already demonstrates awareness of this pattern: toolCounts correctly uses Map (line 321), and uiTelemetry.ts uses Object.create(null) for user-controlled keys with an explicit comment.

— qwen3.7-max via Qwen Code /review

[Suggestion] rebuildFromSessionJsonl silently swallows every per-file error — "my stats are missing sessions" is undebuggable

Both try { files = fs.readdirSync(chatsDir) ... } catch { continue; } (around line 196) and the inner try { ...jsonl.read... } catch { continue; } (around line 243) discard every error: permission denied, ENOENT mid-run, JSON parse errors, truncated JSONL, concurrent-write locks. The newly added NaN guard (line 232) also silently continues. There is no skippedCount, no logWarn, and the function returns only successes. After a rebuild that skips 50 out of 200 sessions, the cache file is written with 150 records and the next loadUsageHistory returns those 150 with no indication anything was lost.

A user who notices "I had a session on project X yesterday but it's not in /stats" has no diagnostic path — qwen --debug emits nothing, and the only workaround is to manually diff ls ~/.qwen/projects/*/chats/*.jsonl against the contents of usage_record.jsonl.

Consider also returning { results, skippedCount } or surfacing a "N sessions could not be parsed" count in the /stats footer so the user has a breadcrumb.

— qwen3.7-max via Qwen Code /review

[Suggestion] buildHeatmapData monthOffset edge case (month-end into short month) has no test coverage

The three-step fix — setDate(1) → setMonth(month - offset) → setMonth(month+1, 0) — was specifically motivated by the March 31 → February overflow bug flagged in R1. But the existing test 'supports monthOffset to shift view backward' in asciiCharts.test.ts uses vi.setSystemTime(new Date('2025-06-04T12:00:00Z')), where June 4 is nowhere near a month end, so it never exercises the overflow path. A regression to the naive single-setMonth approach would still pass this test.

If someone simplifies the three-step pattern back to a single setMonth, the heatmap would silently show the wrong month range when the user scrolls back from March, May, July, August, October, or December (all months with 31 days preceding a shorter month).

Suggested test:

vi.setSystemTime(new Date('2025-03-31T12:00:00Z'));
const result = buildHeatmapData(records, 'all', 1, 10);
// Assert endDate === new Date('2025-02-28') (Feb 28, not Mar 3 overflow)

vi.setSystemTime(new Date('2025-01-31T12:00:00Z'));
// Assert endDate === new Date('2024-12-31') (prior year wrap)

— qwen3.7-max via Qwen Code /review

[Critical] persistSessionUsage write path has no test coverage

The test mock at clearCommand.test.ts:24 returns getMetrics: vi.fn(() => ({ models: {} })), which makes Object.values(metrics.models) an empty array, so hasActivity is always false. No test exercises the new persistSessionUsage call introduced by this PR.

This is the primary write-path for cross-session stats. If it breaks, the /stats dashboard silently shows stale/empty data for sessions ended via /clear, and the bug would go undetected.

— qwen3.7-max via Qwen Code /review

[Suggestion] DST bug: getPreviousRangeBounds('today') uses fixed 24h subtraction

todayStart.getTime() - 24 * 60 * 60 * 1000 subtracts a fixed 24 hours. On DST spring-forward days (23-hour day), this lands at 23:00 two days ago instead of 00:00 yesterday. On fall-back days (25-hour day), it lands at 01:00 yesterday.

— qwen3.7-max via Qwen Code /review

[Suggestion] Token-counting inconsistency: KPI row vs project ranking use different definitions

The KPI row sums m.totalTokens (which includes thought tokens per the provider-reported total_token_count), while the project ranking table in aggregateUsage sums m.inputTokens + m.outputTokens (strictly prompt + candidates, excluding thoughts). Users see contradictory token numbers within the same dashboard view.

Pick one consistent definition and use it everywhere. Either always use inputTokens + outputTokens (simplest), or always use totalTokens.

— qwen3.7-max via Qwen Code /review

[Suggestion] "today" delta compares partial current day against full 24h previous day

When range is 'today', the current period covers [start_of_today, now] (partial day), but getPreviousRangeBounds('today') returns a full 24-hour yesterday. At 9am, users see "Sessions ▼67%" not because they're less productive, but because only 9/24 hours have elapsed. This erodes trust in the delta indicators.

Fix: for the 'today' previous period, compare against the same time window yesterday:

const windowMs = now - todayStart.getTime();
return {
  start: yesterdayStart.getTime(),
  end: yesterdayStart.getTime() + windowMs,
};

Or simply hide delta for the 'today' range since partial-day comparison is inherently noisy.

— qwen3.7-max via Qwen Code /review

[Suggestion] Dead code: if (sorted.length === 0) return null is unreachable

sorted is [...data].sort(...), and data.length === 0 already returned null six lines earlier (line 259). Sorting a non-empty array always produces a non-empty array, so this check can never trigger.

Remove the line to avoid misleading future readers.

— qwen3.7-max via Qwen Code /review

[Suggestion] Heatmap legend/cell visual mismatch for empty cells

The legend renders ' ' (two spaces) for intensity level 0, but actual heatmap cells use HEATMAP_CHARS[0] which is '··' (two middle dots) for empty days. Users see dots in the grid but a blank square in the legend for "Less".

Either use '··' in the legend to match the cells, or change HEATMAP_CHARS[0] to ' ' (spaces) to match the legend.

— qwen3.7-max via Qwen Code /review

[Suggestion] i18n key 'Failed to load stats. Press r to retry.' used via t() but not defined in en.js

This key is absent from packages/cli/src/i18n/locales/en.js. While i18next falls back to the key string (so English works), the key is invisible to the translation pipeline — it will silently remain untranslated in all 7 non-English locales.

Add it to en.js and all locale files.

— qwen3.7-max via Qwen Code /review

[Critical] IME physical cursor positioning feature silently reverted + ink unpinned

This change reverts PR #4652's IME cursor positioning (~75 lines from BaseTextInput.tsx, the ink+7.0.3.patch, and patch-package), and unpins ink from "7.0.3" to "^7.0.3". CJK/IME users will lose physical cursor positioning during composition.

The revert itself appears intentional (fixing a UI layout bug noted by @DragonnZhang), but:

1. The PR description does not mention this feature regression.
2. Unpinning ink to ^7.0.3 risks pulling newer versions with layout regressions in a TUI-critical app.

— qwen3.7-max via Qwen Code /review

IME cursor 功能(#4652)不属于本 PR 范围，是上游合并时意外带入的。该 commit 引入了 ink/dom 的未导出路径导致运行时崩溃(ERR_PACKAGE_PATH_NOT_EXPORTED)，必须 revert。ink 版本改为 ^7.0.3 是标准 semver 范围，不会引入 breaking change。

已确认 main 分支上 ink 版本也是 ^7.0.3，当前分支与 main 保持一致，无需修改。

[Critical] aggregateUsage sums all records without sessionId deduplication — duplicate records silently inflate all metrics

aggregateUsage iterates filtered records and unconditionally sums every counter. No Set or dedup by sessionId exists. If duplicate records appear in usage_record.jsonl (from concurrent rebuilds, concurrent persistSessionUsage calls, or manual file edits), all stats views are silently inflated — session count, token totals, streaks, leaderboard data.

— qwen3.7-max via Qwen Code /review

每个 session 仅在 /clear 或进程退出时 persist 一次(append-only)，正常使用不会产生重复记录。rebuildFromSessionJsonl 已有 seenSessionIds 去重。如果手动编辑文件导致重复，属于用户操作问题，不需要在热路径上增加额外开销。

[Critical] loadUsageHistory reads usage_record.jsonl with no file-size guard — a large file causes OOM on /stats

jsonl.read() accumulates all parsed records into an unbounded array. If the file grows unexpectedly (bug, concurrent writers, or external modification), the next /stats invocation loads everything into memory.