Compression telemetry retention has never deleted a row (same unit bug as #9625) (#10559)

* fix(db): align compression_run_telemetry cleanup cutoff with millisecond column

cleanupCompressionRunTelemetry() computed its cutoff in epoch seconds while
insertCompressionRunTelemetryRow() stamps the timestamp column with Date.now()
(epoch milliseconds). A millisecond timestamp is ~1000x larger than a seconds
cutoff, so DELETE WHERE timestamp < cutoff never matched an old row and the
retention sweep added by #6848 to bound storage.sqlite growth was inert.

This is the same defect as domain_cost_history (#9625), whose fix corrected
cleanupDomainCostHistory() ~90 lines earlier in this file and missed this
sibling call site. The stale docstring asserting a unix-epoch column is
corrected too.

The repro test seeds through the real writer to establish the stored unit, so
it also fails if the producer format diverges from the consumer again.

* docs(changelog): add fragment for the telemetry retention unit fix

changelog.d/fixes/compression-run-telemetry-retention-ms.md

@@ -0,0 +1 @@
+- **fix(db):** the `compression_run_telemetry` retention sweep now actually deletes expired rows. Its cutoff was computed in epoch seconds while the column stores epoch milliseconds, so `WHERE timestamp < cutoff` never matched and the table added by #6848 to bound `storage.sqlite` growth was unbounded in practice. Same unit mismatch as #9625, which corrected the sibling `domain_cost_history` sweep and missed this call site

src/lib/db/cleanup.ts

@@ -344,14 +344,16 @@ export async function cleanupXpAuditLog(): Promise<CleanupResult> {
 
 /**
  * Clean up old compression_run_telemetry based on retention settings. (#6848)
- * Uses unix-epoch `timestamp` column (INTEGER).
+ * The `timestamp` column stores epoch milliseconds (recordCompressionRun stamps
+ * Date.now()), so the cutoff must be in milliseconds to match. Same unit bug as
+ * domain_cost_history (#9625), which this function was missed by.
  */
 export async function cleanupCompressionRunTelemetry(): Promise<CleanupResult> {
   const db = getDbInstance();
   const retention = getRetentionSettings();
 
   const retentionDays = retention.compressionRunTelemetry;
-  const cutoffEpoch = Math.floor(Date.now() / 1000) - retentionDays * 86_400;
+  const cutoffEpoch = Date.now() - retentionDays * 86_400_000;
 
   const result: CleanupResult = { deleted: 0, errors: 0 };
 

tests/unit/repro-compression-run-telemetry-ms.test.ts

@@ -0,0 +1,99 @@
+/**
+ * compression_run_telemetry cleanup cutoff unit mismatch.
+ *
+ * cleanupCompressionRunTelemetry() computed its cutoff in epoch seconds
+ * (Math.floor(Date.now() / 1000)) while the timestamp column stores epoch
+ * milliseconds, stamped by insertCompressionRunTelemetryRow() as Date.now().
+ *
+ * This is the same defect as domain_cost_history (#9625). That fix corrected
+ * cleanupDomainCostHistory() ~90 lines earlier in cleanup.ts and missed this
+ * sibling call site, so the retention sweep added by #6848 to bound
+ * storage.sqlite growth has been permanently inert: a millisecond timestamp is
+ * ~1000x larger than a seconds cutoff, so `WHERE timestamp < cutoff` never
+ * matched an old row.
+ *
+ * The first test uses the REAL writer to establish the stored unit, so it
+ * cannot pass if the producer's format ever changes independently.
+ */
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-crt-ms-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { cleanupCompressionRunTelemetry } = await import("../../src/lib/db/cleanup.ts");
+const { insertCompressionRunTelemetryRow } = await import(
+  "../../src/lib/db/compressionRunTelemetry.ts"
+);
+const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts");
+
+test.after(() => {
+  resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+const DAY_MS = 86_400_000;
+
+function seedRow(requestId: string): void {
+  insertCompressionRunTelemetryRow({
+    requestId,
+    model: "m",
+    provider: "p",
+    source: "s",
+    tokensBefore: 100,
+    tokensAfter: 50,
+    ratio: 0.5,
+  });
+}
+
+test("the writer stamps epoch milliseconds, not seconds", () => {
+  seedRow("req-unit-probe");
+  const db = getDbInstance()!;
+  const row = db
+    .prepare("SELECT timestamp FROM compression_run_telemetry WHERE request_id = ?")
+    .get("req-unit-probe") as { timestamp: number };
+
+  // A seconds stamp is ~1.7e9; a milliseconds stamp is ~1.7e12.
+  assert.ok(
+    row.timestamp > 1e12,
+    `timestamp ${row.timestamp} is not millisecond-scale; the cleanup cutoff unit must match the writer`
+  );
+});
+
+test("cleanupCompressionRunTelemetry deletes rows older than the retention window", async () => {
+  const db = getDbInstance()!;
+  db.exec("DELETE FROM compression_run_telemetry");
+
+  // Insert through the real writer, then backdate to simulate age. Backdating
+  // preserves the producer's unit while letting the test control the age.
+  const now = Date.now();
+  for (const id of ["old-1", "old-2", "old-3"]) {
+    seedRow(id);
+    db.prepare("UPDATE compression_run_telemetry SET timestamp = ? WHERE request_id = ?").run(
+      now - 40 * DAY_MS,
+      id
+    );
+  }
+  for (const id of ["new-1", "new-2"]) {
+    seedRow(id);
+    db.prepare("UPDATE compression_run_telemetry SET timestamp = ? WHERE request_id = ?").run(
+      now - 5 * DAY_MS,
+      id
+    );
+  }
+
+  const result = await cleanupCompressionRunTelemetry();
+
+  // With the pre-fix seconds cutoff this asserted 0 deleted: the sweep was a no-op.
+  assert.strictEqual(result.deleted, 3, "should delete the 3 rows aged 40 days");
+  assert.strictEqual(result.errors, 0);
+
+  const remaining = db
+    .prepare("SELECT COUNT(*) as cnt FROM compression_run_telemetry")
+    .get() as { cnt: number };
+  assert.strictEqual(remaining.cnt, 2, "should keep the 2 rows aged 5 days");
+});

tests/unit/telemetry-auto-cleanup-6848.test.ts

@@ -47,7 +47,6 @@ test.after(() => {
   fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
 });
 
-const DAY_SECONDS = 86_400;
 const DAY_MS = 86_400_000;
 
 /** Ensure compression_run_telemetry table exists (created lazily in production). */
@@ -161,14 +160,15 @@ test("#6848 cleanupCompressionRunTelemetry: deletes rows older than retention wi
   ensureTelemetryTable();
   const db = getDbInstance()!;
   const now = Date.now();
-  const nowSeconds = Math.floor(now / 1000);
   const insert = db.prepare(
     "INSERT INTO compression_run_telemetry (timestamp, tokens_before, tokens_after) VALUES (?, ?, ?)"
   );
 
-  insert.run(nowSeconds - 40 * DAY_SECONDS, 1000, 500);
-  insert.run(nowSeconds - 40 * DAY_SECONDS, 2000, 800);
-  insert.run(nowSeconds - 5 * DAY_SECONDS, 1500, 600);
+  // insertCompressionRunTelemetryRow() stamps Date.now() — epoch MILLISECONDS.
+  // Seeding seconds here encoded the same unit mismatch the cleanup had.
+  insert.run(now - 40 * DAY_MS, 1000, 500);
+  insert.run(now - 40 * DAY_MS, 2000, 800);
+  insert.run(now - 5 * DAY_MS, 1500, 600);
 
   const result = await cleanupCompressionRunTelemetry();
 
@@ -184,7 +184,6 @@ test("#6848 cleanupCompressionRunTelemetry: deletes rows older than retention wi
 test("#6848 no rows deleted when all data is within retention window (calls all 4 real functions)", async () => {
   ensureTelemetryTable();
   const db = getDbInstance()!;
-  const nowSeconds = Math.floor(Date.now() / 1000);
   const nowMilliseconds = Date.now();
   const recentISO = new Date().toISOString();
 
@@ -201,7 +200,7 @@ test("#6848 no rows deleted when all data is within retention window (calls all
   ).run("k", "a", 5, recentISO);
   db.prepare(
     "INSERT INTO compression_run_telemetry (timestamp, tokens_before, tokens_after) VALUES (?, ?, ?)"
-  ).run(nowSeconds - DAY_SECONDS, 100, 50);
+  ).run(nowMilliseconds - DAY_MS, 100, 50);
 
   const r1 = await cleanupDomainCostHistory();
   const r2 = await cleanupCompressionCacheStats();