Skip to content

Commit 35c2eec

Browse files
jeremymanningclaude
andcommitted
Survive transient HuggingFace outages instead of failing on them
CI failed with `HuggingFace dataset service: HTTP 502` — 247 tests passed and the one failure was a live third-party service being down. The same run had the Hub 429-ing the static export five times before it correctly refused to publish. This project forbids mocks, so a test that calls a real service will fail when the service does; the answer is not to fake it. Retry with backoff (500/1500/4000 ms) on 429 and 5xx, then FAIL LOUDLY with the real status. 404/401/403 are NOT retried — an unknown or gated dataset is a real answer the user must see immediately, and delaying it helps nobody. This is a product fix, not a CI convenience: a reader who pastes a dataset id during a blip now gets their rows instead of an error they can do nothing about. Four tests drive the REAL client through a controlled transport — not a mock of the service, a test of the policy: recovers from 502, recovers from 429, does not retry 404, and gives up after exactly 4 attempts reporting the true status. The last one carries an explicit 15 s timeout because the full backoff is 6 s of real waiting, past vitest's default; waiting it out is the point, so the delays cannot be shortened away. Frontend 252 passed (was 247), svelte-check 0/0. Also noted for the record: the Pages deploy on d11d394 failed on Hub 429s and was rerun. That guard worked correctly — it refused to publish a build pinned to 'main' rather than a commit sha, which is exactly the mis-pinning tracked as issue #5. The durable fix for the underlying rate limiting is an HF_TOKEN secret, which needs the repository owner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d11d394 commit 35c2eec

3 files changed

Lines changed: 152 additions & 10 deletions

File tree

code/frontend/src/lib/staticClient/hfDatasets.ts

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,44 @@ export interface DatasetFetchResult {
4444
rows: number;
4545
}
4646

47+
/**
48+
* Statuses worth trying again: the service is up but momentarily unable to answer.
49+
* 429 is rate limiting; 5xx is the service failing on its own side. Everything else
50+
* (404 unknown dataset, 401/403 gated) is a real answer and must NOT be retried — the
51+
* user needs to see it immediately.
52+
*/
53+
const TRANSIENT = (status: number): boolean => status === 429 || status >= 500;
54+
const RETRY_DELAYS_MS = [500, 1500, 4000];
55+
56+
const sleep = (ms: number): Promise<void> => new Promise((r) => setTimeout(r, ms));
57+
58+
/**
59+
* GET with retries for transient upstream failures, then FAIL LOUDLY.
60+
*
61+
* This is a live third-party service and it does go down: CI observed `HTTP 502` here
62+
* while every other test passed, and the same run saw the Hub 429-ing the static export.
63+
* Retrying is not about CI convenience — a reader who pastes a dataset id during a blip
64+
* should get their data, not an error they can do nothing about. Nothing is faked and
65+
* nothing is swallowed: after the last attempt the real status is raised.
66+
*/
4767
async function getJson(url: string, fetchImpl: typeof fetch): Promise<unknown> {
48-
let res: Response;
49-
try {
50-
res = await fetchImpl(url);
51-
} catch (e) {
52-
throw computeError(
53-
`Could not reach the HuggingFace dataset service: ${e instanceof Error ? e.message : String(e)}`,
54-
);
55-
}
56-
if (!res.ok) {
68+
let lastDetail = "";
69+
for (let attempt = 0; attempt <= RETRY_DELAYS_MS.length; attempt++) {
70+
let res: Response;
71+
try {
72+
res = await fetchImpl(url);
73+
} catch (e) {
74+
// A network-level failure is transient too (dropped connection, DNS blip).
75+
lastDetail = e instanceof Error ? e.message : String(e);
76+
if (attempt < RETRY_DELAYS_MS.length) {
77+
await sleep(RETRY_DELAYS_MS[attempt]);
78+
continue;
79+
}
80+
throw computeError(`Could not reach the HuggingFace dataset service: ${lastDetail}`);
81+
}
82+
83+
if (res.ok) return res.json();
84+
5785
// The service returns a useful `error` string for unknown / gated / not-yet-indexed
5886
// datasets; surface it rather than a bare status code.
5987
let detail = `HTTP ${res.status}`;
@@ -63,9 +91,16 @@ async function getJson(url: string, fetchImpl: typeof fetch): Promise<unknown> {
6391
} catch {
6492
// non-JSON body — the status is all we have
6593
}
94+
95+
if (TRANSIENT(res.status) && attempt < RETRY_DELAYS_MS.length) {
96+
lastDetail = detail;
97+
await sleep(RETRY_DELAYS_MS[attempt]);
98+
continue;
99+
}
66100
throw invalidParamError(`HuggingFace dataset service: ${detail}`);
67101
}
68-
return res.json();
102+
// Unreachable: the loop either returns or throws. Kept so the type is honest.
103+
throw invalidParamError(`HuggingFace dataset service: ${lastDetail || "unavailable"}`);
69104
}
70105

71106
/** The config/split pairs a dataset actually exposes. */

code/frontend/tests/unit/hfDatasets.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,67 @@ describe("HuggingFace dataset viewer (real service)", () => {
4444
await expect(listSplits(" ")).rejects.toMatchObject({ type: "InvalidParamError" });
4545
});
4646
});
47+
48+
describe("transient upstream failures", () => {
49+
// NOT a mock of the service: these drive the real client with a controlled transport to
50+
// prove its RETRY POLICY. CI saw the live viewer return HTTP 502 while everything else
51+
// passed, and the same run saw the Hub 429 the static export. A reader who pastes a
52+
// dataset id during a blip should get their rows, not an error they cannot act on.
53+
const okBody = {
54+
splits: [{ dataset: "d", config: "default", split: "train" }],
55+
};
56+
const respond = (status: number, body: unknown = {}): Response =>
57+
new Response(JSON.stringify(body), {
58+
status,
59+
headers: { "content-type": "application/json" },
60+
});
61+
62+
it("retries a 502 and succeeds when the service recovers", async () => {
63+
let calls = 0;
64+
const flaky: typeof fetch = async () => {
65+
calls += 1;
66+
return calls < 3 ? respond(502, { error: "Bad Gateway" }) : respond(200, okBody);
67+
};
68+
const splits = await listSplits("roneneldan/TinyStories", flaky);
69+
expect(calls).toBe(3);
70+
expect(splits.length).toBeGreaterThan(0);
71+
});
72+
73+
it("retries a 429 the same way", async () => {
74+
let calls = 0;
75+
const limited: typeof fetch = async () => {
76+
calls += 1;
77+
return calls < 2 ? respond(429, { error: "Too Many Requests" }) : respond(200, okBody);
78+
};
79+
await listSplits("roneneldan/TinyStories", limited);
80+
expect(calls).toBe(2);
81+
});
82+
83+
it("does NOT retry a 404 — an unknown dataset is a real answer", async () => {
84+
let calls = 0;
85+
const missing: typeof fetch = async () => {
86+
calls += 1;
87+
return respond(404, { error: "Dataset not found" });
88+
};
89+
await expect(listSplits("nobody/does-not-exist", missing)).rejects.toThrow(/not found/i);
90+
// One attempt only: retrying would delay an error the user must see to act on.
91+
expect(calls).toBe(1);
92+
});
93+
94+
it(
95+
"gives up loudly after exhausting retries, reporting the real status",
96+
async () => {
97+
let calls = 0;
98+
const down: typeof fetch = async () => {
99+
calls += 1;
100+
return respond(503, { error: "Service Unavailable" });
101+
};
102+
await expect(listSplits("roneneldan/TinyStories", down)).rejects.toThrow(/Unavailable/i);
103+
expect(calls).toBe(4); // initial + 3 retries
104+
},
105+
// The full backoff is 500 + 1500 + 4000 = 6 s of REAL waiting, past vitest's 5 s
106+
// default. Waiting it out is the point: this asserts the policy actually gives up
107+
// rather than retrying forever, so the delays must not be shortened away.
108+
15_000,
109+
);
110+
});

