From 73553c4c2c1daf4ab9b302e6d427aac52fe07165 Mon Sep 17 00:00:00 2001 From: digua Date: Sat, 4 Jul 2026 14:55:01 +0800 Subject: [PATCH] fix(query): clamp SQL rows and stabilize response stats Clamp CLI SQL output after execution so user-provided LIMIT clauses cannot bypass the requested row cap. Add a deterministic message id tie-breaker for response-time statistics. --- apps/cli/src/query/commands-stats.test.ts | 13 +++++++++++++ apps/cli/src/query/commands-stats.ts | 14 +++++++++----- apps/cli/src/query/commands-topics-sql.test.ts | 11 +++++++++++ apps/cli/src/query/commands-topics-sql.ts | 18 +++++++++++++++--- 4 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 apps/cli/src/query/commands-stats.test.ts diff --git a/apps/cli/src/query/commands-stats.test.ts b/apps/cli/src/query/commands-stats.test.ts new file mode 100644 index 00000000..470811dc --- /dev/null +++ b/apps/cli/src/query/commands-stats.test.ts @@ -0,0 +1,13 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' + +import { responseStatsMessagesSql } from './commands-stats' + +describe('responseStatsMessagesSql', () => { + it('orders same-second messages by id for deterministic response gaps', () => { + const sql = responseStatsMessagesSql() + + assert.match(sql, /SELECT\s+msg\.id,/i) + assert.match(sql, /ORDER BY msg\.ts ASC, msg\.id ASC/i) + }) +}) diff --git a/apps/cli/src/query/commands-stats.ts b/apps/cli/src/query/commands-stats.ts index 26f9b9c7..c9fc4d16 100644 --- a/apps/cli/src/query/commands-stats.ts +++ b/apps/cli/src/query/commands-stats.ts @@ -51,6 +51,14 @@ function toTimeFilter(time: ReturnType): { startTs: num return { startTs: time.startTs ?? 0, endTs: time.endTs ?? Math.floor(Date.now() / 1000) } } +export function responseStatsMessagesSql(): string { + return `SELECT msg.id, msg.sender_id, COALESCE(m.group_nickname, m.account_name) AS name, msg.ts + FROM message msg + JOIN member m ON msg.sender_id = m.id + WHERE msg.type = 0 AND msg.ts >= @startTs + ORDER BY msg.ts ASC, msg.id ASC` +} + export function registerStatsCommands(program: Command): void { const statsCmd = program .command('stats') @@ -259,11 +267,7 @@ export function registerStatsCommands(program: Command): void { const top = parseLimit(options.top, 10, 100, '--top') const rows = executeParameterizedSql<{ sender_id: number; name: string; ts: number }>( ctx.db, - `SELECT msg.sender_id, COALESCE(m.group_nickname, m.account_name) AS name, msg.ts - FROM message msg - JOIN member m ON msg.sender_id = m.id - WHERE msg.type = 0 AND msg.ts >= @startTs - ORDER BY msg.ts ASC`, + responseStatsMessagesSql(), { startTs: time.startTs ?? 0 } ) const items = computeResponseTimeStats( diff --git a/apps/cli/src/query/commands-topics-sql.test.ts b/apps/cli/src/query/commands-topics-sql.test.ts index dc81c5cf..b4c12ffd 100644 --- a/apps/cli/src/query/commands-topics-sql.test.ts +++ b/apps/cli/src/query/commands-topics-sql.test.ts @@ -9,6 +9,7 @@ import assert from 'node:assert/strict' import { assertSqlPrivacyAllowed, assertSqlResultPrivacyAllowed, + clampSqlRows, parseSqlRowLimit, topicsAgentText, type TopicListItem, @@ -58,6 +59,16 @@ describe('parseSqlRowLimit', () => { }) }) +describe('clampSqlRows', () => { + it('clamps SQL rows even when the executed SQL already had a LIMIT', () => { + const rows = [{ id: 1 }, { id: 2 }, { id: 3 }] + const result = clampSqlRows(rows, 2, false) + + assert.deepEqual(result.rows, [{ id: 1 }, { id: 2 }]) + assert.equal(result.truncated, true) + }) +}) + describe('assertSqlPrivacyAllowed', () => { it('requires raw mode before SQL can read message content', () => { assert.throws( diff --git a/apps/cli/src/query/commands-topics-sql.ts b/apps/cli/src/query/commands-topics-sql.ts index e720d65a..482fda9f 100644 --- a/apps/cli/src/query/commands-topics-sql.ts +++ b/apps/cli/src/query/commands-topics-sql.ts @@ -24,6 +24,17 @@ export function parseSqlRowLimit(value: string | undefined): number { return parseLimit(value, 100, 1000, '--limit', 1) } +export function clampSqlRows( + rows: Record[], + maxRows: number, + alreadyTruncated: boolean +): { rows: Record[]; truncated: boolean } { + if (maxRows <= 0 || rows.length <= maxRows) { + return { rows, truncated: alreadyTruncated } + } + return { rows: rows.slice(0, maxRows), truncated: true } +} + function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } @@ -335,7 +346,8 @@ export function registerSqlCommands(program: Command): void { } assertSqlResultPrivacyAllowed(result.columns, !!options.raw) - let rows = result.rows + const clamped = clampSqlRows(result.rows, limit, result.truncated) + let rows = clamped.rows let removed = 0 if (!options.raw) { const rules = enabledRules(ctx) @@ -362,7 +374,7 @@ export function registerSqlCommands(program: Command): void { } return { - data: { columns: result.columns, rows, rowCount: rows.length, truncated: result.truncated }, + data: { columns: result.columns, rows, rowCount: rows.length, truncated: clamped.truncated }, meta: { session: ctx.session, ...(options.raw ? { preprocess: { raw: true } } : {}), @@ -372,7 +384,7 @@ export function registerSqlCommands(program: Command): void { rows.length === 0 ? 'No results.' : rows.map((r) => JSON.stringify(r)).join('\n') + - `\n${rows.length} row(s)${result.truncated ? ' (truncated)' : ''}`, + `\n${rows.length} row(s)${clamped.truncated ? ' (truncated)' : ''}`, } } finally { ctx.close()