Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/backend/convex/_generated/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import type * as lib_politeFetch from "../lib/politeFetch.js";
import type * as lib_publicEventPreviews from "../lib/publicEventPreviews.js";
import type * as lib_quizHelpers from "../lib/quizHelpers.js";
import type * as lib_rateLimit from "../lib/rateLimit.js";
import type * as lib_rateLimitError from "../lib/rateLimitError.js";
import type * as lib_retentionPolicy from "../lib/retentionPolicy.js";
import type * as lib_romanian from "../lib/romanian.js";
import type * as lib_sourceBias from "../lib/sourceBias.js";
Expand Down Expand Up @@ -144,6 +145,7 @@ declare const fullApi: ApiFromModules<{
"lib/publicEventPreviews": typeof lib_publicEventPreviews;
"lib/quizHelpers": typeof lib_quizHelpers;
"lib/rateLimit": typeof lib_rateLimit;
"lib/rateLimitError": typeof lib_rateLimitError;
"lib/retentionPolicy": typeof lib_retentionPolicy;
"lib/romanian": typeof lib_romanian;
"lib/sourceBias": typeof lib_sourceBias;
Expand Down
128 changes: 105 additions & 23 deletions packages/backend/convex/clustering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ const MERGE_LOCK_KEY = "mergeNearDuplicateEvents";
const RECLUSTER_SINGLETONS_LOCK_KEY = "reclusterRecentSingletonEvents";
const MERGE_LOCK_TTL_MS = 20 * 60 * 1000;
const CLUSTER_BATCH_SIZE = 32;
// Self-chaining bounds for clusterEnrichedArticles. 60 x CLUSTER_BATCH_SIZE(32)
// = 1,920 articles per triggered drain, which covers a full day of intake
// (~1,300/day) with headroom while guaranteeing the chain always terminates.
// The delay is long enough that the pipeline lock from the previous batch has
// been released before the next one tries to acquire it.
const MAX_CLUSTER_CHAIN_DEPTH = 60;
const CLUSTER_CHAIN_DELAY_MS = 10_000;
const RECENT_EVENT_WINDOW_MS = 48 * 60 * 60 * 1000;
const MAX_CANDIDATE_EVENTS = 220;
// Each vector-search neighbor is hydrated (candidacy + ~10KB embedding doc),
Expand Down Expand Up @@ -6230,10 +6237,62 @@ export const reclusterRecentSingletonEvents = internalAction({
},
});

/**
* Follow-on passes scheduled once a clustering DRAIN completes.
*
* These are deliberately deferred to the end of the self-chain rather than run
* per batch: previously each clustering batch kicked summarization directly,
* which is what actually drove summary volume (~1,680 processSummaryJob
* invocations/day) and made it the largest line item on the Convex bill.
*
* The totals passed in are CUMULATIVE across the whole chain. Using per-batch
* counters here would strand a drain whose final batch happened to cluster
* nothing — the events created by earlier links would never be summarized, and
* since summaries gate publishing, they would never go public.
*/
async function scheduleClusteringFollowUps(
ctx: ActionCtx,
totals: { clusteredIntoExisting: number; createdEvents: number },
): Promise<void> {
const touchedEvents = totals.clusteredIntoExisting + totals.createdEvents;
if (touchedEvents > 0) {
await ctx.scheduler.runAfter(
MERGE_NEAR_DUPLICATES_DELAY_MS,
internal.clustering.mergeNearDuplicateEvents,
{},
);
}
if (totals.createdEvents > 0) {
await ctx.scheduler.runAfter(
RECLUSTER_RECENT_SINGLETONS_DELAY_MS,
internal.clustering.reclusterRecentSingletonEvents,
{},
);
}
// Events publish only after a summary, so kick summarization once the drain
// finishes rather than waiting up to 6h for the next cron window.
if (touchedEvents > 0) {
await ctx.scheduler.runAfter(
0,
internal.summarizationNode.summarizeQueuedEvents,
{},
);
}
}

