From 3b258971e26d17038219dd338c638f9182bd7e02 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:49:08 -0500 Subject: [PATCH 01/20] fix(viewer): preserve run evidence and recover result reads Keep pending results pending, reject retired statuses and unidentified results, and read terminal outcomes independently of event-poll failures. Honor preview backpressure and reread results when their advertised preview disappears. Long polls now drop at the synthetic outage boundary. https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114609680 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114609691 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114623689 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114688567 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114688570 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114688572 https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4115036579 --- tools/integration-viewer/public/live.js | 14 ++-- tools/integration-viewer/public/model.js | 15 ++-- tools/integration-viewer/relay.js | 12 ++-- tools/integration-viewer/synthetic-api.js | 22 ++++-- tools/integration-viewer/test/live.test.js | 71 +++++++++++++++++++ tools/integration-viewer/test/model.test.js | 15 +++- tools/integration-viewer/test/relay.test.js | 20 ++++++ .../test/synthetic-api.test.js | 23 ++++++ 8 files changed, 167 insertions(+), 25 deletions(-) create mode 100644 tools/integration-viewer/test/synthetic-api.test.js diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index 19ad8e731..04a5fae93 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -1,4 +1,4 @@ -// Follows one capture through the loopback relay: health once, then an events long-poll loop, +// Follows one capture through the loopback relay: health until it answers, then an events long-poll loop, // with status refreshes and the result and preview fetched when the events say they exist. // Every dispatch carries the session it started under; the reducer ignores it after a switch, // and stop() aborts whatever request is in flight. @@ -74,6 +74,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat let waitS = 0; while (!signal.aborted) { + let contactRetryMs = 0; try { // An event or result named a run the status has not named yet: ask the status now. if (getState().statusRefreshWanted) statusAt = -Infinity; @@ -105,9 +106,8 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat send({ type: "contact-failed", error: detail, offline: !online() }); const backoff = BACKOFF_MS[Math.min(failures, BACKOFF_MS.length - 1)]; const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; - await sleep(Math.max(backoff, asked), signal).catch(() => {}); + contactRetryMs = Math.max(backoff, asked); failures += 1; - continue; } const state = getState(); @@ -149,8 +149,8 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat send({ type: "preview", runId, cloud }); } catch (error) { if (signal.aborted) return; - if (error instanceof HttpError && error.status === 409) { - previewKey = null; // the run moved on; the next result read decides what to show + if (error instanceof HttpError && (error.status === 409 || error.code === "preview_not_available")) { + previewKey = revisionKey; // do not retry this artifact until a fresh result is accepted readyKey = null; continue; } @@ -159,10 +159,12 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat // A fetch failure may pass; an unreadable file will not. Only the first is retried. send({ type: "preview-error", runId, error: error instanceof HttpError ? error.message : "network error" }); previewKey = null; - previewRetryAt = now() + PREVIEW_RETRY_MS; + const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; + previewRetryAt = now() + Math.max(PREVIEW_RETRY_MS, asked); } } } + if (contactRetryMs > 0) await sleep(contactRetryMs, signal).catch(() => {}); } } diff --git a/tools/integration-viewer/public/model.js b/tools/integration-viewer/public/model.js index 1b873f5f9..d8a8da354 100644 --- a/tools/integration-viewer/public/model.js +++ b/tools/integration-viewer/public/model.js @@ -117,7 +117,7 @@ export function reduce(state, action) { return applyEvents(state, action.body, action.at, action.catchUp); case "result-loading": // Refreshing a result already shown keeps showing it until the new one arrives. - return { ...state, result: { ...state.result, phase: state.result.phase === "ready" ? "ready" : "loading", error: null } }; + return { ...state, result: { ...state.result, phase: ["ready", "pending"].includes(state.result.phase) ? state.result.phase : "loading", error: null } }; case "result": return applyResult(state, action.body); case "result-error": @@ -157,6 +157,7 @@ function select(state, action) { function applyStatus(state, body) { if (!isObject(body) || (body.captureId && body.captureId !== state.captureId)) return state; + if (typeof body.runId === "string" && state.retiredRuns.has(body.runId)) return state; const next = { ...state, status: typeof body.status === "string" ? body.status : state.status, @@ -167,7 +168,7 @@ function applyStatus(state, body) { }; // The status reports the capture's latest run, so it alone decides which run is shown. const named = { ...next, statusRefreshWanted: false }; - if (typeof body.runId !== "string" || named.retiredRuns.has(body.runId)) return named; + if (typeof body.runId !== "string") return named; return setCurrentRun(named, body.runId); } @@ -331,13 +332,13 @@ const NO_OUTCOME_STATUSES = new Set(["failed", "expired"]); function applyResult(state, body) { if (!isObject(body)) return { ...state, result: { ...state.result, phase: "error", body: null, error: "Result was not a JSON object" } }; + // An outcome without run identity cannot be attributed to the capture's current run. + if (typeof body.runId !== "string" || body.runId === "") return state; // A result for a run other than the one shown is not accepted: a retired run's is dropped, and // an unknown run's asks for a status refresh first. The result read is retried either way. - if (typeof body.runId === "string") { - if (state.retiredRuns.has(body.runId)) return state; - state = sawRun(state, body.runId); - if (body.runId !== state.currentRunId) return state; - } + if (state.retiredRuns.has(body.runId)) return state; + state = sawRun(state, body.runId); + if (body.runId !== state.currentRunId) return state; const ready = body.outcome != null || NO_OUTCOME_STATUSES.has(body.status); // A refreshed result that no longer offers a preview takes the earlier model with it. const preview = !hasPreview(body) && state.preview.runId === body.runId ? { phase: "none", runId: null, cloud: null, error: null } : state.preview; diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index 14ee8806f..23f3e7650 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -111,7 +111,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { async function relayPreview(res, upstream, resultPath, runId, signal) { const result = await upstreamGet(`${upstream.base}${resultPath}/result`, signal); if (result.failure) return refuse(res, result.failure.status, result.failure.code, result.failure.message); - if (result.status !== 200) return refuse(res, result.status, "result_unavailable", "the result could not be read"); + if (result.status !== 200) return refuse(res, result.status, "result_unavailable", "the result could not be read", result.retryAfter ? { "retry-after": result.retryAfter } : {}); if (result.json?.runId !== runId) return refuse(res, 409, "preview_run_changed", "the latest result belongs to another run"); const signed = result.json?.previewUrl; if (typeof signed !== "string" || signed === "") return refuse(res, 404, "preview_not_available", "the result has no preview"); @@ -130,7 +130,11 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { } catch (error) { return refuse(res, 502, "preview_unreachable", error?.name === "TimeoutError" ? "preview fetch timed out" : "preview fetch failed"); } - if (!reply.ok) return refuse(res, 502, "preview_fetch_failed", `storage answered ${reply.status}`); + if (!reply.ok) { + const retryAfter = reply.headers.get("retry-after"); + await reply.body?.cancel(); + return refuse(res, 502, "preview_fetch_failed", `storage answered ${reply.status}`, retryAfter ? { "retry-after": retryAfter } : {}); + } const declared = Number(reply.headers.get("content-length") ?? "0"); if (declared > PREVIEW_MAX_BYTES) return refuse(res, 502, "preview_too_large", "the preview is larger than the viewer reads"); const chunks = []; @@ -220,8 +224,8 @@ async function upstreamGet(url, signal) { return { status: reply.status, json, etag: reply.headers.get("etag"), retryAfter: reply.headers.get("retry-after") }; } -function refuse(res, status, code, message) { - send(res, status, { errors: [{ code, message }] }); +function refuse(res, status, code, message, headers = {}) { + send(res, status, { errors: [{ code, message }] }, headers); return `refused ${code}`; } diff --git a/tools/integration-viewer/synthetic-api.js b/tools/integration-viewer/synthetic-api.js index d34988d0b..f41d45947 100644 --- a/tools/integration-viewer/synthetic-api.js +++ b/tools/integration-viewer/synthetic-api.js @@ -13,10 +13,10 @@ import { DROPOUT, RUN_ID, encodePly, resultBody, syntheticCloud, timeline } from const ID = /^cap_SYNTH_(COMPLETE|FAILED|DROPOUT)_[A-Za-z0-9_]{1,40}$/; -/** @param {{ speed?: number }} [options] speed > 1 plays scenarios faster; tests use it. */ -export function createSyntheticApi({ speed = 1 } = {}) { - const t0 = Date.now(); - const now = () => t0 + (Date.now() - t0) * speed; +/** @param {{ speed?: number, clock?: () => number }} [options] speed > 1 plays scenarios faster; tests use it. */ +export function createSyntheticApi({ speed = 1, clock = Date.now } = {}) { + const t0 = clock(); + const now = () => t0 + (clock() - t0) * speed; const captures = new Map(); let ply = null; let origin = ""; @@ -75,8 +75,11 @@ export function createSyntheticApi({ speed = 1 } = {}) { const entry = m ? capture(m[1]) : null; if (!entry) return send(404, { errors: [{ code: "capture_not_found", pointer: "/", message: "capture not found" }] }); - const elapsed = now() - entry.startedAt; - if (entry.scenario === "dropout" && elapsed >= DROPOUT.from && elapsed < DROPOUT.to) { + const inOutage = () => { + const elapsed = now() - entry.startedAt; + return entry.scenario === "dropout" && elapsed >= DROPOUT.from && elapsed < DROPOUT.to; + }; + if (inOutage()) { req.socket.destroy(); // a dropped connection, not an HTTP error return; } @@ -88,9 +91,14 @@ export function createSyntheticApi({ speed = 1 } = {}) { const deadline = now() + waitS * 1000; let state = stateAt(entry); while (!state.events.some((e) => e.seq > after) && now() < deadline && state.nextAt != null) { - const pause = Math.max(10, (Math.min(deadline, state.nextAt) - now()) / speed); + const outageAt = entry.scenario === "dropout" && state.elapsed < DROPOUT.from ? entry.startedAt + DROPOUT.from : Infinity; + const pause = Math.max(1, (Math.min(deadline, state.nextAt, outageAt) - now()) / speed); await new Promise((resolve) => setTimeout(resolve, pause)); if (res.destroyed) return; + if (inOutage()) { + req.socket.destroy(); + return; + } state = stateAt(entry); } const fresh = state.events.filter((e) => e.seq > after).slice(0, 100); diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index 059f035be..1173aa3f1 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -203,3 +203,74 @@ test("health and a failed preview are read again after a transient failure", asy assert.ok(healthCalls >= 2); assert.equal(previewCalls, 2); }); + +function stubFollower(fetchReply) { + let clock = 0; + let state = reduce(initialState(), { type: "select", mode: "live", source: { key: "s", kind: "synthetic" }, captureId: "cap_STUB_3" }); + const follower = followCapture({ + sourceKey: "s", captureId: "cap_STUB_3", session: state.session, + dispatch: (a) => { state = reduce(state, a); }, getState: () => state, + fetchImpl: (path) => fetchReply(path, clock), now: () => clock, + sleep: (ms) => new Promise((resolve) => { clock += ms; setTimeout(resolve, 1); }), + online: () => true, + }); + return { follower, get state() { return state; } }; +} + +const stubJson = (body, status = 200, headers = {}) => new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json", ...headers } }); +const stubStatus = () => stubJson({ runId: "run_s", status: "complete" }); +const stubEvents = () => stubJson({ events: [], next: 0, status: "complete" }); +const stubResult = (previewUrl = "") => stubJson({ runId: "run_s", status: "complete", outcome: { kind: "eligible" }, previewUrl }); + +test("terminal results are read even while every events request fails", async () => { + let resultReads = 0; + const h = stubFollower((path) => { + if (path.includes("/events")) return stubJson({ errors: [{ code: "busy" }] }, 503); + if (path.endsWith("/result")) { resultReads += 1; return stubResult(); } + return stubStatus(); + }); + try { + await until(() => h.state.result.phase === "ready"); + assert.equal(resultReads, 1); + assert.equal(h.state.connection.phase, "reconnecting"); + } finally { h.follower.stop(); await h.follower.done; } +}); + +test("preview retries wait for Retry-After", async () => { + const previewAt = []; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) return stubEvents(); + if (path.endsWith("/result")) return stubResult("withheld"); + if (path.includes("/preview")) { + previewAt.push(clock); + return previewAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "19" }) : new Response(ply); + } + return stubStatus(); + }); + try { + await until(() => h.state.preview.phase === "ready"); + assert.equal(previewAt.length, 2); + assert.ok(previewAt[1] - previewAt[0] >= 19_000); + } finally { h.follower.stop(); await h.follower.done; } +}); + +test("a preview removed by a result revision triggers a fresh result read", async () => { + let resultReads = 0; + let previewReads = 0; + const h = stubFollower((path) => { + if (path.includes("/events")) return stubEvents(); + if (path.endsWith("/result")) { resultReads += 1; return stubResult(resultReads === 1 ? "withheld" : ""); } + if (path.includes("/preview")) { + previewReads += 1; + return stubJson({ errors: [{ code: "preview_not_available" }] }, 404); + } + return stubStatus(); + }); + try { + await until(() => resultReads >= 2 && h.state.result.body.previewUrl === ""); + assert.equal(resultReads, 2); + assert.equal(previewReads, 1); + assert.equal(h.state.preview.phase, "none"); + } finally { h.follower.stop(); await h.follower.done; } +}); diff --git a/tools/integration-viewer/test/model.test.js b/tools/integration-viewer/test/model.test.js index 3f007e184..ffed17ea9 100644 --- a/tools/integration-viewer/test/model.test.js +++ b/tools/integration-viewer/test/model.test.js @@ -157,6 +157,9 @@ describe("results", () => { test("a minimal result while processing stays pending", () => { const s = run(selected(), { type: "result", body: { runId: "run_a", status: "processing", outcome: null, viewsNeeded: [], memberActions: [] } }); assert.equal(s.result.phase, "pending"); + const refreshing = run(s, { type: "result-loading" }); + assert.equal(refreshing.result.phase, "pending"); + assert.equal(refreshing.result.body, s.result.body); }); test("a model with zero points is empty, not ready", () => { @@ -341,9 +344,19 @@ describe("run identity: the status names the run shown", () => { }); test("a delayed status naming a retired run does not bring it back", () => { - const s = run(selected(), status("run_a"), status("run_b"), eligible("run_b"), status("run_a")); + const current = run(selected(), status("run_a"), status("run_b"), eligible("run_b")); + const s = run(current, { type: "status", body: { runId: "run_a", status: "failed", filesRegistered: 99 } }); assert.equal(s.currentRunId, "run_b"); assert.equal(s.result.phase, "ready"); + assert.equal(s, current, "no retired-run status fields are applied"); + }); + + test("a result without a nonempty run identity is deferred", () => { + const current = run(selected(), status("run_a"), status("run_b"), eligible("run_b")); + for (const runId of [undefined, null, 42, ""]) { + const s = run(current, { type: "result", body: { runId, status: "complete", outcome: { kind: "not_eligible" } } }); + assert.equal(s, current); + } }); test("reproduced: a run the status named without events, then retired, cannot return through a late event", () => { diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index aee506f2a..416e81a65 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -217,6 +217,26 @@ describe("refusals", () => { } }); + test("preview result and storage failures preserve Retry-After", async () => { + for (const failingPath of ["/result", "/v1/throttled.ply"]) { + hostileHandler = (req, res) => { + if (req.url.endsWith(failingPath)) { + res.writeHead(503, { "content-type": "application/json", "retry-after": "19" }); + return res.end(JSON.stringify({ errors: [{ code: "busy" }] })); + } + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ runId: "r", previewUrl: "/v1/throttled.ply" })); + }; + try { + const reply = await raw("/relay/hostile/captures/cap_HOSTILE_1/preview?run=r"); + assert.equal(reply.status, failingPath === "/result" ? 503 : 502); + assert.equal(reply.headers["retry-after"], "19"); + } finally { + hostileHandler = null; + } + } + }); + test("an oversized API reply is refused, not buffered", async () => { hostileHandler = (req, res) => { res.writeHead(200, { "content-type": "application/json" }); diff --git a/tools/integration-viewer/test/synthetic-api.test.js b/tools/integration-viewer/test/synthetic-api.test.js new file mode 100644 index 000000000..e78ae187b --- /dev/null +++ b/tools/integration-viewer/test/synthetic-api.test.js @@ -0,0 +1,23 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createSyntheticApi } from "../synthetic-api.js"; + +test("a long poll started before the outage drops at its boundary", async () => { + let clock = 0; + let onRead = () => {}; + const api = createSyntheticApi({ clock: () => { onRead(); return clock; } }); + const origin = `http://127.0.0.1:${await api.listen()}`; + const path = `${origin}/v1/captures/cap_SYNTH_DROPOUT_BOUNDARY`; + try { + await fetch(path); + clock = 10_900; + const reading = new Promise((resolve) => { onRead = resolve; }); + const poll = fetch(`${path}/events?after=999&wait=20`, { signal: AbortSignal.timeout(2000) }); + const dropped = assert.rejects(poll, /fetch failed/); + await reading; + clock = 11_000; + await dropped; + clock = 19_000; + assert.equal((await fetch(path)).status, 200); + } finally { await api.close(); } +}); From 2ab17087866bfe14cfc569767306a166e77d4944 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:49:26 -0500 Subject: [PATCH 02/20] docs: match review conclusions and examples to source evidence Correct the shortened request example, conditional existing-battery check and reconstruction pair-error figures. Reconcile the app-review summaries with their own source descriptions and recorded audit coverage. https://github.com/SamGu-NRX/BaseScanning/pull/134#discussion_r4114742754 https://github.com/SamGu-NRX/BaseScanning/pull/134#discussion_r4115037523 https://github.com/SamGu-NRX/BaseScanning/pull/134#discussion_r4115037524 https://github.com/SamGu-NRX/BaseScanning/pull/134#discussion_r4115037526 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4113876024 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297481 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297485 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297488 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297490 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297493 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297496 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297497 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297499 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297505 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297509 https://github.com/SamGu-NRX/BaseScanning/pull/61#discussion_r4114297511 --- README.md | 11 ++++++----- docs/00-overview.md | 2 +- docs/app-review/product/README.md | 2 +- docs/app-review/product/bug-triage.md | 3 ++- docs/app-review/ux/review.md | 21 +++++++++++---------- docs/readme/architecture.svg | 2 +- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 4de84535d..6a2ef25d4 100644 --- a/README.md +++ b/README.md @@ -113,14 +113,15 @@ The battery measures 31 × 22 × 39.5 in, a bit bigger than a dishwasher. It sta | Cable route | Can't cross a door, a garage or a gap in the wall | Demo choice | | Driveway | At least 5 ft away | Placeholder, no public value | | Pool | At least 10 ft away | Placeholder, no public value | +| Existing battery, when the scene marks one | At least 3 ft away; the cable also detours around it | Base's help page; IRC 2021 R328, cited in `server/rules.yaml` | -Each check comes back PASS, FAIL or UNSURE, along with what the server measured, how far off that measurement could be and a reason in plain English. NEC is the National Electrical Code and IRC is the International Residential Code, the two building codes behind several of these rules. The numbers themselves live in `server/rules.yaml` on PR #11's branch, each one next to its source, and [docs/04](docs/04-prior-art-and-codes.md) has the full citations. +Each check comes back PASS, FAIL or UNSURE, along with what the server measured, how far off that measurement could be and a reason in plain English. NEC is the National Electrical Code and IRC is the International Residential Code, the two building codes behind several of these rules. The first 13 checks run at each spot; `battery_clearance` is a fourteenth check only when the scene marks an existing battery. The numbers themselves live in `server/rules.yaml`, each one next to its source, and [docs/04](docs/04-prior-art-and-codes.md) has the full citations. The server then gives one of three answers for the whole scan. It says `pass` when a spot passes every check. It says `reject` only when every spot within cable reach fails and the app knows where the wall ends on both sides. Anything in between is `manual_review`, and it goes to a person. ### One request, start to finish -Here's what that looks like in practice. We took the example scene from the server's tests in PR #11 and sent it to the demo server. The scene is made up. We wrote it by hand, with a gas meter, a window and an AC unit spread over two walls. The reply is real, though, trimmed to the parts worth reading. +Here's what that looks like in practice. We took the example scene from the server's tests in PR #11 and sent it to the demo server. The scene is made up. We wrote it by hand, with a gas meter, a window and an AC unit spread over two walls. The reply is real, though, trimmed to the parts worth reading. The abbreviated scene below omits fields needed to reproduce it. Use [the complete test fixture](server/tests/fixtures/example-scene.json) for a runnable request. The server stopped short of placing the battery itself. Its best spot was 9 ft 11 in left of the meter. From there, the AC unit around the corner measured 4 ft 1 in away, against a 3 ft rule. That sounds like a pass until you see the margin of error, which is give or take 3 ft 10 in. It's that wide because the AC unit sits far along the wall from the meter, where tracking error piles up the most. With an error that wide, the result is too close to call. So the answer is a manual review, plus a request to keep walking past the left end of the scan, because a spot within cable reach might be there. @@ -174,7 +175,7 @@ The server stopped short of placing the battery itself. Its best spot was 9 ft 1 { "id": "ac_clearance", "outcome": "unsure", "measured_ft": 4.08, "plus_minus_ft": 3.8, "threshold_ft": 3.0, "comparison": "at_least", - "reason": "objects[3] ac is 4 ft 1 in (± 3 ft 10 in) from the battery against a 3 ft 0 in rule: too close to call." + "reason": "The ac is 4 ft 1 in (± 3 ft 10 in) from the battery against a 3 ft 0 in rule: too close to call." } ], "missing_evidence": [ @@ -277,7 +278,7 @@ A few terms first, in case they're new to you. Tracking drift is how far the pho | Tracking drift, older iPhone | Two to three times over that allowance | ADVIO, iPhone 6s, PR #12 | | Learned depth on its own | Scale 4 to 12% off, which puts walls about 20 in out (p90) | ETH3D, PR #12 | | Learned depth, rescaled with the phone's poses | Walls within about 5 in (p90), or 2.8 in with exact poses. Edges stay at 8 in or worse | ETH3D, PR #12 | -| Reconstruction worker | Walls within 2.1 in (p90) with a laser scan standing in for LiDAR, and 2.8 in from photos only | ETH3D, PR #20 | +| Reconstruction worker | Wall pair-error p90 of 1.8 in with a laser scan standing in for LiDAR, and 1.5 in from photos only. Both fit stretches of the same wall, with different lengths | ETH3D, PR #20 | | Reading the meter number | Read in full on 71 of 73 photos, but the right line on only 21 of 75. A list of three candidates held it on 27 of 34 held-out photos | Photos of real meters, PR #16 | | First run on our phone | Both wall ends landed at the meter, so the server placed no spot. Two of five features came within 4 in of the tape | TestFlight build, PR #23 | @@ -287,7 +288,7 @@ Here's what we know is either missing or broken: - **Most of it isn't merged.** Guided capture is in PR #10, the rules engine in PR #11, reconstruction in PR #20 and the packet spec in PR #22. All four still live on their own branches. - **Our first real phone run placed nothing.** The app put both ends of the wall right at the meter, which left the server a wall with no length to search (PR #23). -- **We haven't settled on how to build the 3D model.** A depth model on its own puts walls about 20 in off. Correcting its scale with the phone's poses brings that down to about 5 in, but edges stay 8 in off or worse, and the clearance rules measure from edges. With a laser scan standing in for LiDAR, walls land within 2.1 in. +- **We haven't settled on how to build the 3D model.** A depth model on its own puts walls about 20 in off. Correcting its scale with the phone's poses brings that down to about 5 in, but edges stay 8 in off or worse, and the clearance rules measure from edges. With a laser scan standing in for LiDAR, the worker reports wall pair-error p90 of 1.8 in, as recorded in [the reconstruction handoff](recon/HANDOFF.md). - **Our best tracking result came from a phone with LiDAR.** An iPhone 14 Pro Max stayed inside the error allowance, but it has LiDAR, and most homeowners' phones don't. An older iPhone 6s ran two to three times over. - **Nobody has taken on hidden walls yet.** A bush or a trash can in front of the wall can hide what's behind it, and on phones without LiDAR, nothing checks for that. - **Two rules use placeholder numbers.** We couldn't find a public value for how far a battery should sit from a pool or a driveway, so for now they're 10 ft and 5 ft. diff --git a/docs/00-overview.md b/docs/00-overview.md index 920b7abc1..add12d5e2 100644 --- a/docs/00-overview.md +++ b/docs/00-overview.md @@ -50,7 +50,7 @@ These results come from public datasets with laser-scanned or surveyed ground tr - **Tracking.** A current iPhone stayed inside the server's error allowance. On the MARViN dataset, an iPhone 14 Pro Max had p90 position error of 8.6, 13.4 and 18.5 in after 10, 20 and 30 ft, against an allowance of 19.2, 38.4 and 57.6 in. That phone has LiDAR, and MARViN's reference may be scaled to its tracking. A 2018 iPhone 6s on ADVIO ran two to three times over, partly from noise in that dataset's reference. - **Learned depth.** A model's own scale is 4 to 12% off. Rescaled with the phone's poses, walls come within about 5 in at p90 at a realistic 2% pose error, and within 2.8 in with exact poses. Edges stay at 8 in or worse, and clearances are measured from edges. MapAnything's scale shifted with input resolution. -- **Reconstruction worker.** On the ETH3D building, wall p90 is 2.1 in with a laser scan standing in for LiDAR, and 2.8 in from photos only. The two runs measured different walls. +- **Reconstruction worker.** On the ETH3D building, wall pair-error p90 is 1.8 in with a laser scan standing in for LiDAR, and 1.5 in from photos only. Both fit stretches of the same wall, with different lengths. The photos-only change includes a different fitted stretch; see `recon/HANDOFF.md`. - **Coverage.** The app's first coverage map claimed 1.1 ft of a 19.3 ft wall that no photo saw. Requiring several view angles removed the false claim but also discarded 6 to 8.5 ft of good wall. A depth test on LiDAR phones removed it cleanly. Checked against depth, the reconstruction worker claims at most about 0.46 ft of unseen wall. - **Meter reading.** Apple's on-device text recognition read the full meter number on 71 of 73 real photos. It picked the right line on only 21 of 75, because nameplates carry several numbers. A barcode held the number on 20 of the 24 photos that had one. A list of three candidates held it on about 4 in 5 held-out photos (27 of 34). So the app offers three candidates to tap, barcode match first, and asks for a retake when the photo is blurry or the number too small. - **Panel labels.** Not tested. A search found 7 usable open photos of US panels, and the test needs 40. diff --git a/docs/app-review/product/README.md b/docs/app-review/product/README.md index 741e678c6..0af25272f 100644 --- a/docs/app-review/product/README.md +++ b/docs/app-review/product/README.md @@ -75,4 +75,4 @@ and Open questions and verification, ending with the commit it was verified agai | [screens/uploading.md](screens/uploading.md) | Sending the scan and waiting for the result, offline and failure | drafted; checked in the Simulator | | [screens/result.md](screens/result.md) | The result, its checks and the 3D and AR views | drafted; checked in the Simulator | | [verification.md](verification.md) | Checks against the running app and their results | current pass at `a39d0a5` | -| [bug-triage.md](bug-triage.md) | Every suspected defect, deduplicated | 16 entries, 2 resolved | +| [bug-triage.md](bug-triage.md) | Every suspected defect, deduplicated | 16 entries, 3 resolved | diff --git a/docs/app-review/product/bug-triage.md b/docs/app-review/product/bug-triage.md index 3c0b08cc3..5c63f4a80 100644 --- a/docs/app-review/product/bug-triage.md +++ b/docs/app-review/product/bug-triage.md @@ -29,8 +29,9 @@ source and not run. Every blocker and major entry was re-read in the source by a **B-03. Failures other than an unsupported phone never reach the screen.** Only the failure screen shows a failure, and the app switches to it only for an unsupported phone (`HouseScan/Runtime/ScanEngine.swift:91-93`). A denied camera, a failed session or an unreadable -replay set the failure without changing screen (lines 105, 379, 381). The homeowner stays on +replay set the failure without changing screen (lines 105, 379, 381). With a denied camera or failed live session, the homeowner stays on "Find your electric meter" with no camera and taps do nothing; "Open Settings" is unreachable. +An unreadable replay can fail while still on onboarding, because it loads in the background. Seen with `-replay /nonexistent`: the app logged "replay unreadable: cannot read /nonexistent/session.json" and stayed on onboarding (report `sim/20260926-044618-5520229-5520229d-preview-bad-replay`). diff --git a/docs/app-review/ux/review.md b/docs/app-review/ux/review.md index 809249331..a03ff6b44 100644 --- a/docs/app-review/ux/review.md +++ b/docs/app-review/ux/review.md @@ -27,8 +27,8 @@ ARKit, so tracking loss, relocalization and the live meter tap are judged from s ## Verdict: Block Blockers that fail: R9 (camera access denied leads nowhere, [B-03](../product/bug-triage.md#blockers)), -T5 (the close-up's way out can fail to appear, B-04), R1 (a stretch the homeowner can't reach -is not recorded, B-08) and, from source, I8 (at `a39d0a5` the meter tap can land on another +T5 (the close-up's way out can fail to appear, B-04), R1 ("Done with this wall" permits uncovered cells without marking them inaccessible, +[wall-walk edge cases](../product/screens/wall-walk.md#edge-cases)) and, from source, I8 (at `a39d0a5` the meter tap can land on another wall's plane extended past its edge, B-11). The feature list's "Add something" doing nothing (B-09) blocks adding a missed feature. I9, the panel close-up's safety wording, is Not verified because panel close-ups are not built; with blockers failing, the verdict stays Block. @@ -40,13 +40,13 @@ because panel close-ups are not built; with blockers failing, the verdict stays | `onboarding` | Pass at default size: one message, one primary button, "About 2 min". At AX5 the pill reads "About 2 mi" and the body runs under the page dots. | UX-01 | | `unsupported` | Pass: "This phone can't measure walls" with what to do instead. Camera denied never reaches this screen. | B-03 | | `findMeter` | Fails I8 from source: with the meter's wall not yet detected, the tap can land on another wall's plane extended past its edge. Otherwise one instruction, the reticle, "This is my meter". At AX5 the button is pushed partly off screen. Refusal wording ("Step a little closer to the wall") also covers tracking refusals. | UX-01, B-11 | -| `meterCloseUp` | Pass for the meter close-up: one instruction, no shutter, a named fix ("Center the meter in the circle"). The problem pill fails contrast; the way out can fail to appear. I9 Not verified: panel close-ups are not built. | UX-03, B-04 | -| `wallWalk` | One instruction with its reply inside the card, as the checklist asks. The strip's states differ by colour alone, the counter fails contrast, and at AX5 the card covers most of the camera and pushes "Mark something" and the strip off screen. At `525ea40` a marked end is followed by "What's at the left end?", so its kind is truthful, but an end closer than 20 ft still can't be marked. | UX-01, UX-02, UX-03, UX-05, UX-08, B-06 | -| `markFeatures` | Pass at default size: clear list, remove buttons labelled, window question with two full-width answers. At AX5 the list is hidden behind "Looks complete". "Add something" does nothing. | UX-01, B-09 | +| `meterCloseUp` | One instruction, no shutter, a named fix ("Center the meter in the circle"). The problem pill passes contrast at `a39d0a5`; the way out can fail to appear. T6 fails from source: none of the named fixes tells the homeowner to fit a cut-off label in the box. I9 Not verified: panel close-ups are not built. | T6, B-04 | +| `wallWalk` | One instruction with its reply inside the card, as the checklist asks. The strip's states differ by colour alone, the counter fails contrast, and at AX5 the card covers most of the camera and pushes "Mark something" and the strip off screen. At `525ea40` a marked end is followed by "What's at the left end?", so its kind is truthful, but an end closer than 20 ft still can't be marked. V4 fails from source: the target ring pulses during capture. I7 fails from source: world-anchored guidance points into unobserved ground. A2 fails from source: VoiceOver reports wall percent seen, including amber cells, but omits completed sections, ground coverage and tracking status. | UX-01, UX-02, UX-03, UX-05, UX-08, B-06, V4, I7, A2 | +| `markFeatures` | Pass at default size: clear list, remove buttons labelled, window question with two full-width answers. At AX5 the list is hidden behind "Looks complete". "Add something" does nothing. I4 fails from source when two windows are marked: each window row shows its own question at once. | UX-01, B-09, I4 | | `gapRequest` | Pass: one line with the reason and target, a progress bar, "I can't get there". The requested stretch reuses the amber that means "seen". | UX-05, UX-06, B-08 | -| `uploading` | Pass: progress in its own steps, apart from coverage (R7); with no server it says "Making a sample result" (`525ea40`). Four text elements do not scale. A failure shows raw error text. | UX-04, B-02 | -| `result` | Pass on honesty: "An installer will take a look", the rules-not-final note, the unseen side disclosed, the borderline window check with measurement, rule and error, and "An installer will check this". A maximum reads like a minimum; the first load leaves the upload screen up for over 1.2 s. | B-14, UX-07 | -| `resultAR` | Pass: the spot and cable over the camera with one headline and "Done". The headline says "The spot an installer will check" unless the result is an approved pass, and "Example spot, not your result" for a sample (seen at `525ea40`). A7 Not verified: not run with Reduce Motion on, and the [product description](../product/screens/result.md) reads from source that the AR battery then rises over 0.2 s, where A7 asks for a crossfade or cut. | A7 (from source) | +| `uploading` | Pass: progress in its own steps, apart from coverage (R7); with no server it says "Making a sample result" (`525ea40`). The current audit records no upload font-scaling failure; the four-element failure belongs to an older run. A failure shows raw error text. | B-02 | +| `result` | Pass for the exercised manual-review and sample results: "An installer will take a look", the rules-not-final note, the unseen side disclosed, the borderline window check with measurement, rule and error, and "An installer will check this". A maximum reads like a minimum; the first load leaves the upload screen up for over 1.2 s. H5 Not verified: no reject was exercised, and its source description supplies neither another candidate nor a promise that an installer will find one. | B-14, UX-07, H5 | +| `resultAR` | The spot and cable over the camera with one headline and "Done". The headline says "The spot an installer will check" unless the result is an approved pass, and "Example spot, not your result" for a sample (seen at `525ea40`). A7 fails from source, not observed with Reduce Motion on. The [product description](../product/screens/result.md) reads from source that the AR battery then rises over 0.2 s, where A7 asks for a crossfade or cut. | A7 (from source) | Checks that passed across the flow: one primary instruction per camera screen (I1); plain words, no jargon in any on-screen string (I3); no shutter where capture is automatic (I5); every control @@ -95,8 +95,9 @@ flagged text on the walk that is drawn rather than exposed to accessibility; the ## Fixed since the preview (`0876e03`) -Seen: the window question answers "It opens" and "It stays shut", stacked (I4), and the result -says the electrical panel still needs an electrician's review (H7). From source: tracking coaching says +Seen: one window question answers "It opens" and "It stays shut", stacked. The multiple-window +I4 failure is recorded from source in the state table. The result says the electrical panel still +needs an electrician's review (H7). From source: tracking coaching says "Slow down" and "Aim at a corner or somewhere with more texture" (T1). While relocalizing, the saved meter photo shows with "Point at the meter like this." (T7). Overlays hide while tracking is not normal (T3). diff --git a/docs/readme/architecture.svg b/docs/readme/architecture.svg index 814a7f353..0d05a6f50 100644 --- a/docs/readme/architecture.svg +++ b/docs/readme/architecture.svg @@ -53,7 +53,7 @@ svg{--bg:#ffffff;--ink:#1a1a1a;--ink2:#333333;--mut:#4d4d4d;--edge:#333333;--df: reached count as seen. FastAPI validates scene.json against its JSON Schema. The solver slides the 31 × 22 in footprint along every observed wall -span and scores 13 checks against rules.yaml. A check is +span and scores all checks against rules.yaml. A check is UNSURE when its error band straddles the threshold or its area went unseen. decision is pass, manual_review or reject. RealityKit parents the spot to an AnchorEntity on the meter's From d06435e1580fb10b2b9fd4de6b605a8f05c4d9de Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:49:40 -0500 Subject: [PATCH 03/20] fix(checks): include viewer tests and reject experiment drift Run the viewer suite from make check, refuse stale meter lockfiles, and check Measure Lab's generated project before its tests and build. Add scoped stub regressions for these root targets without invoking Xcode or dependencies. https://github.com/SamGu-NRX/BaseScanning/pull/124#discussion_r4114688574 https://github.com/SamGu-NRX/BaseScanning/pull/181#discussion_r4115813889 https://github.com/SamGu-NRX/BaseScanning/pull/181#discussion_r4115813895 --- AGENTS.md | 2 +- Makefile | 21 ++++++-- tools/integration-viewer/README.md | 2 +- .../integration-viewer/test/makefile.test.js | 52 +++++++++++++++++++ 4 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 tools/integration-viewer/test/makefile.test.js diff --git a/AGENTS.md b/AGENTS.md index af4a8b6f5..05b7a195b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ A user's instruction outranks a skill. ## Working in the repository -- `make check` runs every suite on the branch. `make ios`, `make server`, `make web`, `make scoring`, `make measure-lab`, `make evals`, `make recon` and `make meter-closeup` run one each. +- `make check` runs every suite on the branch. `make ios`, `make server`, `make web`, `make scoring`, `make measure-lab`, `make evals`, `make recon`, `make meter-closeup` and `make integration-viewer` run one each. - Branch from `main`, keep one writer per branch, and open a pull request. People merge, and agents never push to `main` or merge, so a person sees every change before it lands. - Several agents share one Mac. Exit code 137 means the system killed the process, usually for memory. Find the large allocation before rerunning, because one runaway process can freeze the whole machine. - Put `DEVELOPMENT_TEAM` in `ios/Config/Local.xcconfig` (copy `Local.xcconfig.example`), not in Xcode's Signing & Capabilities pane. The pane writes into `project.pbxproj`, and CI fails on that drift. diff --git a/Makefile b/Makefile index 21d3fd050..b5438d5cf 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ -# Each suite target runs the commands of its CI job in .github/workflows/. +# Suite targets follow their CI jobs where one exists. The viewer has no CI job yet. # `make check` runs server, web and ios, then scoring, measure-lab, evals, recon -# and meter-closeup where the branch has them. +# meter-closeup and the integration viewer where the branch has them. # tests/makefile.sh checks this dispatch with stub suites. XCODEGEN ?= xcodegen @@ -15,20 +15,22 @@ MEASURE_LAB := experiments/measure-lab/Makefile EVALS := experiments/evals/Makefile RECON := recon/Makefile METER_CLOSEUP := experiments/meter-closeup/Makefile +INTEGRATION_VIEWER := tools/integration-viewer/package.json # $(call present,FILE,TARGET) is TARGET when FILE exists, otherwise nothing. present = $(if $(wildcard $(1)),$(2)) # $(call require,FILE,PR): stop before any command runs when the suite's file is missing. require = @test -f $(1) || { echo "make $@: $(1) is missing; the suite arrives with pull request $(2)." >&2; exit 1; } -.PHONY: check server web ios ios-project smoke scoring measure-lab evals recon meter-closeup +.PHONY: check server web ios ios-project smoke scoring measure-lab evals recon meter-closeup integration-viewer check: server web ios \ $(call present,$(SCORING),scoring) \ $(call present,$(MEASURE_LAB),measure-lab) \ $(call present,$(EVALS),evals) \ $(call present,$(RECON),recon) \ - $(call present,$(METER_CLOSEUP),meter-closeup) + $(call present,$(METER_CLOSEUP),meter-closeup) \ + $(call present,$(INTEGRATION_VIEWER),integration-viewer) server: cd server && uv sync --locked @@ -64,9 +66,12 @@ scoring: cd experiments/scoring && uv run ruff format --check . cd experiments/scoring && uv run pytest -q -# The Geometry package's tests, then the app build. +# Regenerate and check the committed project before Geometry tests and the app build. measure-lab: $(call require,$(MEASURE_LAB),#7) + $(MAKE) -C experiments/measure-lab project XCODEGEN="$(XCODEGEN)" + git diff --exit-code -- experiments/measure-lab/MeasureLab.xcodeproj + @test -z "$$(git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj)" || { echo "MeasureLab.xcodeproj contains uncommitted generated files." >&2; exit 1; } $(MAKE) -C experiments/measure-lab check # The folder's `make test` lints and tests but leaves out the `uv sync --locked` that @@ -80,11 +85,17 @@ recon: $(call require,$(RECON),#20) $(MAKE) -C recon test +# Locked sync matches the metric CI job and rejects stale locks before uv run can rewrite them. # Lint and unit tests only. The separate leak scan needs a private key and must not run here. meter-closeup: $(call require,$(METER_CLOSEUP),#16) + cd experiments/meter-closeup && uv sync --locked $(MAKE) -C experiments/meter-closeup check +integration-viewer: + $(call require,$(INTEGRATION_VIEWER),#124) + cd tools/integration-viewer && node --test "test/*.test.js" + # Posts each scene in server/examples to a placement server and prints its decision: # make smoke URL=https://house-scanning-server.vercel.app [KEY_FILE=server/.env.private.local] smoke: diff --git a/tools/integration-viewer/README.md b/tools/integration-viewer/README.md index b7b1df1fe..147bf3ee7 100644 --- a/tools/integration-viewer/README.md +++ b/tools/integration-viewer/README.md @@ -84,7 +84,7 @@ cd tools/integration-viewer node --test "test/*.test.js" ``` -The tests cover the reducer (duplicates, ordering, cursor, stale sessions, unknown events, offline, no claimed result), the PLY reader, the relay through real HTTP requests (allowlist, traversal, Host check, signed-URL handling), and the live loop against the relay and a sped-up synthetic API, including a dropped connection. CI does not run them yet, because no workflow covers `tools/`. +The tests cover the reducer (duplicates, ordering, cursor, stale sessions, unknown events, offline, no claimed result), the PLY reader, the relay through real HTTP requests (allowlist, traversal, Host check, signed-URL handling), and the live loop against the relay and a sped-up synthetic API, including a dropped connection. `make integration-viewer` runs this suite, and `make check` includes it. CI does not run it yet, because no workflow covers `tools/`. `test/browser.test.js` runs the page in headless Chrome with the animation clock stopped and frames drawn only on request, which is how an embedded preview panel behaved. It checks that the file counts match the events drawn in the same frame and that finished stages show their marker. It skips when no Chrome is found; set `CHROME_PATH` to point at one. diff --git a/tools/integration-viewer/test/makefile.test.js b/tools/integration-viewer/test/makefile.test.js new file mode 100644 index 000000000..044e93333 --- /dev/null +++ b/tools/integration-viewer/test/makefile.test.js @@ -0,0 +1,52 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; + +function tree(t) { + const root = mkdtempSync(join(tmpdir(), "viewer-make-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + writeFileSync(join(root, "Makefile"), readFileSync(new URL("../../../Makefile", import.meta.url))); + const bin = join(root, "bin"); + mkdirSync(bin); + const log = join(root, "commands"); + writeFileSync(log, ""); + for (const tool of ["uv", "node", "git", "pnpm", "xcodebuild"]) { + writeFileSync(join(bin, tool), `#!/bin/sh\nprintf '%s %s\\n' '${tool}' "$*" >> "$LOG"\n[ "$FAIL_TOOL" != '${tool}' ]\n`, { mode: 0o755 }); + } + const suite = (path, text) => { + mkdirSync(join(root, path), { recursive: true }); + writeFileSync(join(root, path, "Makefile"), text); + }; + const run = (target, failTool = "") => spawnSync("make", [target], { cwd: root, encoding: "utf8", env: { ...process.env, PATH: `${bin}:${process.env.PATH}`, LOG: log, FAIL_TOOL: failTool } }); + return { root, suite, run, commands: () => readFileSync(log, "utf8") }; +} + +test("meter-closeup refuses a stale lock before running its checks", (t) => { + const h = tree(t); + h.suite("experiments/meter-closeup", 'check:\n\t@echo checks >> "$$LOG"\n'); + assert.notEqual(h.run("meter-closeup", "uv").status, 0); + assert.equal(h.commands(), "uv sync --locked\n"); +}); + +test("measure-lab detects project drift before its tests or build", (t) => { + const h = tree(t); + h.suite("experiments/measure-lab", 'project:\n\t@echo project >> "$$LOG"\ncheck:\n\t@echo checks >> "$$LOG"\n'); + assert.notEqual(h.run("measure-lab", "git").status, 0); + assert.equal(h.commands(), "project\ngit diff --exit-code -- experiments/measure-lab/MeasureLab.xcodeproj\n"); +}); + +test("make check includes the viewer only when its suite exists", (t) => { + const h = tree(t); + mkdirSync(join(h.root, "server")); + mkdirSync(join(h.root, "web")); + assert.equal(h.run("check").status, 0); + assert.ok(!h.commands().includes("node")); + h.suite("tools/integration-viewer", ""); + writeFileSync(join(h.root, "tools/integration-viewer/package.json"), "{}"); + assert.equal(h.run("check").status, 0); + assert.ok(h.commands().includes("node --test test/*.test.js\n")); + assert.notEqual(h.run("check", "node").status, 0); +}); From d0b5444b751462548f6d2948cc552b60bcb9c5e8 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:08:59 -0500 Subject: [PATCH 04/20] fix(checks): test viewer dispatch and fail on project-list errors Cover all 64 suite combinations, exact command order, missing suites and failure propagation for project generation, tracked/untracked drift, locked meter sync and viewer tests. A failed git ls-files now fails the Makefile instead of looking like an empty generated-file list. https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4264927974 --- Makefile | 4 ++- tests/makefile.sh | 72 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index b5438d5cf..52b807c3c 100644 --- a/Makefile +++ b/Makefile @@ -71,7 +71,9 @@ measure-lab: $(call require,$(MEASURE_LAB),#7) $(MAKE) -C experiments/measure-lab project XCODEGEN="$(XCODEGEN)" git diff --exit-code -- experiments/measure-lab/MeasureLab.xcodeproj - @test -z "$$(git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj)" || { echo "MeasureLab.xcodeproj contains uncommitted generated files." >&2; exit 1; } + # A failed listing must not look like a clean generated project. + @untracked="$$(git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj)" || exit 1; \ + test -z "$$untracked" || { echo "MeasureLab.xcodeproj contains uncommitted generated files." >&2; exit 1; } $(MAKE) -C experiments/measure-lab check # The folder's `make test` lints and tests but leaves out the `uv sync --locked` that diff --git a/tests/makefile.sh b/tests/makefile.sh index 9e7552bd5..c87f78501 100755 --- a/tests/makefile.sh +++ b/tests/makefile.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Checks which suites the root Makefile runs, without running any of them. Each case copies the -# Makefile into an empty temporary tree, adds stub experiment folders, and puts stub uv, pnpm and -# xcodebuild first on PATH. Every stub, including the stub folder Makefiles, appends +# Makefile into an empty temporary tree, adds stub suite folders, and puts stub tools +# first on PATH. Every stub, including the stub folder Makefiles, appends # " " to a log, so the log shows which suites ran and with what commands. # Run it from anywhere: bash tests/makefile.sh set -euo pipefail @@ -10,33 +10,44 @@ root=$(cd "$(dirname "$0")/.." && pwd) # Resolved, because the stubs compare it with $PWD, and macOS's /var is a link to /private/var. work=$(cd "$(mktemp -d)" && pwd -P) trap 'rm -rf "$work"' EXIT -export LOG="$work/log" TREE="$work/tree" FAIL_AT="" +export LOG="$work/log" TREE="$work/tree" FAIL_AT="" UNTRACKED="" failures=0 -# A stub fails when FAIL_AT is " " for it, such as "experiments/evals make". +# FAIL_AT names a folder and tool, or one exact invocation including its arguments. mkdir "$work/bin" -for tool in uv pnpm xcodebuild; do +for tool in uv pnpm xcodebuild git node; do cat >"$work/bin/$tool" <<'EOF' #!/bin/sh dir=${PWD#"$TREE"/} [ "$dir" = "$PWD" ] && dir=. echo "$dir $(basename "$0") $*" >>"$LOG" -[ "$dir $(basename "$0")" != "$FAIL_AT" ] +command="$dir $(basename "$0")" +[ "$command" != "$FAIL_AT" ] && [ "$command $*" != "$FAIL_AT" ] || exit 1 +if [ "$(basename "$0")" = git ] && [ "$1" = ls-files ] && [ -n "$UNTRACKED" ]; then + printf '%s\n' "$UNTRACKED" +fi EOF chmod +x "$work/bin/$tool" done export PATH="$work/bin:$PATH" -suites="scoring measure-lab evals recon meter-closeup" +suites="scoring measure-lab evals recon meter-closeup integration-viewer" folder() { case $1 in recon) echo recon ;; + integration-viewer) echo tools/integration-viewer ;; *) echo "experiments/$1" ;; esac } -# Makes a tree with the root Makefile and the named experiment suites. Every suite but scoring -# gets a stub Makefile with the target the root calls; scoring gets only its pyproject.toml. +stub_target() { + local dir=$1 target=$2 + printf '%s:\n\t@echo "%s make %s" >>"$$LOG"\n\t@test "$$FAIL_AT" != "%s make"\n\t@test "$$FAIL_AT" != "%s make %s"\n' \ + "$target" "$dir" "$target" "$dir" "$dir" "$target" +} + +# Each suite gets its dispatch marker: a stub Makefile, scoring's pyproject.toml, +# or the viewer's package.json. Measure Lab has separate project and check targets. make_tree() { rm -rf "$TREE" "$LOG" mkdir -p "$TREE/server" "$TREE/web" @@ -48,11 +59,14 @@ make_tree() { mkdir -p "$TREE/$dir" case $suite in scoring) touch "$TREE/$dir/pyproject.toml"; continue ;; + integration-viewer) touch "$TREE/$dir/package.json"; continue ;; measure-lab | meter-closeup) target=check ;; *) target=test ;; esac - printf '%s:\n\t@echo "%s make %s" >>"$$LOG"\n\t@test "$$FAIL_AT" != "%s make"\n' \ - "$target" "$dir" "$target" "$dir" >"$TREE/$dir/Makefile" + stub_target "$dir" "$target" >"$TREE/$dir/Makefile" + if [ "$suite" = measure-lab ]; then + stub_target "$dir" project >>"$TREE/$dir/Makefile" + fi done } @@ -66,8 +80,8 @@ fail() { } # 1. `make check` runs server, web and ios, then exactly the experiment suites present, in order, -# for all 32 combinations. "." is the ios build, which runs from the root. -for mask in $(seq 0 31); do +# for all 64 combinations. "." is the ios build or project drift check at the root. +for mask in $(seq 0 63); do present=() expected="server web ." i=0 @@ -75,6 +89,7 @@ for mask in $(seq 0 31); do if (((mask >> i) & 1)); then present+=("$suite") expected="$expected $(folder "$suite")" + [ "$suite" != measure-lab ] || expected="$expected . $(folder "$suite")" fi i=$((i + 1)) done @@ -101,11 +116,16 @@ expect_commands scoring "experiments/scoring uv sync --locked experiments/scoring uv run ruff check . experiments/scoring uv run ruff format --check . experiments/scoring uv run pytest -q" -expect_commands measure-lab "experiments/measure-lab make check" +expect_commands measure-lab "experiments/measure-lab make project +. git diff --exit-code -- experiments/measure-lab/MeasureLab.xcodeproj +. git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj +experiments/measure-lab make check" expect_commands evals "experiments/evals uv sync --locked experiments/evals make test" expect_commands recon "recon make test" -expect_commands meter-closeup "experiments/meter-closeup make check" +expect_commands meter-closeup "experiments/meter-closeup uv sync --locked +experiments/meter-closeup make check" +expect_commands integration-viewer "tools/integration-viewer node --test test/*.test.js" # 3. Naming a missing suite fails loudly and runs nothing. for suite in $suites; do @@ -118,17 +138,33 @@ for suite in $suites; do done # 4. A failing command inside a present suite fails `make check`, and nothing runs after it. -for FAIL_AT in "experiments/scoring uv" "experiments/measure-lab make" "experiments/evals uv" \ - "experiments/evals make" "recon make" "experiments/meter-closeup make"; do +for FAIL_AT in "experiments/scoring uv" "experiments/measure-lab make" \ + "experiments/measure-lab make project" \ + "experiments/measure-lab make check" \ + ". git diff --exit-code -- experiments/measure-lab/MeasureLab.xcodeproj" \ + ". git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj" \ + "experiments/evals uv" "experiments/evals make" "recon make" \ + "experiments/meter-closeup uv" "experiments/meter-closeup make" \ + "tools/integration-viewer node"; do make_tree $suites if run_make check; then fail "check passed although $FAIL_AT failed" - elif [ "$(tail -n 1 "$LOG" | cut -d' ' -f1-2)" != "$FAIL_AT" ]; then + elif [ "$(tail -n 1 "$LOG")" != "$FAIL_AT" ] && [ "$(tail -n 1 "$LOG" | cut -d' ' -f1-2)" != "$FAIL_AT" ]; then fail "check kept going after $FAIL_AT failed" fi done FAIL_AT="" +# 5. New generated project files fail the root check before tests or builds. +make_tree measure-lab +UNTRACKED=experiments/measure-lab/MeasureLab.xcodeproj/new-file +if run_make measure-lab; then + fail "measure-lab passed with untracked generated files" +elif [ "$(tail -n 1 "$LOG")" != ". git ls-files --others --exclude-standard -- experiments/measure-lab/MeasureLab.xcodeproj" ]; then + fail "measure-lab kept going after finding untracked generated files" +fi +UNTRACKED="" + if [ "$failures" -ne 0 ]; then echo "$failures Makefile dispatch case(s) failed" >&2 exit 1 From aaa6e8c1be785a7cbc156e1da266c76d314892bd Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:09:24 -0500 Subject: [PATCH 05/20] fix(viewer): honor backoff through preview invalidation Parse HTTP-date Retry-After values and keep contact backoff when a preview requires a fresh result. The controlled outage test now asserts the 100 ms boundary wake and rejects a mutation that instead sleeps until the next event. https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4264951280 https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4264951290 https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4264951301 --- tools/integration-viewer/public/live.js | 8 +++--- tools/integration-viewer/synthetic-api.js | 6 ++--- tools/integration-viewer/test/live.test.js | 25 +++++++++++++++++-- .../test/synthetic-api.test.js | 16 ++++++++---- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index 04a5fae93..b50f10409 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -52,7 +52,9 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat if (!reply.ok) { const body = await reply.json().catch(() => null); const error = body?.errors?.[0]; - const retryAfter = Number(reply.headers.get("retry-after")); + const retryHeader = reply.headers.get("retry-after") ?? ""; + let retryAfter = Number(retryHeader); + if (!/^\d+$/.test(retryHeader)) retryAfter = /[A-Za-z]/.test(retryHeader) ? (Date.parse(retryHeader) - now()) / 1000 : NaN; throw new HttpError(reply.status, error?.code ?? `http_${reply.status}`, error?.message ?? `HTTP ${reply.status}`, Number.isFinite(retryAfter) && retryAfter > 0 ? retryAfter : null); } return as === "json" ? reply.json() : reply.arrayBuffer(); @@ -152,9 +154,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat if (error instanceof HttpError && (error.status === 409 || error.code === "preview_not_available")) { previewKey = revisionKey; // do not retry this artifact until a fresh result is accepted readyKey = null; - continue; - } - if (error instanceof PlyError) send({ type: "preview-error", runId, unsupported: true, error: error.message }); + } else if (error instanceof PlyError) send({ type: "preview-error", runId, unsupported: true, error: error.message }); else { // A fetch failure may pass; an unreadable file will not. Only the first is retried. send({ type: "preview-error", runId, error: error instanceof HttpError ? error.message : "network error" }); diff --git a/tools/integration-viewer/synthetic-api.js b/tools/integration-viewer/synthetic-api.js index f41d45947..32eb57040 100644 --- a/tools/integration-viewer/synthetic-api.js +++ b/tools/integration-viewer/synthetic-api.js @@ -13,8 +13,8 @@ import { DROPOUT, RUN_ID, encodePly, resultBody, syntheticCloud, timeline } from const ID = /^cap_SYNTH_(COMPLETE|FAILED|DROPOUT)_[A-Za-z0-9_]{1,40}$/; -/** @param {{ speed?: number, clock?: () => number }} [options] speed > 1 plays scenarios faster; tests use it. */ -export function createSyntheticApi({ speed = 1, clock = Date.now } = {}) { +/** @param {{ speed?: number, clock?: () => number, sleep?: (ms: number) => Promise }} [options] speed > 1 plays scenarios faster; tests use it. */ +export function createSyntheticApi({ speed = 1, clock = Date.now, sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)) } = {}) { const t0 = clock(); const now = () => t0 + (clock() - t0) * speed; const captures = new Map(); @@ -93,7 +93,7 @@ export function createSyntheticApi({ speed = 1, clock = Date.now } = {}) { while (!state.events.some((e) => e.seq > after) && now() < deadline && state.nextAt != null) { const outageAt = entry.scenario === "dropout" && state.elapsed < DROPOUT.from ? entry.startedAt + DROPOUT.from : Infinity; const pause = Math.max(1, (Math.min(deadline, state.nextAt, outageAt) - now()) / speed); - await new Promise((resolve) => setTimeout(resolve, pause)); + await sleep(pause); if (res.destroyed) return; if (inOutage()) { req.socket.destroy(); diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index 1173aa3f1..cfe314068 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -236,7 +236,7 @@ test("terminal results are read even while every events request fails", async () } finally { h.follower.stop(); await h.follower.done; } }); -test("preview retries wait for Retry-After", async () => { +for (const form of ["seconds", "date"]) test(`preview retries wait for ${form}-form Retry-After`, async () => { const previewAt = []; const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; const h = stubFollower((path, clock) => { @@ -244,7 +244,7 @@ test("preview retries wait for Retry-After", async () => { if (path.endsWith("/result")) return stubResult("withheld"); if (path.includes("/preview")) { previewAt.push(clock); - return previewAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "19" }) : new Response(ply); + return previewAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": form === "date" ? new Date(clock + 19_000).toUTCString() : "19" }) : new Response(ply); } return stubStatus(); }); @@ -274,3 +274,24 @@ test("a preview removed by a result revision triggers a fresh result read", asyn assert.equal(h.state.preview.phase, "none"); } finally { h.follower.stop(); await h.follower.done; } }); + +for (const response of [ + { status: 409, code: "preview_run_changed" }, + { status: 404, code: "preview_not_available" }, +]) test(`events backoff survives preview ${response.code}`, async () => { + const eventsAt = []; + let resultReads = 0; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) { + eventsAt.push(clock); + return eventsAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "15" }) : stubEvents(); + } + if (path.endsWith("/result")) { resultReads += 1; return stubResult(resultReads === 1 ? "withheld" : ""); } + if (path.includes("/preview")) return stubJson({ errors: [{ code: response.code }] }, response.status); + return stubStatus(); + }); + try { + await until(() => eventsAt.length >= 2); + assert.ok(eventsAt[1] - eventsAt[0] >= 15_000, `event reads at ${eventsAt}`); + } finally { h.follower.stop(); await h.follower.done; } +}); diff --git a/tools/integration-viewer/test/synthetic-api.test.js b/tools/integration-viewer/test/synthetic-api.test.js index e78ae187b..fdd206e45 100644 --- a/tools/integration-viewer/test/synthetic-api.test.js +++ b/tools/integration-viewer/test/synthetic-api.test.js @@ -4,20 +4,26 @@ import { createSyntheticApi } from "../synthetic-api.js"; test("a long poll started before the outage drops at its boundary", async () => { let clock = 0; - let onRead = () => {}; - const api = createSyntheticApi({ clock: () => { onRead(); return clock; } }); + let wake = () => {}; + let scheduled; + const waiting = new Promise((resolve) => { scheduled = resolve; }); + const api = createSyntheticApi({ + clock: () => clock, + sleep: (ms) => new Promise((resolve) => { wake = resolve; scheduled(ms); }), + }); const origin = `http://127.0.0.1:${await api.listen()}`; const path = `${origin}/v1/captures/cap_SYNTH_DROPOUT_BOUNDARY`; try { await fetch(path); clock = 10_900; - const reading = new Promise((resolve) => { onRead = resolve; }); const poll = fetch(`${path}/events?after=999&wait=20`, { signal: AbortSignal.timeout(2000) }); const dropped = assert.rejects(poll, /fetch failed/); - await reading; + const delay = await waiting; clock = 11_000; + wake(); + assert.equal(delay, 100, "the poll wakes at 11 s, before the next event"); await dropped; clock = 19_000; assert.equal((await fetch(path)).status, 200); - } finally { await api.close(); } + } finally { clock = 11_000; wake(); await api.close(); } }); From 02d60738d6fe9bcbfea950166456c1e6f5903ad4 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:09:24 -0500 Subject: [PATCH 06/20] docs: state both conditions for checking battery clearance The check also runs without an existing battery when its configured clearance exceeds the AC clearance. Equal 3 ft limits describe the shipped public rules. https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4264951308 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6a2ef25d4..6642ee21f 100644 --- a/README.md +++ b/README.md @@ -113,9 +113,9 @@ The battery measures 31 × 22 × 39.5 in, a bit bigger than a dishwasher. It sta | Cable route | Can't cross a door, a garage or a gap in the wall | Demo choice | | Driveway | At least 5 ft away | Placeholder, no public value | | Pool | At least 10 ft away | Placeholder, no public value | -| Existing battery, when the scene marks one | At least 3 ft away; the cable also detours around it | Base's help page; IRC 2021 R328, cited in `server/rules.yaml` | +| Existing battery, when the scene marks one or its configured clearance exceeds the AC clearance | At least 3 ft away with shipped rules; the cable also detours around it | Base's help page; IRC 2021 R328, cited in `server/rules.yaml` | -Each check comes back PASS, FAIL or UNSURE, along with what the server measured, how far off that measurement could be and a reason in plain English. NEC is the National Electrical Code and IRC is the International Residential Code, the two building codes behind several of these rules. The first 13 checks run at each spot; `battery_clearance` is a fourteenth check only when the scene marks an existing battery. The numbers themselves live in `server/rules.yaml`, each one next to its source, and [docs/04](docs/04-prior-art-and-codes.md) has the full citations. +Each check comes back PASS, FAIL or UNSURE, along with what the server measured, how far off that measurement could be and a reason in plain English. NEC is the National Electrical Code and IRC is the International Residential Code, the two building codes behind several of these rules. The first 13 checks run at each spot. `battery_clearance` also runs when the scene marks an existing battery or the configured battery clearance exceeds the AC clearance. The shipped public rules give both a 3 ft limit, so a scene without a battery omits that extra check. The numbers themselves live in `server/rules.yaml`, each one next to its source, and [docs/04](docs/04-prior-art-and-codes.md) has the full citations. The server then gives one of three answers for the whole scan. It says `pass` when a spot passes every check. It says `reject` only when every spot within cable reach fails and the app knows where the wall ends on both sides. Anything in between is `manual_review`, and it goes to a person. From 224a4572b967cb1b2afb2e99f0d603ecd4f8c2af Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:08:50 -0500 Subject: [PATCH 07/20] fix(viewer): refresh invalidated results and isolate preview retries Reset slow result polling when a preview becomes unavailable. Limit preview backoff to the revision whose artifact failed. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265379978 Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265379981 --- tools/integration-viewer/public/live.js | 9 +++- tools/integration-viewer/test/live.test.js | 62 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index b50f10409..6cf2b6e85 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -69,6 +69,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat let resultAt = -Infinity; let resultNotBefore = -Infinity; let previewKey = null; + let previewRetryKey = null; let previewRetryAt = -Infinity; // Reads start without waiting and repeat while pages come back full. They return what happened // before the viewer connected, which the reducer marks as backlog so it does not animate. @@ -142,7 +143,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const result = getState().result; const runId = result.body?.runId ?? null; const revisionKey = `${runId}|${result.revision}`; - if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && now() >= previewRetryAt) { + if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && (previewRetryKey !== revisionKey || now() >= previewRetryAt)) { previewKey = revisionKey; send({ type: "preview-loading", runId }); try { @@ -154,12 +155,18 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat if (error instanceof HttpError && (error.status === 409 || error.code === "preview_not_available")) { previewKey = revisionKey; // do not retry this artifact until a fresh result is accepted readyKey = null; + // This result is stale; do not carry over its slow polling schedule. + resultTries = 0; + resultAt = -Infinity; + resultNotBefore = -Infinity; + waitS = 0; } else if (error instanceof PlyError) send({ type: "preview-error", runId, unsupported: true, error: error.message }); else { // A fetch failure may pass; an unreadable file will not. Only the first is retried. send({ type: "preview-error", runId, error: error instanceof HttpError ? error.message : "network error" }); previewKey = null; const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; + previewRetryKey = revisionKey; // another revision may reference a different storage object previewRetryAt = now() + Math.max(PREVIEW_RETRY_MS, asked); } } diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index cfe314068..e1609e632 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -295,3 +295,65 @@ for (const response of [ assert.ok(eventsAt[1] - eventsAt[0] >= 15_000, `event reads at ${eventsAt}`); } finally { h.follower.stop(); await h.follower.done; } }); + +for (const response of [ + { status: 409, code: "preview_run_changed" }, + { status: 404, code: "preview_not_available" }, +]) test(`preview ${response.code} restarts result polling after slow reads`, async () => { + const resultAt = []; + let previewReads = 0; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) return stubEvents(); + if (path.endsWith("/result")) { + resultAt.push(clock); + if (resultAt.length <= 8) return stubJson({ runId: "run_s", status: "complete", outcome: null }); + return stubResult(resultAt.length === 9 ? "withheld" : ""); + } + if (path.includes("/preview")) { + previewReads += 1; + return stubJson({ errors: [{ code: response.code }] }, response.status); + } + return stubStatus(); + }); + try { + await until(() => resultAt.length >= 10 && h.state.result.body.previewUrl === ""); + assert.ok(resultAt[8] - resultAt[7] >= 30_000, "the result reached the slow polling interval"); + assert.ok(resultAt[9] - resultAt[8] <= 4_000, `result reads at ${resultAt}`); + assert.equal(previewReads, 1); + assert.equal(h.state.preview.phase, "none"); + } finally { h.follower.stop(); await h.follower.done; } +}); + +for (const change of ["run", "revision"]) test(`preview backoff does not delay a new ${change}`, async () => { + const previewAt = []; + let resultReads = 0; + let revisedAt = null; + const runAt = (clock) => change === "run" && clock >= 6_000 ? "run_b" : "run_s"; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) { + const after = new URL(path, "http://x").searchParams.get("after"); + const changed = clock >= 6_000; + return stubJson({ status: "complete", next: changed ? 1 : 0, events: changed && after === "0" + ? [{ seq: 1, type: "verdict_ready", data: { runId: runAt(clock), kind: "eligible" } }] : [] }); + } + if (path.endsWith("/result")) { + resultReads += 1; + if (resultReads === 2) revisedAt = clock; + return stubJson({ runId: runAt(clock), status: "complete", outcome: { kind: "eligible" }, previewUrl: resultReads === 1 ? "artifact_a" : "artifact_b" }); + } + if (path.includes("/preview")) { + previewAt.push({ clock, run: new URL(path, "http://x").searchParams.get("run") }); + return previewAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "120" }) : new Response(ply); + } + return stubJson({ runId: runAt(clock), status: "complete" }); + }); + try { + await until(() => h.state.preview.phase === "ready"); + assert.equal(resultReads, 2); + assert.equal(previewAt.length, 2, "the failed revision was not retried during its backoff"); + assert.equal(previewAt[1].clock, revisedAt, "the new revision's preview was read as soon as its result was accepted"); + assert.ok(previewAt[1].clock < previewAt[0].clock + 120_000); + assert.equal(previewAt[1].run, change === "run" ? "run_b" : "run_s"); + } finally { h.follower.stop(); await h.follower.done; } +}); From bfbfb64687523c91c3fb760c7c7f97179f13ccd8 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:08:58 -0500 Subject: [PATCH 08/20] docs: include viewer checks in contributor guidance Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265379976 --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 784df22b6..acaa368ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,7 +40,7 @@ The iOS UI tests skip the every-state accessibility audit on pull requests, beca The iOS check runs as three jobs, each under its own 90-minute cap: `screenStates` runs `ScreenStatesUITests`, the every-state audit included under `full-ui`; `photoProcessing` runs an unsigned Release build and `PhotoProcessingUITests` with its own audits; and `journey` runs the package tests, the Debug build and every other UI test class. `ios/Tools/ui-test-groups.py` holds the groups' test selectors; `python3 ios/Tools/ui-test-groups.py check` lists the UI tests from source and fails unless the groups run each exactly once. Run it after adding or renaming a UI test class. Each job keeps its result bundle as the artifact `ui-test-results-`, with `SNAPSHOT.txt` saying whether the copy is complete; a job the cap cancelled can leave it incomplete. -`make check` runs the server, web and iOS suites, then `make scoring`, `make measure-lab`, `make evals`, `make recon` and `make meter-closeup` for each of those folders the branch has. They need uv, Node 24 with pnpm, and Xcode 26 or newer; each directory's README has details. No check is required by branch rules yet. Don't call one required until the rules require its status. +`make check` runs the server, web and iOS suites, then `make scoring`, `make measure-lab`, `make evals`, `make recon`, `make meter-closeup` and `make integration-viewer` for each of those suites the branch has. They need uv, Node 24 with pnpm, and Xcode 26 or newer. The viewer's browser regression runs when Chrome or Chromium is available and skips otherwise; each directory's README has details. The viewer has no GitHub CI job yet. No check is required by branch rules yet. Don't call one required until the rules require its status. Keep workflows that run pull-request code away from production credentials and destructive external systems. A green CI run is evidence for the checks it ran, not proof that a capture works on a real house. From 3ac6e4993021196117ac251516c317457e27393a Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:17:52 -0500 Subject: [PATCH 09/20] fix(viewer): separate server deadlines from polling cadence Result reads honor their endpoint deadline. Preview reads honor the later result and artifact deadlines because the relay rereads the result. Stable opaque artifact IDs preserve delays across result refreshes and renewed signed queries. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265427358 Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265427372 --- tools/integration-viewer/public/live.js | 49 +++++++-------- tools/integration-viewer/relay.js | 5 ++ tools/integration-viewer/test/live.test.js | 66 ++++++++++++++++++++- tools/integration-viewer/test/relay.test.js | 18 ++++++ 4 files changed, 113 insertions(+), 25 deletions(-) diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index 6cf2b6e85..82d88715c 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -63,14 +63,13 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat async function run() { let failures = 0; let statusAt = -Infinity; - let triesKey = null; - let readyKey = null; - let resultTries = 0; - let resultAt = -Infinity; + const resultPoll = { key: null, readyKey: null, tries: 0, at: -Infinity }; + // Server deadlines are independent of polling cadence and survive result refreshes. + // Result reads obey the later of cadence and result delay. Preview reads obey both + // endpoint delays because the relay reads /result before fetching the artifact. let resultNotBefore = -Infinity; + const previewNotBefore = new Map(); let previewKey = null; - let previewRetryKey = null; - let previewRetryAt = -Infinity; // Reads start without waiting and repeat while pages come back full. They return what happened // before the viewer connected, which the reducer marks as backlog so it does not animate. let catchUp = true; @@ -116,34 +115,36 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const state = getState(); if (state.session !== session) return; const key = resultKey(state); - if (key !== triesKey) { - triesKey = key; - resultTries = 0; - resultNotBefore = -Infinity; // backpressure applied to the previous key's reads + if (key !== resultPoll.key) { + resultPoll.key = key; + resultPoll.tries = 0; } - const pending = resultExpected(state) && readyKey !== key; - const interval = resultTries < RESULT_QUICK_TRIES ? RESULT_RETRY_MS : RESULT_SLOW_MS; - if (pending && now() - resultAt >= interval && now() >= resultNotBefore) { - resultTries += 1; - resultAt = now(); + const pending = resultExpected(state) && resultPoll.readyKey !== key; + const interval = resultPoll.tries < RESULT_QUICK_TRIES ? RESULT_RETRY_MS : RESULT_SLOW_MS; + if (pending && now() >= Math.max(resultPoll.at + interval, resultNotBefore)) { + resultPoll.tries += 1; + resultPoll.at = now(); send({ type: "result-loading" }); try { const body = await get(`/captures/${captureId}/result`); send({ type: "result", body }); // Only a response the reducer accepted settles this key; a discarded one is read again. - if (getState().result.body === body && getState().result.phase === "ready") readyKey = key; + if (getState().result.body === body && getState().result.phase === "ready") resultPoll.readyKey = key; } catch (error) { if (signal.aborted) return; if (error instanceof HttpError && error.retryAfterS) resultNotBefore = now() + Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS); send({ type: "result-error", error: error instanceof HttpError ? `${error.code} (${error.status})` : "network error" }); } } - waitS = resultExpected(getState()) && readyKey !== key && resultTries < RESULT_QUICK_TRIES ? WAIT_EXPECTING_S : WAIT_S; + waitS = resultExpected(getState()) && resultPoll.readyKey !== key && resultPoll.tries < RESULT_QUICK_TRIES ? WAIT_EXPECTING_S : WAIT_S; const result = getState().result; const runId = result.body?.runId ?? null; const revisionKey = `${runId}|${result.revision}`; - if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && (previewRetryKey !== revisionKey || now() >= previewRetryAt)) { + const artifactKey = JSON.stringify([runId, result.body?.previewArtifactId ?? result.body?.previewUrl]); + for (const [artifact, deadline] of previewNotBefore) if (deadline <= now()) previewNotBefore.delete(artifact); + const previewDue = Math.max(resultNotBefore, previewNotBefore.get(artifactKey) ?? -Infinity); + if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && now() >= previewDue) { previewKey = revisionKey; send({ type: "preview-loading", runId }); try { @@ -154,11 +155,10 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat if (signal.aborted) return; if (error instanceof HttpError && (error.status === 409 || error.code === "preview_not_available")) { previewKey = revisionKey; // do not retry this artifact until a fresh result is accepted - readyKey = null; + resultPoll.readyKey = null; // This result is stale; do not carry over its slow polling schedule. - resultTries = 0; - resultAt = -Infinity; - resultNotBefore = -Infinity; + resultPoll.tries = 0; + resultPoll.at = -Infinity; waitS = 0; } else if (error instanceof PlyError) send({ type: "preview-error", runId, unsupported: true, error: error.message }); else { @@ -166,8 +166,9 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat send({ type: "preview-error", runId, error: error instanceof HttpError ? error.message : "network error" }); previewKey = null; const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; - previewRetryKey = revisionKey; // another revision may reference a different storage object - previewRetryAt = now() + Math.max(PREVIEW_RETRY_MS, asked); + // The relay identifies failures of its prerequisite result read separately. + if (error instanceof HttpError && error.code === "result_unavailable") resultNotBefore = Math.max(resultNotBefore, now() + asked); + previewNotBefore.set(artifactKey, now() + Math.max(PREVIEW_RETRY_MS, asked)); } } } diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index 23f3e7650..9d619bcd7 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -7,6 +7,7 @@ // supplies a URL. Signed storage URLs in the result stay inside the relay: the page gets a // placeholder, and the relay fetches the preview itself after checking the URL's host. +import { createHash } from "node:crypto"; import { readFile } from "node:fs/promises"; import { createServer } from "node:http"; import { extname, join } from "node:path"; @@ -98,6 +99,10 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { let body = reply.json; if (leaf === "result" && body && typeof body === "object") { body = { ...body }; + // A renewed signed query is not a new file. The digest distinguishes files without + // exposing their URL; query-only changes conservatively keep the same retry delay. + body.previewArtifactId = typeof body.previewUrl === "string" && body.previewUrl !== "" + ? createHash("sha256").update(body.previewUrl.split(/[?#]/, 1)[0]).digest("hex") : null; // Only a real URL is replaced; an empty value still means "no preview" to the page. for (const field of ["previewUrl", "verdictUrl"]) if (typeof body[field] === "string" && body[field] !== "") body[field] = WITHHELD; } diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index e1609e632..c226e5fa0 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -340,7 +340,7 @@ for (const change of ["run", "revision"]) test(`preview backoff does not delay a if (path.endsWith("/result")) { resultReads += 1; if (resultReads === 2) revisedAt = clock; - return stubJson({ runId: runAt(clock), status: "complete", outcome: { kind: "eligible" }, previewUrl: resultReads === 1 ? "artifact_a" : "artifact_b" }); + return stubJson({ runId: runAt(clock), status: "complete", outcome: { kind: "eligible" }, previewUrl: "withheld-by-viewer-relay", previewArtifactId: resultReads === 1 ? "artifact_a" : "artifact_b" }); } if (path.includes("/preview")) { previewAt.push({ clock, run: new URL(path, "http://x").searchParams.get("run") }); @@ -357,3 +357,67 @@ for (const change of ["run", "revision"]) test(`preview backoff does not delay a assert.equal(previewAt[1].run, change === "run" ? "run_b" : "run_s"); } finally { h.follower.stop(); await h.follower.done; } }); + +for (const response of [ + { status: 409, code: "preview_run_changed" }, + { status: 404, code: "preview_not_available" }, +]) test(`result and preview delays survive refreshes and ${response.code}`, async () => { + const resultAt = []; + const previewAt = []; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) { + const after = Number(new URL(path, "http://x").searchParams.get("after")); + const seq = clock >= 10_000 ? 2 : clock >= 6_000 ? 1 : 0; + return stubJson({ status: "complete", next: seq, events: seq > after + ? [{ seq, type: "verdict_ready", data: { runId: "run_s", kind: "eligible" } }] : [] }); + } + if (path.endsWith("/result")) { + resultAt.push(clock); + if (resultAt.length === 3) return stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "40" }); + return stubResult(resultAt.length < 5 ? "withheld-by-viewer-relay" : ""); + } + if (path.includes("/preview")) { + previewAt.push(clock); + if (clock < 21_000) return stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": String((21_000 - clock) / 1000) }); + return stubJson({ errors: [{ code: response.code }] }, response.status); + } + return stubStatus(); + }); + try { + await until(() => resultAt.length >= 5 && h.state.preview.phase === "none"); + assert.equal(resultAt[1], 6_000, "a refreshed result is read while the preview is delayed"); + assert.equal(resultAt[2], 10_000); + assert.deepEqual({ + resultRetriedEarly: resultAt[3] < resultAt[2] + 40_000, + previewRetriedEarly: previewAt.slice(1).some((at) => at < 21_000), + }, { resultRetriedEarly: false, previewRetriedEarly: false }); + assert.equal(previewAt[1], resultAt[3], "preview waits for the later deadline because its relay rereads the result"); + assert.ok(resultAt[4] - resultAt[3] <= 4_000, "invalidation resets cadence without erasing server delays"); + } finally { h.follower.stop(); await h.follower.done; } +}); + +test("a delayed prerequisite result read also delays direct result refreshes", async () => { + const resultAt = []; + const previewAt = []; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) { + const after = new URL(path, "http://x").searchParams.get("after"); + return stubJson({ status: "complete", next: clock >= 6_000 ? 1 : 0, events: clock >= 6_000 && after === "0" + ? [{ seq: 1, type: "verdict_ready", data: { runId: "run_s", kind: "eligible" } }] : [] }); + } + if (path.endsWith("/result")) { + resultAt.push(clock); + return stubResult(resultAt.length === 1 ? "withheld" : ""); + } + if (path.includes("/preview")) { + previewAt.push(clock); + return stubJson({ errors: [{ code: "result_unavailable" }] }, 503, { "retry-after": "19" }); + } + return stubStatus(); + }); + try { + await until(() => resultAt.length === 2 && h.state.preview.phase === "none"); + assert.ok(resultAt[1] >= previewAt[0] + 19_000, `result reads at ${resultAt}`); + assert.equal(previewAt.length, 1); + } finally { h.follower.stop(); await h.follower.done; } +}); diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index 416e81a65..e6f2a8a01 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -105,6 +105,24 @@ describe("allowed reads", () => { const cloud = parsePly(preview.body.buffer.slice(preview.body.byteOffset, preview.body.byteOffset + preview.body.byteLength)); assert.ok(cloud.count > 1000); }); + + test("preview identity survives renewed credentials but changes for another file", async () => { + const read = async (previewUrl) => { + hostileBody = { runId: "run_s", previewUrl, verdictUrl: "https://storage.example/verdict?secret=verdict" }; + const reply = await raw("/relay/hostile/captures/cap_ARTIFACT/result"); + assert.equal(reply.status, 200); + assert.equal(reply.json.previewUrl, "withheld-by-viewer-relay"); + assert.equal(reply.json.verdictUrl, "withheld-by-viewer-relay"); + assert.match(reply.json.previewArtifactId, /^[a-f0-9]{64}$/); + assert.ok(!reply.body.toString().includes("secret=")); + return reply.json.previewArtifactId; + }; + const first = await read("https://storage.example/model-a.ply?secret=first"); + assert.equal(await read("https://storage.example/model-a.ply?secret=renewed"), first); + assert.notEqual(await read("https://storage.example/model-b.ply?secret=renewed"), first); + hostileBody = { runId: "run_s", previewUrl: "" }; + assert.equal((await raw("/relay/hostile/captures/cap_ARTIFACT/result")).json.previewArtifactId, null); + }); }); describe("refusals", () => { From 8bc0729a5bb4f7ef55f6caca1043b60277eac0b7 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:30:30 -0500 Subject: [PATCH 10/20] fix(viewer): suppress unchanged invalidated preview artifacts Keep refreshing the result at its bounded cadence until the invalidated artifact changes or disappears. A new run or artifact can load without repeated requests for the rejected preview. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4265505512 --- tools/integration-viewer/public/live.js | 16 ++++++++--- tools/integration-viewer/test/live.test.js | 33 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index 82d88715c..bf356e243 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -43,6 +43,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const abort = new AbortController(); const { signal } = abort; const base = `/relay/${sourceKey}`; + const artifactKeyFor = (body) => JSON.stringify([body?.runId ?? null, body?.previewArtifactId ?? body?.previewUrl]); const send = (action) => { if (!signal.aborted) dispatch({ ...action, session }); }; @@ -70,6 +71,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat let resultNotBefore = -Infinity; const previewNotBefore = new Map(); let previewKey = null; + let invalidatedArtifactKey = null; // Reads start without waiting and repeat while pages come back full. They return what happened // before the viewer connected, which the reducer marks as backlog so it does not animate. let catchUp = true; @@ -129,7 +131,13 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const body = await get(`/captures/${captureId}/result`); send({ type: "result", body }); // Only a response the reducer accepted settles this key; a discarded one is read again. - if (getState().result.body === body && getState().result.phase === "ready") resultPoll.readyKey = key; + if (getState().result.body === body && getState().result.phase === "ready") { + // Keep refreshing an invalidated artifact at the normal result cadence. + if (!hasPreview(body) || artifactKeyFor(body) !== invalidatedArtifactKey) { + invalidatedArtifactKey = null; + resultPoll.readyKey = key; + } + } } catch (error) { if (signal.aborted) return; if (error instanceof HttpError && error.retryAfterS) resultNotBefore = now() + Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS); @@ -141,10 +149,10 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const result = getState().result; const runId = result.body?.runId ?? null; const revisionKey = `${runId}|${result.revision}`; - const artifactKey = JSON.stringify([runId, result.body?.previewArtifactId ?? result.body?.previewUrl]); + const artifactKey = artifactKeyFor(result.body); for (const [artifact, deadline] of previewNotBefore) if (deadline <= now()) previewNotBefore.delete(artifact); const previewDue = Math.max(resultNotBefore, previewNotBefore.get(artifactKey) ?? -Infinity); - if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && now() >= previewDue) { + if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && artifactKey !== invalidatedArtifactKey && now() >= previewDue) { previewKey = revisionKey; send({ type: "preview-loading", runId }); try { @@ -154,7 +162,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat } catch (error) { if (signal.aborted) return; if (error instanceof HttpError && (error.status === 409 || error.code === "preview_not_available")) { - previewKey = revisionKey; // do not retry this artifact until a fresh result is accepted + invalidatedArtifactKey = artifactKey; // a new read revision does not mean the artifact changed resultPoll.readyKey = null; // This result is stale; do not carry over its slow polling schedule. resultPoll.tries = 0; diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index c226e5fa0..fd4e0c3d3 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -421,3 +421,36 @@ test("a delayed prerequisite result read also delays direct result refreshes", a assert.equal(previewAt.length, 1); } finally { h.follower.stop(); await h.follower.done; } }); + +for (const response of [ + { status: 409, code: "preview_run_changed" }, + { status: 404, code: "preview_not_available" }, +]) for (const change of ["artifact", "run"]) test(`unchanged ${response.code} stays suppressed until the ${change} changes`, async () => { + const resultAt = []; + const previewAt = []; + const runAt = (clock) => change === "run" && clock >= 60_000 ? "run_b" : "run_s"; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) return stubEvents(); + if (path.endsWith("/result")) { + resultAt.push(clock); + return stubJson({ runId: runAt(clock), status: "complete", outcome: { kind: "eligible" }, previewUrl: "withheld-by-viewer-relay", + previewArtifactId: change === "artifact" && clock >= 60_000 ? "artifact_b" : "artifact_a" }); + } + if (path.includes("/preview")) { + previewAt.push(clock); + return clock < 60_000 ? stubJson({ errors: [{ code: response.code }] }, response.status) : new Response(ply); + } + return stubJson({ runId: runAt(clock), status: "complete" }); + }); + try { + await until(() => h.state.preview.phase === "ready"); + assert.equal(previewAt.length, 2, "an identical refreshed result cannot retry the invalidated artifact"); + const beforeChange = resultAt.filter((at) => at < 60_000); + assert.ok(beforeChange.length >= 8, "results keep refreshing while the preview is suppressed"); + assert.ok(beforeChange.length <= 10, `result refreshes remain bounded: ${beforeChange}`); + assert.ok(beforeChange.slice(2).every((at, i) => at - beforeChange[i + 1] >= 4_000)); + assert.ok(previewAt[1] >= 60_000, "a changed identity allows the preview to load"); + assert.equal(h.state.preview.runId, change === "run" ? "run_b" : "run_s"); + } finally { h.follower.stop(); await h.follower.done; } +}); From 2bbfb25eefe25c6750136ce1445c91536aaa9560 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:41:57 -0500 Subject: [PATCH 11/20] docs: correct reconstruction attribution and battery cable routing Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4266841041 Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4266841052 --- README.md | 2 +- docs/00-overview.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6642ee21f..1e412e34a 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ The battery measures 31 × 22 × 39.5 in, a bit bigger than a dishwasher. It sta | Cable route | Can't cross a door, a garage or a gap in the wall | Demo choice | | Driveway | At least 5 ft away | Placeholder, no public value | | Pool | At least 10 ft away | Placeholder, no public value | -| Existing battery, when the scene marks one or its configured clearance exceeds the AC clearance | At least 3 ft away with shipped rules; the cable also detours around it | Base's help page; IRC 2021 R328, cited in `server/rules.yaml` | +| Existing battery, when the scene marks one or its configured clearance exceeds the AC clearance | At least 3 ft away with shipped rules; the cable detours over it | Base's help page; IRC 2021 R328, cited in `server/rules.yaml` | Each check comes back PASS, FAIL or UNSURE, along with what the server measured, how far off that measurement could be and a reason in plain English. NEC is the National Electrical Code and IRC is the International Residential Code, the two building codes behind several of these rules. The first 13 checks run at each spot. `battery_clearance` also runs when the scene marks an existing battery or the configured battery clearance exceeds the AC clearance. The shipped public rules give both a 3 ft limit, so a scene without a battery omits that extra check. The numbers themselves live in `server/rules.yaml`, each one next to its source, and [docs/04](docs/04-prior-art-and-codes.md) has the full citations. diff --git a/docs/00-overview.md b/docs/00-overview.md index add12d5e2..9e25a84ee 100644 --- a/docs/00-overview.md +++ b/docs/00-overview.md @@ -50,7 +50,7 @@ These results come from public datasets with laser-scanned or surveyed ground tr - **Tracking.** A current iPhone stayed inside the server's error allowance. On the MARViN dataset, an iPhone 14 Pro Max had p90 position error of 8.6, 13.4 and 18.5 in after 10, 20 and 30 ft, against an allowance of 19.2, 38.4 and 57.6 in. That phone has LiDAR, and MARViN's reference may be scaled to its tracking. A 2018 iPhone 6s on ADVIO ran two to three times over, partly from noise in that dataset's reference. - **Learned depth.** A model's own scale is 4 to 12% off. Rescaled with the phone's poses, walls come within about 5 in at p90 at a realistic 2% pose error, and within 2.8 in with exact poses. Edges stay at 8 in or worse, and clearances are measured from edges. MapAnything's scale shifted with input resolution. -- **Reconstruction worker.** On the ETH3D building, wall pair-error p90 is 1.8 in with a laser scan standing in for LiDAR, and 1.5 in from photos only. Both fit stretches of the same wall, with different lengths. The photos-only change includes a different fitted stretch; see `recon/HANDOFF.md`. +- **Reconstruction worker.** On the ETH3D building, wall pair-error p90 is 1.8 in with a laser scan standing in for LiDAR, and 1.5 in from photos only. Both fit stretches of the same wall, with different lengths. The photos-only drop from 2.8 to 1.5 in combines the scoring fix with a different fitted stretch; see `recon/HANDOFF.md`. - **Coverage.** The app's first coverage map claimed 1.1 ft of a 19.3 ft wall that no photo saw. Requiring several view angles removed the false claim but also discarded 6 to 8.5 ft of good wall. A depth test on LiDAR phones removed it cleanly. Checked against depth, the reconstruction worker claims at most about 0.46 ft of unseen wall. - **Meter reading.** Apple's on-device text recognition read the full meter number on 71 of 73 real photos. It picked the right line on only 21 of 75, because nameplates carry several numbers. A barcode held the number on 20 of the 24 photos that had one. A list of three candidates held it on about 4 in 5 held-out photos (27 of 34). So the app offers three candidates to tap, barcode match first, and asks for a retake when the photo is blurry or the number too small. - **Panel labels.** Not tested. A search found 7 usable open photos of US panels, and the test needs 40. From 73ad98138c48bf8b174c8a11f3d24cfa43ca32dc Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:41:57 -0500 Subject: [PATCH 12/20] fix(viewer): preserve storage error responses when cancel rejects Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4266841078 --- tools/integration-viewer/relay.js | 2 +- tools/integration-viewer/test/relay.test.js | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index 9d619bcd7..408603dcc 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -137,7 +137,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { } if (!reply.ok) { const retryAfter = reply.headers.get("retry-after"); - await reply.body?.cancel(); + await reply.body?.cancel().catch(() => {}); return refuse(res, 502, "preview_fetch_failed", `storage answered ${reply.status}`, retryAfter ? { "retry-after": retryAfter } : {}); } const declared = Number(reply.headers.get("content-length") ?? "0"); diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index e6f2a8a01..5601cce3e 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -255,6 +255,24 @@ describe("refusals", () => { } }); + test("an errored storage body cannot replace the preview 502 or Retry-After", async (t) => { + const storageUrl = `http://127.0.0.1:${hostile.address().port}/v1/errored.ply`; + hostileBody = { runId: "r", previewUrl: storageUrl }; + const fetchImpl = globalThis.fetch; + let storageReads = 0; + t.mock.method(globalThis, "fetch", async (url, init) => { + if (String(url) !== storageUrl) return fetchImpl(url, init); + storageReads += 1; + const body = new ReadableStream({ start(controller) { controller.error(new Error("storage stream failed")); } }); + return new Response(body, { status: 503, headers: { "retry-after": "19" } }); + }); + const reply = await raw("/relay/hostile/captures/cap_HOSTILE_1/preview?run=r"); + assert.equal(storageReads, 1); + assert.equal(reply.status, 502); + assert.equal(reply.json.errors[0].code, "preview_fetch_failed"); + assert.equal(reply.headers["retry-after"], "19"); + }); + test("an oversized API reply is refused, not buffered", async () => { hostileHandler = (req, res) => { res.writeHead(200, { "content-type": "application/json" }); From 5ebf51f2361ef8b99b0ce2595455ec7c8a93a54b Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:04:55 -0500 Subject: [PATCH 13/20] fix(viewer): bind preview retries to the requested artifact Reject a changed same-run artifact before storage is fetched, so its Retry-After cannot be charged to an older file. The loopback regression switches A to B and requires the full 120-second delay before B is retried. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4272480607 --- tools/integration-viewer/README.md | 3 +- tools/integration-viewer/public/live.js | 6 ++- tools/integration-viewer/relay.js | 17 +++++-- tools/integration-viewer/test/live.test.js | 54 +++++++++++++++++++++ tools/integration-viewer/test/relay.test.js | 33 +++++++++++++ 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/tools/integration-viewer/README.md b/tools/integration-viewer/README.md index 147bf3ee7..e833493a6 100644 --- a/tools/integration-viewer/README.md +++ b/tools/integration-viewer/README.md @@ -60,9 +60,10 @@ GET /relay//captures/ GET /relay//captures//events?after=&wait=<0-25> GET /relay//captures//result GET /relay//captures//preview?run= +GET /relay//captures//preview?run=&artifact= ``` -Sources are fixed at startup. The relay refuses other methods and paths, and any other query. It also refuses encoded or dot segments and any Host header that is not loopback, which stops DNS rebinding. Signed URLs in the result are replaced before the page sees them. For `/preview`, the relay reads the result itself and fetches the signed URL only if it points at the source's own host or at `storage.googleapis.com`. It answers 409 if the latest result belongs to a different run than the page asked about. It follows no redirects, reads at most 64 MiB of preview and 8 MiB of JSON, and passes the API's `Retry-After` through. The log records each route and status, never a URL or query. +Sources are fixed at startup. The relay refuses other methods and paths, and any other query. It also refuses encoded or dot segments and any Host header that is not loopback, which stops DNS rebinding. Signed URLs in the result are replaced before the page sees them. For `/preview`, the relay reads the result itself and fetches the signed URL only if it points at the source's own host or at `storage.googleapis.com`. It answers 409 if the latest result belongs to a different run than the page asked about. The page also sends the result's opaque `previewArtifactId` as `artifact`; a different file in the same run returns 409 before storage is fetched. This keeps a storage `Retry-After` attached to the file it came from. Run-only requests remain supported. It follows no redirects, reads at most 64 MiB of preview and 8 MiB of JSON, and passes the API's `Retry-After` through. The log records each route and status, never a URL or query. ## Files diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index bf356e243..ff9dea537 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -156,8 +156,10 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat previewKey = revisionKey; send({ type: "preview-loading", runId }); try { - // The relay refuses with 409 if the latest result is no longer this run's. - const cloud = parsePly(await get(`/captures/${captureId}/preview?run=${encodeURIComponent(runId)}`, "bytes")); + // Bind the fetch to the file shown, so any Retry-After belongs to this artifact. + const artifact = result.body.previewArtifactId; + const artifactQuery = artifact == null ? "" : `&artifact=${encodeURIComponent(artifact)}`; + const cloud = parsePly(await get(`/captures/${captureId}/preview?run=${encodeURIComponent(runId)}${artifactQuery}`, "bytes")); send({ type: "preview", runId, cloud }); } catch (error) { if (signal.aborted) return; diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index 408603dcc..cdf5a555c 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -20,6 +20,7 @@ const PREVIEW_MAX_BYTES = 64 * 1024 * 1024; const JSON_MAX_BYTES = 8 * 1024 * 1024; const RUN_ID = /^[A-Za-z0-9_-]{1,80}$/; const WITHHELD = "withheld-by-viewer-relay"; +const previewArtifactId = (url) => createHash("sha256").update(url.split(/[?#]/, 1)[0]).digest("hex"); /** * @param {object} options @@ -83,7 +84,12 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { timeoutMs = Number(wait) * 1000 + 15_000; } else if (leaf === "preview") { const run = params.get("run"); - if (run == null || !RUN_ID.test(run) || [...params.keys()].length !== 1) return refuse(res, 400, "query_invalid", "preview takes run="); + const artifact = params.get("artifact"); + if (run == null || !RUN_ID.test(run) || params.getAll("run").length !== 1 || + (artifact != null && (!/^[a-f0-9]{64}$/.test(artifact) || params.getAll("artifact").length !== 1)) || + [...params.keys()].some((key) => key !== "run" && key !== "artifact")) { + return refuse(res, 400, "query_invalid", "preview takes run= and optional artifact="); + } } else if (query !== "") { return refuse(res, 400, "query_invalid", "this route takes no query"); } @@ -92,7 +98,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { res.on("close", () => abort.abort()); const signal = AbortSignal.any([abort.signal, AbortSignal.timeout(timeoutMs)]); - if (leaf === "preview") return relayPreview(res, upstream, upstreamPath, params.get("run"), signal); + if (leaf === "preview") return relayPreview(res, upstream, upstreamPath, params.get("run"), params.get("artifact"), signal); const reply = await upstreamGet(upstream.base + upstreamPath, signal); if (reply.failure) return refuse(res, reply.failure.status, reply.failure.code, reply.failure.message); @@ -102,7 +108,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { // A renewed signed query is not a new file. The digest distinguishes files without // exposing their URL; query-only changes conservatively keep the same retry delay. body.previewArtifactId = typeof body.previewUrl === "string" && body.previewUrl !== "" - ? createHash("sha256").update(body.previewUrl.split(/[?#]/, 1)[0]).digest("hex") : null; + ? previewArtifactId(body.previewUrl) : null; // Only a real URL is replaced; an empty value still means "no preview" to the page. for (const field of ["previewUrl", "verdictUrl"]) if (typeof body[field] === "string" && body[field] !== "") body[field] = WITHHELD; } @@ -113,13 +119,16 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { return `relay ${key} ${leaf ?? (captureId ? "status" : "healthz")}`; } - async function relayPreview(res, upstream, resultPath, runId, signal) { + async function relayPreview(res, upstream, resultPath, runId, artifactId, signal) { const result = await upstreamGet(`${upstream.base}${resultPath}/result`, signal); if (result.failure) return refuse(res, result.failure.status, result.failure.code, result.failure.message); if (result.status !== 200) return refuse(res, result.status, "result_unavailable", "the result could not be read", result.retryAfter ? { "retry-after": result.retryAfter } : {}); if (result.json?.runId !== runId) return refuse(res, 409, "preview_run_changed", "the latest result belongs to another run"); const signed = result.json?.previewUrl; if (typeof signed !== "string" || signed === "") return refuse(res, 404, "preview_not_available", "the result has no preview"); + // A same-run result may move to another file after the page read it. Refuse before + // fetching so a storage Retry-After cannot be attributed to the page's older file. + if (artifactId != null && artifactId !== previewArtifactId(signed)) return refuse(res, 409, "preview_artifact_changed", "the latest result references another preview artifact"); let target; try { target = new URL(signed, new URL(upstream.base).origin); diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index fd4e0c3d3..81ca67451 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -2,6 +2,7 @@ // sped up. This is the same code path a page uses, minus the DOM. import assert from "node:assert/strict"; +import { createServer } from "node:http"; import { dirname, join } from "node:path"; import { after, before, test } from "node:test"; import { fileURLToPath } from "node:url"; @@ -454,3 +455,56 @@ for (const response of [ assert.equal(h.state.preview.runId, change === "run" ? "run_b" : "run_s"); } finally { h.follower.stop(); await h.follower.done; } }); + +test("a same-run file switch cannot misattribute the next file's preview delay", async () => { + let clock = 0; + let resultReads = 0; + const storageReads = []; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const upstream = createServer((req, res) => { + res.setHeader("content-type", "application/json"); + if (req.url.endsWith("/result")) { + resultReads += 1; + // The page receives A, but the relay's prerequisite result read already names B. + return res.end(JSON.stringify({ runId: "run_s", status: "complete", outcome: { kind: "eligible" }, previewUrl: resultReads === 1 ? "/file-a.ply" : "/file-b.ply" })); + } + if (req.url.endsWith(".ply")) { + storageReads.push({ path: req.url, at: clock }); + if (storageReads.length === 1) { + res.writeHead(503, { "retry-after": "120" }); + return res.end("busy"); + } + return res.end(ply); + } + if (req.url.includes("/events")) { + const after = new URL(req.url, "http://x").searchParams.get("after"); + const changed = clock >= 6_000; + return res.end(JSON.stringify({ status: "complete", next: changed ? 1 : 0, events: changed && after === "0" + ? [{ seq: 1, type: "verdict_ready", data: { runId: "run_s", kind: "eligible" } }] : [] })); + } + res.end(JSON.stringify({ runId: "run_s", status: "complete" })); + }); + await new Promise((resolve) => upstream.listen(0, "127.0.0.1", resolve)); + const relay = createViewerServer({ upstreams: { s: { base: `http://127.0.0.1:${upstream.address().port}/v1`, label: "Two files", kind: "synthetic" } }, + publicDir: join(dirname(fileURLToPath(import.meta.url)), "..", "public") }); + let follower; + try { + const relayOrigin = `http://127.0.0.1:${await relay.listen(0)}`; + let state = reduce(initialState(), { type: "select", mode: "live", source: { key: "s", kind: "synthetic" }, captureId: "cap_TWO_FILES" }); + follower = followCapture({ sourceKey: "s", captureId: "cap_TWO_FILES", session: state.session, + dispatch: (a) => { state = reduce(state, a); }, getState: () => state, + fetchImpl: (path, init) => fetch(relayOrigin + path, init), now: () => clock, + sleep: async (ms) => { clock += ms; }, online: () => true }); + await until(() => state.preview.phase === "ready"); + assert.equal(storageReads.length, 2); + assert.ok(storageReads.every((read) => read.path === "/file-b.ply")); + assert.ok(storageReads[1].at - storageReads[0].at >= 120_000, `B was fetched at ${storageReads.map((read) => read.at)}`); + assert.equal(state.result.body.runId, "run_s"); + } finally { + follower?.stop(); + await follower?.done; + await relay.close(); + upstream.closeAllConnections(); + await new Promise((resolve) => upstream.close(resolve)); + } +}); diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index 5601cce3e..59bfc2fda 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -203,6 +203,39 @@ describe("refusals", () => { assert.equal((await raw("/relay/hostile/captures/cap_HOSTILE_1/preview")).json.errors[0].code, "query_invalid"); }); + test("a same-run preview file change is refused before storage is fetched", async () => { + let storageReads = 0; + let previewUrl = "/v1/file-a.ply"; + hostileHandler = (req, res) => { + if (req.url.endsWith("/result")) { + res.writeHead(200, { "content-type": "application/json" }); + return res.end(JSON.stringify({ runId: "r", previewUrl })); + } + storageReads += 1; + res.writeHead(503, { "retry-after": "120" }); + res.end("busy"); + }; + try { + const path = "/relay/hostile/captures/cap_HOSTILE_1"; + const first = await raw(`${path}/result`); + previewUrl = "/v1/file-b.ply"; + const stale = await raw(`${path}/preview?run=r&artifact=${first.json.previewArtifactId}`); + assert.equal(stale.status, 409); + assert.equal(stale.json.errors[0].code, "preview_artifact_changed"); + assert.equal(storageReads, 0); + assert.equal(stale.headers["retry-after"], undefined); + const current = await raw(`${path}/result`); + const failed = await raw(`${path}/preview?run=r&artifact=${current.json.previewArtifactId}`); + assert.equal(failed.status, 502); + assert.equal(failed.headers["retry-after"], "120"); + assert.equal(storageReads, 1); + for (const query of ["run=r&artifact=not-a-digest", `run=r&artifact=${current.json.previewArtifactId}&artifact=${current.json.previewArtifactId}`, "run=r&run=r"]) { + assert.equal((await raw(`${path}/preview?${query}`)).status, 400); + } + assert.equal(storageReads, 1); + } finally { hostileHandler = null; } + }); + test("a preview URL that redirects is not followed", async () => { hostileHandler = (req, res) => { if (req.url.endsWith("/result")) { From 2e8cddd59a7da5cb534ffd50fb96e9a8f0ff731b Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:38:19 -0500 Subject: [PATCH 14/20] fix(viewer): retain resource query selectors in preview identity Exclude only known credential query fields, retain resource selectors and unknown parameters, and ignore fragments. Query versions now have separate artifact delays while renewed signing credentials keep the same identity. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4272874967 --- tools/integration-viewer/relay.js | 19 ++++++++++++++++--- tools/integration-viewer/test/relay.test.js | 10 ++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index cdf5a555c..25db43274 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -20,7 +20,21 @@ const PREVIEW_MAX_BYTES = 64 * 1024 * 1024; const JSON_MAX_BYTES = 8 * 1024 * 1024; const RUN_ID = /^[A-Za-z0-9_-]{1,80}$/; const WITHHELD = "withheld-by-viewer-relay"; -const previewArtifactId = (url) => createHash("sha256").update(url.split(/[?#]/, 1)[0]).digest("hex"); +const PREVIEW_CREDENTIAL_PARAM = /^(?:signature|sig|secret|token|access_token|expires|expiry|expiration|googleaccessid|x-(?:goog|amz)-(?:algorithm|credential|date|expires|signedheaders|signature|security-token))$/i; + +function previewArtifactId(url) { + const withoutFragment = url.split("#", 1)[0]; + const queryAt = withoutFragment.indexOf("?"); + const path = queryAt < 0 ? withoutFragment : withoutFragment.slice(0, queryAt); + const query = queryAt < 0 ? "" : withoutFragment.slice(queryAt + 1); + const params = new URLSearchParams(query); + // Versions such as generation/versionId select the file. Unknown parameters stay too; + // only known signing fields are removed so renewed credentials keep the retry deadline. + for (const name of [...params.keys()]) if (PREVIEW_CREDENTIAL_PARAM.test(name)) params.delete(name); + params.sort(); + const resource = params.size === 0 ? path : `${path}?${params}`; + return createHash("sha256").update(resource).digest("hex"); +} /** * @param {object} options @@ -105,8 +119,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { let body = reply.json; if (leaf === "result" && body && typeof body === "object") { body = { ...body }; - // A renewed signed query is not a new file. The digest distinguishes files without - // exposing their URL; query-only changes conservatively keep the same retry delay. + // The digest distinguishes resource versions without exposing the signed URL. body.previewArtifactId = typeof body.previewUrl === "string" && body.previewUrl !== "" ? previewArtifactId(body.previewUrl) : null; // Only a real URL is replaced; an empty value still means "no preview" to the page. diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index 59bfc2fda..380bca6a9 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -120,6 +120,16 @@ describe("allowed reads", () => { const first = await read("https://storage.example/model-a.ply?secret=first"); assert.equal(await read("https://storage.example/model-a.ply?secret=renewed"), first); assert.notEqual(await read("https://storage.example/model-b.ply?secret=renewed"), first); + const credentialNames = ["signature", "sig", "token", "access_token", "expires", "expiry", "expiration", "GoogleAccessId", + "X-Goog-Algorithm", "X-Goog-Credential", "X-Goog-Date", "X-Goog-Expires", "X-Goog-SignedHeaders", "X-Goog-Signature", "X-Goog-Security-Token", + "X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Date", "X-Amz-Expires", "X-Amz-SignedHeaders", "X-Amz-Signature", "X-Amz-Security-Token"]; + const signed = (value) => new URLSearchParams(credentialNames.map((name) => [name, value])).toString(); + const version1 = await read(`https://storage.example/model-a.ply?generation=1&${signed("first")}`); + assert.equal(await read(`https://storage.example/model-a.ply?${signed("renewed")}&generation=1#new-fragment`), version1); + assert.notEqual(await read(`https://storage.example/model-a.ply?generation=2&${signed("renewed")}`), version1); + assert.notEqual(await read("https://storage.example/model-a.ply?generation=1&versionId=2"), version1); + assert.notEqual(await read("https://storage.example/model-a.ply?generation=1&custom=2"), version1, "unknown parameters remain part of resource identity"); + assert.notEqual(await read("https://storage.example/model-a.ply?custom=one?two"), await read("https://storage.example/model-a.ply?custom=one")); hostileBody = { runId: "run_s", previewUrl: "" }; assert.equal((await raw("/relay/hostile/captures/cap_ARTIFACT/result")).json.previewArtifactId, null); }); From 09581face1ce2ce21521d4c11df21d7d6f286518 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:38:19 -0500 Subject: [PATCH 15/20] fix(viewer): settle forced refreshes answered by retired runs Reject the retired status fields but clear the completed refresh request so the follower returns to its normal status polling interval. Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4272874979 --- tools/integration-viewer/public/model.js | 5 +++- tools/integration-viewer/test/live.test.js | 31 +++++++++++++++++++++ tools/integration-viewer/test/model.test.js | 4 +++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/tools/integration-viewer/public/model.js b/tools/integration-viewer/public/model.js index d8a8da354..d26c196d5 100644 --- a/tools/integration-viewer/public/model.js +++ b/tools/integration-viewer/public/model.js @@ -157,7 +157,10 @@ function select(state, action) { function applyStatus(state, body) { if (!isObject(body) || (body.captureId && body.captureId !== state.captureId)) return state; - if (typeof body.runId === "string" && state.retiredRuns.has(body.runId)) return state; + if (typeof body.runId === "string" && state.retiredRuns.has(body.runId)) { + // The requested refresh completed, even though this replica still names an old run. + return state.statusRefreshWanted ? { ...state, statusRefreshWanted: false } : state; + } const next = { ...state, status: typeof body.status === "string" ? body.status : state.status, diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index 81ca67451..f51ad17e9 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -508,3 +508,34 @@ test("a same-run file switch cannot misattribute the next file's preview delay", await new Promise((resolve) => upstream.close(resolve)); } }); + +test("a forced refresh answered by a retired-run replica returns to normal polling", async () => { + const statusAt = []; + let clock = 0; + let state = reduce(initialState(), { type: "select", mode: "live", source: { key: "s", kind: "synthetic" }, captureId: "cap_STALE_REPLICA" }); + state = reduce(state, { type: "status", session: state.session, body: { runId: "run_retired", status: "processing" } }); + state = reduce(state, { type: "status", session: state.session, body: { runId: "run_s", status: "processing" } }); + const fetchImpl = (path) => { + if (path.endsWith("/healthz")) return stubJson({ status: "ok" }); + if (path.includes("/events")) { + const after = Number(new URL(path, "http://x").searchParams.get("after")); + return stubJson({ status: "processing", next: 1, events: after === 0 + ? [{ seq: 1, type: "stage", data: { runId: "run_unknown", stage: "reconstruct", status: "running" } }] : [] }); + } + if (path.endsWith("/result")) return stubResult(); + statusAt.push(clock); + return stubJson({ runId: "run_retired", status: "failed", filesRegistered: 999 }); + }; + const follower = followCapture({ + sourceKey: "s", captureId: "cap_STALE_REPLICA", session: state.session, + dispatch: (a) => { state = reduce(state, a); }, getState: () => state, fetchImpl, now: () => clock, + sleep: (ms) => new Promise((resolve) => { clock += ms; setTimeout(resolve, 1); }), online: () => true, + }); + try { + await until(() => statusAt.length >= 5); + assert.equal(state.statusRefreshWanted, false); + assert.equal(state.currentRunId, "run_s"); + assert.equal(state.counts.registered, null); + assert.deepEqual(statusAt.slice(0, 5), [0, 0, 5_000, 10_000, 15_000]); + } finally { follower.stop(); await follower.done; } +}); diff --git a/tools/integration-viewer/test/model.test.js b/tools/integration-viewer/test/model.test.js index ffed17ea9..f604d7e1e 100644 --- a/tools/integration-viewer/test/model.test.js +++ b/tools/integration-viewer/test/model.test.js @@ -349,6 +349,10 @@ describe("run identity: the status names the run shown", () => { assert.equal(s.currentRunId, "run_b"); assert.equal(s.result.phase, "ready"); assert.equal(s, current, "no retired-run status fields are applied"); + const forced = run(current, events([runEvent(2, "run_unknown", "retake_request")], 2)); + assert.equal(forced.statusRefreshWanted, true); + const refreshed = run(forced, { type: "status", body: { runId: "run_a", status: "failed", filesRegistered: 99 } }); + assert.deepEqual(refreshed, { ...forced, statusRefreshWanted: false }, "the completed refresh clears only the request flag"); }); test("a result without a nonempty run identity is deferred", () => { From 6c8593ff45bf3930ebdcb3ba4cd9feb6f624db18 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 10:46:42 -0500 Subject: [PATCH 16/20] docs: explain the pinned XcodeGen prerequisite for root checks Review: https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4272994394 --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index acaa368ea..29d94a0ff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,7 +40,9 @@ The iOS UI tests skip the every-state accessibility audit on pull requests, beca The iOS check runs as three jobs, each under its own 90-minute cap: `screenStates` runs `ScreenStatesUITests`, the every-state audit included under `full-ui`; `photoProcessing` runs an unsigned Release build and `PhotoProcessingUITests` with its own audits; and `journey` runs the package tests, the Debug build and every other UI test class. `ios/Tools/ui-test-groups.py` holds the groups' test selectors; `python3 ios/Tools/ui-test-groups.py check` lists the UI tests from source and fails unless the groups run each exactly once. Run it after adding or renaming a UI test class. Each job keeps its result bundle as the artifact `ui-test-results-`, with `SNAPSHOT.txt` saying whether the copy is complete; a job the cap cancelled can leave it incomplete. -`make check` runs the server, web and iOS suites, then `make scoring`, `make measure-lab`, `make evals`, `make recon`, `make meter-closeup` and `make integration-viewer` for each of those suites the branch has. They need uv, Node 24 with pnpm, and Xcode 26 or newer. The viewer's browser regression runs when Chrome or Chromium is available and skips otherwise; each directory's README has details. The viewer has no GitHub CI job yet. No check is required by branch rules yet. Don't call one required until the rules require its status. +`make check` runs the server, web and iOS suites, then `make scoring`, `make measure-lab`, `make evals`, `make recon`, `make meter-closeup` and `make integration-viewer` for each of those suites the branch has. They need uv, Node 24 with pnpm, Xcode 26 or newer, and XcodeGen 2.46.0. Measure Lab regenerates its committed project before testing, so XcodeGen must match CI's pinned version to avoid project drift. The viewer's browser regression runs when Chrome or Chromium is available and skips otherwise; each directory's README has details. The viewer has no GitHub CI job yet. No check is required by branch rules yet. Don't call one required until the rules require its status. + +To install the pinned XcodeGen, download [XcodeGen 2.46.0's xcodegen.zip](https://github.com/yonaskolb/XcodeGen/releases/download/2.46.0/xcodegen.zip). Run `shasum -a 256 xcodegen.zip` and compare it with `XCODEGEN_SHA256` in [the Measure Lab workflow](.github/workflows/measure-lab.yml). Unzip it and add the extracted `xcodegen/bin` directory to your `PATH`. `xcodegen --version` must print `Version: 2.46.0`. To use the binary without changing `PATH`, run `make check XCODEGEN=/absolute/path/to/xcodegen/bin/xcodegen`. Keep workflows that run pull-request code away from production credentials and destructive external systems. A green CI run is evidence for the checks it ran, not proof that a capture works on a real house. From 33db360723cb99036273b3960f3a2f5914ff686a Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:43:44 -0500 Subject: [PATCH 17/20] fix(viewer): preserve deadlines and results on malformed replies Capture Retry-After before reading JSON and retain it on direct and preview prerequisite failures, including HTML and truncated bodies. Malformed result refreshes use the existing refresh-error presentation while retaining an accepted result. Vet follow-up: https://github.com/SamGu-NRX/BaseScanning/pull/238 --- tools/integration-viewer/public/model.js | 8 +++- tools/integration-viewer/relay.js | 16 ++++--- tools/integration-viewer/test/live.test.js | 49 +++++++++++++++++++++ tools/integration-viewer/test/model.test.js | 11 +++++ tools/integration-viewer/test/relay.test.js | 16 +++++++ 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/tools/integration-viewer/public/model.js b/tools/integration-viewer/public/model.js index d26c196d5..96106a743 100644 --- a/tools/integration-viewer/public/model.js +++ b/tools/integration-viewer/public/model.js @@ -122,7 +122,7 @@ export function reduce(state, action) { return applyResult(state, action.body); case "result-error": // A failed refresh keeps a result already shown; it is still what the server last said. - return { ...state, result: { ...state.result, phase: state.result.phase === "ready" ? "ready" : "error", error: action.error } }; + return resultError(state, action.error); case "preview-loading": // Re-reading the same run's model keeps the one on screen until the new one arrives. if (state.preview.phase === "ready" && state.preview.runId === action.runId) return state; @@ -333,8 +333,12 @@ function stage(state, data, base) { /** Statuses after which no outcome will come, so a result without one is final. */ const NO_OUTCOME_STATUSES = new Set(["failed", "expired"]); +function resultError(state, error) { + return { ...state, result: { ...state.result, phase: state.result.phase === "ready" ? "ready" : "error", error } }; +} + function applyResult(state, body) { - if (!isObject(body)) return { ...state, result: { ...state.result, phase: "error", body: null, error: "Result was not a JSON object" } }; + if (!isObject(body)) return resultError(state, "Result was not a JSON object"); // An outcome without run identity cannot be attributed to the capture's current run. if (typeof body.runId !== "string" || body.runId === "") return state; // A result for a run other than the one shown is not accepted: a retired run's is dropped, and diff --git a/tools/integration-viewer/relay.js b/tools/integration-viewer/relay.js index 25db43274..743264bb5 100644 --- a/tools/integration-viewer/relay.js +++ b/tools/integration-viewer/relay.js @@ -115,7 +115,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { if (leaf === "preview") return relayPreview(res, upstream, upstreamPath, params.get("run"), params.get("artifact"), signal); const reply = await upstreamGet(upstream.base + upstreamPath, signal); - if (reply.failure) return refuse(res, reply.failure.status, reply.failure.code, reply.failure.message); + if (reply.failure) return refuse(res, reply.failure.status, reply.failure.code, reply.failure.message, reply.retryAfter ? { "retry-after": reply.retryAfter } : {}); let body = reply.json; if (leaf === "result" && body && typeof body === "object") { body = { ...body }; @@ -134,7 +134,7 @@ export function createViewerServer({ upstreams, publicDir, log = () => {} }) { async function relayPreview(res, upstream, resultPath, runId, artifactId, signal) { const result = await upstreamGet(`${upstream.base}${resultPath}/result`, signal); - if (result.failure) return refuse(res, result.failure.status, result.failure.code, result.failure.message); + if (result.failure) return refuse(res, result.failure.status, "result_unavailable", result.failure.message, result.retryAfter ? { "retry-after": result.retryAfter } : {}); if (result.status !== 200) return refuse(res, result.status, "result_unavailable", "the result could not be read", result.retryAfter ? { "retry-after": result.retryAfter } : {}); if (result.json?.runId !== runId) return refuse(res, 409, "preview_run_changed", "the latest result belongs to another run"); const signed = result.json?.previewUrl; @@ -221,10 +221,12 @@ async function upstreamGet(url, signal) { const timedOut = error?.name === "TimeoutError"; return { failure: { status: timedOut ? 504 : 502, code: timedOut ? "upstream_timeout" : "upstream_unreachable", message: timedOut ? "the API did not answer in time" : "the API could not be reached" } }; } + // Backpressure belongs to the HTTP response, even when its body is unreadable. + const retryAfter = reply.headers.get("retry-after"); const declared = Number(reply.headers.get("content-length") ?? "0"); if (declared > JSON_MAX_BYTES) { await reply.body?.cancel().catch(() => {}); - return { failure: { status: 502, code: "upstream_too_large", message: "the API reply is larger than the viewer reads" } }; + return { retryAfter, failure: { status: 502, code: "upstream_too_large", message: "the API reply is larger than the viewer reads" } }; } let text = ""; try { @@ -234,21 +236,21 @@ async function upstreamGet(url, signal) { size += chunk.length; if (size > JSON_MAX_BYTES) { await reply.body.cancel().catch(() => {}); - return { failure: { status: 502, code: "upstream_too_large", message: "the API reply is larger than the viewer reads" } }; + return { retryAfter, failure: { status: 502, code: "upstream_too_large", message: "the API reply is larger than the viewer reads" } }; } chunks.push(chunk); } text = Buffer.concat(chunks).toString("utf8"); } catch { - return { failure: { status: 502, code: "upstream_unreachable", message: "the API reply was cut off" } }; + return { retryAfter, failure: { status: 502, code: "upstream_unreachable", message: "the API reply was cut off" } }; } let json; try { json = text === "" ? null : JSON.parse(text); } catch { - return { failure: { status: 502, code: "upstream_not_json", message: `the API answered ${reply.status} with a body that is not JSON` } }; + return { retryAfter, failure: { status: 502, code: "upstream_not_json", message: `the API answered ${reply.status} with a body that is not JSON` } }; } - return { status: reply.status, json, etag: reply.headers.get("etag"), retryAfter: reply.headers.get("retry-after") }; + return { status: reply.status, json, etag: reply.headers.get("etag"), retryAfter }; } function refuse(res, status, code, message, headers = {}) { diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index f51ad17e9..60c75c17d 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -539,3 +539,52 @@ test("a forced refresh answered by a retired-run replica returns to normal polli assert.deepEqual(statusAt.slice(0, 5), [0, 0, 5_000, 10_000, 15_000]); } finally { follower.stop(); await follower.done; } }); + +for (const bodyKind of ["non-JSON", "truncated"]) for (const failureRoute of ["result", "preview prerequisite"]) +test(`${bodyKind} ${failureRoute} errors retain the result Retry-After`, async () => { + let clock = 0; + const resultAt = []; + const failureIndex = failureRoute === "result" ? 0 : 1; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const upstream = createServer((req, res) => { + if (req.url.endsWith("/result")) { + resultAt.push(clock); + if (resultAt.length === failureIndex + 1) { + res.writeHead(503, { "retry-after": "120", "content-type": "text/html", ...(bodyKind === "truncated" ? { "content-length": "1000" } : {}) }); + if (bodyKind === "non-JSON") return res.end("busy"); + return res.write("", () => res.destroy()); + } + return res.end(JSON.stringify({ runId: "run_s", status: "complete", outcome: { kind: "eligible" }, previewUrl: failureRoute === "result" ? "" : "/model.ply" })); + } + if (req.url === "/model.ply") return res.end(ply); + if (req.url.includes("/events")) { + const after = new URL(req.url, "http://x").searchParams.get("after"); + const changed = clock >= 6_000; + return res.end(JSON.stringify({ status: "complete", next: changed ? 1 : 0, events: changed && after === "0" + ? [{ seq: 1, type: "verdict_ready", data: { runId: "run_s", kind: "eligible" } }] : [] })); + } + res.end(JSON.stringify({ runId: "run_s", status: "complete" })); + }); + await new Promise((resolve) => upstream.listen(0, "127.0.0.1", resolve)); + const relay = createViewerServer({ upstreams: { s: { base: `http://127.0.0.1:${upstream.address().port}/v1`, label: "Malformed errors", kind: "synthetic" } }, + publicDir: join(dirname(fileURLToPath(import.meta.url)), "..", "public") }); + let follower; + try { + const origin = `http://127.0.0.1:${await relay.listen(0)}`; + let state = reduce(initialState(), { type: "select", mode: "live", source: { key: "s", kind: "synthetic" }, captureId: "cap_BAD_ERROR" }); + follower = followCapture({ sourceKey: "s", captureId: "cap_BAD_ERROR", session: state.session, + dispatch: (a) => { state = reduce(state, a); }, getState: () => state, + fetchImpl: (path, init) => fetch(origin + path, init), now: () => clock, + sleep: async (ms) => { clock += ms; }, online: () => true }); + await until(() => failureRoute === "result" ? state.result.phase === "ready" : state.preview.phase === "ready"); + assert.ok(resultAt.length > failureIndex + 1); + const deadline = resultAt[failureIndex] + 120_000; + assert.ok(resultAt.slice(failureIndex + 1).every((at) => at >= deadline), `result reads at ${resultAt}, deadline ${deadline}`); + assert.equal(state.result.body.outcome.kind, "eligible"); + } finally { + follower?.stop(); await follower?.done; + await relay.close(); + upstream.closeAllConnections(); + await new Promise((resolve) => upstream.close(resolve)); + } +}); diff --git a/tools/integration-viewer/test/model.test.js b/tools/integration-viewer/test/model.test.js index f604d7e1e..356741adf 100644 --- a/tools/integration-viewer/test/model.test.js +++ b/tools/integration-viewer/test/model.test.js @@ -261,6 +261,17 @@ test("a failed refresh keeps the result already shown", () => { assert.equal(s.result.error, "network error"); }); +test("a null result refresh keeps the accepted eligible result and shows a refresh error", () => { + const accepted = run(selected(), { type: "result", body: { runId: "run_a", status: "complete", outcome: { kind: "eligible" } } }); + const refreshed = run(accepted, { type: "result-loading" }, { type: "result", body: null }); + assert.equal(refreshed.result.phase, "ready"); + assert.equal(refreshed.result.body, accepted.result.body); + assert.equal(refreshed.result.revision, accepted.result.revision); + assert.equal(refreshed.preview, accepted.preview); + assert.equal(refreshed.result.error, "Result was not a JSON object"); + assert.equal(run(selected(), { type: "result", body: null }).result.phase, "error"); +}); + test("the eligible illustration leaves no check unsure", async () => { const { resultBody } = await import("../public/scenario.js"); const body = resultBody("complete", "complete"); diff --git a/tools/integration-viewer/test/relay.test.js b/tools/integration-viewer/test/relay.test.js index 380bca6a9..f8bcd3806 100644 --- a/tools/integration-viewer/test/relay.test.js +++ b/tools/integration-viewer/test/relay.test.js @@ -298,6 +298,22 @@ describe("refusals", () => { } }); + for (const bodyKind of ["non-JSON", "truncated"]) test(`${bodyKind} result errors preserve deadlines on direct and preview routes`, async () => { + hostileHandler = (_req, res) => { + res.writeHead(503, { "content-type": "text/html", "retry-after": "120", ...(bodyKind === "truncated" ? { "content-length": "1000" } : {}) }); + if (bodyKind === "non-JSON") return res.end("busy"); + res.write("", () => res.destroy()); + }; + try { + for (const leaf of ["result", "preview?run=r"]) { + const reply = await raw(`/relay/hostile/captures/cap_HOSTILE_1/${leaf}`); + assert.equal(reply.status, 502); + assert.equal(reply.headers["retry-after"], "120"); + assert.equal(reply.json.errors[0].code, leaf === "result" ? (bodyKind === "non-JSON" ? "upstream_not_json" : "upstream_unreachable") : "result_unavailable"); + } + } finally { hostileHandler = null; } + }); + test("an errored storage body cannot replace the preview 502 or Retry-After", async (t) => { const storageUrl = `http://127.0.0.1:${hostile.address().port}/v1/errored.ply`; hostileBody = { runId: "r", previewUrl: storageUrl }; From 92a273557915f572891d04af092c64faef85bb5d Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:00:16 -0500 Subject: [PATCH 18/20] fix(viewer): show errors after pending result refreshes Retain only ready bodies on refresh failure so pending-result errors remain visible. https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4274298187 --- tools/integration-viewer/public/model.js | 5 +- tools/integration-viewer/test/browser.test.js | 60 +++++++++++++++++++ tools/integration-viewer/test/model.test.js | 20 +++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/tools/integration-viewer/public/model.js b/tools/integration-viewer/public/model.js index 96106a743..fae7833f6 100644 --- a/tools/integration-viewer/public/model.js +++ b/tools/integration-viewer/public/model.js @@ -121,7 +121,7 @@ export function reduce(state, action) { case "result": return applyResult(state, action.body); case "result-error": - // A failed refresh keeps a result already shown; it is still what the server last said. + // A failed refresh retains only a ready result. return resultError(state, action.error); case "preview-loading": // Re-reading the same run's model keeps the one on screen until the new one arrives. @@ -334,7 +334,8 @@ function stage(state, data, base) { const NO_OUTCOME_STATUSES = new Set(["failed", "expired"]); function resultError(state, error) { - return { ...state, result: { ...state.result, phase: state.result.phase === "ready" ? "ready" : "error", error } }; + const ready = state.result.phase === "ready"; + return { ...state, result: { ...state.result, phase: ready ? "ready" : "error", body: ready ? state.result.body : null, error } }; } function applyResult(state, body) { diff --git a/tools/integration-viewer/test/browser.test.js b/tools/integration-viewer/test/browser.test.js index 0ef85753a..967a3bf2a 100644 --- a/tools/integration-viewer/test/browser.test.js +++ b/tools/integration-viewer/test/browser.test.js @@ -59,6 +59,66 @@ const READ_PAGE = `(() => { }; })()`; +for (const phase of ["pending", "ready"]) { + test(`${phase} result followed by null shows the error in the page`, { skip: chromePath ? false : "no Chrome found; set CHROME_PATH" }, async () => { + const chrome = await launchChrome(chromePath); + try { + await chrome.send("Page.enable"); + await chrome.send("Page.addScriptToEvaluateOnNewDocument", { source: RARE_FRAMES + ` + const originalFetch = window.fetch; + window.__resultReads = 0; + window.__refreshResult = false; + window.fetch = async (input, init) => { + const path = new URL(input, location.href).pathname; + const json = (body) => new Response(JSON.stringify(body), { headers: { "content-type": "application/json" } }); + if (path.endsWith("/result")) { + window.__resultReads += 1; + return json(window.__resultReads > 1 ? null : { + runId: "run_a", status: "complete", + outcome: ${phase === "ready" ? '{ kind: "eligible", message: "Accepted result" }' : 'null'}, + }); + } + if (path.endsWith("/events")) return json({ status: "complete", events: window.__refreshResult ? [ + { seq: 1, type: "verdict_ready", data: { runId: "run_a", kind: "eligible" } }, + ] : [] }); + if (path.includes("/captures/")) return json({ runId: "run_a", status: "complete" }); + return originalFetch(input, init); + }; + ` }); + await chrome.send("Page.navigate", { url: `${origin}/?mode=live&source=synthetic&capture=cap_TEST_REFRESH` }); + let page; + const started = Date.now(); + while (Date.now() - started < 10_000) { + await new Promise((resolve) => setTimeout(resolve, 100)); + page = await chrome.evaluate(`(() => { + if (!window.__flushFrames || !document.getElementById("outcome")) return null; + window.__flushFrames(); + if (window.__resultReads === 1) window.__refreshResult = true; + const box = document.getElementById("outcome"); + const note = document.getElementById("outcome-note"); + return { reads: window.__resultReads, visible: !box.hidden, + kind: document.getElementById("outcome-kind").textContent, + message: document.getElementById("outcome-msg").textContent, + note: note.hidden ? "" : note.textContent }; + })()`); + if (page?.reads >= 2 && /Result was not a JSON object/.test(page.message + page.note)) break; + } + assert.ok(page?.reads >= 2, "the follower did not refresh the result"); + assert.equal(page.visible, true, "the error must be visible"); + if (phase === "ready") { + assert.equal(page.kind, "A battery fits"); + assert.equal(page.message, "Accepted result"); + assert.match(page.note, /Refreshing the result failed.*Result was not a JSON object/); + } else { + assert.equal(page.kind, "Could not read the result"); + assert.match(page.message, /Result was not a JSON object/); + } + } finally { + await chrome.close(); + } + }); +} + test("with the animation clock stopped, counts and stage marks still follow the server", { skip: chromePath ? false : "no Chrome found; set CHROME_PATH" }, async () => { const chrome = await launchChrome(chromePath); try { diff --git a/tools/integration-viewer/test/model.test.js b/tools/integration-viewer/test/model.test.js index 356741adf..c98e72692 100644 --- a/tools/integration-viewer/test/model.test.js +++ b/tools/integration-viewer/test/model.test.js @@ -272,6 +272,26 @@ test("a null result refresh keeps the accepted eligible result and shows a refre assert.equal(run(selected(), { type: "result", body: null }).result.phase, "error"); }); +for (const body of [null, [], "invalid"]) { + test(`a pending result followed by ${JSON.stringify(body)} clears the body so its error can render`, () => { + const pending = run(selected(), { type: "result", body: { runId: "run_a", status: "processing", outcome: null } }); + assert.equal(pending.result.phase, "pending"); + const failed = run(pending, { type: "result-loading" }, { type: "result", body }); + assert.equal(failed.result.phase, "error"); + assert.equal(failed.result.body, null); + assert.equal(failed.result.error, "Result was not a JSON object"); + assert.equal(failed.result.revision, pending.result.revision); + }); +} + +test("a pending result followed by a fetch failure clears the body so its error can render", () => { + const pending = run(selected(), { type: "result", body: { runId: "run_a", status: "processing", outcome: null } }); + const failed = run(pending, { type: "result-loading" }, { type: "result-error", error: "network error" }); + assert.equal(failed.result.phase, "error"); + assert.equal(failed.result.body, null); + assert.equal(failed.result.error, "network error"); +}); + test("the eligible illustration leaves no check unsure", async () => { const { resultBody } = await import("../public/scenario.js"); const body = resultBody("complete", "complete"); From 3e82e8c0fea5bffeedb1e62f4c594c23a9b2d7f9 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:13:36 -0500 Subject: [PATCH 19/20] fix(viewer): retain prompts and isolate contact retry delays Keep pending result bodies and display refresh errors alongside them. Let result and preview retries proceed while contact requests honor their own backoff. https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4274465693 https://github.com/SamGu-NRX/BaseScanning/pull/238#discussion_r4274500132 --- tools/integration-viewer/public/app.js | 2 +- tools/integration-viewer/public/live.js | 98 +++++++++++-------- tools/integration-viewer/public/model.js | 5 +- tools/integration-viewer/test/browser.test.js | 13 ++- tools/integration-viewer/test/live.test.js | 31 ++++++ tools/integration-viewer/test/model.test.js | 27 +++-- 6 files changed, 122 insertions(+), 54 deletions(-) diff --git a/tools/integration-viewer/public/app.js b/tools/integration-viewer/public/app.js index ede7ae806..580f1fe3b 100644 --- a/tools/integration-viewer/public/app.js +++ b/tools/integration-viewer/public/app.js @@ -350,7 +350,7 @@ function renderResult() { $("review-link").hidden = true; return; } - if (!body || (result.phase !== "ready" && result.phase !== "loading")) { + if (!body || (result.phase !== "ready" && result.phase !== "loading" && !result.error)) { box.hidden = true; $("criteria").replaceChildren(); $("criteria").dataset.key = ""; diff --git a/tools/integration-viewer/public/live.js b/tools/integration-viewer/public/live.js index ff9dea537..7d1f9c257 100644 --- a/tools/integration-viewer/public/live.js +++ b/tools/integration-viewer/public/live.js @@ -72,46 +72,64 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const previewNotBefore = new Map(); let previewKey = null; let invalidatedArtifactKey = null; + let contactNotBefore = -Infinity; + + function resultDue(state) { + const key = resultKey(state); + if (!resultExpected(state) || resultPoll.readyKey === key) return Infinity; + const tries = resultPoll.key === key ? resultPoll.tries : 0; + const interval = tries < RESULT_QUICK_TRIES ? RESULT_RETRY_MS : RESULT_SLOW_MS; + return Math.max(resultPoll.at + interval, resultNotBefore); + } + + function previewDue(result) { + const runId = result.body?.runId; + const artifactKey = artifactKeyFor(result.body); + if (result.phase !== "ready" || !hasPreview(result.body) || typeof runId !== "string" + || previewKey === `${runId}|${result.revision}` || artifactKey === invalidatedArtifactKey) return Infinity; + return Math.max(resultNotBefore, previewNotBefore.get(artifactKey) ?? -Infinity); + } // Reads start without waiting and repeat while pages come back full. They return what happened // before the viewer connected, which the reducer marks as backlog so it does not animate. let catchUp = true; let waitS = 0; while (!signal.aborted) { - let contactRetryMs = 0; - try { - // An event or result named a run the status has not named yet: ask the status now. - if (getState().statusRefreshWanted) statusAt = -Infinity; - if (now() - statusAt >= STATUS_EVERY_MS) { - // Health is read until it answers once, so the build shown survives a failed first read. - if (!getState().identity) get("/healthz").then((body) => send({ type: "health", body }), () => {}); - send({ type: "status", body: await get(`/captures/${captureId}`) }); - statusAt = now(); - send({ type: "contact-ok", at: now() }); - } - const cursor = getState().cursor; - const asked = now(); - // The wait ends by the next status refresh, so a status change with no event still shows. - const untilStatusS = Math.ceil(Math.max(0, STATUS_EVERY_MS - (now() - statusAt)) / 1000); - const body = await get(`/captures/${captureId}/events?after=${cursor}&wait=${catchUp ? 0 : Math.min(waitS, untilStatusS)}`); - const count = Array.isArray(body?.events) ? body.events.length : 0; - send({ type: "events", body, at: now(), catchUp }); - failures = 0; - if (catchUp && count >= PAGE_SIZE) continue; - catchUp = false; - if (count === 0 && now() - asked < MIN_EMPTY_POLL_MS) await sleep(MIN_EMPTY_POLL_MS - (now() - asked), signal).catch(() => {}); - } catch (error) { - if (signal.aborted) return; - if (error instanceof HttpError && (error.status === 404 || error.status === 410)) { - send({ type: "gone", error: error.status === 410 ? "The capture was deleted (410)." : "The API has no capture with this id (404)." }); - return; + if (now() >= contactNotBefore) { + try { + // An event or result named a run the status has not named yet: ask the status now. + if (getState().statusRefreshWanted) statusAt = -Infinity; + if (now() - statusAt >= STATUS_EVERY_MS) { + // Health is read until it answers once, so the build shown survives a failed first read. + if (!getState().identity) get("/healthz").then((body) => send({ type: "health", body }), () => {}); + send({ type: "status", body: await get(`/captures/${captureId}`) }); + statusAt = now(); + send({ type: "contact-ok", at: now() }); + } + const cursor = getState().cursor; + const asked = now(); + // The wait ends by the next status refresh, so a status change with no event still shows. + const untilStatusS = Math.ceil(Math.max(0, STATUS_EVERY_MS - (now() - statusAt)) / 1000); + const body = await get(`/captures/${captureId}/events?after=${cursor}&wait=${catchUp ? 0 : Math.min(waitS, untilStatusS)}`); + const count = Array.isArray(body?.events) ? body.events.length : 0; + send({ type: "events", body, at: now(), catchUp }); + failures = 0; + if (catchUp && count >= PAGE_SIZE) continue; + catchUp = false; + if (count === 0 && now() - asked < MIN_EMPTY_POLL_MS) await sleep(MIN_EMPTY_POLL_MS - (now() - asked), signal).catch(() => {}); + } catch (error) { + if (signal.aborted) return; + if (error instanceof HttpError && (error.status === 404 || error.status === 410)) { + send({ type: "gone", error: error.status === 410 ? "The capture was deleted (410)." : "The API has no capture with this id (404)." }); + return; + } + const detail = error instanceof HttpError ? `${error.code} (${error.status})` : "network error"; + send({ type: "contact-failed", error: detail, offline: !online() }); + const backoff = BACKOFF_MS[Math.min(failures, BACKOFF_MS.length - 1)]; + const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; + contactNotBefore = now() + Math.max(backoff, asked); + failures += 1; } - const detail = error instanceof HttpError ? `${error.code} (${error.status})` : "network error"; - send({ type: "contact-failed", error: detail, offline: !online() }); - const backoff = BACKOFF_MS[Math.min(failures, BACKOFF_MS.length - 1)]; - const asked = error instanceof HttpError && error.retryAfterS ? Math.min(error.retryAfterS * 1000, RETRY_AFTER_CAP_MS) : 0; - contactRetryMs = Math.max(backoff, asked); - failures += 1; } const state = getState(); @@ -121,9 +139,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat resultPoll.key = key; resultPoll.tries = 0; } - const pending = resultExpected(state) && resultPoll.readyKey !== key; - const interval = resultPoll.tries < RESULT_QUICK_TRIES ? RESULT_RETRY_MS : RESULT_SLOW_MS; - if (pending && now() >= Math.max(resultPoll.at + interval, resultNotBefore)) { + if (now() >= resultDue(state)) { resultPoll.tries += 1; resultPoll.at = now(); send({ type: "result-loading" }); @@ -151,8 +167,7 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat const revisionKey = `${runId}|${result.revision}`; const artifactKey = artifactKeyFor(result.body); for (const [artifact, deadline] of previewNotBefore) if (deadline <= now()) previewNotBefore.delete(artifact); - const previewDue = Math.max(resultNotBefore, previewNotBefore.get(artifactKey) ?? -Infinity); - if (result.phase === "ready" && hasPreview(result.body) && typeof runId === "string" && previewKey !== revisionKey && artifactKey !== invalidatedArtifactKey && now() >= previewDue) { + if (now() >= previewDue(result)) { previewKey = revisionKey; send({ type: "preview-loading", runId }); try { @@ -182,7 +197,12 @@ export function followCapture({ sourceKey, captureId, session, dispatch, getStat } } } - if (contactRetryMs > 0) await sleep(contactRetryMs, signal).catch(() => {}); + if (now() < contactNotBefore) { + // Contact throttling must not delay independent result or preview retries. + const current = getState(); + const wakeAt = Math.min(contactNotBefore, resultDue(current), previewDue(current.result)); + await sleep(Math.max(0, wakeAt - now()), signal).catch(() => {}); + } } } diff --git a/tools/integration-viewer/public/model.js b/tools/integration-viewer/public/model.js index fae7833f6..911dda770 100644 --- a/tools/integration-viewer/public/model.js +++ b/tools/integration-viewer/public/model.js @@ -121,7 +121,7 @@ export function reduce(state, action) { case "result": return applyResult(state, action.body); case "result-error": - // A failed refresh retains only a ready result. + // Pending results can carry titled retake prompts, so keep their bodies too. return resultError(state, action.error); case "preview-loading": // Re-reading the same run's model keeps the one on screen until the new one arrives. @@ -334,8 +334,7 @@ function stage(state, data, base) { const NO_OUTCOME_STATUSES = new Set(["failed", "expired"]); function resultError(state, error) { - const ready = state.result.phase === "ready"; - return { ...state, result: { ...state.result, phase: ready ? "ready" : "error", body: ready ? state.result.body : null, error } }; + return { ...state, result: { ...state.result, phase: state.result.body ? state.result.phase : "error", error } }; } function applyResult(state, body) { diff --git a/tools/integration-viewer/test/browser.test.js b/tools/integration-viewer/test/browser.test.js index 967a3bf2a..62f815163 100644 --- a/tools/integration-viewer/test/browser.test.js +++ b/tools/integration-viewer/test/browser.test.js @@ -59,7 +59,7 @@ const READ_PAGE = `(() => { }; })()`; -for (const phase of ["pending", "ready"]) { +for (const phase of ["needs_views", "pending", "ready"]) { test(`${phase} result followed by null shows the error in the page`, { skip: chromePath ? false : "no Chrome found; set CHROME_PATH" }, async () => { const chrome = await launchChrome(chromePath); try { @@ -74,7 +74,8 @@ for (const phase of ["pending", "ready"]) { if (path.endsWith("/result")) { window.__resultReads += 1; return json(window.__resultReads > 1 ? null : { - runId: "run_a", status: "complete", + runId: "run_a", status: ${JSON.stringify(phase === "needs_views" ? "needs_views" : "complete")}, + viewsNeeded: ${phase === "needs_views" ? '[{ id: "view_left", prompt: { title: "Show the left edge" } }]' : '[]'}, outcome: ${phase === "ready" ? '{ kind: "eligible", message: "Accepted result" }' : 'null'}, }); } @@ -99,7 +100,8 @@ for (const phase of ["pending", "ready"]) { return { reads: window.__resultReads, visible: !box.hidden, kind: document.getElementById("outcome-kind").textContent, message: document.getElementById("outcome-msg").textContent, - note: note.hidden ? "" : note.textContent }; + note: note.hidden ? "" : note.textContent, + views: document.getElementById("views").hidden ? "" : document.getElementById("views-list").textContent }; })()`); if (page?.reads >= 2 && /Result was not a JSON object/.test(page.message + page.note)) break; } @@ -110,8 +112,9 @@ for (const phase of ["pending", "ready"]) { assert.equal(page.message, "Accepted result"); assert.match(page.note, /Refreshing the result failed.*Result was not a JSON object/); } else { - assert.equal(page.kind, "Could not read the result"); - assert.match(page.message, /Result was not a JSON object/); + assert.equal(page.kind, "No outcome"); + assert.match(page.note, /Refreshing the result failed.*Result was not a JSON object/); + if (phase === "needs_views") assert.equal(page.views, "Show the left edgeview_left"); } } finally { await chrome.close(); diff --git a/tools/integration-viewer/test/live.test.js b/tools/integration-viewer/test/live.test.js index 60c75c17d..5f7026369 100644 --- a/tools/integration-viewer/test/live.test.js +++ b/tools/integration-viewer/test/live.test.js @@ -237,6 +237,37 @@ test("terminal results are read even while every events request fails", async () } finally { h.follower.stop(); await h.follower.done; } }); +for (const firstResult of ["pending", "throttled"]) test(`events Retry-After leaves ${firstResult} result and preview retries on their own schedules`, async () => { + const eventsAt = []; + const resultAt = []; + const previewAt = []; + const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; + const h = stubFollower((path, clock) => { + if (path.includes("/events")) { + eventsAt.push(clock); + return eventsAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "120" }) : stubEvents(); + } + if (path.endsWith("/result")) { + resultAt.push(clock); + if (resultAt.length > 1) return stubResult("withheld"); + return firstResult === "pending" ? stubJson({ runId: "run_s", status: "complete", outcome: null }) + : stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "11" }); + } + if (path.includes("/preview")) { + previewAt.push(clock); + return previewAt.length === 1 ? stubJson({ errors: [{ code: "busy" }] }, 503, { "retry-after": "19" }) : new Response(ply); + } + return stubStatus(); + }); + try { + await until(() => eventsAt.length >= 2 && h.state.preview.phase === "ready"); + assert.equal(resultAt[1] - resultAt[0], firstResult === "pending" ? 4_000 : 11_000, `result reads at ${resultAt}`); + assert.equal(previewAt[1] - previewAt[0], 19_000, `preview reads at ${previewAt}`); + assert.ok(previewAt[1] < eventsAt[0] + 120_000, "the preview settles while events are still delayed"); + assert.ok(eventsAt[1] - eventsAt[0] >= 120_000, `event reads at ${eventsAt}`); + } finally { h.follower.stop(); await h.follower.done; } +}); + for (const form of ["seconds", "date"]) test(`preview retries wait for ${form}-form Retry-After`, async () => { const previewAt = []; const ply = "ply\nformat ascii 1.0\nelement vertex 1\nproperty float x\nproperty float y\nproperty float z\nend_header\n1 2 3\n"; diff --git a/tools/integration-viewer/test/model.test.js b/tools/integration-viewer/test/model.test.js index c98e72692..2351e9730 100644 --- a/tools/integration-viewer/test/model.test.js +++ b/tools/integration-viewer/test/model.test.js @@ -273,25 +273,40 @@ test("a null result refresh keeps the accepted eligible result and shows a refre }); for (const body of [null, [], "invalid"]) { - test(`a pending result followed by ${JSON.stringify(body)} clears the body so its error can render`, () => { + test(`a pending result followed by ${JSON.stringify(body)} keeps its body with a refresh error`, () => { const pending = run(selected(), { type: "result", body: { runId: "run_a", status: "processing", outcome: null } }); assert.equal(pending.result.phase, "pending"); const failed = run(pending, { type: "result-loading" }, { type: "result", body }); - assert.equal(failed.result.phase, "error"); - assert.equal(failed.result.body, null); + assert.equal(failed.result.phase, "pending"); + assert.equal(failed.result.body, pending.result.body); assert.equal(failed.result.error, "Result was not a JSON object"); assert.equal(failed.result.revision, pending.result.revision); }); } -test("a pending result followed by a fetch failure clears the body so its error can render", () => { +test("a pending result followed by a fetch failure keeps its body with a refresh error", () => { const pending = run(selected(), { type: "result", body: { runId: "run_a", status: "processing", outcome: null } }); const failed = run(pending, { type: "result-loading" }, { type: "result-error", error: "network error" }); - assert.equal(failed.result.phase, "error"); - assert.equal(failed.result.body, null); + assert.equal(failed.result.phase, "pending"); + assert.equal(failed.result.body, pending.result.body); assert.equal(failed.result.error, "network error"); }); +for (const withRetake of [false, true]) { + test(`a needs_views refresh failure retains titled prompts ${withRetake ? "over event IDs" : "without retake events"}`, () => { + const actions = withRetake ? [events([{ seq: 1, type: "retake_request", data: { runId: "run_a", viewsNeeded: ["view_left"] } }])] : []; + const pending = run(selected(), ...actions, { type: "result", body: { + runId: "run_a", status: "needs_views", outcome: null, + viewsNeeded: [{ id: "view_left", prompt: { title: "Show the left edge" } }], + } }); + const failed = run(pending, { type: "result-loading" }, { type: "result", body: null }); + assert.deepEqual(viewsToShow(failed), [{ id: "view_left", title: "Show the left edge" }]); + assert.equal(failed.result.body, pending.result.body); + assert.equal(failed.result.phase, "pending"); + assert.equal(failed.result.error, "Result was not a JSON object"); + }); +} + test("the eligible illustration leaves no check unsure", async () => { const { resultBody } = await import("../public/scenario.js"); const body = resultBody("complete", "complete"); From ce9de6ddd2a717892cffac5826d6fe3af3a63316 Mon Sep 17 00:00:00 2001 From: Sam Gu <127461594+SamGu-NRX@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:41:36 -0500 Subject: [PATCH 20/20] fix(viewer): clear refresh notes when no result body remains Update the note before either early return so a new run cannot inherit the previous result refresh error. https://github.com/SamGu-NRX/BaseScanning/pull/238 --- tools/integration-viewer/public/app.js | 4 +- tools/integration-viewer/test/browser.test.js | 51 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/tools/integration-viewer/public/app.js b/tools/integration-viewer/public/app.js index 580f1fe3b..cbca79a95 100644 --- a/tools/integration-viewer/public/app.js +++ b/tools/integration-viewer/public/app.js @@ -343,6 +343,8 @@ function renderResult() { const box = $("outcome"); const body = result.body; + $("outcome-note").hidden = !body || !result.error; + $("outcome-note").textContent = body && result.error ? `Refreshing the result failed (${plainError(result.error)}). This is the last result received.` : ""; if (result.phase === "error" && !body) { box.hidden = false; setOutcome("", "Could not read the result", `${result.error}. The viewer will try again while connected.`); @@ -357,8 +359,6 @@ function renderResult() { return; } box.hidden = false; - $("outcome-note").hidden = !result.error; - $("outcome-note").textContent = result.error ? `Refreshing the result failed (${plainError(result.error)}). This is the last result received.` : ""; const outcome = body.outcome; if (outcome && typeof outcome.kind === "string") { setOutcome(outcome.kind, OUTCOME_WORDS[outcome.kind] ?? outcome.kind, typeof outcome.message === "string" ? outcome.message : ""); diff --git a/tools/integration-viewer/test/browser.test.js b/tools/integration-viewer/test/browser.test.js index 62f815163..13034dc3f 100644 --- a/tools/integration-viewer/test/browser.test.js +++ b/tools/integration-viewer/test/browser.test.js @@ -59,8 +59,8 @@ const READ_PAGE = `(() => { }; })()`; -for (const phase of ["needs_views", "pending", "ready"]) { - test(`${phase} result followed by null shows the error in the page`, { skip: chromePath ? false : "no Chrome found; set CHROME_PATH" }, async () => { +for (const [phase, switchRun] of [["needs_views", false], ["pending", false], ["ready", false], ["pending", true], ["needs_views", true]]) { + test(`${phase} result followed by null shows the error in the page${switchRun ? " and clears the note on a run switch" : ""}`, { skip: chromePath ? false : "no Chrome found; set CHROME_PATH" }, async () => { const chrome = await launchChrome(chromePath); try { await chrome.send("Page.enable"); @@ -73,6 +73,10 @@ for (const phase of ["needs_views", "pending", "ready"]) { const json = (body) => new Response(JSON.stringify(body), { headers: { "content-type": "application/json" } }); if (path.endsWith("/result")) { window.__resultReads += 1; + if (window.__reportedNextRun) { + await new Promise((resolve) => { window.__finishNextResult = resolve; }); + return json(null); + } return json(window.__resultReads > 1 ? null : { runId: "run_a", status: ${JSON.stringify(phase === "needs_views" ? "needs_views" : "complete")}, viewsNeeded: ${phase === "needs_views" ? '[{ id: "view_left", prompt: { title: "Show the left edge" } }]' : '[]'}, @@ -82,7 +86,10 @@ for (const phase of ["needs_views", "pending", "ready"]) { if (path.endsWith("/events")) return json({ status: "complete", events: window.__refreshResult ? [ { seq: 1, type: "verdict_ready", data: { runId: "run_a", kind: "eligible" } }, ] : [] }); - if (path.includes("/captures/")) return json({ runId: "run_a", status: "complete" }); + if (path.includes("/captures/")) { + window.__reportedNextRun = !!window.__switchRun; + return json({ runId: window.__reportedNextRun ? "run_b" : "run_a", status: "complete" }); + } return originalFetch(input, init); }; ` }); @@ -116,6 +123,44 @@ for (const phase of ["needs_views", "pending", "ready"]) { assert.match(page.note, /Refreshing the result failed.*Result was not a JSON object/); if (phase === "needs_views") assert.equal(page.views, "Show the left edgeview_left"); } + if (switchRun) { + await chrome.evaluate("window.__switchRun = true"); + const readNote = `(() => { + window.__flushFrames(); + const box = document.getElementById("outcome"); + const note = document.getElementById("outcome-note"); + return { reading: !!window.__finishNextResult, hidden: box.hidden, + noteHidden: note.hidden, note: note.textContent, + kind: document.getElementById("outcome-kind").textContent, + message: document.getElementById("outcome-msg").textContent }; + })()`; + let loading; + const switchedAt = Date.now(); + while (Date.now() - switchedAt < 8_000) { + await new Promise((resolve) => setTimeout(resolve, 100)); + loading = await chrome.evaluate(readNote); + if (loading.reading) break; + } + assert.ok(loading?.reading, "the new run did not request its first result"); + await chrome.evaluate("window.__finishNextResult()"); + let failed; + const releasedAt = Date.now(); + while (Date.now() - releasedAt < 2_000) { + await new Promise((resolve) => setTimeout(resolve, 100)); + failed = await chrome.evaluate(readNote); + if (failed.kind === "Could not read the result") break; + } + assert.equal(failed.hidden, false); + assert.equal(failed.kind, "Could not read the result"); + assert.equal((failed.message.match(/Result was not a JSON object/g) ?? []).length, 1); + assert.deepEqual({ + loading: { hidden: loading.hidden, noteHidden: loading.noteHidden, note: loading.note }, + failed: { noteHidden: failed.noteHidden, note: failed.note }, + }, { + loading: { hidden: true, noteHidden: true, note: "" }, + failed: { noteHidden: true, note: "" }, + }, "neither early return may keep the previous run's refresh note"); + } } finally { await chrome.close(); }