extract_llm: rep.Gate from per-candidate goroutines can crash the proxy (concurrent map writes)

[amiddavid]

components.Report.Gate (components/component.go:503) lazily creates and writes r.Gates, an
unlocked map. extract_llm calls it from inside its per-candidate goroutines:

• components/offload/extract_llm.go:1343 — rep.Gate("deduped_inflight_extraction")
• components/offload/extract_llm.go:1371 — rep.Gate("reply_truncated")

Both sit inside the region bounded by sem := make(chan struct{}, llmConcurrency) and a
sync.WaitGroup, so up to llmConcurrency (4) goroutines can reach them on one request.

Why this is worse than a data race

Concurrent map writes are a runtime fatal error, not a panic. pipeline.runOne's per-component
recover() does not catch it — nothing can. So the outcome is not "the component reverts and the
request proceeds"; it is the proxy process dies, taking every in-flight request with it.

That distinction matters for triage. Most failure modes in this component are designed to fail open
and cost a compaction. This one costs the process.

Why it has not been seen

Both branches are rare. deduped_inflight_extraction requires two goroutines to collide on the same
content within one request, and reply_truncated requires a reply to stop exactly at the output cap.
singleflight serialises part of the dedup path. So the window is narrow — but it widens with
candidate count, and a request with many large tool outputs is exactly when llmConcurrency is
saturated and truncation is most likely.

Found by

Building extract_llm_sweep (#118). The first version of that component called rep.Gate from its
own per-call goroutines and the cap test died with fatal error: concurrent map writes; under
-race it reported WARNING: DATA RACE … components.(*Report).Gate(). That component now
accumulates gate names per slot and raises them in the serial phase. extract_llm was left
untouched, so it still has the defect.

The fix, and the choice in it

Report is copied by value across this codebase, so it cannot carry a sync.Mutex — that is the
constraint that makes the obvious fix wrong. Two workable options:

1. Accumulate per-slot, raise serially — what extract_llm_sweep does. Each goroutine records
   its gate names in its own slot of a pre-sized slice; the serial phase after wg.Wait() replays
   them into the report. No locking, no API change, and it keeps Report copyable. Downside:
   every concurrent component has to remember to do it.
2. Give Gate a package-level lock or make Gates a sync.Map — fixes every present and future
   caller at once, but adds contention on a hot path and changes Report's shape, which
   /stats, the Prometheus component counters and the dashboard's components table all read.

Option 1 is the local, proven fix; option 2 is the one that stops this recurring. Worth deciding
deliberately, because the same trap is available to any component that adds concurrency later — and
the failure mode it produces is a process crash, which is not the kind of thing to leave to a
convention nobody is reminded of.

Verification

A reproduction should run extract_llm with several large candidates under -race and assert no
race is reported — and be confirmed to FAIL (report the race) before the fix.

[amiddavid]

Fixed and merged in #118 (squashed as d6efefe).

Verified on main: components/offload/extract_llm.go no longer calls rep.Gate from inside runCall. Both names — deduped_inflight_extraction and reply_truncated — are accumulated into a pre-sized per-slot slice (gateNames[k]) and replayed in the serial phase after wg.Wait().

That is option 1 from this issue: accumulate per slot, raise serially. It keeps Report copyable, which is the constraint that made the obvious fix wrong.

The reproduction was the single-flight follower — four byte-identical candidates share one key, so extractInflight releases three followers at the same instant. Reverted, under -race:

WARNING: DATA RACE
Read at 0x00c00015e158 by goroutine 28:
  components.(*Report).Gate()  components/component.go:507
  offload.(*ExtractLLM).Offload.func2()  components/offload/extract_llm.go:1356
Previous write at 0x00c00015e158 by goroutine 27:  [same two frames]

and without -race, the quiet form on 3 of 20 runs — which is why the test asserts an exact count rather than relying on a flag the suite does not pass:

deduped_inflight_extraction = 2, want 3: a follower's gate was lost, which is the quiet form
of the same unsynchronised write

Option 2 stays open as a question, and is worth someone's judgement rather than being closed with this. Locking Gate once (or making Gates a sync.Map) would stop this recurring for the next component that adds concurrency, at the cost of contention on a hot path and a change to Report's shape — which /stats, the Prometheus component counters and the dashboard's component row all read. The trap is still available: nothing prevents a future component calling rep.Gate from a goroutine, and the failure mode is a runtime fatal error rather than a recoverable panic, so it kills the proxy rather than reverting the component.

If you want that tracked separately rather than lost here, say so and I will open it — I did not want to file a near-duplicate without asking.

[amiddavid]

Tracked separately as #135, so the general trap is not carried in a closed issue's last paragraph.

Recorded there and worth repeating here: the surface is four methods now, not two — Gate, GateN, Event, EventN. The gates/events split (#121) doubled it while fixing a different problem, and it widened the trap in the worst direction: the events counters are exactly the ones a successful concurrent component raises, so the natural place to call Event is inside the worker.