export const clusterEnrichedArticles = internalAction({
args: {},
// These are set only by this action rescheduling itself; the cron,
// enrichment's hand-off, and manual invocations all start at 0. The
// `*SoFar` counters accumulate across the chain so the terminal link can
// decide the follow-on passes from the drain's total work, not its own batch.
args: {
chainDepth: v.optional(v.number()),
clusteredSoFar: v.optional(v.number()),
createdSoFar: v.optional(v.number()),
},
handler: async (
ctx,
{ chainDepth = 0, clusteredSoFar = 0, createdSoFar = 0 },
Comment on lines 6283 to +6295

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider clamping chainDepth and the cumulative counters.

chainDepth, clusteredSoFar, and createdSoFar are plain v.optional(v.number()). A manual invocation with a negative or fractional chainDepth bypasses the MAX_CLUSTER_CHAIN_DEPTH bound. internalAction limits the blast radius to operators, so this is defensive only. Normalize with the existing safeInteger helper if you want the bound to hold for every caller.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/backend/convex/clustering.ts` around lines 6280 - 6292, Normalize
chainDepth, clusteredSoFar, and createdSoFar at the start of
clusterEnrichedArticles using the existing safeInteger helper, applying
nonnegative integer bounds so negative or fractional manual inputs cannot bypass
MAX_CLUSTER_CHAIN_DEPTH or corrupt cumulative totals. Preserve the current
defaults and downstream accumulation behavior.

): Promise<{
clusteredIntoExisting: number;
createdEvents: number;
Expand All @@ -6260,6 +6319,13 @@ export const clusterEnrichedArticles = internalAction({
console.log(
`[clustering] clusterEnrichedArticles already running (owner=${lock.owner}, expiresAt=${new Date(lock.expiresAt).toISOString()})`,
);
// A chained link that loses the lock still has to hand off whatever the
// earlier links accomplished, or that work is stranded unsummarized (and
// therefore unpublished) until a later cron window happens to pick it up.
await scheduleClusteringFollowUps(ctx, {
clusteredIntoExisting: clusteredSoFar,
createdEvents: createdSoFar,
});
return {
clusteredIntoExisting: 0,
createdEvents: 0,
Expand All @@ -6286,6 +6352,12 @@ export const clusterEnrichedArticles = internalAction({
);
if (!hasEnriched) {
console.log("[clustering] No enriched articles to cluster");
// Chain terminator: the previous link may have taken a full batch (and
// so chained) only for the backlog to be empty by the time this ran.
await scheduleClusteringFollowUps(ctx, {
clusteredIntoExisting: clusteredSoFar,
createdEvents: createdSoFar,
});
return {
clusteredIntoExisting: 0,
createdEvents: 0,
Expand All @@ -6302,6 +6374,11 @@ export const clusterEnrichedArticles = internalAction({

if (articles.length === 0) {
console.log("[clustering] No enriched articles to cluster");
// Chain terminator — see the note on the !hasEnriched branch above.
await scheduleClusteringFollowUps(ctx, {
clusteredIntoExisting: clusteredSoFar,
createdEvents: createdSoFar,
});
return {
clusteredIntoExisting: 0,
createdEvents: 0,
Expand Down Expand Up @@ -6862,30 +6939,35 @@ export const clusterEnrichedArticles = internalAction({
clusterRunReservationSettled = true;
}
await flushJobMetrics(ctx, metrics, startedAt);
if (clusteredIntoExisting + createdEvents > 0) {
await ctx.scheduler.runAfter(
MERGE_NEAR_DUPLICATES_DELAY_MS,
internal.clustering.mergeNearDuplicateEvents,
{},
);
}
if (createdEvents > 0) {
await ctx.scheduler.runAfter(
RECLUSTER_RECENT_SINGLETONS_DELAY_MS,
internal.clustering.reclusterRecentSingletonEvents,
{},
);
}
// Events publish only after a summary, so kick summarization immediately
// after a clustering batch instead of waiting up to 45 min for the cron —
// any event that just crossed the summary/publish bar gets its perspective
// summaries + globalImpact (and goes public) as soon as possible.
if (clusteredIntoExisting + createdEvents > 0) {

// Self-chain until the enriched-article backlog is drained.
//
// This action clusters at most CLUSTER_BATCH_SIZE articles per run. That
// kept up when the cron fired every 40 minutes, but the pipeline now runs
// in 4 batched windows per day for cost reasons, and 4 x 32 = 128
// articles/day is far below the ~1,300/day intake. Without chaining the
// backlog would grow forever and the feed would stall.
const likelyMoreWaiting = articles.length === CLUSTER_BATCH_SIZE;
const shouldChain =
likelyMoreWaiting && chainDepth + 1 < MAX_CLUSTER_CHAIN_DEPTH;
const clusteredTotal = clusteredSoFar + clusteredIntoExisting;
const createdTotal = createdSoFar + createdEvents;

if (shouldChain) {
await ctx.scheduler.runAfter(
0,
internal.summarizationNode.summarizeQueuedEvents,
{},
CLUSTER_CHAIN_DELAY_MS,
internal.clustering.clusterEnrichedArticles,
{
chainDepth: chainDepth + 1,
clusteredSoFar: clusteredTotal,
createdSoFar: createdTotal,
},
);
} else {
await scheduleClusteringFollowUps(ctx, {
clusteredIntoExisting: clusteredTotal,
createdEvents: createdTotal,
});
}

return {
Expand Down
40 changes: 27 additions & 13 deletions packages/backend/convex/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -697,13 +697,13 @@ export const seedDefaults = internalMutation({
key: "event_summary_enqueue_limit",
value: 40,
description:
"Maximum number of recent published events inspected for summary eligibility per summarization run.",
"Maximum number of recent published events inspected for summary eligibility per summarization run. This scan (summarization.enqueueEligibleEventSummaries) was the 2nd largest database-I/O consumer in the app at 2.15 GB, but the fix was cadence, not depth: dropping from 32 runs/day to 4 already cuts that I/O ~8x. Lowering this further would starve the queue of eligible events instead, since summaries gate publishing.",
},
{
key: "event_summary_batch_size",
value: 8,
value: 12,
description:
"Maximum number of queued event summary jobs processed per summarization run. Summaries gate publishing, so this is sized to keep pace with clustering rather than trickle.",
"Maximum number of queued event summary jobs processed per summarization run. Summaries gate publishing, so this must keep pace with the eligible-event rate or the feed stalls: at 4 runs/day this allows ~48 summaries/day. Jobs are staggered JOB_STAGGER_MS apart (~7.5 requests/min), which stays inside Gemini's free-tier RPM; the 429 backpressure path defers rather than failing, so overshoot self-corrects instead of burning attempts.",
},
Comment on lines 702 to 707

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

event_summary_batch_size: 12 cannot take effect. The consumer clamps it to 10.

loadSummarySettings in packages/backend/convex/summarizationNode.ts (Lines 492-497) reads this key through safeInteger(args.processLimit ?? cfg.event_summary_batch_size, DEFAULT_BATCH_SIZE, 1, 10). The upper bound is 10, so a stored value of 12 resolves to 10 or to DEFAULT_BATCH_SIZE, never 12.

Two effects follow:

  • The throughput claim in this description (~48 summaries/day at 4 runs/day) does not hold. The real ceiling is 40/day, which is below the eligible-event rate the description says must be matched. Summaries gate publishing, so the feed can fall behind.
  • applyCostReductionConfig in packages/backend/convex/migrations.ts (Line 1217) writes 12 as well, so the stored row and the effective runtime value stay permanently out of sync. The migration will report a change that has no effect.

Raise the safeInteger upper bound in loadSummarySettings to at least 12, or set this default to 10 and correct the arithmetic in both descriptions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/backend/convex/config.ts` around lines 702 - 707, Update
loadSummarySettings so its safeInteger upper bound accepts the configured
event_summary_batch_size value of 12, preserving the intended ~48 summaries/day
throughput and keeping applyCostReductionConfig’s stored value aligned with the
effective runtime setting.

{
key: "event_summary_max_attempts",
Expand All @@ -725,15 +725,15 @@ export const seedDefaults = internalMutation({
},
{
key: "event_summary_max_input_articles",
value: 8,
value: 6,
description:
"Maximum number of recent articles included in one event summarization prompt. Lower values reduce data egress (fewer bodies fetched and sent to the model) and token cost.",
"Maximum number of recent articles included in one event summarization prompt. Reduced in cost mode (12 -> 8 in #60, now 6): prompt size drives both data egress (billed per GB leaving Convex) and model latency, which is itself billed as action compute. 6 articles are still enough for a multi-perspective summary.",
},
{
key: "event_summary_body_fetch_enabled",
value: true,
value: false,
description:
"When true, the summarizer fetches each selected article's body transiently at summarization time (used in memory for the prompt, never stored) instead of relying only on the short extracted summary + RSS snippet.",
"When true, the summarizer fetches each selected article's body transiently at summarization time (used in memory for the prompt, never stored) instead of relying only on the short extracted summary + RSS snippet. DEFAULT OFF (cost mode): Convex bills action compute by wall-clock time including network waits, so fetching one body per selected article (up to event_summary_max_input_articles per job) made this the single most expensive operation in the app. Turning it on again is the biggest cost regression available — measure before doing so.",
},
{
key: "event_summary_body_chars",
Expand Down Expand Up @@ -1169,9 +1169,21 @@ export const seedDefaults = internalMutation({
},
{
key: "pipeline_alert_check_interval_minutes",
value: 20,
value: 720,
description:
"Nominal interval for pipeline alert checks. Must track the check-pipeline-alerts cron in crons.ts (cost mode: 2x daily), otherwise absent-run and staleness alerts fire spuriously.",
},
{
key: "archived_article_retention_days",
value: 90,
description:
"Age after which an ARCHIVED, event-detached article row (and its embeddings) is deleted. Only applies to articles archived by singletonCleanup as stale singletons/processing events, which belong to no event — never to articles reachable from the feed. Hand-labeled clusterPairLabels rows are excluded by the purge job.",
},
{
key: "article_embedding_retention_days",
value: 45,
description:
"Nominal interval for pipeline alert checks.",
"Age after which an article's 512-dimension embedding row is deleted. Clustering only ever compares recent articles, so older embeddings are dead weight — and unbounded articleEmbeddings growth (~1,300 articles/day) was the main driver of database storage cost. Article rows themselves are retained; only the vectors are purged.",
},
];

Expand Down Expand Up @@ -1216,10 +1228,12 @@ export const seedDefaults = internalMutation({
article_fact_extraction_model: ['"gpt-5-nano"'],
article_bias_detection_model: ['"gpt-5-nano"'],
claim_analysis_model: ['"gpt-5-nano"'],
// Migrate the prior summary input cap (12) down to the new egress-reduced
// default (8). This key is intentionally not force-managed, so operator
// overrides (any other value) are preserved.
event_summary_max_input_articles: ["12"],
// Migrate the prior summary input caps down to the current default.
// "12" was the original; "8" was the intermediate egress-reduced value
// from #60, which never reached prod before the deployment was disabled —
// both are listed so either state converges. This key is intentionally
// not force-managed, so operator overrides (any other value) are kept.
event_summary_max_input_articles: ["12", "8"],
};

let created = 0;
Expand Down
Loading
Loading