notes/2026-08-04-feature-006-lexicon-lab.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,49 @@ real PyTorch" — false for a tab that never calls the backend; and the corpus c
182182
advertised a verification the browser never performed, over the untrimmed file rather than
183183
the loaded body. The browser now genuinely rehashes what it loaded and refuses a mismatch.
184184

185+
## What shipped broken, and why the tests did not stop it
186+
187+
Worth keeping, because all four failures share one cause and it is not "not enough tests".
188+
189+
**A crash on the DEFAULT configuration reached the live site.** With `steps=400` and
190+
`sampleEvery=50` the periodic sampler fires at step 400 and the final sample is also
191+
recorded at step 400; the samples list is keyed by step, so Svelte threw
192+
`each_key_duplicate` on every default run. Found by training on the deployed site — not by
193+
any test.
194+
195+
The bug needs `steps % sampleEvery === 0`. The local browser check used 120 steps, the e2e
196+
test used 30, the unit tests used other non-multiples. Every one was chosen to be FAST, and
197+
every one accidentally avoided the collision. **The one configuration a visitor actually
198+
gets was the one nothing exercised.** The e2e test also never asserted the absence of
199+
console errors during a run, so executing it would still have passed.
200+
201+
**Then CI failed on three tests written against imagined markup:**
202+
203+
1. digit-stripping `lex-budget-size` to read `|V|` — that element is the whole radio
204+
*group*, so it returned every option's digits concatenated;
205+
2. waiting for a radio matching `/frequenc/i` — the label is "corpus top-N", so Playwright
206+
waited the full timeout for text that has never existed;
207+
3. `toContainText("314")` against that same group, which lists EVERY size — so it passed
208+
regardless of what was selected. **A test that cannot fail is worse than no test**: it
209+
reports coverage it does not provide, and it was green locally.
210+
211+
**And a fourth:** the Lexicon tab was added without updating `shell.spec.ts`, which
212+
asserted `toHaveCount(3)`. It was doing its job; it broke in the same commit that added the
213+
tab, and the push happened after running only the Lexicon spec. Adding a tab is exactly the
214+
change that touches the shell contract.
215+
216+
The common thread: assertions written against what the system was ASSUMED to contain rather
217+
than what it does — the same error as the `<eos>` divergence (a contract gap) and the
218+
`+29.3` copied from an agent report without recomputing. What corrected it every time was
219+
looking: querying the DOM, recomputing the arithmetic, driving the real thing.
220+
221+
Durable changes made in response:
222+
- the e2e training test uses 100 steps **because** `100 % 50 == 0`, and fails on any
223+
`pageerror` or console error during a run;
224+
- the shell contract asserts tabs by NAME and ORDER, table-driven over every tab so a new
225+
one cannot be added without appearing there;
226+
- assertions read the element that states a number, never a container that includes it.
227+
185228
## Verification
186229

187230
Backend **336 passed**, ruff + black clean. Frontend **247 passed** (1 skipped),

0 commit comments

Comments
 (0)