[matthewberry]

Part of #17. Built against mmli-backend #148, which has since merged and is deployed to staging and prod. The /v1 OpenAPI snapshot here comes from its final commit (aac06e6), and nothing in /v1 changed after it.

What changes

The template's input → loading → results flow now uses mmli-backend's /v1 API instead of the legacy per-type routes.

• Submit: the form posts the tool's input document as multipart (an inputs part plus any files), with the notification address in X-Notify-Email. When the backend rejects the input, the form shows the problem document, including each field's JSON Pointer and reason.
• Loading: shows queued (waiting for a compute node) separately from processing, and shows error and canceled states. The queued status comes from mmli-backend #153.
• Results: the route is now /result/:id, which is what the backend's default notification email (mmli-backend #151) links to. Nothing linked to the old test/result/:id (I grepped the repo; no template app is deployed), so there's no redirect.
• Generated client: src/app/api/v1, plus npm run openapi:v1 and openapi:v1:sync. It uses openapi-generator 7.24.0 (openapitools.v1.json), because 6.6.0, which the legacy client uses, types FastAPI's OpenAPI 3.1 parameters as any. The legacy client is unchanged.
• Job types come from the tool's schemas: npm run schemas and schemas:sync generate ToolInput and ToolResults from schemas/{input,output}.schema.json, and schemas:sync fetches those through the tool descriptor's links. CI checks that the generated types are current. The template's copies are job-template's sample tool.
• app.yaml job_type now reaches the app as APP_IDENTITY.jobType. The template's is job-template-sample, a name I chose for the sample tool. The backend hasn't registered it; when it does, it should use that name, or this value should change.
• frontendOnly mode and the Storybook mock now serve the sample tool's example job and results.

Findings for mmli-backend #148

1. The generated client can't send multipart. POST /tools/{tool}/jobs declares both application/json and multipart/form-data. typescript-angular (6.6.0 and 7.24.0 alike) generates a method that always sets Content-Type: application/json, so uploads go through HttpClient directly (TestService.submit).
2. No response schemas. Every /v1 response is a plain dict, so the generated client returns any for jobs and results. The job document, results envelope and problem document are typed by hand in src/app/models/v1-job.ts, mirroring _job_document. Response models on the routes would let the client generate these.
3. The status values aren't in the OpenAPI, for the same reason.
4. Not a defect: started_at is always null today because the watcher never sets time_start. The progress estimate falls back to submitted_at.

How it was verified

• Unit tests: 52 pass (33 before), with ng test --watch=false --browsers=ChromeHeadless. New tests cover:
  • the multipart body and headers;
  • results unwrapping;
  • queued, processing, error and canceled on Loading;
  • polling stopping on cancel;
  • problem display;
  • FASTA round-trips;
  • the resubmit rule with files.
• e2e: nightwatch/src/job_flow_spec.ts starts its own mock /v1 API and points the app at it by answering the envvars.json request over CDP. It checks submit (multipart, the tool name, the input document, the email header), then /result/e2e-job, queued, processing, and the results table. I ran it locally with Chrome 155 against npm start, and both suites passed. nightwatch/tsconfig.json now targets es2017. At es5, async tests compile to generators, so Nightwatch doesn't recognize them as async (it checks fn.constructor.name === 'AsyncFunction', lib/utils/index.js:77), and awaited commands don't wait.
• ng build, npm run app:sync -- --check, npm run schemas -- --check, and npm run test:scripts (20 pass) all succeed.
• Not run: lint. ng lint isn't configured in this repo.

Deferred acceptance criteria

• Submit with file upload against staging with the sample tool: needs the sample tool registered on staging and mmli-backend #146 (browser-origin proof through oauth2-proxy and CORS). The staging and prod CORS preflights don't allow X-Notify-Email or Idempotency-Key yet; the fix is in mmli2 MR !7.
• Email links land on /result/:id: the route exists, but the link needs mmli-backend #151 deployed and the tool's frontend_url set.

Not in this PR

• Storybook isn't installed (@storybook/angular is missing), so .storybook/tsconfig.json doesn't compile, before this change and after. The mock was ported anyway.
• The nightly cron job (disabled until the template is deployed) runs every spec against staging, including this mocked one. Apps may want it to run only real smoke tests.

[matthewberry]

