Skip to content

Latest commit

 

History

History
309 lines (238 loc) · 13.8 KB

File metadata and controls

309 lines (238 loc) · 13.8 KB

LoopGain Bench — Engineering Lessons

Forensic record of bugs surfaced while shipping the registered n=200 bench. The bench's value proposition is reproducibility + honesty, so the engineering failures that almost corrupted the data deserve to be on the record alongside the results — including the wrong diagnoses, not just the right ones. Sanitizing the trail to "we went straight to the fix" would be the wrong kind of polish.

Lesson 1 — The wrong diagnosis (LangGraph cache thread-safety)

Symptom round 1

Registered bench v2 (concurrent runner, first attempt) wrote 200/200 W1-langgraph trials with input_tokens == 0, output_tokens == 0, every iteration in failed_iters, worst-case error markers populating error_history. LG terminated as "diverged" after 1–2 iters with empty output; B20 burned through 20 iters of the same.

Hypothesis

W1 uses LangGraph via framework_invoke._invoke_langgraph, which cached the compiled StateGraph at module level (_LANGGRAPH_CACHE). With 4-thread condition concurrency inside run_trial, 4 threads were invoking graph.invoke() on the same compiled graph simultaneously. LangGraph compiled graphs aren't documented as thread-safe — concurrent invokes might corrupt internal channel/checkpointer state, leading to the observed exception storm.

"Fix" applied

Moved the LangGraph cache to threading.local(), so each worker thread gets its own compiled graph instance. Wrote a stress test: 8 concurrent _invoke_langgraph calls against real Haiku, verified 0/8 zero-token responses post-fix.

Why we kept this change anyway

It's correct prophylactic engineering. Even if the actual v2/v3 corruption was elsewhere (and it was — see Lesson 2), sharing a stateful framework object across threads is fragile by default. The threading.local pattern is the right design.

What gave us false confidence

The stress test for the LangGraph cache hypothesis hit _invoke_langgraph directly. It did NOT exercise workload.run_iteration end-to-end, which would have included the actual culprit (signal-based timeout in worker threads — see Lesson 2). The stress test was correct for the hypothesis it tested, but the hypothesis was wrong about which code path was broken.

Generalizable rule

When a stress test "fixes" a bug, run the same stress test through the full call path the production code uses, not just the suspected hot spot. A test that hits the suspected adapter directly proves the adapter is fine — it doesn't prove the bench is fine, because the bench's call path includes everything around the adapter too.

Lesson 2 — The right diagnosis (signal.SIGALRM in worker threads)

Symptom round 2

Registered bench v3 (with the threading.local LangGraph fix applied) ran W1-langgraph again. Same symptom: 200/200 trials with input_tokens=0, failed_iters populated. The Lesson-1 "fix" had not actually fixed anything.

This time, we looked at stderr, not just the JSONLs. Stderr had been emitting for both v2 and v3:

[iter 1 failed: ValueError('signal only works in main thread of the main interpreter')]

We had missed it because the per-iteration failures only showed up in the runner's stderr stream, not in the tee'd stdout that the run-progress output went to. The forensic moral: read stderr when the data is wrong; the failure message was already in front of us, we just weren't looking in the right pipe.

Root cause

bench/workloads/_shared/codegen_base.py::_run_tests enforced an exec timeout via signal.signal(SIGALRM, …) + signal.alarm(N). Python's signal module raises ValueError("signal only works in main thread of the main interpreter") when signal.signal is called from a non-main thread.

