Frontend bug-fix plan (October 2026)¶

This plan fixes the defects that the architecture review found in the Angular app (Appendix A of architecture-review-2026-10.md). It is planning only: no code changes start until Chris approves the plan and answers the decisions in §8.

Every bug listed below was re-checked against main@eb93caffa (3 Oct 2026) by reading the code, except where it says otherwise.

1. Bugs in scope¶

ID	Bug	User impact	Evidence	Issue
B1	After a SignalR automatic reconnect, project, project-listing and export subscriptions are never re-established	Live updates stop silently after any network blip	`signal-r.service.ts:633` (`withAutomaticReconnect`), `:762`/`:811` (`connected` only goes false on close), `:1140-1190`; server `NotificationHub.cs:99-122`	#3976
B2	The reconnect handler leaves a permanent store listener (`switchMap(() => _currentProjectId$)`)	Every later project change dispatches an extra `loadProject`, which feeds B6	`signal-r.service.ts:1231-1243`	#3976
B3	Unawaited hub `invoke` promises; the `connected` flag sticks at false when the summaries subscribe fails after `start()`	Unhandled rejections; no project subscriptions until reload	`signal-r.service.ts:1153,1165,1181,1187,730-768`	#3976
B4	`addSearch$` is `{dispatch: false}`, but its stream carries the progress, started and error actions	The progress bar stays at 0; an S3 failure leaves the dialog stuck on "Uploading…"	`project-detail.effects.ts` `addSearch$`; `s3-file.service.ts:37-55,151,204`	#3977
B5	The data-export download effect uses `exhaustMap`	A second export started while the first downloads is never downloaded	`data-export.effects.ts:119-147`	#3977
B6	Project navigation race: an exhaust strategy plus a reducer that replaces the per-project loading state, and a "current project" written by whichever load completes	Navigation to project B can hang; or B's URL shows project A	`requests.ts:22`, `project-ui.reducer.ts:75-108`, `project-guard.service.ts:85-185`, `global-request-state.reducer.ts:194-213`	#3977
B7	The request-state reducer discards switch-cancel results and never records exhausted triggers	`dispatchRequest.*Promise` never settles for those requests	`global-request-state.reducer.ts:194-213`	#3977
B8	`search-ui.reducer` has `props ?? 0 < 100`, which always evaluates to "uploading"	Progress never reaches "processing"	`search-ui.reducer.ts:50`	#3977
B9	API effects that use `switchMap` for non-idempotent writes, and effects without `catchError`	Double submits create duplicates on the server; one error permanently kills an effect after NgRx's retry budget	Operator scan of `project-detail.effects.ts` (each effect is re-checked in its PR; some route errors through helper operators)	new
B10	Effects read `err.error`, which is undefined on NSwag's `ApiException` (it has `status`/`response`/`result`)	Error messages and status are lost in failure actions	`project-detail.effects.ts:240,323,352,381,438,485`; `api-client.generated.ts:17572-17582`	new
B11	Constructor `store.select(...).subscribe` with no teardown	Destroyed components keep running callbacks; memory leaks	`study-filters.component.ts:71`, `stage-permissions.component.ts:71`, `question-selection.component.ts:96,112`, `annotation.component.ts:127`	new
B12	The `combineWithNext` helper keeps a live subscription to `selectLoadedProjectId`	After one visit to Studies, every project switch runs a background `getTableData`	`study-table.effects.ts:18-45`	new
B13	`projectDeleted` removes the project entity but not its UI loading state, so `selectMemberStatusForProject` then throws; 11 throwing selectors in 6 files	When a project is deleted elsewhere, the selector throws inside subscriptions and guards	`project.reducer.ts:141-143`, `project.selectors.ts:155-190`, no `projectDeleted` handler in `project-ui.reducer.ts`	new
B14	Production bootstrap debug settings: `withDebugTracing()` always on, `withComponentInputBinding()` registered twice, store devtools always on with `trace: true`	Router events logged in production; trace overhead whenever devtools attach	`src/main.ts:733-746`	new
B15	3 `debugger;` statements in shipped code; `.sort()` with no comparator on objects; `jobs.sort` mutating a memoized selector result	Debugger pauses for anyone with devtools open; keyword options unsorted; selector-cache corruption	`annotation-question-tree-drag-drop.feature.ts:543`, `core/actions/util.ts:231`, `stage-study.viewmodel.ts:96`; `project-index-ui.selectors.ts:48`; `risk-of-bias-job.selectors.ts:70`	new
B16	A 2,387-line unused stub contains 4 real personal email addresses	Personal data committed to the repository (GDPR)	`core/state/entities/project/project-detail.viewmodel.stub.ts` (0 importers)	new
B17	No CI job runs `ng lint`; production builds are non-strict	The lint baseline and editor strictness are not enforced; new defects land unseen	`pr-tests.yml` `test-web`/`test-web-fork` steps; `tsconfig.build.json`	#3981

