From cff50aef86d90725a11922aa921a2a15fd49d9e0 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 29 Mar 2026 01:10:11 -0700 Subject: [PATCH] fix: guard disabled immersion retention windows --- ...-PR-36-latest-CodeRabbit-review-round-2.md | 28 +++++++++ .../immersion-tracker/maintenance.test.ts | 59 +++++++++++++++++++ .../services/immersion-tracker/maintenance.ts | 40 +++++++------ 3 files changed, 108 insertions(+), 19 deletions(-) create mode 100644 backlog/tasks/task-244 - Assess-and-address-PR-36-latest-CodeRabbit-review-round-2.md diff --git a/backlog/tasks/task-244 - Assess-and-address-PR-36-latest-CodeRabbit-review-round-2.md b/backlog/tasks/task-244 - Assess-and-address-PR-36-latest-CodeRabbit-review-round-2.md new file mode 100644 index 0000000..089900a --- /dev/null +++ b/backlog/tasks/task-244 - Assess-and-address-PR-36-latest-CodeRabbit-review-round-2.md @@ -0,0 +1,28 @@ +--- +id: TASK-244 +title: 'Assess and address PR #36 latest CodeRabbit review round 2' +status: In Progress +assignee: [] +created_date: '2026-03-29 08:09' +labels: + - code-review + - pr-36 +dependencies: [] +references: + - 'https://github.com/ksyasuda/SubMiner/pull/36' +priority: high +ordinal: 3610 +--- + +## Description + + +Inspect the newest CodeRabbit review round on PR #36, verify the actionable comment against the current branch, implement the confirmed fix, and verify the touched path. + + +## Acceptance Criteria + +- [ ] #1 The actionable review comment is implemented or explicitly deferred with rationale. +- [ ] #2 Touched path is verified with the smallest sufficient test lane. +- [ ] #3 Current PR feedback is reduced to resolved or intentionally deferred suggestions. + diff --git a/src/core/services/immersion-tracker/maintenance.test.ts b/src/core/services/immersion-tracker/maintenance.test.ts index a17482f..cdb6225 100644 --- a/src/core/services/immersion-tracker/maintenance.test.ts +++ b/src/core/services/immersion-tracker/maintenance.test.ts @@ -82,6 +82,65 @@ test('pruneRawRetention uses session retention separately from telemetry retenti } }); +test('pruneRawRetention skips disabled retention windows', () => { + const dbPath = makeDbPath(); + const db = new Database(dbPath); + + try { + ensureSchema(db); + const nowMs = 1_000_000_000; + + db.exec(` + INSERT INTO imm_videos ( + video_id, video_key, canonical_title, source_type, duration_ms, CREATED_DATE, LAST_UPDATE_DATE + ) VALUES ( + 1, 'local:/tmp/video.mkv', 'Video', 1, 0, ${nowMs}, ${nowMs} + ); + INSERT INTO imm_sessions ( + session_id, session_uuid, video_id, started_at_ms, ended_at_ms, status, CREATED_DATE, LAST_UPDATE_DATE + ) VALUES ( + 1, 'session-1', 1, ${nowMs - 1_000}, ${nowMs - 500}, 2, ${nowMs}, ${nowMs} + ); + INSERT INTO imm_session_telemetry ( + session_id, sample_ms, total_watched_ms, active_watched_ms, CREATED_DATE, LAST_UPDATE_DATE + ) VALUES ( + 1, ${nowMs - 2_000}, 0, 0, ${nowMs}, ${nowMs} + ); + INSERT INTO imm_session_events ( + session_id, event_type, ts_ms, payload_json, CREATED_DATE, LAST_UPDATE_DATE + ) VALUES ( + 1, 1, ${nowMs - 3_000}, '{}', ${nowMs}, ${nowMs} + ); + `); + + const result = pruneRawRetention(db, nowMs, { + eventsRetentionMs: Number.POSITIVE_INFINITY, + telemetryRetentionMs: Number.POSITIVE_INFINITY, + sessionsRetentionMs: Number.POSITIVE_INFINITY, + }); + + const remainingSessionEvents = db + .prepare('SELECT COUNT(*) AS count FROM imm_session_events') + .get() as { count: number }; + const remainingTelemetry = db + .prepare('SELECT COUNT(*) AS count FROM imm_session_telemetry') + .get() as { count: number }; + const remainingSessions = db + .prepare('SELECT COUNT(*) AS count FROM imm_sessions') + .get() as { count: number }; + + assert.equal(result.deletedSessionEvents, 0); + assert.equal(result.deletedTelemetryRows, 0); + assert.equal(result.deletedEndedSessions, 0); + assert.equal(remainingSessionEvents.count, 1); + assert.equal(remainingTelemetry.count, 1); + assert.equal(remainingSessions.count, 1); + } finally { + db.close(); + cleanupDbPath(dbPath); + } +}); + test('toMonthKey floors negative timestamps into the prior UTC month', () => { assert.equal(toMonthKey(-1), 196912); assert.equal(toMonthKey(-86_400_000), 196912); diff --git a/src/core/services/immersion-tracker/maintenance.ts b/src/core/services/immersion-tracker/maintenance.ts index b2cda9b..1ed9bc9 100644 --- a/src/core/services/immersion-tracker/maintenance.ts +++ b/src/core/services/immersion-tracker/maintenance.ts @@ -53,25 +53,27 @@ export function pruneRawRetention( sessionsRetentionMs: number; }, ): RawRetentionResult { - const eventCutoff = currentMs - policy.eventsRetentionMs; - const telemetryCutoff = currentMs - policy.telemetryRetentionMs; - const sessionsCutoff = currentMs - policy.sessionsRetentionMs; - - const deletedSessionEvents = ( - db.prepare(`DELETE FROM imm_session_events WHERE ts_ms < ?`).run(toDbMs(eventCutoff)) as { - changes: number; - } - ).changes; - const deletedTelemetryRows = ( - db - .prepare(`DELETE FROM imm_session_telemetry WHERE sample_ms < ?`) - .run(toDbMs(telemetryCutoff)) as { changes: number } - ).changes; - const deletedEndedSessions = ( - db - .prepare(`DELETE FROM imm_sessions WHERE ended_at_ms IS NOT NULL AND ended_at_ms < ?`) - .run(toDbMs(sessionsCutoff)) as { changes: number } - ).changes; + const deletedSessionEvents = Number.isFinite(policy.eventsRetentionMs) + ? ( + db.prepare(`DELETE FROM imm_session_events WHERE ts_ms < ?`).run( + toDbMs(currentMs - policy.eventsRetentionMs), + ) as { changes: number } + ).changes + : 0; + const deletedTelemetryRows = Number.isFinite(policy.telemetryRetentionMs) + ? ( + db + .prepare(`DELETE FROM imm_session_telemetry WHERE sample_ms < ?`) + .run(toDbMs(currentMs - policy.telemetryRetentionMs)) as { changes: number } + ).changes + : 0; + const deletedEndedSessions = Number.isFinite(policy.sessionsRetentionMs) + ? ( + db + .prepare(`DELETE FROM imm_sessions WHERE ended_at_ms IS NOT NULL AND ended_at_ms < ?`) + .run(toDbMs(currentMs - policy.sessionsRetentionMs)) as { changes: number } + ).changes + : 0; return { deletedSessionEvents,