A TOCTOU Race in a Dedup Pipeline, and the One-Line Upsert Fix

I added a Celery task that ingests crawled web content: crawl a site, chunk every page, deduplicate chunks against anything already ingested for that tenant, embed the new ones, and upsert them into a vector store. The dedup step lives in filter_new_chunks — given a batch of chunk texts, figure out which ones are actually new, so only those get embedded and inserted.

The first implementation looked completely reasonable, passed every test, and still had a bug the test suite couldn't see: a classic check-then-act race between reading "what's already there" and writing "what's new."

The pattern

filter_new_chunks hashes every chunk (SHA-256, so identical text always maps to the same value regardless of which page it came from), looks up which hashes already exist for the tenant, and records the rest so a future crawl treats them as seen:

def compute_hash(text: str) -> str:
    return hashlib.sha256(text.encode("utf-8")).hexdigest()

The naive version of the write step - the one the implementation plan didn't flag as a problem - looked like this:

# SELECT: which of these hashes have we already seen for this tenant?
existing_hashes: set[str] = set()
for i in range(0, len(hashes), BATCH):
    batch = hashes[i : i + BATCH]
    result = (
        schema.table("chunk_hashes")
        .select("hash")
        .eq("tenant_id", tenant_id)
        .in_("hash", batch)
        .execute()
    )
    existing_hashes.update(row["hash"] for row in result.data)

# compute the new ones in application code
rows = [
    {"hash": h, "tenant_id": tenant_id, "source_id": source_id}
    for h in hashes
    if h not in existing_hashes
]

# INSERT: record them so future calls see them as seen
if rows:
    schema.table("chunk_hashes").insert(rows).execute()

chunk_hashes has a composite primary key on (hash, tenant_id). Nothing here is wrong in isolation - the SELECT is correct, the INSERT is correct, the logic connecting them is correct. It's also exactly the shape you'd write for a one-off script, and it was covered by tests like test_filter_new_chunks_excludes_hashes_seen_in_a_previous_call and test_filter_new_chunks_keeps_only_unseen_chunks_on_partial_overlap in tests/core/test_dedup.py - all of which called filter_new_chunks sequentially, one call finishing before the next started. Every one of them passed.

The race

The bug only exists between two calls that overlap in time. That's not a hypothetical for this task: ingest_url_task runs as a Celery job, and nothing prevents two crawls of the same tenant's site from being dispatched close together - a user re-triggering an ingest, a retry racing the original attempt, or two different pages of the same crawl both containing a shared boilerplate chunk (nav bars and footers are famous for being byte-identical across every page of a site).

The failure sequence for two concurrent calls, A and B, both processing a chunk that hashes to the same value and is genuinely new to the tenant:

1. A runs its SELECT — hash not in chunk_hashes yet.
2. B runs its SELECT — hash still not in chunk_hashes (A hasn't written yet).
3. A runs its INSERT — succeeds, row now exists.
4. B runs its INSERT — same (hash, tenant_id) pair, primary-key violation.

Step 4 raises an uncaught exception from the postgrest client. There's no transaction spanning the SELECT and INSERT, and no conflict guard on the INSERT itself, so the window between "check" and "act" is wide open to any other call touching the same tenant. This is a time-of-check-to-time-of-use (TOCTOU) race: the fact you checked something a moment ago tells you nothing about what's true right now.

It's also the kind of bug that's structurally invisible to a normal test suite. Tests call the function once, observe the result, and move on — there's no reason for a sequential test to ever have two calls' SELECT and INSERT interleave. This one surfaced during a task review pass done by a separate reviewer subagent going through the implementation independently of whoever wrote it — not from the plan (which didn't anticipate it) and not from the tests (which couldn't have caught it without deliberately simulating concurrency).

The fix: push the check into the write

The real insight isn't "add a try/except around the insert" — that papers over the race without removing it, and still requires deciding what to do when the fallback fires. The actual fix is to stop doing a separate check at all. Postgres already has a primitive for "insert this, and if it conflicts, do something sane instead of erroring": INSERT ... ON CONFLICT. postgrest-py exposes it as upsert():

if rows:
    schema.table("chunk_hashes").upsert(
        rows, on_conflict="hash,tenant_id", ignore_duplicates=True
    ).execute()

on_conflict="hash,tenant_id" tells Postgres which constraint to match against — the same composite primary key — and ignore_duplicates=True maps to ON CONFLICT DO NOTHING rather than DO UPDATE, which is exactly what you want here: if the row already exists, there's nothing to update, just skip it silently. This isn't a guess at the API — the fix was checked against the installed postgrest client's actual upsert() signature (json, count, returning, ignore_duplicates, on_conflict, default_to_null) rather than assumed from docs or memory, the same verify-against-the-real-library discipline that had already caught wrong API assumptions elsewhere in this project (a nonexistent crawl4ai attribute, a wrong embedding dimension from a ticket's sample code).

With the upsert in place, the same race from before plays out differently:

1. A's SELECT — hash not seen.
2. B's SELECT — hash not seen.
3. A's upsert — inserts the row.
4. B's upsert — same (hash, tenant_id), conflict detected, DO NOTHING, no error.

The SELECT is still there — it's what turns hashes into new_indices so the caller knows which chunks to actually embed and insert into the vector store this call. But the correctness of chunk_hashes no longer depends on the SELECT being accurate at write time. Even if two calls both think a hash is new and both try to record it, the write itself is atomic and idempotent. The race window between check and act still exists, but it no longer matters, because the "act" step can survive being executed twice.

One more dedup bug touched this same file a session later (CAN-40 bug #3: within-batch duplicates weren't gated identically for vector inserts vs. DB inserts) — different bug, different root cause, covered in its own post; not going deeper into it here.

Wrap-up

"Check if it exists, then insert if it doesn't" is one of the most common patterns written against a shared datastore, and it's subtly broken the moment more than one caller can run it concurrently — a second Celery worker, a retried request, two overlapping crawls, a horizontally scaled API. The check is always answering a question about the past; by the time the write happens, the answer may no longer be true. No amount of testing the function in isolation will catch this, because the bug lives in the gap between two calls, not inside either one.

The fix is rarely "add a lock" or "catch the exception" — both add complexity without removing the underlying race. Prefer collapsing the check and the write into a single atomic operation the datastore itself guarantees: an upsert with an explicit conflict target, INSERT ... ON CONFLICT, a unique constraint plus ignore_duplicates, whatever your database calls it. If two callers can legitimately both believe something is new, let the database be the one to decide who's first — that's a problem it already solves correctly, and your application code doesn't have to.