Review of the /v1 job flow. Nothing was run. Each finding comes from reading the diff, mmli-backend origin/main's /v1 code, and the installed ngx-matomo-client@8.0.0 source. Treat them as hypotheses.

What checks out:

• The OpenAPI snapshot matches origin/main's /v1 routes, operations, parameters (including X-Notify-Email) and status enum.
• The multipart body (an inputs part plus files parts) matches what the backend parses.
• The tests call the handlers the templates actually bind: (ngSubmit) calls onSubmit, and (submitted) calls onResubmitted.
• The PR body still says #148 isn't merged; it is.

The async entry points: there is no generation token or abort signal anywhere. Cancellation rests on switchMap and shareReplay, and that's where findings 1, 2, 3 and 5 come from.

1. src/app/models/job-result.ts:40: polling outlives the page. jobResultResponse$ uses shareReplay(1) without refCount. So once something subscribes, statusResponse$ never drops to zero subscribers, and the 10s timer keeps calling GET /v1/jobs/:id after the component is destroyed. That contradicts the docstring. Every unfinished job a user visits adds another poll that runs until the job ends. test-result.polling.spec.ts:38 calls discardPeriodicTasks() after fixture.destroy(), which hides this. Asserting that getJob is not called after destroy would fail.
2. src/app/models/job-result.ts:23: any getJob error freezes the page. That includes a 404 "Unknown job" for a mistyped or deleted id from an email link, and a 502/504 during a deploy. There's no catchError or retry, so polling stops for good, isLoading$ stays true, and the page shows "Processing. Estimated processing time…" forever.
3. src/app/models/job-result.ts:35: getResults errors aren't handled either. The backend returns 409 JOB_FAILED when a job completes with no results, and 502 when results fail the output schema. Either one leaves a full progress bar over "Processing…", and the error only reaches the console.
4. src/app/components/test/test.component.ts:111: double-submit. There's no in-flight guard and no Idempotency-Key (which /v1 supports), and Submit is disabled only while the form is invalid. Two clicks during a slow multipart upload create two jobs and send two emails.
5. src/app/components/test/test.component.ts:111: the submit subscription isn't tied to the component's lifetime. A response that arrives after the user has navigated away still calls router.navigate and pulls them to /result/<id>.
6. src/main.ts:46: MatomoRouterModule.forRoot() became MatomoRouterModule. In 8.0.0, the bare module only provides MATOMO_ROUTER_ENABLED=true. It never registers MatomoRouter or its ENVIRONMENT_INITIALIZER. And because core sets trackAppInitialLoad = !MATOMO_ROUTER_ENABLED, even the initial page load isn't tracked. Result: no page views at all (ngx-matomo-client-router.mjs:364-435, core.mjs:96). This change looks unrelated to the PR.
7. src/app/components/test-result/test-result.component.ts:47: no check that the job belongs to this tool. job.tool isn't compared with APP_IDENTITY.jobType, and job.inputs is cast straight to ToolInput. /v1 job ids are global, so /result/<another tool's id> throws in toFasta when the user opens the Input tab.
8. src/app/components/loading/loading.component.html:9: queued jobs briefly show as processing. status is undefined until the first poll returns, so the template falls through to the processing branch. A queued job shows the processing estimate before switching to "Queued".
9. src/app/components/test/test.component.ts:114: the consent checkbox is ignored. X-Notify-Email is sent whenever the email field is filled. agreeToSubscription is never read.
10. .github/workflows/e2e.yml:32: CI only checks the schema types. Nothing checks src/app/api/v1 against the OpenAPI snapshot. Separately, the hand-written Job in v1-job.ts leaves out provenance and links, which _job_document returns.

[matthewberry]

Responses to the review. The branch is rebased onto main, and the fixes are in 59e6462, 7455141, 076e8fd and 764b906. Each fixed finding has a test that failed before its fix.

