mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-07 04:12:04 +00:00
fix(coach): cap proactive nudges by the persisted conversation, not push success
The proactive-nudge frequency gate only counted delivered push attempts. A user with no push channel never wrote a delivered row, so the cap was inert for them: every cron tick that hit a persistent trigger minted a fresh nudge conversation in the rail. Count a recently persisted nudge conversation toward the same rolling window so the once-per-window contract holds regardless of push outcome. Add a covering index on coach messages for the assistant-message scans the FAB poll and the gate both run, and drop the unused providerType parameter from the persistence helper now that the tag is fixed.
This commit is contained in:
1 parent
442eb83e2b
commit
ddbb25d5d4
5 files changed
+98
-7
No files matched your search
@@ -0,0 +1,24 @@
|
||||
-- v1.18.6 (L1) — covering index for the Coach assistant-message scans.
|
||||
--
|
||||
-- Two hot reads walk a user's assistant messages newest-first:
|
||||
-- * the global Coach FAB poll (`GET /api/insights/coach/nudge-status`) —
|
||||
-- the newest assistant message across the caller's conversations, used
|
||||
-- to decide whether the unread dot paints; and
|
||||
-- * the proactive-nudge cron's frequency gate (CCH-02 / M1) — the newest
|
||||
-- `provider_type = 'nudge'` assistant message inside the rolling window.
|
||||
--
|
||||
-- The pre-existing `(conversation_id, created_at)` index does not cover the
|
||||
-- `role` / `provider_type` filter, so Postgres gathered every assistant row
|
||||
-- in the user's threads and sorted. `coach_messages` grows unbounded, and the
|
||||
-- FAB poll fires on every authenticated page with refetch-on-focus.
|
||||
--
|
||||
-- This index orders by `created_at DESC` after `(conversation_id, role)` so
|
||||
-- the planner can do an index-backed per-conversation merge for the
|
||||
-- newest-first reads. Additive; no data touched.
|
||||
--
|
||||
-- Reversibility (down):
|
||||
-- DROP INDEX IF EXISTS "coach_messages_conversation_id_role_created_at_idx";
|
||||
|
||||
-- CreateIndex
|
||||
CREATE INDEX IF NOT EXISTS "coach_messages_conversation_id_role_created_at_idx"
|
||||
ON "coach_messages"("conversation_id", "role", "created_at" DESC);
|
||||
@@ -3632,6 +3632,12 @@ model CoachMessage {
|
||||
feedback RecommendationFeedback[]
|
||||
|
||||
@@index([conversationId, createdAt])
|
||||
// v1.18.6 (L1) — the FAB nudge-status poll and the cron's persisted-nudge
|
||||
// frequency gate both scan a user's assistant messages newest-first
|
||||
// (optionally filtered to provider_type = 'nudge'). This index lets the
|
||||
// planner do an index-backed per-conversation merge instead of gathering +
|
||||
// sorting every assistant row in the user's threads on each global poll.
|
||||
@@index([conversationId, role, createdAt(sort: Desc)])
|
||||
@@map("coach_messages")
|
||||
}
|
||||
|
||||
|
||||
@@ -194,8 +194,6 @@ export interface RecordProactiveNudgeParams {
|
||||
title: string;
|
||||
/** The nudge body, persisted as the initial ASSISTANT message. */
|
||||
body: string;
|
||||
/** Trigger tag for the wide-event annotation on the caller side. */
|
||||
providerType?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -228,7 +226,9 @@ export async function recordProactiveNudge(
|
||||
role: "assistant",
|
||||
encryptedContent: encryptToBytes(params.body),
|
||||
metricSourceJson: null,
|
||||
providerType: params.providerType ?? "nudge",
|
||||
// Tags the message as a proactive nudge — the cron's frequency gate
|
||||
// reads this back to cap rail conversations for no-push-channel users.
|
||||
providerType: "nudge",
|
||||
promptVersion: null,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -524,6 +524,7 @@ describe("runCoachNudgeTick — gates and prefs", () => {
|
||||
users?: unknown[];
|
||||
intakeRows?: unknown[];
|
||||
recentNudge?: unknown;
|
||||
recentPersistedNudge?: unknown;
|
||||
coachFocusEncrypted?: Uint8Array | null;
|
||||
}) {
|
||||
return {
|
||||
@@ -539,6 +540,12 @@ describe("runCoachNudgeTick — gates and prefs", () => {
|
||||
return overrides.recentNudge ?? null;
|
||||
}),
|
||||
},
|
||||
coachMessage: {
|
||||
findFirst: vi.fn(async (args?: unknown) => {
|
||||
void args;
|
||||
return overrides.recentPersistedNudge ?? null;
|
||||
}),
|
||||
},
|
||||
medicationIntakeEvent: {
|
||||
findMany: vi.fn(async () => overrides.intakeRows ?? []),
|
||||
},
|
||||
@@ -643,6 +650,42 @@ describe("runCoachNudgeTick — gates and prefs", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("caps on a recently persisted nudge conversation even with no push-success row (M1)", async () => {
|
||||
// A no-push-channel user never writes an `ok` push-attempt row, so the
|
||||
// ledger side of gate 5 stays empty. The persisted nudge conversation
|
||||
// from a prior tick must still pin the window so the rail does not get a
|
||||
// brand-new nudge conversation every cron run.
|
||||
const dispatch = vi.fn();
|
||||
const recordNudge = vi.fn();
|
||||
const prisma = prismaMock({
|
||||
users: [userRow(null)],
|
||||
intakeRows: failingIntakes,
|
||||
recentNudge: null,
|
||||
recentPersistedNudge: { id: "msg_recent" },
|
||||
});
|
||||
const summary = await runCoachNudgeTick(
|
||||
prisma as unknown as PrismaClient,
|
||||
now,
|
||||
{ dispatch: dispatch as never, recordNudge: recordNudge as never },
|
||||
);
|
||||
expect(summary.skippedRecentNudge).toBe(1);
|
||||
expect(dispatch).not.toHaveBeenCalled();
|
||||
expect(recordNudge).not.toHaveBeenCalled();
|
||||
// The persisted-nudge query honours the same rolling cutoff as the ledger.
|
||||
const arg = vi.mocked(prisma.coachMessage.findFirst).mock.calls[0]?.[0] as {
|
||||
where: {
|
||||
providerType: string;
|
||||
role: string;
|
||||
createdAt: { gte: Date };
|
||||
};
|
||||
} | undefined;
|
||||
expect(arg?.where.providerType).toBe("nudge");
|
||||
expect(arg?.where.role).toBe("assistant");
|
||||
expect(arg?.where.createdAt.gte.getTime()).toBe(
|
||||
now.getTime() - 7 * MS_PER_DAY,
|
||||
);
|
||||
});
|
||||
|
||||
it("skips a disabled group's trigger queries entirely", async () => {
|
||||
const prisma = prismaMock({
|
||||
users: [
|
||||
|
||||
@@ -782,10 +782,15 @@ export async function runCoachNudgeTick(
|
||||
}
|
||||
|
||||
// Gate 5 — one nudge per rolling window (7 d default, 14 d when
|
||||
// the user picked "biweekly"), anchored on the push-attempts
|
||||
// ledger. A delivered nudge writes an `ok` row per succeeding
|
||||
// channel; a fully failed dispatch leaves the slot free so
|
||||
// tomorrow's tick can retry.
|
||||
// the user picked "biweekly"). Anchored on BOTH the push-attempts
|
||||
// ledger (a delivered nudge writes an `ok` row per succeeding
|
||||
// channel) AND the persisted nudge conversation (v1.18.6 CCH-02
|
||||
// writes one regardless of push outcome). A user with no push
|
||||
// channel never gets an `ok` row, so without the persisted-side
|
||||
// check the cap would be inert for them and every tick would mint a
|
||||
// fresh rail conversation. A fully failed dispatch on a push-capable
|
||||
// user still leaves the ledger slot free, but the persisted nudge
|
||||
// already pins the window — exactly the once-per-window contract.
|
||||
const capCutoff = new Date(
|
||||
now.getTime() - nudgePrefs.minIntervalDays * MS_PER_DAY,
|
||||
);
|
||||
@@ -802,6 +807,19 @@ export async function runCoachNudgeTick(
|
||||
summary.skippedRecentNudge += 1;
|
||||
continue;
|
||||
}
|
||||
const recentPersistedNudge = await prisma.coachMessage.findFirst({
|
||||
where: {
|
||||
providerType: "nudge",
|
||||
role: "assistant",
|
||||
conversation: { userId: user.id },
|
||||
createdAt: { gte: capCutoff },
|
||||
},
|
||||
select: { id: true },
|
||||
});
|
||||
if (recentPersistedNudge) {
|
||||
summary.skippedRecentNudge += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Gate 6 — Rest Mode pause (v1.18.1 P4). A user with an active
|
||||
// illness/condition episode should not be told to measure more often;
|
||||
|
||||
Reference in new issue
Block a user