Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/ui/utils/statsDataService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export async function loadStatsData(
range: TimeRange,
currentSession?: UsageSummaryRecord,
): Promise<StatsData> {
const persisted = await loadUsageHistory();
const persisted = await loadUsageHistory(currentSession?.sessionId);
let records = persisted;
if (currentSession) {
records = persisted.filter((r) => r.sessionId !== currentSession.sessionId);
Expand Down
231 changes: 229 additions & 2 deletions packages/core/src/services/usageHistoryService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, it, expect } from 'vitest';
import { metricsToUsageRecord, aggregateUsage } from './usageHistoryService.js';
import { afterEach, beforeEach, describe, it, expect } from 'vitest';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {
metricsToUsageRecord,
aggregateUsage,
loadUsageHistory,
persistSessionUsage,
} from './usageHistoryService.js';
import { ToolCallDecision } from '../telemetry/tool-call-decision.js';
import type { SessionMetrics } from '../telemetry/uiTelemetry.js';
import type { UsageSummaryRecord } from './usageHistoryService.js';
Expand Down Expand Up @@ -349,3 +357,222 @@ describe('aggregateUsage', () => {
expect(report.tools.topTools).toEqual([]);
});
});

// Regression coverage for issue #4994: opening /stats during the first-ever
// turn followed by /clear or process exit used to write the same sessionId
// twice into usage_record.jsonl, permanently inflating every aggregate 2x.
describe('loadUsageHistory + persistSessionUsage (issue #4994 regression)', () => {
let tmpHome: string;
let originalQwenHome: string | undefined;

beforeEach(() => {
tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-usage-history-'));
originalQwenHome = process.env['QWEN_HOME'];
process.env['QWEN_HOME'] = path.join(tmpHome, '.qwen');
fs.mkdirSync(process.env['QWEN_HOME'], { recursive: true });
});

afterEach(() => {
if (originalQwenHome === undefined) delete process.env['QWEN_HOME'];
else process.env['QWEN_HOME'] = originalQwenHome;
fs.rmSync(tmpHome, { recursive: true, force: true });
});

function plantChatJsonl(sessionId: string, tokens: number) {
const cwd = '/repro/project';
const start = new Date('2026-06-11T00:00:00Z').toISOString();
const mid = new Date('2026-06-11T00:01:00Z').toISOString();
const end = new Date('2026-06-11T00:02:00Z').toISOString();
const projDir = path.join(
process.env['QWEN_HOME']!,
'projects',
'repro-project',
);
fs.mkdirSync(path.join(projDir, 'chats'), { recursive: true });
const records = [
{
sessionId,
cwd,
uuid: 'u1',
parentUuid: null,
timestamp: start,
type: 'user',
message: { role: 'user', content: 'hi' },
},
{
sessionId,
cwd,
uuid: 'u2',
parentUuid: 'u1',
timestamp: mid,
type: 'system',
subtype: 'ui_telemetry',
systemPayload: {
uiEvent: {
'event.name': 'qwen-code.api_response',
'event.timestamp': mid,
response_id: 'r1',
model: 'qwen-max',
duration_ms: 1200,
input_token_count: tokens * 0.6,
output_token_count: tokens * 0.3,
cached_content_token_count: 0,
thoughts_token_count: tokens * 0.1,
total_token_count: tokens,
prompt_id: 'p1',
},
},
},
{
sessionId,
cwd,
uuid: 'u3',
parentUuid: 'u2',
timestamp: end,
type: 'assistant',
message: { role: 'assistant', content: 'ok' },
},
];
fs.writeFileSync(
path.join(projDir, 'chats', `${sessionId}.jsonl`),
records.map((r) => JSON.stringify(r)).join('\n') + '\n',
);
}

function makeLiveMetrics(tokens: number): SessionMetrics {
return {
models: {
'qwen-max': {
api: { totalRequests: 1, totalErrors: 0, totalLatencyMs: 1200 },
tokens: {
prompt: tokens * 0.6,
candidates: tokens * 0.3,
total: tokens,
cached: 0,
thoughts: tokens * 0.1,
},
bySource: {},
},
},
tools: {
totalCalls: 0,
totalSuccess: 0,
totalFail: 0,
totalDurationMs: 0,
totalDecisions: {
[ToolCallDecision.ACCEPT]: 0,
[ToolCallDecision.REJECT]: 0,
[ToolCallDecision.MODIFY]: 0,
[ToolCallDecision.AUTO_ACCEPT]: 0,
},
byName: {},
},
files: { totalLinesAdded: 0, totalLinesRemoved: 0 },
};
}

it('read-side: dedups duplicate sessionId records already on disk (last-wins)', async () => {
// Simulate a usage_record.jsonl already corrupted by the pre-fix bug:
// two records with the same sessionId.
const sessionId = 'sess-dup-1';
const usagePath = path.join(
process.env['QWEN_HOME']!,
'usage_record.jsonl',
);
const rec = (totalTokens: number) => ({
version: 1 as const,
sessionId,
timestamp: Date.now(),
startTime: Date.now() - 60000,
project: '/p',
durationMs: 60000,
totalLatencyMs: 1200,
models: {
'qwen-max': {
requests: 1,
inputTokens: totalTokens * 0.6,
outputTokens: totalTokens * 0.3,
cachedTokens: 0,
thoughtsTokens: totalTokens * 0.1,
totalTokens,
},
},
tools: { totalCalls: 0, totalSuccess: 0, totalFail: 0, byName: {} },
files: { linesAdded: 0, linesRemoved: 0 },
});
fs.writeFileSync(
usagePath,
JSON.stringify(rec(1000)) + '\n' + JSON.stringify(rec(1600)) + '\n',
);

const records = await loadUsageHistory();

expect(records).toHaveLength(1);
// Last-wins: the second record (1600 tokens) survives.
expect(records[0]!.models['qwen-max']!.totalTokens).toBe(1600);

const report = aggregateUsage(records, 'all');
expect(report.sessionCount).toBe(1);
});

it('write-side: rebuildFromSessionJsonl skips the in-progress session when skipSessionInRebuild is passed', async () => {
const sessionId = 'sess-in-progress';
plantChatJsonl(sessionId, 1600);
const usagePath = path.join(
process.env['QWEN_HOME']!,
'usage_record.jsonl',
);

// First /stats open during the live session.
const first = await loadUsageHistory(sessionId);
expect(first).toHaveLength(1);
// Critically: the file must NOT contain the in-progress session.
expect(fs.existsSync(usagePath)).toBe(false);

// /clear or process exit writes the authoritative record exactly once.
persistSessionUsage({
sessionId,
startTime: new Date('2026-06-11T00:00:00Z'),
endTime: new Date('2026-06-11T00:02:00Z'),
project: '/repro/project',
metrics: makeLiveMetrics(1600),
});
const lines = fs.readFileSync(usagePath, 'utf8').trim().split('\n');
expect(lines).toHaveLength(1);

// Subsequent /stats open after session end aggregates exactly one record.
const second = await loadUsageHistory();
expect(second).toHaveLength(1);
const report = aggregateUsage(second, 'all');
expect(report.sessionCount).toBe(1);
let totalTokens = 0;
for (const m of Object.values(report.models)) totalTokens += m.totalTokens;
expect(totalTokens).toBe(1600);
});

it('end-to-end: /stats during first turn + /clear must not 2x the session', async () => {
const sessionId = 'sess-e2e';
plantChatJsonl(sessionId, 1600);

// Step 1: open /stats (first time) during the live session.
await loadUsageHistory(sessionId);

// Step 2: /clear or exit.
persistSessionUsage({
sessionId,
startTime: new Date('2026-06-11T00:00:00Z'),
endTime: new Date('2026-06-11T00:02:00Z'),
project: '/repro/project',
metrics: makeLiveMetrics(1600),
});

// Step 3: re-open /stats.
const records = await loadUsageHistory();
const report = aggregateUsage(records, 'all');

expect(report.sessionCount).toBe(1);
let totalTokens = 0;
for (const m of Object.values(report.models)) totalTokens += m.totalTokens;
expect(totalTokens).toBe(1600);
});
});
31 changes: 27 additions & 4 deletions packages/core/src/services/usageHistoryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ export function metricsToUsageRecord(
};
}