1. Polling outlives the page: fixed in 59e6462. Reproduced: after fixture.destroy(), getJob ran 3 more times over 30s. shareReplay(1) defaults to refCount: false in the installed rxjs 7.8.2 (internal/operators/shareReplay.js). It's now { bufferSize: 1, refCount: true }. The new test doesn't call discardPeriodicTasks(), so a timer left running would fail it too.
2. A getJob error freezes the page: fixed in 59e6462. Network, 502, 503 and 504 errors are retried twice, 5s apart, so a deploy doesn't end the poll. Anything else, such as 404 "Unknown job", stops polling and shows the problem document (data-testid="load-error") instead of "Processing".
3. getResults errors: fixed in 59e6462. 409 JOB_FAILED and 502 invalid results now show the problem the same way.
4. Double submit: fixed in 7455141. Submit is ignored and the button disabled, with PrimeNG's loading spinner, until the backend answers, and it's enabled again after a refusal. The test dispatches the form's submit event twice, so it goes through the (ngSubmit) binding. I didn't add an Idempotency-Key: the in-flight guard covers the double click, and the key's CORS allowance isn't live yet (mmli2 MR !7).
5. Navigation after leaving: fixed in 7455141. The submit subscription now ends with the component (takeUntilDestroyed). Test: destroy the form, then let the response arrive; it doesn't navigate.
6. MatomoRouterModule.forRoot(): real, but not from this PR. The branch predated Fix Matomo page view tracking (MatomoRouterModule.forRoot()) #26 (6ab8102), which restored forRoot() on main. The diff showed the old line only because the base was stale. After the rebase, src/main.ts has MatomoRouterModule.forRoot(). I confirmed in ngx-matomo-client@8.0.0 that the bare module provides only MATOMO_ROUTER_ENABLED.
7. Another tool's job: fixed in 59e6462. A job whose tool isn't APP_IDENTITY.jobType shows "This job belongs to another tool" instead of rendering inputs the page can't read.
8. Queued flashes as processing: fixed in 076e8fd. Before the first status arrives, Loading shows neither message.
9. Consent checkbox: not changed. It's a product question, not a template bug. ezspecificity-frontend also sends the email without reading agreeToSubscription. The backend has nowhere to store consent, and the checkbox's label mixes "notifications" with "updates". It's open as question 13 in the EZKinetics design review (where consent goes, and who owns the list). Gating the notification on it would change behavior the existing apps share, so it should follow that answer.
10. CI and Job type: fixed in 764b906.
    • CI now regenerates src/app/api/v1 from the snapshot and fails on any diff. A second regeneration is stable; the first committed FILES listing had one extra line, which this commit corrects.
    • Job gains provenance and links, checked against _job_document on mmli-backend main.

Verification:

• Unit tests: 61 passed (52 before).
• Script tests: 20 passed.
• ng build, app:sync -- --check and schemas -- --check pass.
• e2e: both suites pass, 13 assertions, headless Chrome 156 against the mock /v1 API. Locally that needed a ChromeDriver matching Chrome 156, because the installed one was 130.

[matthewberry]

Re-review at head 764b906 (rebased on af0989e). I read the dispositions comment and the code; nothing was run.

The prior findings, checked against the code:

• 1 (fixed): both streams now use shareReplay({bufferSize: 1, refCount: true}). The new destroy test has no discardPeriodicTasks(), and a leftover poll would fail its call-count assertion.
• 4 (fixed): the double-click guard and the [disabled] binding are in place. The test dispatches submit on the <form>, so it goes through (ngSubmit), the binding the template uses.
• 5 (fixed): takeUntilDestroyed. See new finding 2 for a side effect.
• 6 (fixed): src/main.ts:46 is back to MatomoRouterModule.forRoot().
• 7 (fixed): the tool check runs before the loadedJob cast.
• 8 (fixed): the queued and processing messages are each gated on their own status.
• 10 (fixed): CI diffs src/app/api/v1 after regenerating it. provenance and links match _job_document on mmli-backend origin/main.
• 2 and 3 (fixed except for retries): a 404, a tool mismatch, and getResults 409/502 now reach loadProblem$ and the error UI. The retry gaps are new findings 1 and 3.
• 9 (declined as a product question): noted.
• Async entry points: each one (the poll tick, getResults, submit, the unchanged-request navigate) has a cancellation path. The only readers of loadProblem$ and submitting are the result template, onSubmit and the button, and each is consistent with every write site.

New findings:

1. src/app/models/job-result.ts:44: the transient retry can never finish. The retry (count 2, 5 s apart) sits inside switchMap on the 10 s timer. Retry 2 is due at about 10 s plus twice the request latency, but the tick at 10 s switches it out and starts over with a fresh count. So a lasting 0/502/503/504 never reaches fail(), and the page shows "Processing" forever, which is the original finding 2. The only test fails once and then succeeds, so it can't catch this. Moving the retry outside switchMap, or counting consecutive failures across ticks, would fix it.
2. src/app/components/test/test.component.ts:126: takeUntilDestroyed aborts a POST the server may already have accepted. On /result/A, a user edits the inputs, submits, then clicks the Results tab before the upload finishes. The @if destroys <app-test> and the XHR is cancelled. If the backend already got the body, job B exists and its email goes out, but the user is never sent to it. With no Idempotency-Key, resubmitting creates a third job.
3. src/app/models/job-result.ts:67: any 502 from getResults is treated as final. 502 is both the backend's "Job produced invalid results" and a gateway error during a deploy. A job that completes mid-redeploy shows a permanent error, though the same outage on getJob would be retried. The problem type (JOB_FAILED / invalid-results) is what tells the two apart, not the status code.
4. src/app/components/test/test.component.ts:124: the double-submit guard is incomplete. finalize clears submitting when the POST completes, before router.navigate resolves, so the button re-enables in that window. With no Idempotency-Key, retries and refreshes during a slow upload still create duplicates. This is the second half of the original finding 4.
5. src/app/services/test.examples.ts:10: EXAMPLE_JOB.tool is hard-coded to job-template-sample. A team that sets job_type: my-tool in app.yaml then sees "This job belongs to another tool" on every /result page in frontend-only mode. Using APP_IDENTITY.jobType there would fix it.
6. src/app/components/loading/loading.component.ts:73: the error and canceled screens appear only after the first interval(1000) emission. They're also read via .value rather than async. For a job that already failed, the page shows an empty bar with no message for a second. The specs tick 1000 ms before checking, which hides this.
7. src/app/components/test-result/test-result.polling.spec.ts:38: the first polling test still calls discardPeriodicTasks() after fixture.destroy(). With refCount in place the call does nothing. If refCount were removed, this test would still pass, so delete the call.

[matthewberry]

Responses to the second pass, fixed in 9cd5ea8, c220cd7 and 77d05e6. Each fixed finding has a test that failed before its fix, except 5 (see below).

1. A lasting outage never ended the poll: fixed in 9cd5ea8. Reproduced with a test where getJob returns 502 on every tick for 50s: the page stayed on "Processing". The per-request retry is gone. Consecutive transient failures are now counted across ticks (3 polls, about 30s), the count resets on success, and the third one shows the error. The brief-outage test (one 502, then success) still passes.
2. takeUntilDestroyed aborted an accepted POST: fixed in c220cd7. Decision: the request always completes, and only the navigation is skipped once the form is destroyed, so the job the backend created isn't hidden. The test destroys the form mid-request and asserts that the request is still subscribed (response.observed) and that no navigation happens when it resolves.
3. Every getResults 502 was final: fixed in 9cd5ea8. The backend's own 502 for invalid results (ProblemException(502, 'Job produced invalid results', type_uri=UPSTREAM_FAILURE) in mmli-backend app/routers/v1/jobs.py) always carries a problem document. A gateway's 502 doesn't. So "transient" is now 0/502/503/504 without a problem type, for both streams. getResults retries those twice, 5s apart; problem-document 502s stay final. (Writing this test showed that a fake must be cold, via defer, like HttpClient; otherwise retry resubscribes an observable that has already failed.)
4. The guard re-enabled before navigation finished: fixed in c220cd7. submitting is now cleared when router.navigate settles, or on error, not when the POST completes. Test: with navigation pending, a second submit is ignored. Idempotency-Key still isn't sent: browsers can't send it until mmli2 MR !7 adds it to the CORS allow-list. Refresh-and-resubmit duplicates need the key, so that waits on !7.
5. EXAMPLE_JOB.tool was hard-coded: fixed in 77d05e6. It's now APP_IDENTITY.jobType, and so is its links.tool. No failing test was possible: in the template job_type is job-template-sample, so the old and new values are equal.
6. Error and canceled screens were a second late: fixed in 77d05e6. Reproduced with a test that checks without ticking. The showError$/showCanceled$ flags were set as a side effect of the progress interval and read with .value. They're gone, and the template now gates all four screens on the status it already reads from status$.
7. discardPeriodicTasks() in the first polling test: removed in 9cd5ea8, though the premise doesn't hold. In zone.js 0.15.1, fakeAsync defaults to flush: true. That flushes leftover periodic timers and skips the "periodic timer(s) still in the queue" check (zone-testing.js, fakeAsync), so removing the call doesn't make that test catch a missing refCount. I confirmed it with a mutation: with refCount removed from jobResultResponse$, the first test still passes and the destroy test fails on its call-count assertion. That assertion is the leak check. I also corrected the comment from the first round that claimed otherwise.

