add pipeline enhancements - #33
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR simplifies the auth prompt UI, refactors quiz grading to async per-question mutations with locking, adds config gates for backfill flows, implements feed/sitemap snapshotting and pipeline IO rollups, enhances ingestion deduping and hot-embedding handling, and updates schema/cron/vector-search reservation logic. ChangesPlatform optimization and UI simplification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/components/auth-prompt-banner.tsx (1)
6-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
descriptionprop from the type definition.The
descriptionprop is declared inAuthPromptBannerPropsbut is no longer used by the component implementation. This creates a type/implementation mismatch.🧹 Proposed fix
type AuthPromptBannerProps = { redirectTo: AuthRedirectPath; title?: string; - description?: string; compact?: boolean; };As per coding guidelines, TypeScript code should maintain proper type safety and avoid unused type declarations.
🤖 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 `@apps/web/src/components/auth-prompt-banner.tsx` around lines 6 - 11, AuthPromptBannerProps declares a description property that is unused by the component implementation; remove the unused description field from the AuthPromptBannerProps type declaration so the prop types match the component (update the type where AuthPromptBannerProps is defined and ensure any usages of the now-removed description prop elsewhere are updated or removed accordingly).apps/web/src/routes/quiz.tsx (1)
248-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock submission until the last grading request settles.
answersis updated beforegradeQuestionresolves, so the last selection can makeansweredCounthit the total and enable Finish while grading is still in flight. If that grading call then fails, the catch path still deletes the answer and shows an error toast after the quiz was already submitted.Suggested fix
const handleSubmit = async () => { - if (isSubmitting || activeResult) return; + if (isSubmitting || isGrading || !currentFeedback || activeResult) return; try { const response = await submitQuiz.mutateAsync({ quizId: quiz._id, answers: Object.entries(answers).map(([questionId, choiceId]) => ({ questionId, choiceId, })), }); @@ <Button type="button" onClick={handleSubmit} disabled={ - answeredCount < quiz.questions.length || isSubmitting + answeredCount < quiz.questions.length || + isSubmitting || + isGrading || + !currentFeedback } >Also applies to: 413-418
🤖 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 `@apps/web/src/routes/quiz.tsx` around lines 248 - 259, The submit handler allows submitting while the last gradeQuestion call is still in flight, causing answers to be removed on a later grading error; modify handleSubmit (and the other submit path around the gradeQuestion usage) to block until any in-flight grading settles by tracking and awaiting a grading promise/ref (e.g., isGrading or pendingGradePromise) before proceeding to call submitQuiz.mutateAsync; ensure you await that pending promise (or Promise.all of pending graders) after the last answer update and before using answers/answeredCount, then proceed to setResult and setCurrentIndex only after submitQuiz completes.packages/backend/convex/quiz.ts (1)
290-318:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMove
gradeQuizQuestionto a Convexquery(avoid write path for read-only grading)
packages/backend/convex/quiz.tsgradeQuizQuestionis a mutation that only reads/validates (ctx.db.get(...), checks quiz/question/choice existence) and never persists anything. Inapps/web/src/routes/quiz.tsx, it’s called on every choice click viauseMutation(...).mutateAsync(...), so each click unnecessarily goes through Convex’s mutation/write path. ConvertgradeQuizQuestionto aqueryand call it from the UI with a one-offconvex.query(...)instead.🤖 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/quiz.ts` around lines 290 - 318, gradeQuizQuestion is declared as a mutation but only performs reads (ctx.db.get, validation of quiz/questions/choices) so change its declaration from mutation(...) to query(...) — update export const gradeQuizQuestion = mutation({...}) to export const gradeQuizQuestion = query({...}) and keep the handler logic unchanged; then update the UI call site in apps/web/src/routes/quiz.tsx to call the Convex read path (use convex.query(...) or the useQuery hook for a one-off call) instead of useMutation(...).mutateAsync(...), and remove any now-unused mutation imports so the grading uses the read-only Convex path.packages/backend/convex/clustering.ts (1)
2439-2461:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the event’s creation time here, not the candidacy row’s.
chooseCanonicalEvent()breaks ties oncreationTime, and this projection now feeds iteventCandidacy._creationTime. Backfills or recreated candidacy rows can be much newer than the underlying event, so a merge can keep the wrong canonical event/slug after projection data is rebuilt.Suggested direction
function projectClusterCandidate(args: { title: string; slug: string; + creationTime: number; embeddingRow: Doc<"eventEmbeddings">; candidacy: CandidacyWithProjection; }): ClusterCandidateQueryResult { return { eventId: args.candidacy.eventId, embeddingId: args.embeddingRow._id, title: args.title, slug: args.slug, @@ - creationTime: args.candidacy._creationTime, + creationTime: args.creationTime, }; }Pass
event._creationTimefrom callers that already load the event, or fetch/persist that timestamp for the merge/recluster hydration paths.🤖 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 2439 - 2461, projectClusterCandidate currently sets creationTime from candidacy._creationTime which breaks chooseCanonicalEvent tie-breaking; update projectClusterCandidate to accept and use the event's _creationTime (event._creationTime) instead of args.candidacy._creationTime, and update callers that already load the event to pass event._creationTime (or, for hydration/merge/recluster flows that don't load the event, fetch or persist the event's _creationTime and pass it) so chooseCanonicalEvent receives the true event creation timestamp.
🤖 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 `@apps/web/src/lib/i18n/strings.ts`:
- Around line 288-291: Remove the unused i18n key feed.authBody from the strings
file and any related references; specifically delete the "feed.authBody" entry
in strings.ts because routes/feed.tsx only supplies title to AuthPromptBanner
and the banner does not use a description, and optionally remove the now-unused
description prop from the AuthPromptBanner component (and its type/prop usage)
to keep props and translations in sync.
In `@apps/web/src/routes/quiz.tsx`:
- Around line 226-233: The code is casting mutation results (e.g., using "as
QuestionFeedback" and "as SubmitResult") which hides type mismatches; update the
mutation definitions so gradeQuestion and submitAnswer (or the corresponding
useMutation hooks) have correct generic TData types (or ensure their mutationFn
returns the exact Convex-generated types) so that gradeQuestion.mutateAsync(...)
and submitAnswer.mutateAsync(...) already return properly typed results and
remove the "as QuestionFeedback"/"as SubmitResult" assertions; adjust the
hook/generic signatures where gradeQuestion and the other mutation are created
so TypeScript enforces the correct return shape and then delete the casts in
setQuestionFeedback and the other call sites.
In `@apps/web/src/routes/sitemap`[.]xml.ts:
- Around line 50-53: The GET route currently calls
client.query(api.sitemap.getPublicSitemapXml) without catching exceptions so a
thrown error causes a 500 instead of serving the fallback; wrap the
ConvexHttpClient query call in a try/catch inside the GET handler (around the
call to client.query and assignment to snapshot) and on any error log it and set
xml = buildFallbackSitemapXml() (or otherwise set snapshot to undefined) so the
route returns the minimal sitemap when Convex is unavailable; reference the GET
handler, ConvexHttpClient, client.query, api.sitemap.getPublicSitemapXml,
snapshot, and buildFallbackSitemapXml to locate where to add the try/catch.
In `@packages/backend/convex/quizNode.ts`:
- Around line 296-315: buildInputSignature currently hashes only event/source
content so changes to prompt/schema/model or question-count won't invalidate the
signature; update buildInputSignature to include generation-relevant
config/version metadata (e.g., quiz_generation_model,
questionCount/question_count setting, any schema/prompt version identifiers) in
the payload before JSON.stringify so the hash changes when those settings
change; make the same change in the other signature function referenced around
482-487 so both skip-paths respect generation-versioning.
In `@packages/backend/convex/sitemap.ts`:
- Around line 81-109: The sitemap generation currently only grabs the newest
items by calling
ctx.db.query("publicEventPreviews").withIndex("by_last_updated_at").order("desc").take(limit)
and
ctx.db.query("sources").withIndex("by_rolling_bias_updated_at").order("desc").take(limit),
which makes the sitemap a rolling “recent updates” feed; change the logic in the
sitemap routine that builds entries (the block that calls toSitemapUrl for
events and sources) to page through the full result sets instead of a single
.take(limit) call: loop through the queries using the database
cursor/continuation mechanism (or repeated .skip/.take if supported) until no
more rows, accumulating all event and source rows, then emit either a single
full sitemap or produce chunked sitemap files and a sitemap index pointing to
them; ensure you update the places that reference .take(limit) and use the same
timestamp fields (event.lastUpdatedAt, source.rollingBiasUpdatedAt /
mbfcLastChecked / _creationTime) when creating each toSitemapUrl entry.
In `@packages/backend/convex/vectorSearchBudget.ts`:
- Around line 710-750: The cleanup job (cleanupExpiredVectorSearchReservations)
frees budget only when the cron runs, causing expired reservations
(VECTOR_SEARCH_RESERVATION_TTL_MS) to still count toward vectorSearchDaily; fix
by making reserveUsage ignore already-expired reservations instead of relying
solely on cleanup: when reserveUsage (or any place computing current reserved
budget) queries vectorSearchReservations, add a filter to only count records
with status "reserved" AND expiresAt > Date.now() (or compare against the same
now logic), so expired rows don't block new reservations even before cleanup
runs; alternatively, ensure the cron/scheduler (where
VECTOR_SEARCH_RUN_CLEANUP_CONTINUATION_DELAY_MS or the cron in crons.ts triggers
cleanup) runs more frequently than VECTOR_SEARCH_RESERVATION_TTL_MS, but prefer
updating reserveUsage's query to exclude expiresAt <= now for immediate
correctness.
---
Outside diff comments:
In `@apps/web/src/components/auth-prompt-banner.tsx`:
- Around line 6-11: AuthPromptBannerProps declares a description property that
is unused by the component implementation; remove the unused description field
from the AuthPromptBannerProps type declaration so the prop types match the
component (update the type where AuthPromptBannerProps is defined and ensure any
usages of the now-removed description prop elsewhere are updated or removed
accordingly).
In `@apps/web/src/routes/quiz.tsx`:
- Around line 248-259: The submit handler allows submitting while the last
gradeQuestion call is still in flight, causing answers to be removed on a later
grading error; modify handleSubmit (and the other submit path around the
gradeQuestion usage) to block until any in-flight grading settles by tracking
and awaiting a grading promise/ref (e.g., isGrading or pendingGradePromise)
before proceeding to call submitQuiz.mutateAsync; ensure you await that pending
promise (or Promise.all of pending graders) after the last answer update and
before using answers/answeredCount, then proceed to setResult and
setCurrentIndex only after submitQuiz completes.
In `@packages/backend/convex/clustering.ts`:
- Around line 2439-2461: projectClusterCandidate currently sets creationTime
from candidacy._creationTime which breaks chooseCanonicalEvent tie-breaking;
update projectClusterCandidate to accept and use the event's _creationTime
(event._creationTime) instead of args.candidacy._creationTime, and update
callers that already load the event to pass event._creationTime (or, for
hydration/merge/recluster flows that don't load the event, fetch or persist the
event's _creationTime and pass it) so chooseCanonicalEvent receives the true
event creation timestamp.
In `@packages/backend/convex/quiz.ts`:
- Around line 290-318: gradeQuizQuestion is declared as a mutation but only
performs reads (ctx.db.get, validation of quiz/questions/choices) so change its
declaration from mutation(...) to query(...) — update export const
gradeQuizQuestion = mutation({...}) to export const gradeQuizQuestion =
query({...}) and keep the handler logic unchanged; then update the UI call site
in apps/web/src/routes/quiz.tsx to call the Convex read path (use
convex.query(...) or the useQuery hook for a one-off call) instead of
useMutation(...).mutateAsync(...), and remove any now-unused mutation imports so
the grading uses the read-only Convex path.
🪄 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
Run ID: 6c8db821-67e6-4ed1-9930-48166a55d1ee
⛔ Files ignored due to path filters (1)
packages/backend/convex/_generated/api.d.tsis excluded by!**/_generated/**,!**/_generated/**
📒 Files selected for processing (20)
apps/web/src/components/auth-prompt-banner.tsxapps/web/src/lib/i18n/strings.tsapps/web/src/routes/admin.pipeline.tsxapps/web/src/routes/feed.tsxapps/web/src/routes/quiz.tsxapps/web/src/routes/sitemap[.]xml.tspackages/backend/convex/claimDivergence.tspackages/backend/convex/claimDivergenceNode.tspackages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/crons.tspackages/backend/convex/enrichmentNode.tspackages/backend/convex/ingestion.tspackages/backend/convex/pipeline.tspackages/backend/convex/quiz.tspackages/backend/convex/quizNode.tspackages/backend/convex/schema.tspackages/backend/convex/sitemap.tspackages/backend/convex/summarizationNode.tspackages/backend/convex/vectorSearchBudget.ts
…r crons, prune sweeper - events/feed: trending first-page snapshot now hands pagination back to the live ranked query (no more 24-item dead-end); recent feed stays live - add lib/feedSerialization as the single source of truth for the feed-card shape + ranked cursors (snapshot can no longer drift from live payload) - move snapshot rebuild off the per-write hot path to a cron (rebuild-public-feed-snapshots) to remove write amplification + OCC contention - wire pipelineRuntimeConfig: refresh cron + invalidate on admin config writes + warn when falling back to per-key reads - add pruneHotEventEmbeddings sweeper + hourly cron so the hot vector table stays small for quiet events - drop dead filterFields from eventEmbeddingHot vector index + the matching search filter - make representative-search reuse deterministic (best match) and O(n) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/backend/convex/singletonCleanup.ts (1)
346-368: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueHot embedding deletion count is not tracked.
The return value of
deleteByEventIndex(ctx, "eventEmbeddingHot", ...)is discarded, making it invisible in thedeletedEmbeddingsordeletedChildrencounters. Consider adding todeletedEmbeddingsfor consistency witheventEmbeddings.♻️ Suggested fix
const deletedEmbeddings = await deleteByEventIndex( ctx, "eventEmbeddings", args.eventId, ); - await deleteByEventIndex(ctx, "eventEmbeddingHot", args.eventId); + const deletedHotEmbeddings = await deleteByEventIndex(ctx, "eventEmbeddingHot", args.eventId);Then include
deletedHotEmbeddingsin return value or add todeletedEmbeddings.🤖 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/singletonCleanup.ts` around lines 346 - 368, The deletion of the "eventEmbeddingHot" index is ignored: call deleteByEventIndex(ctx, "eventEmbeddingHot", args.eventId) returns a count that should be captured and accounted for; assign it to a variable (e.g., deletedHotEmbeddings) and add it into the aggregate counters so it’s included in deletedEmbeddings or deletedChildren as appropriate (update the returned object or the deletedEmbeddings sum) to keep hot-embedding deletions visible; reference deleteByEventIndex, deletedEmbeddings, deletedHotEmbeddings, eventEmbeddingHot, and deletedChildren when making the change.packages/backend/convex/vectorSearchBudget.ts (1)
755-766:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent status for expired reservations.
The inline
releaseExpiredReservationshelper (line 335) sets status to"expired", while this cleanup mutation sets status to"released". Both handle the same expired reservations, creating inconsistent state depending on which path runs first. Consider using"expired"here for consistency with the inline helper and the new schema status.🐛 Proposed fix
await ctx.db.patch(reservation._id, { - status: "released", + status: "expired", updatedAt: now, });🤖 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/vectorSearchBudget.ts` around lines 755 - 766, The cleanup loop that patches reservation records sets status to "released", causing inconsistent states with the inline helper releaseExpiredReservations which uses "expired"; update the mutation that patches reservation._id in vectorSearchBudget.ts to set status: "expired" (and ensure any other places in this file using "released" for expired reservations are changed accordingly) so both the adjustDailyUsage loop (which uses reservation.date/shard/qgbReserved/vectorSearchesReserved and calls adjustDailyUsage) and releaseExpiredReservations use the same "expired" status value.
🤖 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 `@apps/web/src/routes/quiz.tsx`:
- Around line 277-281: There is a race where handleSelect sets
gradingLockRef.current = true but pendingGradePromiseRef.current may not yet be
assigned, so handleSubmit can proceed without awaiting the in-flight grading;
fix by assigning the grade promise to pendingGradePromiseRef.current immediately
when grading begins (create gradePromise and set pendingGradePromiseRef.current
= gradePromise before toggling gradingLockRef/current or before any asynchronous
work), or alternatively change the grading start logic in the function that
creates gradePromise (the code around gradePromise, pendingGradePromiseRef,
gradingLockRef in handleSelect/grade flow) so the promise reference is set
atomically before marking gradingLockRef true so handleSubmit always awaits the
correct promise.
In `@packages/backend/convex/clustering.ts`:
- Around line 6230-6263: loadCandidatesForRepresentative may return a cached
representative result even when the attach is on the forced-retry path; update
the function to skip/ignore representativeSearchCache when the incoming payload
indicates a forced fresh vector search (check
payload.pending?.needsFreshVectorSearch or equivalent flag used by the caller)
so it always calls loadCandidatesForEmbedding in that case, then continue to set
the cache only when not skipping; apply the same bypass logic to the other cache
reads referenced (the other occurrences around the regions noted) so
forced-retry/pending.needsFreshVectorSearch never reuses stale representative
candidates.
- Around line 6564-6570: The finally block currently releases the Vector Search
batch reservation even if some searches already completed but the reservation
wasn't consumed, risking undercounted usage; modify the control flow around
clusterRunReservationId so that consumeVectorSearchBatchReservation(ctx,
metrics, clusterRunReservationId) is called and awaited immediately after the
successful vector-search operations and clusterRunReservationSettled is set to
true before entering the finally, or alternatively guard the finally release so
it only releases when clusterRunReservationSettled is false and no successful
searches ran; update uses of clusterRunReservationId,
clusterRunReservationSettled, and consumeVectorSearchBatchReservation to ensure
the reservation is consumed on success paths and not released erroneously in the
finally block.
In `@packages/backend/convex/config.ts`:
- Around line 293-299: The code currently triggers an eager refresh of the
pipeline runtime-config snapshot on updates but not on deletions, so calling
remove() (e.g., deleting keys like clustering_*, topic_inference_*,
feed_page_size) leaves the old value in the live snapshot until the cron runs;
add the same eager refresh call (ctx.scheduler.runAfter(0,
internal.config.refreshPipelineRuntimeConfig, {})) to the delete/remove handlers
alongside the existing update hooks so getPipelineRuntimeConfig sees deletions
immediately — update the delete/remove implementations that mirror the update
paths (the places that call ctx.scheduler.runAfter for updates) to invoke
internal.config.refreshPipelineRuntimeConfig after a successful remove().
---
Outside diff comments:
In `@packages/backend/convex/singletonCleanup.ts`:
- Around line 346-368: The deletion of the "eventEmbeddingHot" index is ignored:
call deleteByEventIndex(ctx, "eventEmbeddingHot", args.eventId) returns a count
that should be captured and accounted for; assign it to a variable (e.g.,
deletedHotEmbeddings) and add it into the aggregate counters so it’s included in
deletedEmbeddings or deletedChildren as appropriate (update the returned object
or the deletedEmbeddings sum) to keep hot-embedding deletions visible; reference
deleteByEventIndex, deletedEmbeddings, deletedHotEmbeddings, eventEmbeddingHot,
and deletedChildren when making the change.
In `@packages/backend/convex/vectorSearchBudget.ts`:
- Around line 755-766: The cleanup loop that patches reservation records sets
status to "released", causing inconsistent states with the inline helper
releaseExpiredReservations which uses "expired"; update the mutation that
patches reservation._id in vectorSearchBudget.ts to set status: "expired" (and
ensure any other places in this file using "released" for expired reservations
are changed accordingly) so both the adjustDailyUsage loop (which uses
reservation.date/shard/qgbReserved/vectorSearchesReserved and calls
adjustDailyUsage) and releaseExpiredReservations use the same "expired" status
value.
🪄 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
Run ID: ad1750f1-206b-4f35-a958-d08f0c0b51c0
⛔ Files ignored due to path filters (1)
packages/backend/convex/_generated/api.d.tsis excluded by!**/_generated/**,!**/_generated/**
📒 Files selected for processing (17)
apps/web/src/components/auth-prompt-banner.tsxapps/web/src/lib/i18n/strings.tsapps/web/src/routes/quiz.tsxapps/web/src/routes/sitemap[.]xml.tspackages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/crons.tspackages/backend/convex/events.tspackages/backend/convex/ingestion.tspackages/backend/convex/lib/feedSerialization.tspackages/backend/convex/lib/publicEventPreviews.tspackages/backend/convex/quiz.tspackages/backend/convex/quizNode.tspackages/backend/convex/schema.tspackages/backend/convex/singletonCleanup.tspackages/backend/convex/sitemap.tspackages/backend/convex/vectorSearchBudget.ts
💤 Files with no reviewable changes (2)
- apps/web/src/components/auth-prompt-banner.tsx
- apps/web/src/lib/i18n/strings.ts
Comment on lines
277
to
+281
| const handleSubmit = async () => { | ||
| if (isSubmitting || activeResult) return; | ||
| setIsSubmitting(true); | ||
| try { | ||
| const response = await submitQuiz({ | ||
| await pendingGradePromiseRef.current; | ||
| const settledAnswers = answersRef.current; |
Contributor
There was a problem hiding this comment.
Race window between lock acquisition and promise assignment.
If handleSubmit is called after handleSelect sets gradingLockRef.current = true (line 242) but before pendingGradePromiseRef.current = gradePromise (line 273), the submit will proceed with a stale or null promise while grading is in-flight.
Consider assigning the promise ref immediately when starting the grade:
Proposed fix
gradingLockRef.current = true;
setAnswers((current) => {
const next = { ...current, [questionId]: choiceId };
answersRef.current = next;
return next;
});
- const gradePromise = (async () => {
+ let resolveGrade: () => void;
+ pendingGradePromiseRef.current = new Promise<void>((resolve) => {
+ resolveGrade = resolve;
+ });
+ (async () => {
try {
const feedback = await gradeQuestion.mutateAsync({
quizId: quiz._id,
questionId,
choiceId,
});
setQuestionFeedback((current) => ({
...current,
[questionId]: feedback,
}));
} catch (error) {
console.error("Failed to grade quiz answer:", error);
toast.error(t("quiz.submit.error"));
setAnswers((current) => {
const next = { ...current };
delete next[questionId];
answersRef.current = next;
return next;
});
} finally {
gradingLockRef.current = false;
- pendingGradePromiseRef.current = null;
+ resolveGrade!();
}
})();
- pendingGradePromiseRef.current = gradePromise;
- await gradePromise;
+ await pendingGradePromiseRef.current;🤖 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 `@apps/web/src/routes/quiz.tsx` around lines 277 - 281, There is a race where
handleSelect sets gradingLockRef.current = true but
pendingGradePromiseRef.current may not yet be assigned, so handleSubmit can
proceed without awaiting the in-flight grading; fix by assigning the grade
promise to pendingGradePromiseRef.current immediately when grading begins
(create gradePromise and set pendingGradePromiseRef.current = gradePromise
before toggling gradingLockRef/current or before any asynchronous work), or
alternatively change the grading start logic in the function that creates
gradePromise (the code around gradePromise, pendingGradePromiseRef,
gradingLockRef in handleSelect/grade flow) so the promise reference is set
atomically before marking gradingLockRef true so handleSubmit always awaits the
correct promise.
…n-error, config remove refresh, cleanup counters/status - clustering: forced-fresh retry now bypasses the representative-search cache so it never reuses stale candidates created earlier in the batch - clustering: on an aborted run, settle (consume) the vector-search reservation when searches already ran instead of releasing, so usage isn't undercounted - config: remove() now eagerly refreshes the pipeline runtime-config snapshot, matching set()/setTopicInferenceSettings() - singletonCleanup: count eventEmbeddingHot deletions into deletedEmbeddings - vectorSearchBudget: expiry cleanup marks holds "expired" (not "released") to match releaseExpiredReservations (quiz.tsx grading-race finding verified not real and skipped: the grade promise is assigned synchronously before handleSelect yields.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 10, 2026
Merged
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.
Summary by CodeRabbit
New Features
Improvements