From 3ecd2a1b33a21b7c898ce4d000b942c21bc1ebb2 Mon Sep 17 00:00:00 2001 From: Halil Tezcan KARABULUT Date: Fri, 29 May 2026 22:34:07 +0300 Subject: [PATCH 1/2] fix(analytics): address merged review regressions --- src/app/api/usage/analytics/route.ts | 40 ++++++++++---- src/lib/db/cleanup.ts | 27 ++++++---- src/lib/usage/aggregateHistory.ts | 4 +- src/shared/validation/settingsSchemas.ts | 7 ++- tests/unit/usage-analytics-route.test.ts | 69 ++++++++++++++++++++++++ 5 files changed, 126 insertions(+), 21 deletions(-) diff --git a/src/app/api/usage/analytics/route.ts b/src/app/api/usage/analytics/route.ts index 2d3f8f3c2d54..540f1f93d770 100644 --- a/src/app/api/usage/analytics/route.ts +++ b/src/app/api/usage/analytics/route.ts @@ -361,17 +361,29 @@ export async function GET(request: Request) { const whereClause = conditions.length > 0 ? `WHERE ${conditions.join(" AND ")}` : ""; // Build a UNION data source that merges recent raw rows with aggregated history. - // daily_usage_summary rows are included only when the query window extends before rawCutoffIso. - // The api_key filter is intentionally NOT applied to daily_usage_summary (api_key not stored there). - const needsAggregated = !sinceIso || sinceIso < rawCutoffIso; - + // daily_usage_summary rows are included only when the query window extends before + // rawCutoffIso. They are gated off entirely when an api_key filter is active: + // daily_usage_summary does not store api_key/connection, so including it under a + // key filter would leak other keys' aggregated usage. With a key filter we serve + // only raw rows (older key-scoped data beyond retention is intentionally unavailable). + const rawCutoffDate = rawCutoffIso.split("T")[0]; + const needsAggregated = (!sinceIso || sinceIso < rawCutoffDate) && apiKeyIds.length === 0; + + // Raw leg: when aggregated rows are also included, lower-bound the raw leg at the + // raw cutoff so the two legs never overlap (prevents double-counting). const rawConditions: string[] = []; - if (sinceIso) rawConditions.push("timestamp >= @since"); + if (needsAggregated) { + rawConditions.push("timestamp >= @rawCutoff"); + params.rawCutoff = rawCutoffDate; + } else if (sinceIso) { + rawConditions.push("timestamp >= @since"); + } if (untilIso) rawConditions.push("timestamp <= @until"); if (apiKeyWhere) rawConditions.push(apiKeyWhere); const rawWhere = rawConditions.length > 0 ? `WHERE ${rawConditions.join(" AND ")}` : ""; - // Aggregated rows only span dates within the requested window (no api_key filter). + // Aggregated rows span the requested window but strictly before the raw cutoff, + // so they never overlap the raw leg above (no api_key filter — see note above). const aggConditions: string[] = []; if (sinceIso) { // Use date comparison on the summary's date column (YYYY-MM-DD). @@ -384,6 +396,8 @@ export async function GET(request: Request) { aggConditions.push("date <= @untilDate"); params.untilDate = untilDate; } + aggConditions.push("date < @rawCutoffDate"); + params.rawCutoffDate = rawCutoffDate; const aggWhere = aggConditions.length > 0 ? `WHERE ${aggConditions.join(" AND ")}` : ""; // Unified source CTE: columns aligned to usage_history shape needed by analytics queries. @@ -1209,10 +1223,16 @@ export async function GET(request: Request) { const presetParams: Record = {}; // Build unified source for preset cost queries (same UNION logic as main query). - const presetNeedsAggregated = !presetSinceIso || presetSinceIso < rawCutoffIso; + // Aggregated rows are gated off when an api_key filter is active (leakage) and + // bounded strictly before the raw cutoff (overlap / double-count) — see main query. + const presetNeedsAggregated = + (!presetSinceIso || presetSinceIso < rawCutoffDate) && apiKeyIds.length === 0; const presetRawConds: string[] = []; - if (presetSinceIso) { + if (presetNeedsAggregated) { + presetRawConds.push("timestamp >= @presetRawCutoff"); + presetParams.presetRawCutoff = rawCutoffDate; + } else if (presetSinceIso) { presetRawConds.push("timestamp >= @presetSince"); presetParams.presetSince = presetSinceIso; } @@ -1229,6 +1249,8 @@ export async function GET(request: Request) { presetAggConds.push("date >= @presetSinceDate"); presetParams.presetSinceDate = presetSinceDate; } + presetAggConds.push("date < @presetRawCutoffDate"); + presetParams.presetRawCutoffDate = rawCutoffDate; const presetAggWhere = presetAggConds.length > 0 ? `WHERE ${presetAggConds.join(" AND ")}` : ""; @@ -1252,7 +1274,7 @@ export async function GET(request: Request) { FROM daily_usage_summary ${presetAggWhere} )` - : `(SELECT timestamp, provider, model, service_tier, + : `(SELECT timestamp, provider, model, service_tier, tokens_input, tokens_output, tokens_cache_read, tokens_cache_creation, tokens_reasoning FROM usage_history diff --git a/src/lib/db/cleanup.ts b/src/lib/db/cleanup.ts index 93c1d1d7e5f4..e49fd2b92f35 100644 --- a/src/lib/db/cleanup.ts +++ b/src/lib/db/cleanup.ts @@ -28,6 +28,7 @@ export async function cleanupQuotaSnapshots(): Promise { const cutoffDate = new Date(); cutoffDate.setDate(cutoffDate.getDate() - retentionDays); const cutoffISO = cutoffDate.toISOString(); + const cutoffDateStr = cutoffISO.split("T")[0]; const result: CleanupResult = { deleted: 0, errors: 0 }; @@ -86,22 +87,30 @@ export async function cleanupUsageHistory(): Promise { const cutoffDate = new Date(); cutoffDate.setDate(cutoffDate.getDate() - retentionDays); const cutoffISO = cutoffDate.toISOString(); - const cutoffDateStr = cutoffISO.split("T")[0]; const result: CleanupResult = { deleted: 0, errors: 0 }; - try { - // Roll up rows that are about to be deleted into daily_usage_summary so that - // the analytics route can still surface historical data via the UNION query. - await rollupUsageHistoryBeforeDate(cutoffDateStr); - } catch (err: unknown) { - // Non-fatal: log but continue with deletion so cleanup still runs. - console.error("[Cleanup] Error rolling up usage_history before deletion:", err); + // Roll up rows that are about to be deleted into daily_usage_summary so that the + // analytics route can still surface historical data via the UNION query. The rollup + // uses the exact same day boundary as the DELETE below, so every deleted row + // is guaranteed to have been aggregated first. + // + // rollupUsageHistoryBeforeDate catches its own errors and reports them via the + // returned result, so we inspect that rather than relying on a thrown exception. + // If the rollup failed, abort the DELETE to avoid permanently losing raw usage data + // that was never aggregated. + const rollupResult = await rollupUsageHistoryBeforeDate(cutoffDateStr); + if (rollupResult.errors > 0) { + console.error( + "[Cleanup] Aborting usage_history deletion because the pre-delete rollup failed." + ); + result.errors += rollupResult.errors; + return result; } try { const stmt = db.prepare("DELETE FROM usage_history WHERE timestamp < ?"); - const runResult = stmt.run(cutoffISO); + const runResult = stmt.run(cutoffDateStr); result.deleted = runResult.changes; console.log( diff --git a/src/lib/usage/aggregateHistory.ts b/src/lib/usage/aggregateHistory.ts index 9841ad1db5a6..b54154001617 100644 --- a/src/lib/usage/aggregateHistory.ts +++ b/src/lib/usage/aggregateHistory.ts @@ -138,7 +138,7 @@ export async function rollupHourlyQuota( * The ON CONFLICT clause uses SUM so re-running is additive-safe: if a date already * has a partial rollup (e.g. from a previous partial cleanup), new rows accumulate. * - * @param beforeDate - ISO date string (YYYY-MM-DD). Rows strictly before this date are rolled up. + * @param beforeDate - ISO timestamp/date boundary. Rows strictly before this value are rolled up. * @returns Aggregation result with counts */ export async function rollupUsageHistoryBeforeDate(beforeDate: string): Promise { @@ -162,7 +162,7 @@ export async function rollupUsageHistoryBeforeDate(beforeDate: string): Promise< COALESCE(SUM(tokens_output), 0) as total_output_tokens, 0.0 as total_cost FROM usage_history - WHERE DATE(timestamp) < ? + WHERE timestamp < ? AND provider IS NOT NULL AND provider != '' AND model IS NOT NULL AND model != '' GROUP BY LOWER(provider), LOWER(model), DATE(timestamp) diff --git a/src/shared/validation/settingsSchemas.ts b/src/shared/validation/settingsSchemas.ts index 9cefbab6bfcb..d1746d953dec 100644 --- a/src/shared/validation/settingsSchemas.ts +++ b/src/shared/validation/settingsSchemas.ts @@ -8,7 +8,7 @@ import { z } from "zod"; import { COMBO_CONFIG_MODES } from "@/shared/constants/comboConfigMode"; import { MAX_REQUEST_BODY_LIMIT_MB, MIN_REQUEST_BODY_LIMIT_MB } from "@/shared/constants/bodySize"; -import { HIDEABLE_SIDEBAR_ITEM_IDS } from "@/shared/constants/sidebarVisibility"; +import { HIDEABLE_SIDEBAR_ITEM_IDS, SIDEBAR_SECTIONS } from "@/shared/constants/sidebarVisibility"; import { ACCOUNT_FALLBACK_STRATEGY_VALUES } from "@/shared/constants/routingStrategies"; const signatureCacheModeValues = ["enabled", "bypass", "bypass-strict"] as const; @@ -38,6 +38,11 @@ export const updateSettingsSchema = z.object({ autoRefreshProviderQuotaInterval: z.number().int().min(10).max(3600).optional(), debugMode: z.boolean().optional(), hiddenSidebarItems: z.array(z.enum(HIDEABLE_SIDEBAR_ITEM_IDS)).optional(), + sidebarSectionOrder: z + .array(z.enum(SIDEBAR_SECTIONS.map((s) => s.id) as [string, ...string[]])) + .optional(), + sidebarItemOrder: z.record(z.string(), z.array(z.string().max(100))).optional(), + sidebarActivePreset: z.enum(["all", "minimal", "developer", "admin"]).nullable().optional(), comboConfigMode: z.enum(COMBO_CONFIG_MODES).optional(), codexServiceTier: z .object({ diff --git a/tests/unit/usage-analytics-route.test.ts b/tests/unit/usage-analytics-route.test.ts index 59905699eeaf..f7989d4b36ab 100644 --- a/tests/unit/usage-analytics-route.test.ts +++ b/tests/unit/usage-analytics-route.test.ts @@ -333,6 +333,75 @@ test("GET /api/usage/analytics includes cost by API key", async () => { assertClose(body.byApiKey[0].cost, body.summary.totalCost); }); +test("GET /api/usage/analytics does not double-count raw and aggregated rows", async () => { + const db = core.getDbInstance(); + const today = new Date(); + const todayStr = today.toISOString().split("T")[0]; + const cutoffDate = new Date(); + cutoffDate.setDate(cutoffDate.getDate() - 30); + const olderDate = new Date(cutoffDate); + olderDate.setDate(olderDate.getDate() - 1); + const olderDateStr = olderDate.toISOString().split("T")[0]; + + db.prepare( + `INSERT INTO usage_history (provider, model, connection_id, tokens_input, tokens_output, success, latency_ms, timestamp) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + ).run("openai", "gpt-4o", "raw-current", 100, 50, 1, 200, today.toISOString()); + + const insertSummary = db.prepare( + `INSERT INTO daily_usage_summary (provider, model, date, total_requests, total_input_tokens, total_output_tokens, total_cost) + VALUES (?, ?, ?, ?, ?, ?, ?)` + ); + insertSummary.run("openai", "gpt-4o", todayStr, 99, 9900, 9900, 0); + insertSummary.run("openai", "gpt-4o", olderDateStr, 1, 25, 10, 0); + + const response = await analyticsRoute.GET( + makeRequest("http://localhost/api/usage/analytics?range=all") + ); + const body = await response.json(); + + assert.equal(response.status, 200); + assert.equal(body.summary.totalRequests, 2); + assert.equal(body.summary.totalTokens, 185); +}); + +test("GET /api/usage/analytics omits global aggregates when filtering by API key", async () => { + const apiKey = await apiKeysDb.createApiKey("Scoped Key", "machine1234567890"); + const db = core.getDbInstance(); + + db.prepare( + `INSERT INTO usage_history (provider, model, connection_id, api_key_id, api_key_name, tokens_input, tokens_output, success, latency_ms, timestamp) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ).run( + "openai", + "gpt-4o", + "scoped-conn", + apiKey.id, + "Scoped Key", + 100, + 50, + 1, + 200, + new Date().toISOString() + ); + + db.prepare( + `INSERT INTO daily_usage_summary (provider, model, date, total_requests, total_input_tokens, total_output_tokens, total_cost) + VALUES (?, ?, ?, ?, ?, ?, ?)` + ).run("openai", "gpt-4o", "2024-01-01", 99, 9900, 9900, 0); + + const response = await analyticsRoute.GET( + makeRequest(`http://localhost/api/usage/analytics?range=all&apiKeyIds=${apiKey.id}`) + ); + const body = await response.json(); + + assert.equal(response.status, 200); + assert.equal(body.summary.totalRequests, 1); + assert.equal(body.summary.totalTokens, 150); + assert.equal(body.byApiKey.length, 1); + assert.equal(body.byApiKey[0].apiKeyId, apiKey.id); +}); + test("GET /api/usage/analytics groups renamed API key usage by stable ID", async () => { const apiKey = await apiKeysDb.createApiKey("Averyanov", "machine1234567890"); await apiKeysDb.updateApiKeyPermissions(apiKey.id, { name: "Alexander Averyanov" }); From b5f0d72b5c243f5f270fbf1e65fe819ba65f0288 Mon Sep 17 00:00:00 2001 From: Halil Tezcan KARABULUT Date: Fri, 29 May 2026 23:08:34 +0300 Subject: [PATCH 2/2] fix(cleanup): restore usage history cutoff boundary --- src/lib/db/cleanup.ts | 2 +- .../database-settings-maintenance.test.ts | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/lib/db/cleanup.ts b/src/lib/db/cleanup.ts index e49fd2b92f35..9d67fb91516c 100644 --- a/src/lib/db/cleanup.ts +++ b/src/lib/db/cleanup.ts @@ -28,7 +28,6 @@ export async function cleanupQuotaSnapshots(): Promise { const cutoffDate = new Date(); cutoffDate.setDate(cutoffDate.getDate() - retentionDays); const cutoffISO = cutoffDate.toISOString(); - const cutoffDateStr = cutoffISO.split("T")[0]; const result: CleanupResult = { deleted: 0, errors: 0 }; @@ -87,6 +86,7 @@ export async function cleanupUsageHistory(): Promise { const cutoffDate = new Date(); cutoffDate.setDate(cutoffDate.getDate() - retentionDays); const cutoffISO = cutoffDate.toISOString(); + const cutoffDateStr = cutoffISO.split("T")[0]; const result: CleanupResult = { deleted: 0, errors: 0 }; diff --git a/tests/unit/database-settings-maintenance.test.ts b/tests/unit/database-settings-maintenance.test.ts index fa553eb68772..449ee5a4eaa5 100644 --- a/tests/unit/database-settings-maintenance.test.ts +++ b/tests/unit/database-settings-maintenance.test.ts @@ -186,3 +186,36 @@ test("usage aggregation upserts replace recomputed totals instead of adding them assert.equal(hourly.total_output_tokens, 10); assert.equal(hourly.total_cost, 0.75); }); + +test("cleanupUsageHistory rolls up and deletes old rows using the same day boundary", async () => { + const db = core.getDbInstance(); + const oldTimestamp = "2024-01-01T12:00:00.000Z"; + const recentTimestamp = new Date().toISOString(); + + databaseSettings.updateDatabaseSettings({ + retention: { + ...databaseSettings.getUserDatabaseSettings().retention, + usageHistory: 30, + }, + }); + + const insertUsage = db.prepare( + `INSERT INTO usage_history (provider, model, timestamp, tokens_input, tokens_output, success, latency_ms) + VALUES (?, ?, ?, ?, ?, ?, ?)` + ); + insertUsage.run("openai", "gpt-test", oldTimestamp, 100, 40, 1, 200); + insertUsage.run("openai", "gpt-test", recentTimestamp, 7, 3, 1, 100); + + const result = await cleanup.cleanupUsageHistory(); + + assert.equal(result.errors, 0); + assert.equal(result.deleted, 1); + + const remaining = db.prepare("SELECT COUNT(*) AS count FROM usage_history").get() as CountRow; + assert.equal(remaining.count, 1); + + const daily = db.prepare("SELECT * FROM daily_usage_summary").get() as UsageSummaryRow; + assert.equal(daily.total_requests, 1); + assert.equal(daily.total_input_tokens, 100); + assert.equal(daily.total_output_tokens, 40); +});