async function rebuildFromSessionJsonl(): Promise<UsageSummaryRecord[]> {
async function rebuildFromSessionJsonl(
skipSessionInRebuild?: string,
): Promise<UsageSummaryRecord[]> {
const projectsDir = path.join(Storage.getGlobalQwenDir(), 'projects');
try {
if (!fs.existsSync(projectsDir)) return [];
Expand Down Expand Up @@ -261,23 +263,44 @@ async function rebuildFromSessionJsonl(): Promise<UsageSummaryRecord[]> {
if (results.length > 0) {
const usagePath = getUsageHistoryPath();
for (const record of results) {
// Skip the in-progress current session: persistSessionUsage() will write
// its authoritative record on /clear or exit. Writing here would create
// a permanent duplicate in usage_record.jsonl (issue #4994).
if (skipSessionInRebuild && record.sessionId === skipSessionInRebuild)
continue;
jsonl.writeLineSync(usagePath, record);
}
}

return results;
}

export async function loadUsageHistory(): Promise<UsageSummaryRecord[]> {
function dedupBySessionId(records: UsageSummaryRecord[]): UsageSummaryRecord[] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] dedupBySessionId silently discards duplicate sessionId records with no diagnostic trace. When duplicates are present (evidence of the #4994 bug having affected a user's data), there is no way to measure how many users were affected or detect recurrence.

Suggested change
function dedupBySessionId(records: UsageSummaryRecord[]): UsageSummaryRecord[] {
function dedupBySessionId(records: UsageSummaryRecord[]): UsageSummaryRecord[] {
const map = new Map<string, UsageSummaryRecord>();
for (const r of records) map.set(r.sessionId, r);
if (map.size < records.length) {
debugLogger.debug(
`dedupBySessionId: removed ${records.length - map.size} duplicate record(s)`,
);
}
return [...map.values()];
}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted in a7c7206 — added a debugLogger.debug line that fires only when duplicates were actually removed:

if (map.size < records.length) {
  debugLogger.debug(
    `dedupBySessionId: removed ${records.length - map.size} duplicate record(s)`,
  );
}

This gives us a way to measure #4994's blast radius on real user data without changing aggregate behavior. Thanks for the catch.

// Last-wins by sessionId. Protects existing users whose usage_record.jsonl
// already contains duplicates produced by the bug fixed in this change
// (issue #4994) — without this, every aggregate stays inflated forever.
const map = new Map<string, UsageSummaryRecord>();
for (const r of records) map.set(r.sessionId, r);
if (map.size < records.length) {
debugLogger.debug(
`dedupBySessionId: removed ${records.length - map.size} duplicate record(s)`,
);
}
return [...map.values()];
}

export async function loadUsageHistory(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new currentSessionId parameter in loadUsageHistory is silently ignored on the hot read path — it only takes effect in the rebuildFromSessionJsonl fallback (when usage_record.jsonl doesn't exist or has no v1 records). On the main path, the parameter is passed but the current session is still returned. loadStatsData handles its own filtering (line 251), so correctness is fine, but the API is misleading: a future caller passing currentSessionId would reasonably expect the session to be excluded from results regardless of file state.

Consider either: (a) applying the skip consistently on both paths inside loadUsageHistory and removing the redundant filter from loadStatsData, or (b) renaming the parameter to clarify its limited scope (e.g., skipSessionInRebuild).

— DeepSeek/deepseek-v4-pro via Qwen Code /review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point — the parameter was misleading. Went with renaming over consolidating, in commit a7c7206:

  • currentSessionIdskipSessionInRebuild in both loadUsageHistory and rebuildFromSessionJsonl
  • Zero behavior change, zero test-assertion change

I considered option (a) — apply the skip on both paths and drop the filter in loadStatsData — but kept (b) because:

  1. loadStatsData does defense-in-depth: it strips the persisted current-session record and pushes the live in-memory currentSession (with up-to-date metrics) back. Even if loadUsageHistory filtered the current session out on the read path, the caller would still need to push the live record. So option (a) wouldn't actually let us delete the filter, only relocate it.
  2. The existing read-side dedupBySessionId already protects against a stale persisted current-session record being double-counted (last-wins). So the file-path skip in option (a) would be redundant with dedup.
  3. Option (b) is a pure rename with no test-assertion diff and no public-API surface change.

Let me know if you'd still prefer (a) — happy to follow up.

skipSessionInRebuild?: string,
): Promise<UsageSummaryRecord[]> {
try {
const records = await jsonl.read<UsageSummaryRecord>(getUsageHistoryPath());
const filtered = records.filter((r) => r.version === 1);
if (filtered.length > 0) return filtered;
if (filtered.length > 0) return dedupBySessionId(filtered);
} catch (e) {
debugLogger.debug(`loadUsageHistory: failed to read usage file: ${e}`);
}

return rebuildFromSessionJsonl();
return dedupBySessionId(await rebuildFromSessionJsonl(skipSessionInRebuild));
}

export function getTimeRangeBounds(range: TimeRange): {
Expand Down
Loading