Verification:

• Unit tests: 65 passed (61 before).
• Script tests: 20 passed.
• ng build, app:sync -- --check and schemas -- --check pass.
• Regenerating the /v1 client produces no diff.
• e2e: both suites pass, 13 assertions, headless Chrome 156.

[matthewberry]

Re-review at head 77d05e6. Fixes 2, 4, 5, 6 and 7 hold. The new code for 1 and 3 has a regression (finding 1 below). I read the code, plus the installed node_modules in a checkout; nothing was run.

The fixes:

• 1 (consecutive failures): the counter rises per failed tick and resets on success. The 50 s test would fail on the old code: I traced zone.js's scheduler, and the interval tick at t=10000 was queued ahead of the retry timer due at the same time. There are three gaps (findings 1–3).
• 2: the POST is no longer cancelled, and submitted.emit and navigation are skipped after destroy. The test would fail on the old code.
• 3: the backend half checks out. On mmli-backend origin/main, every /v1 502 goes through problem() and always carries type, including the legacy HTTPException(502) converted by http_exception_handler. /v1 sends no 503 or 504 problem documents. The client half has finding 1.
• 4–6: fixed as described.
• 7: you're right, and my premise was wrong. zone.js 0.15.1 is installed, matching the lockfile. Its fakeAsync defaults flush = true and calls flush(20, true). The "periodic timer(s) still in the queue" error is only raised when flush is false.

New findings:

1. src/app/models/job-result.ts:13: status-0 failures are no longer retried. The new !error.error?.type check is the problem.
   • The app uses Angular's XHR backend (withInterceptorsFromDi(), no withFetch). In @angular/common 20.3.0, a network error builds new HttpErrorResponse({error, status: xhr.status || 0}), where error is the ProgressEvent. Its .type is 'error', so the check treats it as final.
   • Status 0 is what the app sees for a Wi-Fi blip, a DNS failure, or a cross-origin gateway 502/503 that has no Access-Control-Allow-Origin header. Now the poll ends at once with "The job could not be loaded", and getResults stops retrying. Before this round, status 0 was always retried.
   • The specs build errors with error: null, so no test sees this.
   • Fix: check for a problem object with a title, as fail() does, or skip the type check when status is 0.
2. src/app/models/job-result.ts:53: failures slower than the 10 s tick never reach the counter. switchMap cancels the in-flight getJob at every tick. A hung pod, nginx's default 60 s proxy_read_timeout 504, or a blackholed connection (no XHR timeout is set) never errors before it's cancelled. The count stays 0, and the page shows "Processing" forever. A timeout(...) shorter than the tick inside the switchMap, or exhaustMap, would make these count.
3. src/app/models/job-result.ts:48: transientFailures is an instance field and isn't reset when refCount restarts the poll. After two failures, opening the Input tab removes every subscriber. Coming back starts a fresh timer(0, …) with the count still at 2, so the next failure shows the error page. Fix: reset it per subscription, e.g. defer(() => { this.transientFailures = 0; return timer(0, 10000); }).
4. src/app/models/job-result.ts:16: off by one tick. The TRANSIENT_POLLS doc and the disposition say the page gives up after 30 s. With ++count < 3, failures at t=0, 10 and 20 s throw at 20 s.
5. src/app/models/job-result.ts:48: the shared-poll JSDoc now documents the wrong member. private transientFailures = 0; sits between statusResponse$'s JSDoc and statusResponse$, so the doc now attaches to the counter. Move the field above the JSDoc.
6. src/app/components/test/test.component.ts:63: the doc on submitting is stale. It says "True from submit until the backend answers", but submitting now stays true until router.navigate settles. Someone trusting the doc could move the reset back to the response, which would bring back round-2's finding 4.
7. src/app/components/test/test.component.ts:66: the hand-rolled destroyed flag duplicates DestroyRef.destroyed. Angular 20.3.0 already provides it (abstract get destroyed(): boolean), so check this.destroyRef.destroyed instead.
8. src/app/components/loading/loading.component.spec.ts:59: stale test comment and helper. "The terminal states are set by the progress stream, which ticks once a second" and the showAfterATick helper describe the mechanism this round removed. The tick(1000) in the older error and canceled tests now does nothing, and those tests duplicate the new "as soon as the status arrives" test.

