From 9b2b137a6cc26c2ac69807c4147f3a3bc871c987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 4 Jun 2026 10:38:44 +0200 Subject: [PATCH] =?UTF-8?q?fix(coach,rollups):=20QA=20reconcile=20?= =?UTF-8?q?=E2=80=94=20decrypt=20isolation=20+=20collapse=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - conversation-summary: fault-isolate the per-message decrypt in the fold loop so one undecryptable historic turn (e.g. mid key-rotation) no longer permanently stalls a conversation's summary refresh. - source collapse: unify the no-ladder tiebreak on alphabetically smallest source across the JS reader and the all-time rollup SQL, so a ranked vital whose day carries only off-ladder sources resolves to the same canonical source on the warm and the live-SQL read paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/ai/coach/conversation-summary.ts | 18 ++++++++++++++---- src/lib/analytics/source-rank-sql.ts | 2 +- src/lib/analytics/summaries-slice.ts | 2 +- .../collapse-rollup-rows-by-source.test.ts | 12 +++++++----- src/lib/rollups/measurement-read.ts | 12 ++++++------ 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/lib/ai/coach/conversation-summary.ts b/src/lib/ai/coach/conversation-summary.ts index 083cb07fe..f94d5bc93 100644 --- a/src/lib/ai/coach/conversation-summary.ts +++ b/src/lib/ai/coach/conversation-summary.ts @@ -174,10 +174,20 @@ export async function refreshConversationSummary( const priorSummary = decryptPriorSummary(conversation.summaryEncrypted); const foldedTurns: SummaryFoldTurn[] = turns .slice(0, foldHighWater) - .map((m) => ({ - role: m.role === "assistant" ? "assistant" : "user", - content: decryptFromBytes(m.encryptedContent), - })); + .flatMap((m) => { + let content: string; + try { + content = decryptFromBytes(m.encryptedContent); + } catch { + // Skip an undecryptable message (e.g. a row mid key-rotation) instead + // of throwing and permanently stalling this conversation's summary — + // fail-closed like every other decrypt in the Coach stack. + return []; + } + return [ + { role: m.role === "assistant" ? "assistant" : "user", content }, + ]; + }); // 5. Run the user's provider chain. Best-effort: non-ok leaves the old // summary in place. diff --git a/src/lib/analytics/source-rank-sql.ts b/src/lib/analytics/source-rank-sql.ts index e2bcc4314..5aac2e1c4 100644 --- a/src/lib/analytics/source-rank-sql.ts +++ b/src/lib/analytics/source-rank-sql.ts @@ -14,7 +14,7 @@ * single-round-trip `GROUP BY type` shape (no six-figure row transfer). * * Every spliced literal is a closed-enum value (`MeasurementType` / - * `MeasurementSource`) asserted against `/^[A-Z_]+$/`, the same whitelist + * `MeasurementSource`) asserted against `/^[A-Z0-9_]+$/`, the same whitelist * convention the rollup writer uses for its date-trunc + type-list splices. * The user id is always passed as a bound parameter, never spliced. */ diff --git a/src/lib/analytics/summaries-slice.ts b/src/lib/analytics/summaries-slice.ts index 33045b551..1c5f16dc6 100644 --- a/src/lib/analytics/summaries-slice.ts +++ b/src/lib/analytics/summaries-slice.ts @@ -415,7 +415,7 @@ async function computeFromRollups(userId: string): Promise { FROM "measurement_rollups" WHERE "user_id" = $1 AND "granularity" = 'DAY' - ORDER BY "type", "bucket_start", (${rankUnqualified}), "count" DESC, "source" + ORDER BY "type", "bucket_start", (${rankUnqualified}), "source" ) SELECT "type"::text AS type, diff --git a/src/lib/rollups/__tests__/collapse-rollup-rows-by-source.test.ts b/src/lib/rollups/__tests__/collapse-rollup-rows-by-source.test.ts index cd37fb4c0..3108cae1b 100644 --- a/src/lib/rollups/__tests__/collapse-rollup-rows-by-source.test.ts +++ b/src/lib/rollups/__tests__/collapse-rollup-rows-by-source.test.ts @@ -136,19 +136,21 @@ describe("collapseRollupRowsBySource", () => { expect(out[0].source).toBe("APPLE_HEALTH"); }); - it("falls back to max-count when no ladder source is present", () => { - // IMPORT is not on the restingHeartRate ladder; keep one row (most data). + it("falls back to the alphabetically-smallest source when none is on the ladder", () => { + // Neither IMPORT nor MANUAL is on the restingHeartRate ladder; keep one + // row deterministically by source name, matching the live-SQL paths' + // `ORDER BY … source` tiebreak (live/rollup parity). const out = collapseRollupRowsBySource( [ - row(DAY1, "IMPORT", 60, 2), row(DAY1, "MANUAL", 58, 5), + row(DAY1, "IMPORT", 60, 2), ], "RESTING_HEART_RATE", null, ); expect(out).toHaveLength(1); - expect(out[0].source).toBe("MANUAL"); - expect(out[0].count).toBe(5); + // "IMPORT" < "MANUAL" alphabetically. + expect(out[0].source).toBe("IMPORT"); }); it("returns empty input untouched", () => { diff --git a/src/lib/rollups/measurement-read.ts b/src/lib/rollups/measurement-read.ts index 9db48f5ef..67d657470 100644 --- a/src/lib/rollups/measurement-read.ts +++ b/src/lib/rollups/measurement-read.ts @@ -114,13 +114,13 @@ export function collapseRollupRowsBySource( } } if (!picked) { - // No ladder match — keep one row deterministically (most data wins, - // tie-broken by source name) so the bucket neither doubles nor goes dark. + // No ladder match — keep one row deterministically by alphabetically + // smallest source name, so the bucket neither doubles nor goes dark AND + // the pick matches the live-SQL paths' `ORDER BY … source` tiebreak + // (live/rollup parity for a ranked type whose day carries only + // non-ladder sources). picked = bucketRows.reduce((best, r) => - r.count > best.count || - (r.count === best.count && r.source < best.source) - ? r - : best, + r.source < best.source ? r : best, ); } out.push(picked);