Measured enforcement gap (worktree with a clean install; web source as of c4ae26d5, so re-measure at slice start):

ng lint with the committed suppression baseline: 22 errors in 6 files (11 @angular-eslint/template/eqeqeq, 11 @typescript-eslint/naming-convention), plus 58 warnings.
Strict compile using the editor config (tsconfig.json; TypeScript 6 makes strict the default), app code only, specs excluded: about 1,423 TypeScript errors.
The largest groups: TS2564 uninitialised properties 421, TS2769 overload mismatches 210, TS2345 argument types 167, TS7006 implicit any 160.
Strict template errors cannot be counted until those are cleared, because the Angular compiler stops after the TypeScript phase. A full strict flip of the production build is therefore not a quick win (see D3).

2. MVP boundary and outcome¶

MVP (releases R0–R2): - realtime updates survive reconnects; - search imports show progress and errors; - exports are never dropped; - navigating between projects always shows the project in the URL; - the production bootstrap is clean; - personal data is removed; - lint is enforced in CI.

Hardening (R3–R4): effect error handling, write semantics, subscription leaks, and selectors that throw.

Not in scope (tracked elsewhere): - state-management convergence and splitting up the god files (#3989); - Angular modernisation migrations; - the zoneless flip (its own programme); - a strict production build (D3); - Auth0-era auth effects (connectAccount$, disconnectAccount$, signupUser$), which retire with #3988 and the migration session; - AF2 internals beyond the one persistence timeout.

Flag decision (every PR): not flagged. These are defect fixes that restore intended behaviour, with no new feature surface. The riskier UI-visible changes (PR-4, PR-6, PR-10) are verified by Chris on a PR preview before merge (C7), which replaces a kill switch.

3. Common acceptance criteria (every PR)¶

#	Criterion	Verification
C1	The regression test is written first and fails on `main`, then passes with the fix	Red and green runs quoted in the PR body
C2	Focused specs pass via `pnpm exec ng test --no-watch --include=<touched>`, plus the three repo-wide guard specs (`syrf-theme.spec.ts`, `hot-hook-zoneless-discipline.spec.ts`, `no-hardcoded-help-urls.spec.ts`)	Commands and output in the PR body; CI `Test Web (Angular)` green
C3	No new ESLint errors and no new suppressions in touched files	`ng lint` (CI-enforced once PR-2 lands)
C4	Touched files gain no new strict-TypeScript errors	`tsc -p tsconfig.json --noEmit` error count for touched files, before and after, in the PR body
C5	Modernisation only in the code being touched (e.g. `takeUntilDestroyed`, `selectSignal`), called out separately; no unrelated refactors	PR review
C6	Any `/docs/` or `/user-guide/` page describing the behaviour is updated; the PR body states the flag decision and links its bug IDs and issues	PR review
C7	Reviews settled on the head (`pr-review-settled.sh` exit 0) and CI green. PR-4, PR-6 and PR-10 also get Chris's acceptance on a PR preview	Settled-gate output; preview sign-off comment
C8	The change works under the current default test providers and introduces no zone-only assumption	Spec suite; zoneless guard spec

4. Releases and pull requests¶

Numeric values marked PROPOSAL need Chris's confirmation.

R0: quick wins and guardrails (two PRs, in parallel)¶

PR-1: production bootstrap and quick fixes (B14, B15, B16; effort S)

Files: src/main.ts, core/actions/util.ts, annotation-question-tree-drag-drop.feature.ts, stage-study.viewmodel.ts, project-index-ui.selectors.ts, risk-of-bias-job.selectors.ts, the stub file (deleted), and the eslint config (no-debugger).

#	Acceptance criterion (condition → result)	Verification
1.1	Production build boots → no router debug tracing is registered and `withComponentInputBinding` is registered once	Spec over the factored bootstrap provider list; preview console check
1.2	Production configuration → store devtools are not registered (or `logOnly` with `maxAge` ≤ 25, PROPOSAL)	Spec over the provider list
1.3	Non-spec source → contains no `debugger;`	`no-debugger` at error level (enforced by PR-2)
1.4	Repository → the stub file is gone, and a guard spec fails if a non-fixture email domain appears in a TS source file	Guard spec (allowlist: `example.com`, `example.org`, `test`)
1.5	Keyword filter options → sorted alphabetically, case-insensitive	Selector spec
1.6	The RoB job summary selector runs → its input array is not mutated	Spec with `Object.freeze` input

PR-2: enforce lint in CI (B17; effort S)

Files: - pr-tests.yml (test-web, test-web-fork) - .github/scripts/test-pr-web-runner-routing.sh, which pins the web lane's steps - the 22 lint errors

#	Acceptance criterion	Verification
2.1	A PR introducing a new web ESLint error → `Test Web (Angular)` and therefore the required `Test Summary` fail	Contract script pins the lint step; one throwaway red commit shown in the PR, then reverted
2.2	`main` after merge → `ng lint` exits 0 with the committed baseline	CI log
2.3	The lint step adds ≤ 3 minutes to `test-web` (PROPOSAL)	CI timing on the PR
2.4	`validate-workflows.sh` and the web routing contract pass; no `runs-on` changes	Script output

R1: realtime and data-flow correctness (three PRs, in parallel; disjoint files)¶

PR-3: SignalR reconnect (B1, B2, B3; #3976; effort M)

File: core/services/signal-r/signal-r.service.ts, plus a new spec harness. That harness is a fake HubConnection that raises onreconnecting/onreconnected with a new connection ID, and it uses the real SignalRStore. The existing wiring spec replaced connected$ and so hid the bug.

Approach: 1. Set connected false on onreconnecting, so the existing subscription pipelines re-run on onreconnected. 2. Re-subscribe project listings on reconnect. 3. Send unsubscribe invokes only while connected, and catch and log every invoke rejection. 4. Take the current project once per reconnect instead of switchMap into the store. 5. In _connect, keep connected true if the hub is connected and retry the failed subscription.

Coordination: #3932 adds an InboxChanged handler to this file. Whichever lands second rebases (trivial).

#	Acceptance criterion	Verification
3.1	A member viewing project P; the hub auto-reconnects with a new connection ID → `SubscribeToProject(P)` is invoked exactly once on the new connection	Unit (harness)
3.2	Same reconnect → `SubscribeToProjectSummaries` is invoked once, and `SubscribeToDataExportJob(j)` once per unresolved job	Unit
3.3	Two browser contexts on P; the reviewer goes offline for 10 s (PROPOSAL) and back → an edit made by the other context appears without a reload within 10 s (PROPOSAL)	E2E, hermetic stack (the `setOffline` pattern exists in `materialized-statistics-two-api.spec.ts`)
3.4	Invokes reject while reconnecting → no unhandled promise rejection	Unit (`unhandledrejection` spy)
3.5	After a reconnect, switching projects → exactly one `loadProject`, from the guard, and none from the reconnect handler	Unit
3.6	`SubscribeToProjectSummaries` fails after `start()` → `connected` stays true and the subscription is retried	Unit

PR-4: search import (B4, B8; #3977; effort M)

Files: addSearch$ only in project-detail.effects.ts, plus s3-file.service.ts, search-ui.reducer.ts, and the create-search dialog and action (add projectId to the payload).

Approach: 1. Make the effect dispatch, using concatMap, so a second import queues instead of cancelling the first upload. 2. Replace the thrown error with an error action, and add catchError. 3. Take the project from the payload. 4. Fix the reducer's operator precedence.

#	Acceptance criterion	Verification
4.1	S3 upload in progress → the dialog's progress bar advances from 0 to 100	Effect spec plus component spec
4.2	S3 rejects the upload → the dialog shows the error and Cancel/Previous are enabled again	Component spec; E2E with a Playwright route that returns 403 for the S3 PUT
4.3	Two imports started back to back → both uploads complete and both import jobs appear	Effect spec
4.4	No project ID in the payload → a `searchUploadError` action, and the effect stays alive	Effect spec
4.5	Progress below 100% → status `uploading`; at 100% → `processing`	Reducer spec
4.6	The import → targets the project in the payload, never the "current project" selector	Effect spec

PR-5: data export (B5; #3977; effort S)

File: core/state/ui/data-export/data-export.effects.ts. Change exhaustMap to mergeMap, keyed by job.

#	Acceptance criterion	Verification
5.1	A second export is created while the first is still downloading → both GETs run and both files download	Effect spec with a delayed first response
5.2	Every export → reaches a terminal state (success or error)	Effect spec

PR-6: project navigation race (B6, B7; #3977; effort L)

Files: project-ui.reducer.ts, project-guard.service.ts, core/requests.ts, global-request-state.reducer.ts/.actions.ts, plus a new projectActivated action. Starts after PR-4, because both touch project loading.

Why not simply switch to "latest wins": loadProject is also dispatched by systematic-searches.component.ts:387, processing-page.component.ts:288 and the reconnect handler. A background refresh of project A would then cancel B's load, which is the same bug in another form.

Approach: 1. One writer for the current project. The guard dispatches projectActivated(projectId) once access is decided. detailLoaded, created and the other completions stop writing loadedProjectId. This keeps all 64 consumers unchanged. 2. Loading state merged per project instead of replaced. 3. LoadProjectRequest strategy merge, so different projects load in parallel; the guard skips a duplicate if that project is already loading. 4. Request-state reducer records switch cancellations and dropped exhaust triggers, so every promise settles. 5. The guard returns an Observable, so the router cancels it when navigation is superseded. Its wait is bounded at 30 s (PROPOSAL); on timeout the state becomes Error with the existing error UI.

Moving fully to a router-derived current project is follow-up work under #3989.

#	Acceptance criterion	Verification
6.1	A's load is slow; the user navigates to B → B renders, and after A's response lands the current project is still B, with URL and data agreeing	Reducer/guard specs; E2E with a Playwright route delaying `GET /api/projects/{A}`
6.2	As 6.1, but B was visited earlier (details cached)	Spec plus E2E
6.3	B's load fails (404 or 403) → the guard denies and the existing not-found/forbidden UI shows	Spec
6.4	A background `loadProject(A)` while viewing B → the current project never changes	Spec
6.5	Switch-cancelled and exhaust-dropped triggers → every `dispatchRequest` promise settles	Request-state spec
6.6	A superseded navigation → no guard subscription is left alive	Spec (subscription torn down)
6.7	Existing journeys → no regression in the E2E smoke run and the project-related specs	E2E run quoted in the PR

R3: effect hygiene (sequential PRs, because they share `project-detail.effects.ts`)¶

PR-7: one error normaliser and complete error handling (B9 error half, B10; effort M)

Approach: 1. A shared toApiError() covering ApiException, HttpErrorResponse and raw bodies. 2. catchError on every API effect in project-detail.effects.ts, each verified individually. 3. warnOnErrorWithRetryOption$ emits a terminal action when the snackbar is dismissed. 4. A bounded wait in the AF2 persistence dispatchAndWait, coordinated with the AF2 owner.

#	Acceptance criterion	Verification
7.1	Any API-calling effect in the file; its request fails → a failure action is emitted, and the next action is still handled	Parameterised spec over the effect list
7.2	An `ApiException` or `HttpErrorResponse` → the failure payload carries status and message	Normaliser unit spec
7.3	The retry snackbar is dismissed → a terminal action is emitted and correlation waiters settle	Spec
7.4	An AF2 save is waiting on a correlation ID → it gives up after 30 s (PROPOSAL) with a visible error	Spec

PR-8: write semantics (B9 operator half; effort M)

Change switchMap to exhaustMap for create/submit, and to concatMap for ordered updates. Candidates, each re-verified in the PR: created$, stageAdded$, questionAdded$, questionSaved$, questionCopied$, projectFavouriteSet$, updateMemberships$, inviteMembersResponse$, updateProjectPermissions$, updateStagePermissions$, resentInvitations$, updateMembership$.

#	Acceptance criterion	Verification
8.1	Create project/stage/question is double-submitted → exactly one entity is created on the server	Effect spec (exhaust semantics)
8.2	Two rapid edits to the same entity → both are sent in order, and client state equals the last edit	Effect spec
8.3	Every changed effect → has a spec pinning its operator semantics	Spec per effect

PR-9: stage-review effects (B9 in review-effects.ts: nextStudy$, commit$, deleteSession$, getStudy$, submitScreening*; effort M). Pending decision D2. Open PR #2643 also touches this file.

Acceptance criteria follow 7.1 and 8.⅛.2, applied to review-effects.ts, plus:

#	Acceptance criterion	Verification
9.1	"Next study" is triggered twice quickly → the server claims at most one study for the reviewer, and the client shows that study	Effect spec plus E2E (review journey)

R4: leaks and resilience (one PR, after PR-6)¶

PR-10: subscription leaks and throwing selectors (B11, B12, B13; effort M)

Approach: 1. Move the 4 components to takeUntilDestroyed/selectSignal. 2. Make combineWithNext take the current project once per fetch. 3. project-ui.reducer handles projectDeleted: clear its loading state, and if it is the current project, navigate to the index with a message. 4. Rewrite the 11 throwing selectors to return explicit Error/NotFound statuses and report to Sentry instead of throwing.

#	Acceptance criterion	Verification
10.1	The user visits stage admin or export, then leaves → no store subscription from those components remains	Component specs (subscription torn down on destroy)
10.2	After one visit to Studies, the user switches projects → no `getTableData` call until the Studies page asks for one	Effect spec
10.3	The project being viewed is deleted elsewhere → a "project deleted" message shows and the app goes to the project index, with no console errors	Spec plus E2E (two contexts)
10.4	Any combination of loading and listing states → no selector throws	Table-driven selector spec

5. Order and critical path¶

flowchart LR
  PR1[PR-1 quick wins] --> R0done((R0))
  PR2[PR-2 lint in CI] --> R0done
  PR3[PR-3 SignalR] --> R1done((R1))
  PR4[PR-4 search import] --> R1done
  PR5[PR-5 export] --> R1done
  PR4 --> PR6[PR-6 navigation race]
  PR6 --> PR7[PR-7 error handling]
  PR7 --> PR8[PR-8 write semantics]
  PR6 --> PR10[PR-10 leaks + selectors]
  PR9[PR-9 review effects, if D2 = yes]

PR-1, PR-2, PR-3, PR-4 and PR-5 can all start immediately, in separate worktrees with one worker each; their files do not overlap.
Critical path: PR-4 → PR-6 → PR-7 → PR-8, because they all touch project loading and project-detail.effects.ts.
PR-10 follows PR-6, because both touch project-ui.reducer.ts.
PR-9 is independent of the rest but waits for D2.
This host is also the CI runner, so workers run only focused ng test includes plus the guard specs, never the full suite.

6. Issues¶

These existing issues map to PRs: #3976 → PR-3; #3977 → PR-4, PR-5 and PR-6 (to be split into three on approval); #3981 → PR-2.

New issues are created on approval for PR-1, PR-7, PR-8, PR-9 and PR-10. Each copies its acceptance-criteria table from this plan.

7. Risks¶

PR-6 changes core navigation state. Mitigations:
single-writer design;
existing consumers unchanged;
E2E with delayed responses;
Chris's preview acceptance before merge.
The SignalR reconnect sequence differs between client versions (@microsoft/signalr ^8). The harness pins the callback order the library documents. E2E 3.3 proves real behaviour.
E2E capacity. The main E2E lane has a known red set and a budget limit. New E2E cases run as targeted specs (e2e/run-local.sh --spec), not the full suite.
Parallel work in the same files. Check open PRs before each slice starts: #3932 (signal-r.service.ts), #2643 and #3288 (review-effects.ts), #2786 (main.ts, the zoneless flip).

8. Decisions needed from Chris¶

D1. Personal data in the stub (B16): delete it only (recommended), or also purge it from git history? A purge rewrites shared history across about 60 worktrees and every open PR, so do it only if policy requires.
D2. Stage-review effects (PR-9): fix them now, or leave them to the stage-review programme owner (handed to Codex on 2026-09-08)?
D3. Strict TypeScript: add a CI ratchet that fails on any new strict error (baseline about 1,423, burned down over time), or defer? A full strict production build is not realistic as a near-term goal.
D4. Preview acceptance: confirm you want to preview-test PR-4, PR-6 and PR-10 yourself before merge.
D5. PROPOSAL values: reconnect recovery ≤ 10 s (3.3), guard wait 30 s (6), AF2 save timeout 30 s (7.4), lint CI cost ≤ 3 min (2.3), devtools maxAge 25 (1.2).