From 6e6b1849e7d07ab19d9e7e7064abd5708ec7e6bd Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Tue, 21 Apr 2026 23:26:19 -0400 Subject: [PATCH 1/4] fix(sidepanel): recompute daily summary on conv writes [GET-18] Turn writes land under conv: keys and did not trigger loadToday(). TODAY lagged the active conversation by up to the background alarm interval (~30 min). When hasConvChange fires, now also call computeDailySummary so the daily: key is fresh. The existing hasDailyChange arm picks it up and calls loadToday() immediately. --- entrypoints/sidepanel/hooks/useDashboardData.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/entrypoints/sidepanel/hooks/useDashboardData.ts b/entrypoints/sidepanel/hooks/useDashboardData.ts index c7d7143..4e65182 100644 --- a/entrypoints/sidepanel/hooks/useDashboardData.ts +++ b/entrypoints/sidepanel/hooks/useDashboardData.ts @@ -316,6 +316,15 @@ export function useDashboardData(): DashboardData { if (tabIdRef.current !== null) { loadActiveConversation(tabIdRef.current); } + // Recompute today's daily summary so TODAY stays in sync with turn + // writes. Without this, TODAY lags the active conversation until the + // background alarm fires (~30 min). computeDailySummary writes + // daily:{orgId}:{date}, which the hasDailyChange arm picks up below + // and feeds to loadToday(). [GET-18] + const orgId = orgIdRef.current; + if (orgId) { + void computeDailySummary(orgId, todayDateString()).catch(() => {}); + } } if (hasDailyChange) { loadToday(); From a3cb81d70d48f19a7022fdb7e775284d4b7e3b72 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Tue, 21 Apr 2026 23:26:24 -0400 Subject: [PATCH 2/4] test(today-rollup): GET-18 invariant tests Three tests encode the product-level invariants GET-18 depends on: daily summary totalTurns >= any single conv contribution, fresh computeDailySummary after a new turn reflects it immediately, and no double-counting across two conversations in the same day. --- tests/unit/today-rollup.test.ts | 147 ++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 tests/unit/today-rollup.test.ts diff --git a/tests/unit/today-rollup.test.ts b/tests/unit/today-rollup.test.ts new file mode 100644 index 0000000..99e31aa --- /dev/null +++ b/tests/unit/today-rollup.test.ts @@ -0,0 +1,147 @@ +// tests/unit/today-rollup.test.ts +// GET-18: TODAY rollup invariants. +// Proves that computeDailySummary always reflects the current state of all +// conversations, so the TODAY card is never behind the active conversation. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { + setStorage, + recordTurn, + computeDailySummary, + todayDateString, + type StorageArea, +} from '../../lib/conversation-store'; + +const TEST_ORG = 'org-get18'; +const CONV_A = 'conv-get18-a'; +const CONV_B = 'conv-get18-b'; + +function createStoreMock(): StorageArea { + const data: Record = {}; + return { + get: async (keys: string | string[] | null) => { + if (keys === null) return { ...data }; + const keyList = typeof keys === 'string' ? [keys] : keys; + const result: Record = {}; + for (const k of keyList) { + if (k in data) result[k] = data[k]; + } + return result; + }, + set: async (items: Record) => { + Object.assign(data, items); + }, + remove: async (keys: string | string[]) => { + const keyList = typeof keys === 'string' ? [keys] : keys; + for (const k of keyList) delete data[k]; + }, + }; +} + +beforeEach(() => { + setStorage(createStoreMock()); +}); + +describe('GET-18: TODAY rollup invariants', () => { + it('daily summary totalTurns >= any single conversation contribution', async () => { + const today = todayDateString(); + const now = Date.now(); + + for (let i = 0; i < 5; i++) { + await recordTurn(TEST_ORG, CONV_A, { + inputTokens: 100, + outputTokens: 50, + model: 'claude-sonnet-4-6', + contextPct: 5, + cost: 0.001, + completedAt: now, + }); + } + + for (let i = 0; i < 12; i++) { + await recordTurn(TEST_ORG, CONV_B, { + inputTokens: 200, + outputTokens: 80, + model: 'claude-sonnet-4-6', + contextPct: 10, + cost: 0.002, + completedAt: now, + }); + } + + const summary = await computeDailySummary(TEST_ORG, today); + + expect(summary.totalTurns).toBe(17); + expect(summary.totalTurns).toBeGreaterThanOrEqual(12); + expect(summary.totalTurns).toBeGreaterThanOrEqual(5); + expect(summary.totalInputTokens).toBeGreaterThanOrEqual(12 * 200); + expect(summary.estimatedCost).not.toBeNull(); + expect(summary.estimatedCost!).toBeGreaterThanOrEqual(12 * 0.002); + expect(summary.conversationCount).toBe(2); + }); + + it('computeDailySummary reflects a new turn immediately after recordTurn', async () => { + const today = todayDateString(); + const now = Date.now(); + const baseTurn = { + inputTokens: 500, + outputTokens: 100, + model: 'claude-sonnet-4-6', + contextPct: 8, + cost: 0.005, + completedAt: now, + }; + + for (let i = 0; i < 3; i++) { + await recordTurn(TEST_ORG, CONV_A, baseTurn); + } + const stale = await computeDailySummary(TEST_ORG, today); + expect(stale.totalTurns).toBe(3); + + const extraTurn = { ...baseTurn, inputTokens: 800, outputTokens: 150, cost: 0.009 }; + await recordTurn(TEST_ORG, CONV_A, extraTurn); + const fresh = await computeDailySummary(TEST_ORG, today); + + expect(fresh.totalTurns).toBe(stale.totalTurns + 1); + expect(fresh.totalInputTokens).toBe(stale.totalInputTokens + extraTurn.inputTokens); + expect(fresh.totalOutputTokens).toBe(stale.totalOutputTokens + extraTurn.outputTokens); + expect(fresh.estimatedCost).not.toBeNull(); + expect(fresh.estimatedCost!).toBeCloseTo(stale.estimatedCost! + extraTurn.cost, 6); + }); + + it('no double-counting: two conversations sum to exact totals', async () => { + const today = todayDateString(); + const now = Date.now(); + + for (let i = 0; i < 3; i++) { + await recordTurn(TEST_ORG, CONV_A, { + inputTokens: 100, + outputTokens: 50, + model: 'claude-haiku-4-5', + contextPct: 3, + cost: 0.001, + completedAt: now, + }); + } + + for (let i = 0; i < 2; i++) { + await recordTurn(TEST_ORG, CONV_B, { + inputTokens: 200, + outputTokens: 100, + model: 'claude-haiku-4-5', + contextPct: 5, + cost: 0.003, + completedAt: now, + }); + } + + const summary = await computeDailySummary(TEST_ORG, today); + + expect(summary.conversationCount).toBe(2); + expect(summary.totalTurns).toBe(5); + expect(summary.totalInputTokens).toBe(3 * 100 + 2 * 200); + expect(summary.totalOutputTokens).toBe(3 * 50 + 2 * 100); + expect(summary.estimatedCost).not.toBeNull(); + expect(summary.estimatedCost!).toBeCloseTo(3 * 0.001 + 2 * 0.003, 6); + }); +}); From 11c2f9eee0948ed8d9a1b31f4afa7aa4e0d8bf26 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Tue, 21 Apr 2026 23:31:44 -0400 Subject: [PATCH 3/4] refactor(get-18): sharpen comments, fix all pipeline issues Hook comment now names the exact trade-offs (dual writers, full-scan cost). Test file rewritten: WHY comments per test, hook integration gap documented, null-cost test added for the unknown-model case. --- .../sidepanel/hooks/useDashboardData.ts | 11 +- tests/unit/today-rollup.test.ts | 169 +++++++++--------- 2 files changed, 94 insertions(+), 86 deletions(-) diff --git a/entrypoints/sidepanel/hooks/useDashboardData.ts b/entrypoints/sidepanel/hooks/useDashboardData.ts index 4e65182..3893bb0 100644 --- a/entrypoints/sidepanel/hooks/useDashboardData.ts +++ b/entrypoints/sidepanel/hooks/useDashboardData.ts @@ -316,11 +316,12 @@ export function useDashboardData(): DashboardData { if (tabIdRef.current !== null) { loadActiveConversation(tabIdRef.current); } - // Recompute today's daily summary so TODAY stays in sync with turn - // writes. Without this, TODAY lags the active conversation until the - // background alarm fires (~30 min). computeDailySummary writes - // daily:{orgId}:{date}, which the hasDailyChange arm picks up below - // and feeds to loadToday(). [GET-18] + // conv: writes never trigger hasDailyChange, so TODAY drifts from + // the active conversation until the background alarm fires (~30 min). + // Eagerly recompute: computeDailySummary writes daily:{orgId}:{date}, + // which the hasDailyChange arm below catches and feeds to loadToday(). + // Trade-off: reads all org conv records on each turn. Cost grows with + // history depth but is fire-and-forget and fine at 90-day scale. [GET-18] const orgId = orgIdRef.current; if (orgId) { void computeDailySummary(orgId, todayDateString()).catch(() => {}); diff --git a/tests/unit/today-rollup.test.ts b/tests/unit/today-rollup.test.ts index 99e31aa..c70b93d 100644 --- a/tests/unit/today-rollup.test.ts +++ b/tests/unit/today-rollup.test.ts @@ -1,7 +1,15 @@ // tests/unit/today-rollup.test.ts -// GET-18: TODAY rollup invariants. -// Proves that computeDailySummary always reflects the current state of all -// conversations, so the TODAY card is never behind the active conversation. +// +// GET-18 invariant: the TODAY daily summary must always be >= the active +// conversation's numbers. computeDailySummary is the function that produces +// this value; these tests prove its correctness properties directly. +// +// Hook integration gap: the useDashboardData.ts change that calls +// computeDailySummary on conv: storage events cannot be unit-tested here +// without renderHook + full Chrome API mocks (not present in this test setup). +// Manual verification covers it: open the side panel mid-conversation and +// confirm TODAY matches ACTIVE CONVERSATION to the exact turn and token count +// within one turn cycle, without waiting for the 30-min background alarm. import { describe, it, expect, beforeEach } from 'vitest'; import { @@ -12,136 +20,135 @@ import { type StorageArea, } from '../../lib/conversation-store'; -const TEST_ORG = 'org-get18'; +const ORG = 'org-get18'; const CONV_A = 'conv-get18-a'; const CONV_B = 'conv-get18-b'; -function createStoreMock(): StorageArea { +// Inline mock matching the StorageArea interface. Plain async functions are +// sufficient here because these tests assert data values, not call counts. +function makeStoreMock(): StorageArea { const data: Record = {}; return { get: async (keys: string | string[] | null) => { if (keys === null) return { ...data }; - const keyList = typeof keys === 'string' ? [keys] : keys; - const result: Record = {}; - for (const k of keyList) { - if (k in data) result[k] = data[k]; - } - return result; - }, - set: async (items: Record) => { - Object.assign(data, items); + const ks = typeof keys === 'string' ? [keys] : keys; + const out: Record = {}; + for (const k of ks) if (k in data) out[k] = data[k]; + return out; }, + set: async (items: Record) => { Object.assign(data, items); }, remove: async (keys: string | string[]) => { - const keyList = typeof keys === 'string' ? [keys] : keys; - for (const k of keyList) delete data[k]; + for (const k of typeof keys === 'string' ? [keys] : keys) delete data[k]; }, }; } -beforeEach(() => { - setStorage(createStoreMock()); -}); +beforeEach(() => { setStorage(makeStoreMock()); }); describe('GET-18: TODAY rollup invariants', () => { - it('daily summary totalTurns >= any single conversation contribution', async () => { - const today = todayDateString(); + it('sums all conversations, not just the active one', async () => { + // The naive Math.max(today.totalTurns, activeConv.turnCount) fix is wrong + // for multi-conversation days: if conv A has 5 turns and conv B (active) + // has 12, Math.max(17, 12) = 17 when it should be 5+12=17 -- correct by + // coincidence. But if conv B has fewer turns than the daily total, Math.max + // silently drops conv A's contribution. computeDailySummary must sum all + // conversations, so it is immune to that class of bug. const now = Date.now(); for (let i = 0; i < 5; i++) { - await recordTurn(TEST_ORG, CONV_A, { - inputTokens: 100, - outputTokens: 50, - model: 'claude-sonnet-4-6', - contextPct: 5, - cost: 0.001, - completedAt: now, + await recordTurn(ORG, CONV_A, { + inputTokens: 100, outputTokens: 50, model: 'claude-sonnet-4-6', + contextPct: 5, cost: 0.001, completedAt: now, }); } - for (let i = 0; i < 12; i++) { - await recordTurn(TEST_ORG, CONV_B, { - inputTokens: 200, - outputTokens: 80, - model: 'claude-sonnet-4-6', - contextPct: 10, - cost: 0.002, - completedAt: now, + await recordTurn(ORG, CONV_B, { + inputTokens: 200, outputTokens: 80, model: 'claude-sonnet-4-6', + contextPct: 10, cost: 0.002, completedAt: now, }); } - const summary = await computeDailySummary(TEST_ORG, today); + const summary = await computeDailySummary(ORG, todayDateString()); - expect(summary.totalTurns).toBe(17); - expect(summary.totalTurns).toBeGreaterThanOrEqual(12); - expect(summary.totalTurns).toBeGreaterThanOrEqual(5); + expect(summary.totalTurns).toBe(17); // 5 + 12, exact + expect(summary.totalTurns).toBeGreaterThanOrEqual(12); // invariant: >= conv B + expect(summary.totalTurns).toBeGreaterThanOrEqual(5); // invariant: >= conv A expect(summary.totalInputTokens).toBeGreaterThanOrEqual(12 * 200); expect(summary.estimatedCost).not.toBeNull(); expect(summary.estimatedCost!).toBeGreaterThanOrEqual(12 * 0.002); expect(summary.conversationCount).toBe(2); }); - it('computeDailySummary reflects a new turn immediately after recordTurn', async () => { - const today = todayDateString(); + it('reflects a new turn immediately without waiting for the alarm', async () => { + // Simulates the exact GET-18 scenario: daily summary was last written by + // the background alarm when the conversation had N turns. One more turn + // completes. The next computeDailySummary call must include that turn + // immediately, not on the next 30-min alarm cycle. const now = Date.now(); - const baseTurn = { - inputTokens: 500, - outputTokens: 100, - model: 'claude-sonnet-4-6', - contextPct: 8, - cost: 0.005, - completedAt: now, + const base = { + inputTokens: 500, outputTokens: 100, model: 'claude-sonnet-4-6', + contextPct: 8, cost: 0.005, completedAt: now, }; - for (let i = 0; i < 3; i++) { - await recordTurn(TEST_ORG, CONV_A, baseTurn); - } - const stale = await computeDailySummary(TEST_ORG, today); - expect(stale.totalTurns).toBe(3); + for (let i = 0; i < 3; i++) await recordTurn(ORG, CONV_A, base); + const stale = await computeDailySummary(ORG, todayDateString()); - const extraTurn = { ...baseTurn, inputTokens: 800, outputTokens: 150, cost: 0.009 }; - await recordTurn(TEST_ORG, CONV_A, extraTurn); - const fresh = await computeDailySummary(TEST_ORG, today); + // One more turn lands (the one the alarm has not seen yet). + const extra = { ...base, inputTokens: 800, outputTokens: 150, cost: 0.009 }; + await recordTurn(ORG, CONV_A, extra); + const fresh = await computeDailySummary(ORG, todayDateString()); expect(fresh.totalTurns).toBe(stale.totalTurns + 1); - expect(fresh.totalInputTokens).toBe(stale.totalInputTokens + extraTurn.inputTokens); - expect(fresh.totalOutputTokens).toBe(stale.totalOutputTokens + extraTurn.outputTokens); + expect(fresh.totalInputTokens).toBe(stale.totalInputTokens + extra.inputTokens); + expect(fresh.totalOutputTokens).toBe(stale.totalOutputTokens + extra.outputTokens); expect(fresh.estimatedCost).not.toBeNull(); - expect(fresh.estimatedCost!).toBeCloseTo(stale.estimatedCost! + extraTurn.cost, 6); + expect(fresh.estimatedCost!).toBeCloseTo(stale.estimatedCost! + extra.cost, 6); }); - it('no double-counting: two conversations sum to exact totals', async () => { - const today = todayDateString(); + it('no double-counting when two conversations share a day', async () => { + // computeDailySummary iterates the conv index and aggregates. A bug that + // counted the same conv twice would produce 2x totals here. Fixed turn + // counts make the exact expected values unambiguous. const now = Date.now(); for (let i = 0; i < 3; i++) { - await recordTurn(TEST_ORG, CONV_A, { - inputTokens: 100, - outputTokens: 50, - model: 'claude-haiku-4-5', - contextPct: 3, - cost: 0.001, - completedAt: now, + await recordTurn(ORG, CONV_A, { + inputTokens: 100, outputTokens: 50, model: 'claude-haiku-4-5', + contextPct: 3, cost: 0.001, completedAt: now, }); } - for (let i = 0; i < 2; i++) { - await recordTurn(TEST_ORG, CONV_B, { - inputTokens: 200, - outputTokens: 100, - model: 'claude-haiku-4-5', - contextPct: 5, - cost: 0.003, - completedAt: now, + await recordTurn(ORG, CONV_B, { + inputTokens: 200, outputTokens: 100, model: 'claude-haiku-4-5', + contextPct: 5, cost: 0.003, completedAt: now, }); } - const summary = await computeDailySummary(TEST_ORG, today); + const summary = await computeDailySummary(ORG, todayDateString()); expect(summary.conversationCount).toBe(2); - expect(summary.totalTurns).toBe(5); - expect(summary.totalInputTokens).toBe(3 * 100 + 2 * 200); - expect(summary.totalOutputTokens).toBe(3 * 50 + 2 * 100); + expect(summary.totalTurns).toBe(5); // 3 + 2, not 6 + expect(summary.totalInputTokens).toBe(3 * 100 + 2 * 200); // 700 + expect(summary.totalOutputTokens).toBe(3 * 50 + 2 * 100); // 350 expect(summary.estimatedCost).not.toBeNull(); expect(summary.estimatedCost!).toBeCloseTo(3 * 0.001 + 2 * 0.003, 6); }); + + it('propagates null estimatedCost when no turn has a known cost', async () => { + // An unrecognized model returns null from the pricing agent. The daily + // summary must stay null rather than coercing to 0: null means "cost + // unknown" and 0 means "this session was free". Conflating the two would + // silently misreport usage to any downstream consumer (traction exports, + // BIP posts, YC metrics). + await recordTurn(ORG, CONV_A, { + inputTokens: 300, outputTokens: 100, model: 'claude-unknown-future-model', + contextPct: 5, cost: null, completedAt: Date.now(), + }); + + const summary = await computeDailySummary(ORG, todayDateString()); + + expect(summary.estimatedCost).toBeNull(); + expect(summary.totalTurns).toBe(1); + expect(summary.totalInputTokens).toBe(300); + }); }); From a9a55bf70cd434d7441a564d090821a221e602b2 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Wed, 22 Apr 2026 00:06:53 -0400 Subject: [PATCH 4/4] fix(sidepanel): log computeDailySummary errors instead of swallowing [GET-18] Silent catch made TODAY stay stale with zero indication when the recompute failed. Consistent with every other error path in this hook. --- entrypoints/sidepanel/hooks/useDashboardData.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/entrypoints/sidepanel/hooks/useDashboardData.ts b/entrypoints/sidepanel/hooks/useDashboardData.ts index 3893bb0..2a904e7 100644 --- a/entrypoints/sidepanel/hooks/useDashboardData.ts +++ b/entrypoints/sidepanel/hooks/useDashboardData.ts @@ -324,7 +324,9 @@ export function useDashboardData(): DashboardData { // history depth but is fire-and-forget and fine at 90-day scale. [GET-18] const orgId = orgIdRef.current; if (orgId) { - void computeDailySummary(orgId, todayDateString()).catch(() => {}); + void computeDailySummary(orgId, todayDateString()).catch((err) => { + console.error('[Saar] Failed to recompute daily summary on turn:', err); + }); } } if (hasDailyChange) {