fix: routing-service crashed under load, and marketing pages failed contrast in both themes - #197
Merged
Conversation
…not collide `commitCalibrationBatch` replaces the live router profile tables with `deleteMany()` + `createMany()`. That pair is unsafe to run concurrently under READ COMMITTED: if the second transaction's DELETE takes its snapshot before the first COMMITs, it removes only the rows it can see, never observes the rows the first is inserting, and its own INSERT then violates `router_model_profiles_provider_model_task_family_topic_key_key`. The exposure is not occasional. `rebuildCalibrationSnapshot()` runs on EVERY routing outcome, so every pair of concurrent generations is a chance to hit it, and the P2002 is an unhandled rejection that kills routing-service outright — `/routing/models` then 502s and no message can be routed at all. Reproduced twice on 2026-08-30 during a long-thread stress run: all in-flight threads timed out at the same instant because the routing tier had died. It also degrades with age — `findEducationWindow` returns a growing set of decisions, so the transaction lengthens the more the system has been used. 24 concurrent generations on a young database were clean; it crashed reliably after ~1,400. Load-testing a fresh install does not surface this. Both replace paths now take a transaction-scoped advisory lock as their FIRST statement, released automatically on COMMIT or ROLLBACK. restoreCalibrationSnapshot gets it too — a rollback replaces the same tables the same way. The lock matches the three seed repositories in this service, including the `::text` cast. That cast is load-bearing: pg_advisory_xact_lock returns void, which the Prisma pg adapter cannot decode, and a first attempt without it crashed the service faster than the bug it was fixing. Verified at runtime, not only in tests — lock 740040004 was observed held in pg_locks during live load. Key is 740_040_004, next in this service's documented 740_040_00N block. Not addressed here: rebuilding the entire calibration window on every single generation is O(accumulated decisions) per message. The lock makes that correct, not cheap.
`.editorial-comparison__cta` is an inverted panel — its ground is `--editorial-ink`, not `--editorial-paper`. The inversion is theme-safe on its own, because ink and paper swap together, so the panel is always "ink ground, paper text". Two children then re-set `color` using tokens tuned for the PAPER ground: __verdict-label --editorial-signal 3.41:1 light 2.26:1 dark __body --editorial-graphite 2.98:1 light 1.76:1 dark All four are under the 4.5:1 WCAG AA requirement for normal-size text, so this was broken in BOTH themes. Dark merely fails loudly enough for axe to report it, which is why it looked like a dark-mode-only bug. Both now take their colour from the inverted set. Scope explains itself: the compare pages put bare text in the panel and inherit the panel's own colour, so they were never affected. Only the learn and integration topic pages nest `__verdict-label` + `__body` inside it — which is exactly the 32 URLs Lighthouse failed, and no others. The hub pages passed. Verified with real Lighthouse against a rendered page, not by reasoning about token values: `color-contrast` scores 1 and `categories.accessibility` is 1.0 on a learn topic page, an integrations topic page and a compare page. Two false starts worth recording, because both produced a confident wrong answer: a hand-rolled contrast sweep in the browser reported zero failures (it does not reproduce what axe measures), and the first two sweeps ran against a dev container still serving a 404 shell for the new routes, so they were scanning an error page. Restarting the container and running the actual audit is what found it.
…nt passes
Adds `stress-context.mjs`: threads run to hundreds of messages, past the point
where the token budget must start evicting, because that is where a context
system either degrades honestly or lies.
It measures two things and refuses to conflate them, since conflating them is
how the original context defect survived so long:
DELIVERY did the composer hand the model every message it had budget for,
and omit ONLY for budget? The product's promise.
RECALL did the model then use what it was given? The model's job — a weak
model failing here is not a context bug.
Measured on claw.local: 4,400 messages over 2,200 turns across 6 free models,
0 failed, recall 220/220, delivery violations 0, threads reaching 432 messages,
selection 1 ms p50 (6 ms max) and retrieval 8 ms p50. A second run with verbose
filler filled the budget to 24,510 of 24,515 tokens and evicted for real: recall
stayed 43/43 at up to 211 messages back.
Three flaws found in this harness, each of which turned a red result green —
the same silent-degradation shape it exists to catch:
1. The delivery check counted a MISSING manifest as "no violation". A run where
84 of 127 probes produced no receipt still printed "DELIVERY violations 0".
Unmeasured probes are now counted and reported separately; they prove nothing.
2. A probe scored 0 when the upstream provider answered 200 OK with its own
usage-limit notice as the assistant message. That read as a context collapse
past ~200 messages — it was the provider's billing state. Detected now, and
it aborts the run, because the results are not comparable afterwards.
3. The probe selector cycled through planted facts, which keeps landing on
recent ones: a 432-message thread was probed at a maximum distance of 31.
A distance test bounded by its own indexing measures nothing. It now always
re-asks the first fact, so probe distance grows with the thread.
Also in client.mjs: `loginAs` / `createIsolatedUser`, because memories are
USER-scoped and inject into every thread. A shared lab account accumulates
fixtures that compete with the facts a scenario plants — a stored "the internal
project codename is ORCHID-…" made three recall probes fail at distances 4, 24
and 56 at exactly the same rate, which is the signature of a constant competing
fact rather than of context loss. Isolation costs quota headroom (a fresh
account lands on the Free plan), so it is opt-in.
And memory-experiment.mjs: forgetting a memory is confirmation-gated, and this
cleanup omitted `?confirm=FORGET` AND ignored the 400. Every run left its
standing INSTRUCTION behind, so the next run had two conflicting "always end
your reply with …" memories live and obeyed the older one — reporting its own
litter as a product failure.
ihabkhaled
deleted the
fix/context-stress-and-routing-calibration-race
branch
September 1, 2026 10:48
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
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.
Three changes, found by building a long-thread context stress test and running it against
claw.local. Two are production bugs; the third is the harness that found them.1. routing-service crashes under sustained load
commitCalibrationBatchreplaces the live router profile tables withdeleteMany()+createMany(). Under READ COMMITTED that pair races itself: if the second transaction's DELETE takes its snapshot before the first COMMITs, it removes only the rows it can see, never observes the rows the first is inserting, and its own INSERT violatesrouter_model_profiles_provider_model_task_family_topic_key_key.Severity is the part that matters.
rebuildCalibrationSnapshot()runs on every routing outcome — after every single generation. So every pair of concurrent messages is an opportunity, and the P2002 is an unhandled rejection that kills the process. routing-service down means/routing/models502s and no message can be routed at all.Reproduced twice during the stress run: every in-flight thread timed out at the same instant because the routing tier had died.
It degrades with age, which is why nobody caught it.
findEducationWindowreturns a growing window of decisions, so the transaction lengthens the more the system is used. 24 concurrent generations on a young database were clean. It crashed reliably after ~1,400. A load test on a fresh install passes.Fix: a transaction-scoped advisory lock as the first statement of both replace paths (
restoreCalibrationSnapshottoo — a rollback replaces the same tables the same way). Released on COMMIT or ROLLBACK, so a crashed transaction cannot strand it.The
::textcast is load-bearing and matches the three existing seed repositories in this service:pg_advisory_xact_lockreturnsvoid, which the Prisma pg adapter cannot decode. My first attempt omitted it and crashed the service faster than the bug it was fixing. Verified at runtime rather than only in tests — lock740040004was observed held inpg_locksduring live load.Not addressed here: rebuilding the entire calibration window once per message is O(accumulated decisions) per generation. The lock makes that correct, not cheap. Worth a separate decision.
2. Marketing pages fail colour contrast — in both themes
.editorial-comparison__ctais an inverted panel (background: var(--editorial-ink)). The inversion itself is theme-safe, because ink and paper swap together. But two children re-setcolorfrom tokens tuned for the paper ground:__verdict-label--editorial-signal__body--editorial-graphiteAll four are under 4.5:1, so this was broken in both themes — dark just fails loudly enough for axe to report it.
Scope explains itself: compare pages put bare text in the panel and inherit its colour, so they were never affected. Only learn and integration topic pages nest these two inside it — exactly the 32 URLs Lighthouse failed, and no others. The hub pages passed.
Verified with real Lighthouse against rendered pages:
color-contrast= 1 andcategories.accessibility= 1.0 on a learn topic page, an integrations topic page, and a compare page.This gate is currently red on main (it began failing when the SEO cluster landed; main's own run fails identically). This PR should turn it green.
3. The stress harness
scripts/qa-lab/stress-context.mjs. Threads run to hundreds of messages, past the point where the token budget must start evicting — that is where a context system either degrades honestly or lies.It measures two things and refuses to conflate them, because conflating them is how the original context defect survived so long:
Results on
claw.local, free models only (the harness refuses metered providers in code):Three flaws in the harness itself
Each turned a red result green — the same silent-degradation shape the harness exists to catch. Recording them because the numbers above are only trustworthy because these were found:
DELIVERY violations 0. Unmeasured probes are now counted separately and prove nothing.Lab hygiene, same family of bug
client.mjsgainsloginAs/createIsolatedUser. Memories are user-scoped and inject into every thread, so a shared lab account accumulates fixtures that compete with planted facts — a storedThe internal project codename is ORCHID-2728made three recall probes fail at distances 4, 24 and 56 at identical rates, which is the signature of a constant competing fact, not of context loss. Isolation is opt-in because a fresh account lands on the Free plan and exhausts its daily quota in ~15 turns.memory-experiment.mjs: forgetting is confirmation-gated, and the cleanup omitted?confirm=FORGETand ignored the 400. Every run left its standing INSTRUCTION behind, so the next run had two conflicting "always end your reply with …" memories live and obeyed the older one — reporting its own litter as a product failure.Testing
routing-education.repository.spec.ts8/8 (new test asserts the lock is the first operation — one taken after the DELETE would block nothing)Not in this PR
package-lock.jsonversion drift (1.36.2vs1.59.0), deliberately left alone🤖 Generated with Claude Code