[matthewberry]

What

JobResult.statusResponse$ was a cold stream and jobResultResponse$ gated on a second subscription to it through skipUntil. That is two independent 10 s polls racing for the terminal status. When the source's request came back "completed" before the gate's, the inclusive takeWhile completed the source with the gate still closed, jobResultResponse$ completed without ever emitting, and the page stayed on the loading screen. Every statusResponse$ | async in a template also ran its own poll.

How

• statusResponse$ now carries shareReplay({ bufferSize: 1, refCount: true }): one poll for every consumer, stopped when nothing is listening.
• jobResultResponse$ reads completion off that same stream with filter + take(1); the skipUntil gate and its second subscription are gone.
• isLoading$ and resultLoaded$ keep their types and meaning, so no result page or loading component changes.

This is the shape mutagenesis-frontend and crispr-copies-frontend already moved to, for the same reason (their job-result.ts documents the race).

Verification

src/app/models/job-result.spec.ts pins the lifecycle: loading clears and the result is fetched exactly once on completion, loading stays up with no fetch on error, an already-complete first poll works, and several subscribers share a single status request.

npx ng test --watch=false --browsers=ChromeHeadless --include='src/app/models/job-result.spec.ts'

I did not reproduce the race against a live job; the finding is from reading rxjs 7's skipUntil (gate subscribed before source) and the two repos that already hit and fixed it. Result pages were not exercised in a browser.

Found while surveying the moleculemaker and ibiofoundry frontends after moleculemaker/CLEANDB-frontend#59.

Run in this repo

4 of 4 pass under ChromeHeadless. The full spec build does not compile on main today: the job-tab spec imports a .template file, and the test styles in angular.json point at PrimeNG resources/ paths that the installed PrimeNG 20 no longer ships. So this spec was run with a temporary spec tsconfig limited to this file and those style entries dropped for the run. Neither change is in this PR.

[matthewberry]

Checked: I reproduced the race outside Angular with rxjs 7. With the old shape (skipUntil over a second subscription to the cold poll), making the source's terminal status request resolve before the gate's yields 0 result emissions and the stream completes; swapping the latencies yields 1. With the new shape (shareReplay({bufferSize: 1, refCount: true}) + filter + take(1)) both orderings yield exactly 1 result emission, and the status endpoint is called 3 times per job instead of 4. JobStatus.Error, used only by the new spec, exists in this repo's generated enum.

The docker and E2E on Chrome failures are the pre-existing ones already documented on #7 and fixed in #8; docker is green on #8.

Findings: none. One property worth knowing about rather than changing: with refCount: true, if every subscriber drops before the job reaches a terminal state (navigate away and back to a route that reuses the same JobResult instance), the poll restarts from timer(0). That is the desired behavior for a live job and it costs one extra request.

[matthewberry]

Review of 7240024. I read the diff, the PR body and the earlier "Checked" comment. I'm not re-raising its refCount restart note. I have one finding, reproduced.

Verified

• The race fix works on the real page. I rendered TestResultComponent in TestBed under fakeAsync, with a stub TestService. The stub returns Processing for the first 25 s of fake time and Completed after that. getResult is called exactly once, and <app-loading> disappears once the result arrives. The four new job-result.spec.ts specs pass under ChromeHeadless. I ran them the way the body describes: a temporary spec tsconfig, PrimeNG resources/ styles dropped, and neither change committed.
• Requests drop by half. Status requests at each 10 s poll on the result page (the currentPage is set to the results tab):

main                                   0,0,0,0, 10000×4, 20000×4, 30000×4   → 4 per poll
this PR                                0,0,     10000×2, 20000×2, 30000×2   → 2 per poll
this PR, <app-loading> not rendered    0,       10000,   20000,   30000     → 1 per poll

• I checked rxjs 7.8.2 in node_modules. shareReplay({refCount: true}) passes resetOnComplete: false, and share resets on refCount zero only when the source has neither completed nor errored. So a subscriber that arrives after the terminal status gets the replayed status and does not start a new poll.

Finding (reproduced): the loading screen still runs its own poll, but the new comments say it shares this one.

• LoadingComponent extends JobResult (loading.component.ts:26), so each <app-loading> is a separate JobResult instance with its own timer(0, 10000). test-result.component.html renders <app-loading> while isLoading$ is true, and its value$ subscribes to that instance's statusResponse$. That is the second request in each pair above.
• Two comments in the PR say otherwise:
  • the statusResponse$ docstring: "every consumer (template | async, jobResultResponse$, the loading component) rides this one subscription";
  • the spec comment in runs one poll no matter how many consumers subscribe: "The header, the loading screen and the result pipeline all read the status."
• That spec subscribes to a single instance three times, so it can't see the second instance.
• The correctness fix doesn't depend on this, because the race was inside jobResultResponse$. Two things do, though:
  • This is the template that new frontends copy. Its comment would tell the next reader the double poll is gone.
  • The two instances can disagree for up to one interval about whether the job has finished.
• Two options:
  • narrow the docstring and the spec comment to "every consumer of this instance";
  • pass the parent's statusResponse$ into <app-loading> as an input instead of having it extend JobResult. That's a bigger change and may be better left to a separate PR.

[matthewberry]

Re review 5577463890: reproduced, and fixed by narrowing the comments in 18a2ba4. The statusResponse$ docstring and the spec comment now say the poll is shared per JobResult instance, and that <app-loading> (which extends JobResult) still runs its own poll while it's shown. That's 4 to 2 requests per 10 s, as you measured.

The structural fix (pass the parent's statusResponse$ into <app-loading> rather than having it extend JobResult) is left for the template refresh, #11. I've noted it there.

[matthewberry]

Re-review of 18a2ba4. My finding is resolved, and I have nothing new.

• The docs-only commit is accurate. It changes only the two comments (git show --stat 18a2ba4: job-result.ts and job-result.spec.ts, 6+/3−). "One shared poll per JobResult instance" and " … still runs its own poll while it is shown" match what I measured: 2 requests per 10 s while loading, and the loading screen's poll stops when it is removed, because of refCount: true.
• The structural follow-up is recorded on #11. It says to pass the parent's status into <app-loading>, so it isn't lost.