[matthewberry]

Responses to the third pass, fixed in 4710035 and 888549d. Findings 1–3 each have a test that failed before its fix.

1. Status 0 treated as final (regression): fixed in 4710035. Confirmed in @angular/common 20.3.0 (fesm2022/module.mjs, HttpXhrBackend's onError): a network error becomes new HttpErrorResponse({ error, status: xhr.status || 0 }), where error is the XHR ProgressEvent, type === 'error'. A problem document is now defined as an object whose title is a string (isProblemDocument). That's the check fail() already relied on, and a ProgressEvent has no title. The new test uses new HttpErrorResponse({ error: new ProgressEvent('error'), status: 0 }). fail() uses the same check, and also handles the poll's TimeoutError.
2. Failures slower than the tick: fixed in 4710035. switchMap is now exhaustMap, so a tick no longer cancels a request in flight, and each request has timeout(10 s). A TimeoutError counts as transient. Test: getJob returns NEVER, and the page gives up. (fakeAsync flushes the remaining timers, so the test asserts on loadProblem$.)
3. Count not reset when refCount restarts the poll: fixed in 4710035. The poll is now defer(() => { this.transientFailures = 0; return timer(0, POLL_MS); }). Test: two failures, unsubscribe everything, resubscribe, one more failure, and the page hasn't given up.
4. Give-up time: fixed in 4710035. Three consecutive transient failures is intended, and the doc now says what that means in time. I measured both cases with a throwaway spec: 20 s after the first failure when polls fail fast, and 30 s when each poll hangs until its timeout.
5. Shared-poll JSDoc attached to the counter: fixed in 4710035. The field, with its own doc, now sits above that JSDoc.
6. Stale doc on submitting: fixed in 888549d. It now says the guard lasts until navigation settles or the submit fails, and why clearing it on the response would reintroduce the double submit.
7. Hand-rolled destroyed flag: fixed in 888549d. It now uses this.destroyRef.destroyed, which Angular 20.3.0 provides (get destroyed() in core.mjs).
8. Stale Loading test comment and helper: fixed in 888549d. showAfterATick, its comment, and the two tick-based error and canceled tests are removed; the immediate test covers both states.

Verification:

• Unit tests: 66 passed (3 new, 2 duplicates removed).
• Script tests: 20 passed.
• ng build, app:sync -- --check and schemas -- --check pass.
• Regenerating the /v1 client produces no diff.
• e2e: both suites pass, 13 assertions.

[matthewberry]

Round-4 re-review at head 888549d. All four round-3 fixes are correct. Nothing was run. I read the code, plus the rxjs 7.8.2 source (timeout.ts, exhaustMap.ts, AsyncAction.ts), the version in the lockfile.

Your question: is the getResults download bound by the 10 s timeout? No. timeout(POLL_MS) wraps only this.service.getJob(this.jobId) inside the exhaustMap callback (job-result.ts:77-78). jobResultResponse$ waits for 'completed', then calls getResults with only retry attached (:107-108). So large downloads aren't cut off, but a hung one is never bounded (finding 2).

The fixes:

• 1: isProblemDocument requires a string title, so a status-0 ProgressEvent is now transient. The test uses a real new ProgressEvent('error') and would have failed before the fix. The backend's problem() (app/routers/v1/problems.py:40, origin/main) takes title as a required argument, so every /v1 problem document has one.
• 2: exhaustMap drops ticks while a request is in flight. timeout(number) is {each} in rxjs 7, and TimeoutError is checked first in isTransient. The inclusive takeWhile still ends the poll on a terminal status, and refCount teardown unsubscribes the timer and the in-flight request.
• 3: defer resets the counter on every resubscribe. The restart test goes through the refCount reset.
• 4–8: done.

Findings:

1. src/app/models/job-result.ts:78: the per-request timeout equals the poll period, so tie-breaking with the timer decides the give-up time. Reasoned from the rxjs source; not reproduced in a browser.
   • In AsyncAction, timer(0, POLL_MS) creates its setInterval after the first tick runs, then reuses it with deadlines at 10, 20, 30 s.
   • Each timeout(POLL_MS) gets its own interval, created just after the tick that started its request. Request 1's timeout was created before the timer's interval, so it fires first at 10 s. But request 2's timeout is due at 20 s plus a sliver, after the 20 s tick, so exhaustMap drops that tick.
   • Request 3 then starts at 30 s, and the page gives up at about 40 s, not the documented 30 s. While the backend is slow, every other poll is skipped.
   • fakeAsync re-queues an interval after its callback runs, which reverses the tie. That's why the spec measures 30 s.
   • Fix: make the timeout strictly shorter than POLL_MS, or schedule each poll after the previous request finishes, e.g. repeat({delay: POLL_MS}).
2. src/app/models/job-result.ts:107: getResults has no bound at all. If the connection stalls after 'completed', retry never sees an error, so isLoading$ stays true and the spinner runs forever. The TimeoutError branch of isTransient can never trigger on this path either. If you want a bound, give the download its own, much longer timeout.
3. src/app/models/job-result.spec.ts:127: the timeout test doesn't pin the documented 30 s. "Gives up on a request that never answers" advances 60 s and only checks that a problem was set, so a 40 s or 50 s give-up still passes. Asserting loadProblem$ is null at 29.9 s and set at 30 s would pin it, though fakeAsync's tie-breaking still differs from the browser's (finding 1).
4. src/app/components/test/test.component.ts:138: the submit handler has its own problem check. It tests error.error?.title for truthiness instead of using isProblemDocument. Export the helper and share it, so the two definitions can't drift apart.
5. src/app/models/job-result.ts:54: the counter could be local to each subscription. Declaring let failures = 0 inside the defer callback, with the exhaustMap pipe built there, ties the count to each subscription. That removes the field and the reset line, so correctness no longer depends on shareReplay holding exactly one source subscription.
6. src/app/models/job-result.ts:48: a timeout and a network failure look the same. fail() reports a poll timeout as status 0. An overloaded backend that answers in 12 s then shows the same "could not be loaded" with status 0 as being offline. A distinct title or detail for TimeoutError would separate them.

[matthewberry]

Round 4, fixed in 18d568d:

1. The timeout tied with the tick: fixed. Requests now time out after 8 s (REQUEST_TIMEOUT_MS), clearly shorter than the 10 s poll, so no tick arrives while a request is open. The test now pins the give-up time exactly: loadProblem$ is still null at 27,999 ms and set at 28,000 ms (polls at 0, 10 and 20 s each time out after 8 s). The doc says the same.
2. The form's own title check: fixed. isProblemDocument moved to models/v1-job.ts, and both JobResult and the submit form use it.
3. A bound on getResults: fixed (one line). timeout(RESULTS_TIMEOUT_MS), 2 minutes. A timeout counts as transient, so a stalled download gets the same two retries as a gateway error, then shows the error.
4. Declined as nits:
   • The counter stays a field reset in defer. Moving it into a closure would change behavior nowhere.
   • A timeout is reported as status: 0. That only feeds the fallback "The job could not be loaded" message, which doesn't show the status.

Verification:

• Unit tests: 66 passed.
• Script tests: 20 passed.
• ng build, app:sync -- --check and schemas -- --check pass.
• Regenerating the /v1 client produces no diff.
• e2e: both suites pass, 13 assertions.

[matthewberry]

Round-5 re-review of 18d568d: the round-4 findings are handled, and there are no new findings. It's good from my side. I read the diff; nothing was run.

• 1: REQUEST_TIMEOUT_MS = 8000 is shorter than POLL_MS. That removes the tie between the timer's tick and the request's timeout, so exhaustMap no longer drops a tick while a request is open. The test now pins the exact boundary: no error at 27,999 ms, an error at 28,000 ms. The arithmetic matches the doc: polls at 0, 10 and 20 s, each timing out 8 s later.
• 2: isProblemDocument now lives in models/v1-job.ts as a type guard, and both the result page and the submit form use it.
• 3: timeout(RESULTS_TIMEOUT_MS) sits before retry, so each attempt gets its own 2 minutes. isTransient counts a TimeoutError, so a stalled download is retried twice and then reaches fail(). The worst case is about 6 minutes, not forever.
• 4: the declines are fine; both were nits.