Misconception pipeline: concurrent wrong answers on the same topic can insert duplicate weakness rows

[aaronashby]

Overview

runMisconceptionPipeline (app/tutor/conversation.ts) decides whether a classified misconception is novel by reading the student's existing weaknesses for the topic (getWeaknessesForTopic), asking Haiku to match it (matchWeakness), and inserting a new row if there's no match. That read-decide-write sequence is not serialized per (student_id, topic_id).

The pipeline runs detached specifically so it never blocks the reply (see feat(misconception): run classify+dedup+write as a detached pipeline) — which means two wrong answers on the same topic close together (a slow gap-check answer overlapping a solve attempt on a different problem sharing a prerequisite topic, or a double-submit) can both read the same "no match" existing list and both independently decide "novel," producing two rows for what should be one misconception, with observed_count split across them.

There's no unique constraint on (student_id, topic_id) to backstop this — deliberately, since a student can genuinely have multiple distinct misconceptions for one topic. Dedup is semantic (an LLM classification, not embeddings or exact-string), so a DB-level uniqueness constraint can't enforce it.

Why this isn't a quick fix

A Postgres advisory lock only protects a critical section held within a single DB transaction/connection. Supabase's client gives each .rpc()/.from() call its own transaction (PgBouncer transaction pooling) — there's no way to hold pg_advisory_xact_lock open across the two Claude round-trips (classifyMisconception, matchWeakness) that make up the decision. Pushing the dedup decision into SQL doesn't work either, since matchWeakness is inherently a Haiku call.

A lock that actually spans the whole classify→dedup→write sequence needs to live above the DB: either an app-level mutex keyed on ${studentId}:${topicId} (correct only within a single Node process — same limitation lib/historyCache.ts already documents for its in-memory implementation) or a Redis-backed lock once Redis exists for this app (see the Redis history-cache tickets — #42/#43/#44 — which are already moving this codebase toward having a shared cache/lock backend).

Options to evaluate

1. Do nothing for now — accept occasional duplicate rows as a known limitation; observed_count being split across two rows for the same misconception degrades the "how often has this shown up" signal but doesn't corrupt anything else.
2. App-level mutex, scoped to a single instance — meaningfully reduces the race in the current (likely single-instance) deployment, but silently stops working the moment this scales horizontally, with no signal that it stopped working.
3. Redis-backed distributed lock around the critical section, once the Redis migration lands — the only option that's correct under horizontal scaling. Should probably be sequenced after the Redis history-cache work rather than introducing a second piece of Redis infra ad hoc.
4. Shrink the window instead of closing it: re-fetch existing immediately before the insert (not just once at the top of the pipeline). Doesn't eliminate the race, just narrows it from "a full Claude round-trip" to "one extra read plus the insert" — worth doing regardless of which longer-term option is chosen, since it's cheap.

Notes for whoever picks this up

• This surfaced during review of the detached misconception pipeline (branch feat/async-misconception-write-path); see that PR's review thread for the original writeup.
• Related: #4a9e11-labeled matchWeakness fail-open behavior (separate issue) also writes duplicate rows on Haiku failure — same symptom, different cause. Worth keeping in mind if these are fixed independently, since a fix here shouldn't be considered a fix for that one too.