perf(convex): cut prod bill from ~$41/mo to ~$1-3/mo - #63
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
WalkthroughChangesPipeline execution and operations
Ranked feed pagination
Storage retention
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Cron
participant enrichUnprocessedArticles
participant clusterEnrichedArticles
participant FollowUpJobs
Cron->>enrichUnprocessedArticles: start batch chain
enrichUnprocessedArticles->>enrichUnprocessedArticles: schedule delayed next batch
enrichUnprocessedArticles->>clusterEnrichedArticles: schedule clustering after terminal batch
clusterEnrichedArticles->>clusterEnrichedArticles: schedule delayed next batch
clusterEnrichedArticles->>FollowUpJobs: schedule merge, recluster, and summarization
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 3📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/convex/enrichmentNode.ts (1)
1258-1268: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winChain-terminating early exits drop the cumulative chain state in both pipeline actions. Both actions now defer downstream scheduling to the terminal chain link, but only the success and empty-backlog exits were updated to hand off the accumulated counters. Every other exit ends the chain and discards the work of earlier links, so those articles and events wait for the next cron window, which is now up to 6 hours away.
packages/backend/convex/enrichmentNode.ts#L1258-L1268: scheduleclusterEnrichedArticleswhenenrichedSoFar > 0in thecatchblock at Line 1299, and before thepipeline_pausedreturn at Line 1160 and theai_budget_exhaustedreturn at Line 1184.packages/backend/convex/clustering.ts#L6939-L6968: callscheduleClusteringFollowUpswithclusteredSoFarandcreatedSoFarbefore the vector-search budget-exhausted return at Line 6492.🤖 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/enrichmentNode.ts` around lines 1258 - 1268, The chain-terminating exits must preserve cumulative work before returning. In packages/backend/convex/enrichmentNode.ts lines 1160, 1184, and 1299, schedule clusterEnrichedArticles when enrichedSoFar is greater than zero before the pipeline_paused return, ai_budget_exhausted return, and catch-block termination; retain the existing terminal scheduling behavior. In packages/backend/convex/clustering.ts lines 6939-6968, call scheduleClusteringFollowUps with clusteredSoFar and createdSoFar before the vector-search budget-exhausted return.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/backend/convex/clustering.ts`:
- Around line 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.
In `@packages/backend/convex/config.ts`:
- Around line 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.
In `@packages/backend/convex/crons.ts`:
- Around line 20-21: Update the freshness-cost comment near the feed scheduling
configuration to state the snapshot-inclusive worst-case delay of approximately
7 hours 30 minutes, replacing the inaccurate ~6-hour figure while preserving the
explanation that this is an intentional availability tradeoff.
In `@packages/backend/convex/events.ts`:
- Around line 195-223: Add an absolute row cap for the widened tie-run window
and clamp each growth step to that cap. Update the loop around readRankedWindow
so it stops growing once the cap is reached while preserving the existing
attempt and reachedEnd conditions; ensure the final read never requests more
than the capped number of rows.
In `@packages/backend/convex/lib/aiCall.ts`:
- Around line 118-130: Update isRateLimitError to guard against nullish error
values before invoking errorStatus, returning false for null or undefined
inputs. Preserve the existing status and message-based detection for non-null
errors, and ensure callers such as isRetryableError and
requeueRateLimitedSummaryJobs cannot encounter a TypeError from missing
lastError values.
In `@packages/backend/convex/migrations.ts`:
- Around line 1387-1391: Bound the eventShareAssets read in the migration’s
referenced-set reconstruction instead of calling collect() unconditionally. Use
an explicit take limit with a loud failure when the cap is exceeded, or paginate
through the query, while preserving collection of every storageId needed to
protect referenced assets.
- Around line 1418-1429: Update the return object in the migration function to
always include both stable keys, wouldDelete and deleted, instead of using the
computed [dryRun ? "wouldDelete" : "deleted"] key. Set the inactive key to the
appropriate empty or zero value while preserving the existing removed count for
the active operation, so TypeScript retains precise property types and
excess-property checking.
- Around line 1316-1352: Update the migration flow around the capped `failed`
query and returned summary so operators can distinguish a complete scan from the
1000-row truncation. Expose the scan cap explicitly in the result, and use it
with the existing `failed.length`/candidate counts to indicate when additional
older failed jobs may remain; do not claim `remaining` is zero as a complete
pass when the scan reached the cap.
In `@packages/backend/convex/pipeline.ts`:
- Around line 1336-1348: Update getAlertCadence to detect non-finite values from
readConfigNumber before applying the clamp, falling back to the established
default cadence so alert windows always remain numeric. Also verify config.ts
declares 720 minutes as the default and update the relevant migration to write
720 for pipeline_alert_check_interval_minutes, ensuring the stored cadence
matches the check-pipeline-alerts cron schedule.
- Around line 71-78: Replace the bounded table scans in getPipelineDoctor with
scalable queue-depth counters, preferably using a supported Sharded Counter or
Aggregate component and updating it wherever the source records change. If that
is not practical, move the five counts into a cron-refreshed snapshot document
using the refresh-pipeline-runtime-config pattern, so the reactive admin query
reads the snapshot instead of scanning articles and events on every write.
- Around line 1272-1329: Update countProcessingEventsOlderThan so the primary
by_status_last_article_at query excludes events with undefined lastArticleAt
before counting, preventing overlap with the legacy scan; preserve the separate
legacy-row handling and saturation behavior. Also revise the surrounding
documentation to describe the job as twice-daily rather than a 20-minute alert
cron.
In `@packages/backend/convex/retention.test.ts`:
- Around line 287-292: Update the default-retention test around
purgeStaleArticleEmbeddings to delete the existing 180-day config row, then
invoke the mutation without retentionDays. Keep the assertion that one embedding
is purged, so the test validates the getConfig fallback to
STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays rather than explicit-argument
override behavior.
In `@packages/backend/convex/retention.ts`:
- Around line 419-434: Move the clusterPairLabels guard query and
labeledArticleIds construction in the retention handler to occur only after
confirming that candidate reasons exist in the current range; return through the
existing no-op path before loading labels when no candidates are available,
while preserving the overflow guard behavior for non-empty candidate batches.
- Around line 296-311: Update the stale article embeddings purge flow around
purgeStaleArticleEmbeddings and its scheduler invocation to carry a
_creationTime watermark for batches with no deletions. When deleted === 0,
advance the watermark past the scanned candidates and pass it into the next
scheduled run so skipped head rows are not rescanned indefinitely; preserve
normal head scanning and completion behavior when progress is made or the index
is exhausted.
In `@packages/backend/convex/summarization.ts`:
- Around line 1421-1449: Update the handler containing ownsLease to return {
updated: false } without patching when a caller supplies a runId that does not
match the job’s processingRunId, preserving the existing behavior for matching
or absent runId values. In the summarization catch block in summarizationNode,
handle this result by continuing through the normal failure path rather than
returning skipped.
In `@packages/backend/convex/summarizationNode.ts`:
- Around line 1613-1617: Update the rate-limit handling around isRateLimitError
and rateLimitedForMs to track time spent under rate limiting rather than total
queue age from job.requestedAt. Add and persist a dedicated firstRateLimitedAt
timestamp (or equivalent consecutive-deferral state), set it on the first
rate-limit defer, use it for the 24-hour ceiling, and clear it after successful
completion; ensure revived jobs can enter the defer path normally without
consuming maxAttempts.
---
Outside diff comments:
In `@packages/backend/convex/enrichmentNode.ts`:
- Around line 1258-1268: The chain-terminating exits must preserve cumulative
work before returning. In packages/backend/convex/enrichmentNode.ts lines 1160,
1184, and 1299, schedule clusterEnrichedArticles when enrichedSoFar is greater
than zero before the pipeline_paused return, ai_budget_exhausted return, and
catch-block termination; retain the existing terminal scheduling behavior. In
packages/backend/convex/clustering.ts lines 6939-6968, call
scheduleClusteringFollowUps with clusteredSoFar and createdSoFar before the
vector-search budget-exhausted return.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94080725-a646-4f4d-b592-adb0175e5a97
📒 Files selected for processing (14)
packages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/crons.tspackages/backend/convex/enrichmentNode.tspackages/backend/convex/events.tspackages/backend/convex/feedPagination.test.tspackages/backend/convex/lib/aiCall.tspackages/backend/convex/lib/feedSerialization.tspackages/backend/convex/migrations.tspackages/backend/convex/pipeline.tspackages/backend/convex/retention.test.tspackages/backend/convex/retention.tspackages/backend/convex/summarization.tspackages/backend/convex/summarizationNode.ts
Comment on lines
6280
to
+6292
| 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 }, |
Contributor
There was a problem hiding this comment.
📐 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.
Comment on lines
702
to
707
| { | ||
| 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.", | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 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.
applyCostReductionConfiginpackages/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.
Comment on lines
+20
to
+21
| // Freshness cost: worst-case ~6h from publication to appearing in the feed. | ||
| // This is a deliberate, authorised trade to keep the app online. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the stated worst-case freshness.
The ingest window is 6 hours, summarize lands at :45, and rebuild-public-feed-snapshots runs 45 minutes later (Lines 379-384). An item published just after an ingest window therefore reaches an anonymous/cold feed load about 7 h 30 min later, not ~6 h. Operators read this block to judge the trade, so state the snapshot-inclusive figure.
📝 Proposed doc fix
-// Freshness cost: worst-case ~6h from publication to appearing in the feed.
+// Freshness cost: worst-case ~6h from publication to a summarized event, and
+// ~7h30m to appearing in the anonymous snapshot feed (snapshot rebuild runs
+// 45 min after each summarize step).
// This is a deliberate, authorised trade to keep the app online.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Freshness cost: worst-case ~6h from publication to appearing in the feed. | |
| // This is a deliberate, authorised trade to keep the app online. | |
| // Freshness cost: worst-case ~6h from publication to a summarized event, and | |
| // ~7h30m to appearing in the anonymous snapshot feed (snapshot rebuild runs | |
| // 45 min after each summarize step). | |
| // This is a deliberate, authorised trade to keep the app online. |
🤖 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/crons.ts` around lines 20 - 21, Update the
freshness-cost comment near the feed scheduling configuration to state the
snapshot-inclusive worst-case delay of approximately 7 hours 30 minutes,
replacing the inaccurate ~6-hour figure while preserving the explanation that
this is an intentional availability tradeoff.
Comment on lines
+118
to
+130
| export function isRateLimitError(error: unknown): boolean { | ||
| if (errorStatus(error) === 429) return true; | ||
| const message = | ||
| error instanceof Error | ||
| ? error.message | ||
| : typeof error === "string" | ||
| ? error | ||
| : ""; | ||
| return ( | ||
| message.length > 0 && | ||
| /\b429\b|RESOURCE_EXHAUSTED|rate[ _-]?limit|quota/i.test(message) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Guard against null and undefined before calling errorStatus.
errorStatus at Line 88 casts the argument and reads candidate.status without a nullish check. For null or undefined input that read throws TypeError: Cannot read properties of undefined.
The existing caller isRetryableError only receives values from a catch block, so this was not reachable. The new call site in packages/backend/convex/migrations.ts at Line 1325 passes job.lastError, which is optional on eventSummaryJobs. A failed job row without lastError therefore aborts the whole requeueRateLimitedSummaryJobs mutation.
🐛 Proposed fix
export function isRateLimitError(error: unknown): boolean {
+ if (error === null || error === undefined) return false;
if (errorStatus(error) === 429) return true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function isRateLimitError(error: unknown): boolean { | |
| if (errorStatus(error) === 429) return true; | |
| const message = | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === "string" | |
| ? error | |
| : ""; | |
| return ( | |
| message.length > 0 && | |
| /\b429\b|RESOURCE_EXHAUSTED|rate[ _-]?limit|quota/i.test(message) | |
| ); | |
| } | |
| export function isRateLimitError(error: unknown): boolean { | |
| if (error === null || error === undefined) return false; | |
| if (errorStatus(error) === 429) return true; | |
| const message = | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === "string" | |
| ? error | |
| : ""; | |
| return ( | |
| message.length > 0 && | |
| /\b429\b|RESOURCE_EXHAUSTED|rate[ _-]?limit|quota/i.test(message) | |
| ); | |
| } |
🤖 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/lib/aiCall.ts` around lines 118 - 130, Update
isRateLimitError to guard against nullish error values before invoking
errorStatus, returning false for null or undefined inputs. Preserve the existing
status and message-based detection for non-null errors, and ensure callers such
as isRetryableError and requeueRateLimitedSummaryJobs cannot encounter a
TypeError from missing lastError values.
Comment on lines
+287
to
+292
| // With the default 45-day window the same row is beyond retention. | ||
| const purged = await t.mutation( | ||
| internal.retention.purgeStaleArticleEmbeddings, | ||
| { retentionDays: STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays }, | ||
| ); | ||
| expect(purged.deleted).toBe(1); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The test name claims default behavior, but the assertion exercises the explicit argument.
The 180-day config row is still present at line 288. Passing retentionDays explicitly proves that the argument overrides config. It does not prove that the getConfig fallback returns STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays. Delete the config row and call the mutation with no arguments to cover the default path.
💚 Proposed change to cover the default path
- // With the default 45-day window the same row is beyond retention.
- const purged = await t.mutation(
- internal.retention.purgeStaleArticleEmbeddings,
- { retentionDays: STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays },
- );
+ // Remove the override so the default 45-day window applies.
+ await t.run(async (ctx) => {
+ const row = await ctx.db
+ .query("config")
+ .withIndex("by_key", (q) =>
+ q.eq("key", STORAGE_RETENTION_CONFIG_KEYS.articleEmbeddingDays),
+ )
+ .unique();
+ await ctx.db.delete(row!._id);
+ });
+ const purged = await t.mutation(
+ internal.retention.purgeStaleArticleEmbeddings,
+ {},
+ );
expect(purged.deleted).toBe(1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // With the default 45-day window the same row is beyond retention. | |
| const purged = await t.mutation( | |
| internal.retention.purgeStaleArticleEmbeddings, | |
| { retentionDays: STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays }, | |
| ); | |
| expect(purged.deleted).toBe(1); | |
| // Remove the override so the default 45-day window applies. | |
| await t.run(async (ctx) => { | |
| const row = await ctx.db | |
| .query("config") | |
| .withIndex("by_key", (q) => | |
| q.eq("key", STORAGE_RETENTION_CONFIG_KEYS.articleEmbeddingDays), | |
| ) | |
| .unique(); | |
| await ctx.db.delete(row!._id); | |
| }); | |
| const purged = await t.mutation( | |
| internal.retention.purgeStaleArticleEmbeddings, | |
| {}, | |
| ); | |
| expect(purged.deleted).toBe(1); |
🤖 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/retention.test.ts` around lines 287 - 292, Update the
default-retention test around purgeStaleArticleEmbeddings to delete the existing
180-day config row, then invoke the mutation without retentionDays. Keep the
assertion that one embedding is purged, so the test validates the getConfig
fallback to STORAGE_RETENTION_DEFAULTS.articleEmbeddingDays rather than
explicit-argument override behavior.
Source: Path instructions
Comment on lines
+296
to
+311
| // Stop when the head of the index is exhausted, or when a full batch | ||
| // produced no deletions (otherwise skipped rows would loop forever). | ||
| const done = candidates.length < batchSize || deleted === 0; | ||
| await logPurgeRun(ctx, "article_embeddings_stale", deleted, done, { | ||
| counters: { scanned: candidates.length, orphaned, skipped }, | ||
| metadata: { retentionDays, cutoff }, | ||
| }); | ||
|
|
||
| if (!done) { | ||
| await ctx.scheduler.runAfter( | ||
| 0, | ||
| internal.retention.purgeStaleArticleEmbeddings, | ||
| { retentionDays, batchSize }, | ||
| ); | ||
| } | ||
| return { deleted, scanned: candidates.length, skipped, orphaned, done }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
A full batch of skipped rows permanently stalls this purge.
done becomes true when deleted === 0. The scan always restarts at the head of by_creation_time. Rows that are held back by the publishedAt >= cutoff guard stay at the head. When batchSize such rows accumulate, every run scans the same rows, deletes nothing, and reports done. The purge can then never reach the rows behind them, so storage keeps growing.
This is the same hazard that purgeArchivedDetachedArticles documents and solves with a per-reason watermark. Apply the same pattern here: pass a _creationTime floor forward when a batch makes no progress.
♻️ Proposed fix using a creation-time watermark
export const purgeStaleArticleEmbeddings = internalMutation({
args: {
retentionDays: v.optional(v.number()),
batchSize: v.optional(v.number()),
+ /** `_creationTime` floor carried forward past batches that deleted nothing. */
+ from: v.optional(v.number()),
},+ const from = args.from ?? 0;
const candidates = await ctx.db
.query("articleEmbeddings")
- .withIndex("by_creation_time", (q) => q.lt("_creationTime", cutoff))
+ .withIndex("by_creation_time", (q) =>
+ q.gte("_creationTime", from).lt("_creationTime", cutoff),
+ )
.take(batchSize);- const done = candidates.length < batchSize || deleted === 0;
+ const done = candidates.length < batchSize;
+ const nextFrom =
+ deleted === 0 && candidates.length > 0
+ ? candidates[candidates.length - 1]!._creationTime + 1
+ : from;
await logPurgeRun(ctx, "article_embeddings_stale", deleted, done, {
counters: { scanned: candidates.length, orphaned, skipped },
metadata: { retentionDays, cutoff },
});
if (!done) {
await ctx.scheduler.runAfter(
0,
internal.retention.purgeStaleArticleEmbeddings,
- { retentionDays, batchSize },
+ { retentionDays, batchSize, from: nextFrom },
);
}🤖 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/retention.ts` around lines 296 - 311, Update the
stale article embeddings purge flow around purgeStaleArticleEmbeddings and its
scheduler invocation to carry a _creationTime watermark for batches with no
deletions. When deleted === 0, advance the watermark past the scanned candidates
and pass it into the next scheduled run so skipped head rows are not rescanned
indefinitely; preserve normal head scanning and completion behavior when
progress is made or the index is exhausted.
Source: Path instructions
Comment on lines
+419
to
+434
| // Guard 4: never delete an article that the clustering gold set cites. | ||
| const labels = await ctx.db | ||
| .query("clusterPairLabels") | ||
| .take(CLUSTER_LABEL_GUARD_LIMIT); | ||
| if (labels.length >= CLUSTER_LABEL_GUARD_LIMIT) { | ||
| // Can't prove the guard set is complete — do nothing rather than guess. | ||
| await logPurgeRun(ctx, "articles_archived_detached", 0, true, { | ||
| metadata: { skippedReason: "cluster_label_guard_overflow" }, | ||
| }); | ||
| return { deleted: 0, done: true, skippedReason: "guard_overflow" }; | ||
| } | ||
| const labeledArticleIds = new Set<string>(); | ||
| for (const label of labels) { | ||
| labeledArticleIds.add(label.leftArticleId); | ||
| labeledArticleIds.add(label.rightArticleId); | ||
| } |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Load the label guard set only when candidates exist.
The handler reads up to CLUSTER_LABEL_GUARD_LIMIT (5000) clusterPairLabels rows on every invocation, including every self-chained batch and every no-op cron run. This PR targets Convex read bandwidth, so pay this cost only when there is work to do.
Move the guard load after a cheap candidate probe, or return early when no reason has candidates in range.
♻️ Proposed reordering
- // Guard 4: never delete an article that the clustering gold set cites.
- const labels = await ctx.db
- .query("clusterPairLabels")
- .take(CLUSTER_LABEL_GUARD_LIMIT);
+ // Cheap probe: skip the guard scan entirely when nothing is eligible.
+ let hasWork = false;
+ for (const [index, reason] of ARCHIVED_REASONS.entries()) {
+ const probe = await ctx.db
+ .query("articles")
+ .withIndex("by_archived_reason", (q) =>
+ q
+ .eq("archivedReason", reason)
+ .gte("archivedAt", args.cursors?.[index] ?? 0)
+ .lt("archivedAt", cutoff),
+ )
+ .first();
+ if (probe !== null) {
+ hasWork = true;
+ break;
+ }
+ }
+ if (!hasWork) {
+ await logPurgeRun(ctx, "articles_archived_detached", 0, true);
+ return { deleted: 0, deletedEmbeddings: 0, scanned: 0, done: true };
+ }
+
+ // Guard 4: never delete an article that the clustering gold set cites.
+ const labels = await ctx.db
+ .query("clusterPairLabels")
+ .take(CLUSTER_LABEL_GUARD_LIMIT);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Guard 4: never delete an article that the clustering gold set cites. | |
| const labels = await ctx.db | |
| .query("clusterPairLabels") | |
| .take(CLUSTER_LABEL_GUARD_LIMIT); | |
| if (labels.length >= CLUSTER_LABEL_GUARD_LIMIT) { | |
| // Can't prove the guard set is complete — do nothing rather than guess. | |
| await logPurgeRun(ctx, "articles_archived_detached", 0, true, { | |
| metadata: { skippedReason: "cluster_label_guard_overflow" }, | |
| }); | |
| return { deleted: 0, done: true, skippedReason: "guard_overflow" }; | |
| } | |
| const labeledArticleIds = new Set<string>(); | |
| for (const label of labels) { | |
| labeledArticleIds.add(label.leftArticleId); | |
| labeledArticleIds.add(label.rightArticleId); | |
| } | |
| // Cheap probe: skip the guard scan entirely when nothing is eligible. | |
| let hasWork = false; | |
| for (const [index, reason] of ARCHIVED_REASONS.entries()) { | |
| const probe = await ctx.db | |
| .query("articles") | |
| .withIndex("by_archived_reason", (q) => | |
| q | |
| .eq("archivedReason", reason) | |
| .gte("archivedAt", args.cursors?.[index] ?? 0) | |
| .lt("archivedAt", cutoff), | |
| ) | |
| .first(); | |
| if (probe !== null) { | |
| hasWork = true; | |
| break; | |
| } | |
| } | |
| if (!hasWork) { | |
| await logPurgeRun(ctx, "articles_archived_detached", 0, true); | |
| return { deleted: 0, deletedEmbeddings: 0, scanned: 0, done: true }; | |
| } | |
| // Guard 4: never delete an article that the clustering gold set cites. | |
| const labels = await ctx.db | |
| .query("clusterPairLabels") | |
| .take(CLUSTER_LABEL_GUARD_LIMIT); | |
| if (labels.length >= CLUSTER_LABEL_GUARD_LIMIT) { | |
| // Can't prove the guard set is complete — do nothing rather than guess. | |
| await logPurgeRun(ctx, "articles_archived_detached", 0, true, { | |
| metadata: { skippedReason: "cluster_label_guard_overflow" }, | |
| }); | |
| return { deleted: 0, done: true, skippedReason: "guard_overflow" }; | |
| } | |
| const labeledArticleIds = new Set<string>(); | |
| for (const label of labels) { | |
| labeledArticleIds.add(label.leftArticleId); | |
| labeledArticleIds.add(label.rightArticleId); | |
| } |
🤖 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/retention.ts` around lines 419 - 434, Move the
clusterPairLabels guard query and labeledArticleIds construction in the
retention handler to occur only after confirming that candidate reasons exist in
the current range; return through the existing no-op path before loading labels
when no candidates are available, while preserving the overflow guard behavior
for non-empty candidate batches.
Comment on lines
+1421
to
+1449
| handler: async ( | ||
| ctx, | ||
| { jobId, reason, retryAfterMs, runId, refundAttempt }, | ||
| ) => { | ||
| const job = await ctx.db.get(jobId); | ||
| if (!job || job.status === "succeeded" || job.status === "skipped") { | ||
| return { updated: false as const }; | ||
| } | ||
|
|
||
| // Only refund the attempt we actually own — if another worker has since | ||
| // re-leased the job, leave its attempt counter alone. | ||
| const ownsLease = | ||
| job.status === "processing" && | ||
| runId !== undefined && | ||
| job.processingRunId === runId; | ||
| const attempts = | ||
| refundAttempt && ownsLease ? Math.max(0, job.attempts - 1) : job.attempts; | ||
|
|
||
| await ctx.db.patch(jobId, { | ||
| status: job.status === "failed" ? "failed" : "queued", | ||
| attempts, | ||
| processingRunId: undefined, | ||
| leaseExpiresAt: undefined, | ||
| nextAttemptAt: Date.now() + Math.max(60_000, retryAfterMs), | ||
| lastError: reason.slice(0, 1000), | ||
| updatedAt: Date.now(), | ||
| }); | ||
|
|
||
| return { updated: true as const }; | ||
| return { updated: true as const, attempts }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not clear another worker's lease when runId does not match.
ownsLease currently gates only the attempt refund. The patch itself still runs, so a caller that passes a stale runId sets status: "queued" and clears processingRunId and leaseExpiresAt for the worker that now owns the job.
This is reachable through the new rate-limit path in packages/backend/convex/summarizationNode.ts at Line 1621. JOB_LEASE_TTL_MS is 10 minutes. If a summary run exceeds the lease, another worker re-leases the job, and the first run's catch block then defers it. The active run loses its lease, the job returns to queued, and a third worker can start the same event concurrently. That duplicates billed model calls, which works against the goal of this PR.
markSummaryJobFailed at Line 1337 already returns { updated: false } on a processingRunId mismatch. Apply the same rule here whenever the caller supplies a runId.
🔒 Proposed fix
const job = await ctx.db.get(jobId);
if (!job || job.status === "succeeded" || job.status === "skipped") {
return { updated: false as const };
}
- // Only refund the attempt we actually own — if another worker has since
- // re-leased the job, leave its attempt counter alone.
- const ownsLease =
- job.status === "processing" &&
- runId !== undefined &&
- job.processingRunId === runId;
+ // A caller that leased the job may only act on the lease it owns; if
+ // another worker has re-leased the job, leave the row untouched.
+ const ownsLease =
+ job.status === "processing" &&
+ runId !== undefined &&
+ job.processingRunId === runId;
+ if (runId !== undefined && !ownsLease) {
+ return { updated: false as const };
+ }
const attempts =
refundAttempt && ownsLease ? Math.max(0, job.attempts - 1) : job.attempts;Handle the { updated: false } result in the summarization catch block: fall through to the normal failure path instead of returning skipped.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| handler: async ( | |
| ctx, | |
| { jobId, reason, retryAfterMs, runId, refundAttempt }, | |
| ) => { | |
| const job = await ctx.db.get(jobId); | |
| if (!job || job.status === "succeeded" || job.status === "skipped") { | |
| return { updated: false as const }; | |
| } | |
| // Only refund the attempt we actually own — if another worker has since | |
| // re-leased the job, leave its attempt counter alone. | |
| const ownsLease = | |
| job.status === "processing" && | |
| runId !== undefined && | |
| job.processingRunId === runId; | |
| const attempts = | |
| refundAttempt && ownsLease ? Math.max(0, job.attempts - 1) : job.attempts; | |
| await ctx.db.patch(jobId, { | |
| status: job.status === "failed" ? "failed" : "queued", | |
| attempts, | |
| processingRunId: undefined, | |
| leaseExpiresAt: undefined, | |
| nextAttemptAt: Date.now() + Math.max(60_000, retryAfterMs), | |
| lastError: reason.slice(0, 1000), | |
| updatedAt: Date.now(), | |
| }); | |
| return { updated: true as const }; | |
| return { updated: true as const, attempts }; | |
| handler: async ( | |
| ctx, | |
| { jobId, reason, retryAfterMs, runId, refundAttempt }, | |
| ) => { | |
| const job = await ctx.db.get(jobId); | |
| if (!job || job.status === "succeeded" || job.status === "skipped") { | |
| return { updated: false as const }; | |
| } | |
| // A caller that leased the job may only act on the lease it owns; if | |
| // another worker has re-leased the job, leave the row untouched. | |
| const ownsLease = | |
| job.status === "processing" && | |
| runId !== undefined && | |
| job.processingRunId === runId; | |
| if (runId !== undefined && !ownsLease) { | |
| return { updated: false as const }; | |
| } | |
| const attempts = | |
| refundAttempt && ownsLease ? Math.max(0, job.attempts - 1) : job.attempts; | |
| await ctx.db.patch(jobId, { | |
| status: job.status === "failed" ? "failed" : "queued", | |
| attempts, | |
| processingRunId: undefined, | |
| leaseExpiresAt: undefined, | |
| nextAttemptAt: Date.now() + Math.max(60_000, retryAfterMs), | |
| lastError: reason.slice(0, 1000), | |
| updatedAt: Date.now(), | |
| }); | |
| return { updated: true as const, attempts }; |
🤖 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/summarization.ts` around lines 1421 - 1449, Update
the handler containing ownsLease to return { updated: false } without patching
when a caller supplies a runId that does not match the job’s processingRunId,
preserving the existing behavior for matching or absent runId values. In the
summarization catch block in summarizationNode, handle this result by continuing
through the normal failure path rather than returning skipped.
Comment on lines
+1613
to
+1617
| const rateLimitedForMs = Date.now() - job.requestedAt; | ||
| if ( | ||
| isRateLimitError(error) && | ||
| rateLimitedForMs < RATE_LIMIT_DEFER_CEILING_MS | ||
| ) { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The ceiling measures queue age, not rate-limited duration. Revived jobs skip the defer path.
job.requestedAt is the timestamp when the job was first enqueued. It is never reset. rateLimitedForMs therefore grows with total queue age, not with time spent under rate limits.
Two consequences:
- A job that waited more than 24 hours for any other reason (budget deferral at Line 1225, or an idle window) fails on its first 429 instead of deferring. The attempt is not refunded. This is the exact behavior the PR removes.
requeueRateLimitedSummaryJobsinpackages/backend/convex/migrations.tsresetsattempts,nextAttemptAt, andlastError, but notrequestedAt. The ~395 revived prod jobs all carry an oldrequestedAt, so every one of them exceeds the ceiling immediately and dies again on the next 429. The migration does not achieve its stated purpose.
Track rate-limit deferral time explicitly. Add a dedicated field, for example firstRateLimitedAt, set it on the first defer, and clear it on success. Alternatively count consecutive rate-limit deferrals in a separate counter that maxAttempts does not consume.
🤖 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/summarizationNode.ts` around lines 1613 - 1617,
Update the rate-limit handling around isRateLimitError and rateLimitedForMs to
track time spent under rate limiting rather than total queue age from
job.requestedAt. Add and persist a dedicated firstRateLimitedAt timestamp (or
equivalent consecutive-deferral state), set it on the first rate-limit defer,
use it for the 24-hour ceiling, and clear it after successful completion; ensure
revived jobs can enter the defer path normally without consuming maxAttempts.
Prod was disabled for exceeding Convex free-plan limits. Action compute was
81% of the bill, and summarizationNode.processSummaryJob alone was 92% of that
(~75% of the total). Convex bills action compute by WALL-CLOCK time including
time spent awaiting the network, so the cost was waiting, not computing.
Summarization (the dominant cost):
- Default event_summary_body_fetch_enabled to false. The key had no row in
prod, so it silently ran on the `true` code default, fetching an article
body per selected article on every job.
- Treat provider 429s as backpressure, not failure: defer without consuming an
attempt. Previously a 429 burned 1 of 3 attempts, which is why ~395 jobs sat
permanently dead and only ~31% of events were ever summarized. Bounded at
~24h so a permanently rate-limited job still surfaces as failed.
- Stop sleeping inside the action on 429 (maxRetries: 1 on the summary path).
- Body-fetch deadline 60s -> 12s; parallelize pre-work round trips.
Per job: ~16s -> ~2-4s of billed wall clock.
Pipeline cadence -> 4 staggered windows/day (:00 ingest, :15 enrich, :30
cluster, :45 summarize). Worst case ~6h to feed, a deliberate trade. Claim
divergence and daily-quiz crons disabled; both were already off at the config
layer and were waking only to find nothing to do.
Self-chaining (required by the cadence change): neither enrichment nor
clustering drained its backlog — each did one batch and stopped, keeping up
only because crons fired every 30/40min. At 4 windows/day that is 160+128
against ~1,300 articles/day. Both now chain until drained with a bounded
chainDepth, and hand off on cumulative totals so a terminal empty batch cannot
strand the drain. Clustering now fires follow-on passes once per drain rather
than once per batch — that per-batch kick, not the cron, was what produced
~1,680 summary jobs/day.
Database I/O:
- events.getPublishedEvents (top consumer): cursor-anchored ranked pagination.
Deep pages previously rescanned the top 250 rows, and because recency
dominates trendingScore, every publish invalidated every open page-2+
subscription. The cursor bounds the range above so new events fall outside.
- pipeline.countProcessingEventsOlderThan: saturating count, 10k docs -> 400.
- Bound assorted unbounded collect()/scan paths.
Storage (previously unbounded, so the bill compounded monthly): retention for
articleEmbeddings (45d) and archived detached articles (90d).
Alerting retuned for the batched cadence, including two rules whose windows
contained no pipeline run at all and so could never fire.
Fixes a pre-existing ranked-pagination bug surfaced by the new tie-run tests:
compareRankedPayload tie-broke on ASCENDING eventId against a descending index
traversal, so a tie run longer than the page buffer left the window holding a
biased sample and the feed silently dead-ended mid-list. Every comparator key
now descends, and the tiebreak uses the preview row's own _id (the index is on
publicEventPreviews, whose implicit last key is that table's _id).
Migrations (run after deploy; prod config rows override code defaults):
migrations:applyCostReductionConfig
migrations:requeueRateLimitedSummaryJobs
migrations:purgeOrphanedStorageFiles (reclaims ~992MB of orphaned blobs;
eventShareAssets is the only _storage reference and it is empty)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
migrations.ts imported isRateLimitError from lib/aiCall.ts, which is a "use node" module. Convex bundles every module without that directive for the V8 runtime, so the import dragged posthog-node's node built-ins into the V8 bundle and `convex codegen` failed with "Could not resolve node:async_hooks". Type checking does not catch this — it only surfaces at codegen, which is why it reached CI. Extract the predicate into a dependency-free lib/rateLimitError module that either runtime can import; aiCall re-exports it so existing callers are unchanged. Also commits the regenerated _generated/api.d.ts for the new retention and migration functions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flvvius
force-pushed
the
fix/convex-cost-reduction
branch
from
August 1, 2026 13:31
50e6c19 to
7150ca2
Compare
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Prod (
quirky-panda-609) is currently disabled for exceeding Convex free-plan limits. Audit of the usage dashboard against 19 days of prod telemetry (2026-07-07 DB reset → 07-25):Compute was 81% of the bill, and
summarizationNode.processSummaryJobwas 92% of compute (~75% of the total). Convex bills action compute by wall-clock time, including time spentawaiting the network — the cost was waiting, not computing. Of the last 800 summary jobs: 507 failed, 194 skipped, only 44 succeeded (5.5%); 391 of the failures were literally429 status code (no body).Projected after this PR: ~$1–3/mo, with compute back inside the 20 GB-h free allowance.
What changed
Summarization — body fetch off by default (the key had no prod row, so it ran on the
truecode default); 429s treated as backpressure that defers without consuming an attempt (bounded ~24h so a permanently limited job still surfaces as failed); no more in-action retry sleeps; body-fetch deadline 60s → 12s. Per job ~16s → ~2–4s.Cadence — 4 staggered windows/day (
:00ingest →:15enrich →:30cluster →:45summarize). Worst case ~6h to feed, a deliberate trade. Claim-divergence and daily-quiz crons disabled; both were already off at the config layer and woke only to find nothing to do.Self-chaining — required by the cadence change. Neither enrichment nor clustering drained its backlog; each did one batch and stopped, keeping up only because crons fired every 30/40 min. At 4 windows/day that is 160+128 against ~1,300 articles/day. Both now chain until drained, and hand off on cumulative totals so a terminal empty batch cannot strand the drain. Clustering now fires follow-on passes once per drain rather than once per batch — that per-batch kick, not the cron, is what produced ~1,680 summary jobs/day.
Database I/O —
getPublishedEventsmoved to cursor-anchored pagination. Deep pages rescanned the top 250 rows, and since recency dominatestrendingScore, every publish invalidated every open page-2+ subscription.countProcessingEventsOlderThan: 10k docs → 400.Storage — retention for
articleEmbeddings(45d) and archived detached articles (90d). Previously unbounded, so the bill compounded monthly.Alerting — retuned for the batched cadence, including two rules whose windows contained no pipeline run at all and could never fire.
Pre-existing bug fixed
The new tie-run tests surfaced a real feed bug:
compareRankedPayloadtie-broke on ascendingeventIdagainst a descending index traversal. A tie run longer than the page buffer left the window holding a biased sample, the cursor advanced past its max, and the feed silently dead-ended mid-list. Every comparator key now descends, and the tiebreak uses the preview row's own_id(the index is onpublicEventPreviews, whose implicit last key is that table's_id— uncorrelated witheventId).Deploy sequence
Blocked until the Convex plan limit clears. Then, in order — step 2 is required, since prod config rows override the new code defaults:
npx convex deploymigrations:applyCostReductionConfigconfig:refreshPipelineRuntimeConfigmigrations:requeueRateLimitedSummaryJobs— revives ~395 events killed by 429smigrations:purgeOrphanedStorageFiles— reclaims ~992 MB;eventShareAssetsis the only_storagereference in the schema and it is empty, so every stored blob is unreachableExpect a one-off I/O spike the first day retention runs — deleting rows means reading them.
Testing
tsc --noEmitclean · 36 test files, 289 passed, 4 skipped · all 25 cron expressions validated by loading the module.Reviewed by CodeRabbit (23 findings); both criticals and all majors addressed.
Not included
The working tree also holds an unrelated, pre-existing
MiezOnboardingremoval (4 files, 256 deletions). Deliberately left out to keep this reviewable.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Performance