From 3f22ebd5294846c822632e77be2f229f6ad93511 Mon Sep 17 00:00:00 2001 From: Mikaal Naik Date: Wed, 15 Jul 2026 12:13:52 -0400 Subject: [PATCH] Guard bill summarization against unknown DB state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On July 14 the deploy env lost MONGO_URI, and getBillByIdFromDB silently treated that as "bill not in DB" — every bill page view fell through to the Civics API + GPT-5 summarization path, unpersisted, on loop, until it surfaced as an OpenAI cost spike. Make LLM summarization fail-safe: - lookupBillInDB returns found / not-found / unavailable instead of an ambiguous null, logging loudly when the DB can't be checked. - fromCivicsProjectApiBill only summarizes when the DB is confirmed reachable AND the bill is new or its source/billTextsCount actually changed (the old "source unchanged" branch never compared source). - Fallback analyses are tagged isFallback and are never persisted or Slack-notified; the admin reprocess route returns 502 instead of overwriting stored data with a fallback. - Backstop: [LLM_SUMMARIZE] log line on every OpenAI call plus a 25-calls/hour/process soft cap; the reprocess route and evals runner bypass the cap explicitly. - Placeholder analysis judgment changed from "no" to "abstain" so unanalyzed bills don't render a "Vote No" determination. - Document MONGO_URI and NEXTAUTH_SECRET in .env.local.example. Co-Authored-By: Claude Fable 5 --- .env.local.example | 17 ++- src/app/bills/api/[id]/reprocess/route.ts | 11 +- src/app/bills/evals/run.ts | 4 +- .../bills/server/get-bill-by-id-from-db.ts | 47 +++++-- src/app/bills/services/billApi.ts | 50 ++++++- src/app/bills/utils/billConverters.ts | 128 ++++++++++-------- 6 files changed, 181 insertions(+), 76 deletions(-) diff --git a/.env.local.example b/.env.local.example index 56612df..68cf778 100644 --- a/.env.local.example +++ b/.env.local.example @@ -20,7 +20,18 @@ YF_OAUTH_CALLBACK_URL=http://localhost:5050/api/auth/callback YORK_FACTORY_API_URL=http://localhost:3000/api/v1 # Bill data sources. -CIVICS_PROJECT_API_KEY= -OPENAI_API_KEY= +CIVICS_PROJECT_API_KEY= +OPENAI_API_KEY= -BILLS_SLACK_WEBHOOK_URL= \ No newline at end of file +BILLS_SLACK_WEBHOOK_URL= + +# Bills persistence. Must be a mongodb:// or mongodb+srv:// URI. Without it, +# bill pages render from the Civics Project API with no stored analysis and +# LLM summarization is disabled (analyses can't be persisted). +# MONGODB_URI is accepted as an alias. +MONGO_URI= + +# Bills admin auth (NextAuth). getServerSession throws in production without a +# secret — set one of these in every deployed environment. +NEXTAUTH_SECRET= +# AUTH_SECRET is accepted as an alias. \ No newline at end of file diff --git a/src/app/bills/api/[id]/reprocess/route.ts b/src/app/bills/api/[id]/reprocess/route.ts index 4384ff9..b089dc8 100644 --- a/src/app/bills/api/[id]/reprocess/route.ts +++ b/src/app/bills/api/[id]/reprocess/route.ts @@ -95,7 +95,16 @@ export async function POST( ); } - const analysis = await summarizeBillText(markdown); + // Explicit admin action — exempt from the organic-traffic rate cap. + const analysis = await summarizeBillText(markdown, { bypassCap: true }); + if (analysis.isFallback) { + // Never overwrite a good stored analysis with a degraded placeholder + // (missing OpenAI key, parse failure, API error). + return NextResponse.json( + { error: "AI analysis unavailable; existing analysis left untouched" }, + { status: 502 }, + ); + } const latestStageDate = apiBill.stages && apiBill.stages.length > 0 diff --git a/src/app/bills/evals/run.ts b/src/app/bills/evals/run.ts index ea661a8..ee0ad43 100644 --- a/src/app/bills/evals/run.ts +++ b/src/app/bills/evals/run.ts @@ -79,12 +79,12 @@ async function main() { if (runAnalysis) { // In fallback mode the result is deterministic and free — skip the cache. const { value: analysis, cached } = args.fallback - ? { value: await summarizeBillText(text), cached: false } + ? { value: await summarizeBillText(text, { bypassCap: true }), cached: false } : await runCached( "analysis", text, SUMMARY_AND_VOTE_PROMPT, - () => summarizeBillText(text), + () => summarizeBillText(text, { bypassCap: true }), { refresh: args.refresh, stats }, ); report.cached = cached; diff --git a/src/app/bills/server/get-bill-by-id-from-db.ts b/src/app/bills/server/get-bill-by-id-from-db.ts index 27fef31..be87558 100644 --- a/src/app/bills/server/get-bill-by-id-from-db.ts +++ b/src/app/bills/server/get-bill-by-id-from-db.ts @@ -5,20 +5,49 @@ import { Bill } from "@/app/bills/models/Bill"; import type { BillDocument } from "@/app/bills/models/Bill"; import { env } from "@/app/bills/env"; +// Distinguishes "the bill isn't in the DB" from "we couldn't check". Callers +// that trigger expensive side effects (LLM summarization, persistence) must +// treat "unavailable" as unknown state and skip them — a missing MONGO_URI +// must never look like an empty database. +export type BillDbLookup = + | { status: "found"; bill: BillDocument } + | { status: "not-found" } + | { status: "unavailable"; reason: "missing-uri" | "connect-or-query-error" }; + // Wrapped in React.cache so repeated lookups for the same bill within a single // server render (e.g. the page body and a helper both resolving the same id) -// share one query instead of re-hitting Mongo. -export const getBillByIdFromDB = cache( - async (billId: string): Promise => { +// share one query instead of re-hitting Mongo. Never throws, so the cached +// entry is always a settled result. +export const lookupBillInDB = cache( + async (billId: string): Promise => { const uri = env.MONGO_URI || ""; const hasValidMongoUri = uri.startsWith("mongodb://") || uri.startsWith("mongodb+srv://"); - if (!hasValidMongoUri) return null; + if (!hasValidMongoUri) { + console.error( + `[bills] DB unavailable for ${billId}: MONGO_URI is missing or malformed`, + ); + return { status: "unavailable", reason: "missing-uri" }; + } - await connectToDatabase(); - const existing = (await Bill.findOne({ billId }) - .lean() - .exec()) as BillDocument | null; - return existing; + try { + await connectToDatabase(); + const existing = (await Bill.findOne({ billId }) + .lean() + .exec()) as BillDocument | null; + return existing + ? { status: "found", bill: existing } + : { status: "not-found" }; + } catch (error) { + console.error(`[bills] DB unavailable for ${billId}:`, error); + return { status: "unavailable", reason: "connect-or-query-error" }; + } }, ); + +export async function getBillByIdFromDB( + billId: string, +): Promise { + const result = await lookupBillInDB(billId); + return result.status === "found" ? result.bill : null; +} diff --git a/src/app/bills/services/billApi.ts b/src/app/bills/services/billApi.ts index 45a6213..6ce7d64 100644 --- a/src/app/bills/services/billApi.ts +++ b/src/app/bills/services/billApi.ts @@ -78,6 +78,10 @@ export interface BillAnalysis { missing_details: string[]; steel_man: string; question_period_questions?: Array<{ question: string }>; + // Set when this is a degraded placeholder (no OpenAI key, parse failure, + // API error, rate cap) rather than a real analysis. Callers must not + // persist or Slack-notify a fallback. Never written to Mongo. + isFallback?: true; } export async function getBillFromCivicsProjectApi( @@ -121,7 +125,27 @@ export async function getBillFromCivicsProjectApi( return data; } -export async function summarizeBillText(input: string): Promise { +// Soft cap on organic (non-admin) OpenAI calls per process. The primary guard +// lives in fromCivicsProjectApiBill; this backstop bounds the cost of any +// future regression that reopens a rerun loop (July 14 incident). +const SUMMARIZE_CAP_PER_HOUR = 25; +let summarizeWindowStart = 0; +let summarizeCallsInWindow = 0; + +function summarizeCapExceeded(): boolean { + const now = Date.now(); + if (now - summarizeWindowStart > 60 * 60 * 1000) { + summarizeWindowStart = now; + summarizeCallsInWindow = 0; + } + summarizeCallsInWindow += 1; + return summarizeCallsInWindow > SUMMARIZE_CAP_PER_HOUR; +} + +export async function summarizeBillText( + input: string, + options?: { bypassCap?: boolean }, +): Promise { if (!process.env.OPENAI_API_KEY) { console.log("No OPENAI API key, using fallback analysis"); // Fallback analysis @@ -140,11 +164,31 @@ export async function summarizeBillText(input: string): Promise { steel_man: "The steel man for this bill is the bill that aligns with the tenets of Build Canada.", question_period_questions: [], + isFallback: true, + }; + } + + if (!options?.bypassCap && summarizeCapExceeded()) { + console.error( + `[LLM_SUMMARIZE_CAP] exceeded ${SUMMARIZE_CAP_PER_HOUR} calls/hour — refusing OpenAI call`, + ); + const text = input?.trim() || ""; + return { + summary: text.length <= 500 ? text : `${text.slice(0, 500)}…`, + short_title: undefined, + tenet_evaluations: makeFallbackTenets("Analysis rate cap exceeded"), + final_judgment: "abstain", + rationale: undefined, + needs_more_info: true, + missing_details: ["Analysis rate cap exceeded"], + steel_man: "Analysis rate cap exceeded", + question_period_questions: [], + isFallback: true, }; } try { - console.log("Analyzing bill text with AI"); + console.warn("[LLM_SUMMARIZE] starting", { inputChars: input.length }); const OpenAIClient = new OpenAI(); const prompt = `${SUMMARY_AND_VOTE_PROMPT}\n\nBill Text:\n${input}`; @@ -204,6 +248,7 @@ export async function summarizeBillText(input: string): Promise { missing_details: ["Valid AI response format"], steel_man: "Analysis parsing failed", question_period_questions: [], + isFallback: true, }; } } catch (error) { @@ -223,6 +268,7 @@ export async function summarizeBillText(input: string): Promise { missing_details: ["Technical issue resolution"], steel_man: "Technical error during analysis", question_period_questions: [], + isFallback: true, }; } } diff --git a/src/app/bills/utils/billConverters.ts b/src/app/bills/utils/billConverters.ts index 16cb121..92eccc1 100644 --- a/src/app/bills/utils/billConverters.ts +++ b/src/app/bills/utils/billConverters.ts @@ -8,6 +8,7 @@ import { } from "@/app/bills/services/billApi"; import { socialIssueGrader } from "@/app/bills/services/social-issue-grader"; import { notifyNewBillAnalysis } from "@/app/bills/services/slack-notifier"; +import { lookupBillInDB } from "@/app/bills/server/get-bill-by-id-from-db"; // Unified bill data structure export interface UnifiedBill { @@ -106,11 +107,6 @@ export function fromBuildCanadaDbBill(bill: BillDocument): UnifiedBill { export async function fromCivicsProjectApiBill( bill: ApiBillDetail, ): Promise { - const { env } = await import("@/app/bills/env"); - const uri = env.MONGO_URI || ""; - const hasValidMongoUri = - uri.startsWith("mongodb://") || uri.startsWith("mongodb+srv://"); - let existingBill: BillDocument | null = null; const latestStageDate = bill.stages && bill.stages.length > 0 ? bill.stages[bill.stages.length - 1].date @@ -184,61 +180,69 @@ export async function fromCivicsProjectApiBill( explanation: "Not analyzed", }, ], - final_judgment: "no", + final_judgment: "abstain", rationale: undefined, needs_more_info: false, missing_details: [], steel_man: "Not analyzed", }; - // Check existing bill in database to see if source changed - try { - const { connectToDatabase } = await import("@/app/bills/lib/mongoose"); - const { Bill } = await import("@/app/bills/models/Bill"); + // LLM summarization is fail-safe: it may only run when the DB is confirmed + // reachable AND the bill is genuinely new or its source text actually + // changed. An unavailable DB (missing MONGO_URI, connection error) must + // never be treated as "bill not found" — that turns every page view into an + // OpenAI call (July 14 incident). + const dbLookup = await lookupBillInDB(bill.billID); + const existingBill = dbLookup.status === "found" ? dbLookup.bill : null; + const dbAvailable = dbLookup.status !== "unavailable"; - if (hasValidMongoUri) { - await connectToDatabase(); - existingBill = (await Bill.findOne({ billId: bill.billID }) - .lean() - .exec()) as BillDocument | null; - - if (existingBill && !analysis.rationale) { - // Source hasn't changed, use existing analysis - analysis = { - summary: existingBill.summary, - tenet_evaluations: - existingBill.tenet_evaluations || analysis.tenet_evaluations, - final_judgment: (() => { - const raw = String(existingBill.final_judgment || "") - .trim() - .toLowerCase(); - if (raw === "yes" || raw === "no") return raw as "yes" | "no"; - // Treat legacy "neutral" and any unknown value as "abstain" - if (raw === "abstain" || raw === "neutral") return "abstain"; - return analysis.final_judgment; - })(), - rationale: existingBill.rationale || analysis.rationale, - needs_more_info: - existingBill.needs_more_info || analysis.needs_more_info, - missing_details: - existingBill.missing_details || analysis.missing_details, - steel_man: existingBill.steel_man || analysis.steel_man, - }; - console.log( - `Using existing analysis for ${bill.billID} (source unchanged)`, - ); - } - } - } catch (error) { - console.error("Error checking existing bill:", error); - // Continue with regeneration if DB check fails + if (existingBill) { + analysis = { + summary: existingBill.summary, + tenet_evaluations: + existingBill.tenet_evaluations || analysis.tenet_evaluations, + final_judgment: (() => { + const raw = String(existingBill.final_judgment || "") + .trim() + .toLowerCase(); + if (raw === "yes" || raw === "no") return raw as "yes" | "no"; + // Treat legacy "neutral" and any unknown value as "abstain" + if (raw === "abstain" || raw === "neutral") return "abstain"; + return analysis.final_judgment; + })(), + rationale: existingBill.rationale || analysis.rationale, + needs_more_info: existingBill.needs_more_info || analysis.needs_more_info, + missing_details: existingBill.missing_details || analysis.missing_details, + steel_man: existingBill.steel_man || analysis.steel_man, + }; } + const billTextsCount = Array.isArray(bill.billTexts) + ? bill.billTexts.length + : 0; + const sourceChanged = existingBill + ? (existingBill.source || null) !== (bill.source || null) + : false; + const countChanged = existingBill + ? existingBill.billTextsCount !== billTextsCount + : false; + const shouldRegenerate = + dbAvailable && + (dbLookup.status === "not-found" || sourceChanged || countChanged); + let generatedNewAnalysis = false; - if (!analysis?.rationale && billMarkdown) { - console.log(`Regenerating analysis for ${bill.billID} (source changed)`); + if (shouldRegenerate && billMarkdown) { + console.log( + `Regenerating analysis for ${bill.billID} (${ + existingBill ? "source changed" : "new bill" + })`, + ); analysis = await summarizeBillText(billMarkdown); generatedNewAnalysis = true; + } else if (existingBill) { + console.log( + `Using existing analysis for ${bill.billID} (source unchanged)`, + ); } // Only classify if missing (new bill or classification absent). Avoid calling otherwise. @@ -247,7 +251,7 @@ export async function fromCivicsProjectApiBill( ? ((existingBill as BillDocument).isSocialIssue as boolean) : false; if ( - hasValidMongoUri && + dbAvailable && (existingBill === null || typeof existingBill.isSocialIssue !== "boolean") ) { isSocialIssueFinal = await socialIssueGrader( @@ -255,17 +259,23 @@ export async function fromCivicsProjectApiBill( ); } - await onBillNotInDatabase({ - billId: bill.billID, - source: bill.source, - markdown: billMarkdown, - bill, - analysis, - billTextsCount: Array.isArray(bill.billTexts) ? bill.billTexts.length : 0, - isSocialIssue: isSocialIssueFinal, - }); + // A fallback analysis (no OpenAI key, parse failure, API error) must never + // overwrite stored data or ping Slack. + const analysisUsable = !generatedNewAnalysis || !analysis.isFallback; + + if (dbAvailable && analysisUsable) { + await onBillNotInDatabase({ + billId: bill.billID, + source: bill.source, + markdown: billMarkdown, + bill, + analysis, + billTextsCount, + isSocialIssue: isSocialIssueFinal, + }); + } - if (generatedNewAnalysis) { + if (generatedNewAnalysis && analysisUsable) { await notifyNewBillAnalysis({ billId: bill.billID, title: bill.title,