The bench runner uses condition-level concurrency: every trial's four conditions (B5/B10/B20/LG) run in a ThreadPoolExecutor(max_workers=4) inside run_trial. Worker threads invoking _run_tests raise immediately on signal.signal. The exception was caught by _run_baseline / _run_loopgain's except Exception (per Methodology Lockdown #5: never silently drop), recorded as failed_iters[i] = i, and the iteration's LLM call never happened — hence input_tokens=0.

This is a textbook "the standard-library timeout primitive doesn't survive the parallelism model your harness needs" failure.

Why dry-run didn't catch this either

The dry-run executed before concurrency was added to the runner (commit a7754af). Dry-run conditions ran serially in the main thread, where signal.SIGALRM works fine. The condition-concurrent path was first exercised by registered bench v2/v3.

Fix

Replaced SIGALRM-based timeout with a daemon threading.Thread + join(timeout=N) pattern in _run_tests. Daemon threads die with the process, so an LLM that writes an infinite loop leaks a thread but doesn't prevent process exit. Verified with a stress test: 4 concurrent _run_tests('while True: pass', ...) calls all timed out at ~3.2s (within the 3s exec budget + thread.join overhead). 8 concurrent valid fizzbuzz tests all passed.

concurrent.futures.ThreadPoolExecutor.submit(...).result(timeout=N) was tried first but doesn't work — its workers are non-daemon by default, so the executor's __exit__ blocks forever on a hung task. Daemon threading.Thread is the right primitive here.

Generalizable rule

If a Python harness uses threading, no part of it can use signals. SIGALRM, signal.signal, signal.alarm — any of them. If you need a timeout that survives in a worker thread, use threading.Thread(daemon= True) + join(timeout=N) and accept the thread leak. concurrent. futures.ThreadPoolExecutor looks like it should work but its non-daemon workers will bite you.

Lesson 3 — Per-cell tripwire after first N trials

Why

Both bugs above silently wrote 200 broken JSONL records before being caught by manual inspection. By construction, the bench harness writes something per trial — there's no built-in failure mode for "trial completed but every iteration failed." The corruption looks like a successful trial to the file writer.

Mitigation

run_cell now monitors the first 5 trials' token counts. If all five have input_tokens == 0 across both LG and B20, the cell aborts with a loud stderr message naming the workload, tag, and likely cause:

"first 5 trials all have 0 input/output tokens. This indicates a harness-level bug (e.g. thread-safety in the adapter, broken framework_invoke, or per-iteration exception swallowing all LLM calls). Stopping cell."

Cost of the tripwire: ~5 trials × ~$0.01 each = ~$0.05 wasted per broken cell. Versus letting a corrupted cell run to n=200 and burn ~$5 — easily 100× ROI.

Generalizable rule

Bench harnesses need early sanity checks on aggregate signals that should never plausibly be zero. Token counts, error magnitudes, latency — anything where "zero across the board" is a smoking gun for a harness bug, not a workload result.

Lesson 4 — Concurrency math: count inflight calls, not threads

What we tried first

make bench initially used --trials-parallel 8 --cells-parallel 2 on top of always-on condition-level concurrency. The math: 2 cells × 8 trials × 4 conditions = 64 inflight LLM calls at peak.

What happened

After ~1h, two cells had a header written and 0 trials landed. Process got SIGKILL'd (likely OOM or HTTP connection-pool exhaustion). 64 inflight calls is over httpx's default pool size and probably triggered queueing-induced deadlock or memory pressure.

Fix

Dropped to --cells-parallel 2 --trials-parallel 1 (default). Math: 2 cells × 1 trial × 4 conditions = 8 inflight LLM calls. Well within both provider rate limits and HTTP library defaults. Wall-clock projection: 58h serial → ~7-14h with this concurrency level.

Generalizable rule

Count inflight calls end-to-end, not just thread count. Nested concurrency multiplies. The right peak number depends on the slowest shared resource (HTTP connection pool, provider rate limit, memory) — not on how many CPU cores are available.

Lesson 5 — Bench data → dashboard upload was a missed step

Symptom

The public bench tenant cust_7931de9f766452ac was provisioned in D1 on 2026-05-21, but when the dashboard /benchmark route was being built two weeks later it surfaced as "tenant exists, 0 rows." The 2,000 registered bench trials lived only as local data/raw/*-registered.jsonl files. Telemetry-posting was never part of make bench.

Why it slipped through

The bench's job is to produce a raw dataset. The dashboard's job is to display that dataset. Neither side owned the bridge between "the JSONL exists" and "the receiver has the rows." The protocol-lockdown discipline in BENCH_PROTOCOL.md pre-registered everything about how data is collected and analyzed — nothing about how it gets published to the hosted dashboard.

Fix

Wrote bench/upload_to_dashboard.py. Reads every data/raw/*-registered.jsonl, extracts the LG-condition record from each trial (B5/B10/B20 baselines skipped — they aren't LoopGain runs and don't belong in a LoopGain dashboard), maps it to the v3 telemetry aggregate payload shape loopgain.telemetry.build_payload produces, and POSTs to telemetry.loopgain.ai/v1/aggregate against the bench tenant's bearer token. 2,000 LG-condition trials uploaded. The script is in the bench repo so anyone reproducing the bench can populate their own tenant.

Two implementation gotchas worth recording:

  1. The bench JSONLs do not record the per-iteration Aβ trajectory — only the error history and the derived gain margin. To populate the dashboard's Loop Detail scrubber, the uploader synthesizes the smoothed Aβ profile from the error history (Aβ_raw_i = |e_i / e_{i-1}|, then the same EMA-window=3 smoothing the live library applies). This matches what a live loop would have transmitted, with the caveat that the smoothed values are reconstructed from the bench's recorded error history rather than captured in-flight. Adequate for the dashboard, but loop_events.profile_* summary stats for this dataset should be read as derived, not directly observed.

  2. The Cloudflare per-customer AGGREGATE_RL ceiling is 60 rpm (see wrangler.toml). At concurrency=8 with a 15-second cumulative retry budget, the first full-run attempt left 157/2,000 trials in a 429-then-give-up state. Dropping concurrency to 2 and extending the retry budget to ~10 minutes per failing payload (12 attempts × up to 60s backoff) cleared the backlog cleanly. The rate limit is correct policy for a free hosted tier; the uploader just needed to respect it.

Generalizable rule

Every dataset that the public is supposed to see needs an explicit publish step, owned by someone, run as part of the artifact-production pipeline. A "shipped" bench is local JSONL + dashboard rows + the publish step that bridges them, not just the first two. Filed as a v0.2 enhancement for the bench harness: make bench should optionally make publish against a configured tenant, so future bench runs populate both surfaces in one go.

Lesson 6 — Upload step shipped, but a load-bearing field was nulled out

Symptom

After Lesson 5's bench/upload_to_dashboard.py shipped all 2,000 LG trials to D1 cleanly, the dashboard /benchmark Spotlight card read "$0 saved · 0 iterations saved." That directly contradicts the landing page headline ("93.5% cost reduction vs max_iter=20") and RESULTS.md — the most jarring credibility hit on the prospect journey.

Cause

The uploader built the v3 payload shape correctly but hardcoded savings_vs_fixed_cap: None on every event. The receiver dutifully wrote 2,000 rows with savings_vs_fixed_cap IS NULL, the /v1/public/benchmark/stats aggregator summed those nulls to 0, and the dashboard rendered the truthful $0. The bench JSONLs do record LG.iters per trial — the value needed to compute savings was right there; the uploader just didn't compute it.

Fix

savings_vs_fixed_cap = max(0, 20 - LG.iters) per event, using the bench's measured baseline (max_iter=20) rather than the library default (max_iter=10). That's a per-tenant convention — a real customer running with defaults still sees vs-10; the bench tenant uses vs-20 because it's what the bench actually measured and what the public claims reconcile against. Cleared D1, re-uploaded. Aggregate savings landed at ~37K iterations (≈ 2,000 trials × ~18 mean), which matches the landing-page narrative.

Why this is the same lesson as #5

Lesson 5 was "the publish step was missing." Lesson 6 is "the publish step existed but was incomplete." Both stem from the same blind spot: the bench treats data collection as the artifact, but the public artifact is "data correctly visible on the dashboard." Every field the dashboard reads is part of the publish contract, not an optional extra. The fix in #5 added a publish script; the fix in #6 ensures that script populates every column the dashboard depends on. A publish-step checklist (one row per dashboard panel → what column/field it reads → is the uploader populating it?) would have caught this; filed as a v0.2 follow-up alongside the make publish target.

What stayed clean

  • All 5 non-langgraph framework adapters (langchain, crewai, autogen, openai-agents, claude-agent-sdk) passed the 8-concurrent stress test with zero zero-token responses. Each is stateless or uses fresh per-call instances — no module-level mutable state to share.
  • Methodology lockdowns held throughout. Each broken run was discarded per Lockdown #9 ("raw data is immutable" applies to real data, not to harness-failure artifacts).
  • No protocol amendments were needed for the engineering fixes — Methodology Lockdown #4 (same seeds across conditions) and #7 (same wall-clock environment) survive concurrent execution; the lockdowns describe scientific identity, not execution strategy.