Round-2 programme review RT — active reviewer tracking, claims and presence (verbatim report)
Temporary planning review record. The report below is reproduced verbatim as returned by the independent read-only reviewer (Plan agent, Opus model, launched 3 October 2026 about 15:00 BST at Chris's request to review in-flight programmes and their integration with the plan). Only this front matter and note were added. Line numbers refer to the package as it stood when the reviewer read it. Resolutions are in the round-2 resolution matrix.
I'm the reviewer for active reviewer tracking (RT). Baselines: main at 0f5c61073, cluster-gitops at 7c377aa4, plan package as of 09:47 on 3 October, round-2 verifier V2. I made no writes and ran no database queries.
Path roots used below: MAIN = /home/chris/workspace/syrf/main; PMC = MAIN/src/libs/project-management/SyRF.ProjectManagement.Core; PMM = MAIN/src/libs/project-management/SyRF.ProjectManagement.Mongo.Data; API = MAIN/src/services/api/SyRF.API.Endpoint; WEB = MAIN/src/services/web/src/app; PKG = /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10; LEDGER = /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/review-form-owner-decisions-2026-10-02.md; GITOPS = /home/chris/workspace/cluster-gitops/syrf/environments.
1. Verdict¶
The plan treats active reviewer tracking as a small amendment: re-key claims to study + form + reviewer in R2b, and optionally switch tracking on for R4a. The code says otherwise.
- It is the only claim and capacity mechanism, and it is off everywhere real. With its fleet-wide flag off, which is the case in every deployed environment, no claim is ever created, annotation and screening saves are unguarded, and
EnforceAnnotationTargetdoes nothing. The E2E stack runs with the flag on. - Reconciliation is excluded at every layer. So "tracking enabled" cannot deliver RA1's rule that active-work tracking stops two reconcilers editing the same study. Yet that is one of the two routes the plan accepts for R4a production.
- Turning it on is a statistics transition, not a flag flip. Enabling it alters FEAT-024's durable reviewer mode, and the code that moves that mode has no caller (open item M15). Staging already runs FEAT-024 writes, so switching tracking on there today would make capacity writes fail with a typed 503.
- It is keyed by stage everywhere. The reservation key, the two-activity page, connections, the presence index, hub method signatures, DTOs, scheduled commands and the statistics derivations all assume a stage.
- Its release rules predate server drafts. R2a, not only R2b, therefore has to change it.
The direction is right: one claim per study and form, existing machinery extended, existing owners. But as written, several criteria can't be tested in production, R4a can pass its gate without meeting a confirmed decision (Blocker RT-01), and the Study summary projection plus R0's floor would leave capacity guards wrong while legacy and canonical data coexist.
The five most important changes: 1. Replace X-TRACK with a reconciliation-task editor claim that works with tracking off, and add RA1 acceptance criteria. Correct Q-25's premise and give claims an explicit production route, owned with the FEAT-024 owner. 2. At F1, freeze a claim contract: typed claims per scope (form, profile, task, requested review), shared across stage tabs and released when the last one closes, with versioned hub methods, DTOs and commands. "Re-key in R2b" is not enough. 3. Key the Study summary projection by form, add per-reviewer markers, put the tally merge into R0's floor, and add the tracking writers and readers to R0's inventory. 4. Decide whether a server draft holds the reviewer's place. Build the draft tab lease on tracking's connection model, without depending on the tracking flag. 5. Run the affected E2E flows in both tracking modes, and add concurrency, reconnect, failover and scale criteria.
2. Current state¶
| Component | State | Evidence | Known gaps |
|---|---|---|---|
Flag activeReviewerTrackingEnabled; effective value is ActiveReviewerTrackingAvailable = flag && signalRActive |
Merged. Default false. Unset in production, staging and preview. True in the E2E stack. Read by API and PM. | MAIN/src/libs/kernel/SyRF.SharedKernel/Settings/FeatureFlags.cs:24-36; MAIN/src/charts/syrf-common/env-mapping.yaml:1596-1605; GITOPS/production/web/values.yaml:61-72, /staging/web/values.yaml:48-62, /preview/preview.values.yaml:54-56; API/appsettings.e2etest.json:129-131, MAIN/src/services/project-management/SyRF.ProjectManagement.Endpoint/appsettings.e2etest.json:82-84 |
Never enabled in a deployed environment (Chris, recorded in MAIN/docs/planning/review-eligibility-policy.md:743-746). Fleet-wide only: per-stage tracking (#3876) was deferred on 1 Oct. The PM host ignores runtime toggles (#3360). |
SlotReservation, the claim on Study |
Merged: #2467 (30 Aug); typed v1 claims in #3579 and #3719. Dark. | PMC/Model/StudyAggregate/Study.cs:215; PMC/Model/StudyAggregate/SlotReservation.cs:23-163; ReviewActivity.cs:4-8 |
Natural key is (InvestigatorId, StageId). A v1 page holds one screening and one annotation claim on one timer; legacy v0 claims are untyped. Claims are only created when tracking is on. |
| Capacity guards: atomic claim pipeline, guarded annotation save, guarded screening save, direct-navigation claim | Merged. Inert when tracking is off. | PMM/Repositories/StudyRepository.cs:2024-2140, 2502-2526, 2913-2916; API/Controllers/ReviewController.cs:986; API/Services/SubmitAnnotationSessionService.cs:321-337 |
No guard at all in production. "Already owns a place" checks embedded sessions only. Screening capacity uses the annotation target, a defect decision D6 already rules out (review-eligibility-policy.md:704-718, :1085). |
ReviewerPresence (pmReviewerPresence) |
Merged. Dark. | PMC/Model/ReviewerPresence.cs:6-144; PMM/Repositories/ReviewerPresenceRepository.cs:46-79 |
Unique "current presence" index is keyed by stage. AnnotationSessionId points at the embedded session. Records are kept indefinitely. No admin view (#2470's admin-visibility item is open). |
ReviewSessionConnection plus hub methods (Join, Leave, Started/Stopped annotating, Heartbeat, presence subscription) |
Merged. Every method is gated by the tracking flag. | PMC/Model/ReviewSessionConnection.cs:15-90; API/SignalR/NotificationHub.cs:143-1090 |
A connection holds one study/stage context at a time (:413-457). Hub methods can't take extra parameters without breaking older clients (:782-787). Reconciliation is excluded. Snapshots name claim holders to every member who can view studies (#3892). |
| Timers and cleanup consumers (idle mark 5 min, idle removal per stage timeout, default 120 min, suspension 2 h, liveness 2 min) | Merged. Consumers keep cleaning up whatever the flag says. | PMC/Model/ReviewSessionConstants.cs:15-26; MAIN/src/services/project-management/SyRF.ProjectManagement.Endpoint/Consumers/*SessionConsumer.cs, CheckConnectionLivenessConsumer.cs; NotificationHub :1568-1606 |
Scheduled commands are keyed by stage and sit in Quartz for up to 2 h. Nothing catches an orphaned claim if a scheduled message is lost. A dirty form holds its place indefinitely (MAIN/docs/planning/session-copy-review.md:264-283). |
| Access result shared by REST and hub (ADR-008); denial reasons 1–7 | Merged. ADR still Draft. | API/SignalR/Dtos/StudyReviewAccessResult.cs:14-92; MAIN/docs/decisions/ADR-008-review-access-state-and-timing.md |
No step, task or completed-stage denial reasons yet. |
| Typed admission, D8 claim release, claim-revoked outbox and event | Merged behind reviewEligibilityPolicy (off) and tracking. |
PMM/Repositories/StudyRepository.ActivityReservations.cs:18-115; #3719, #3720 |
Programme paused 25 Sep. #3746 open; #3742 is an empty draft; #3741 is a draft. |
| FEAT-024 coupling: durable reviewer mode and its epoch; reservation statistics; fold protocol 4 | Merged. FEAT-024 writes on in staging, off in production. | MAIN/.claude/rules/materialized-stats.md ("Durable reviewer-mode epoch"); PMC/Services/ProjectStatistics/Control/ProjectStatisticsWriteEpochLifecycleService.cs:115-118, 370-399; GITOPS/staging/api/values.yaml:110, /staging/project-management/values.yaml:85; #3730, #3738, #3763, #3820, #3838, #3926 |
AdvanceModeEpochAsync has no production caller (M15, MAIN/docs/features/materialized-project-statistics/phase0-mutation-ownership-matrix.md:1617-1633). The "enough reviewers" counts depend on reservations and the stage target. |
| Web presence store and reviewer banners | Merged. Dark. | WEB/study-presence/study-presence.store.ts; WEB/stage/stage-review/stage-review-presence-policy.ts:12-18 |
Only the stage-review route joins presence. Reconciliation mode turns it off. No admin sessions table on main. |
| Reconciliation concurrency | Legacy, merged | PMC/Services/StageReviewService.cs:67-70, 153, 259-262; ReviewController :1449-1452 |
No editor exclusion. Reconciliation "Next" is an unclaimed random pick. One shared session per study and stage, overwritten. |
| Production over-allocation report | Open | #2446 open; #2565 closed with synthetic test coverage only; epic #2470 open, environment acceptance outstanding | The #2467 fixes run only when tracking is on. |
| Tracking design documents | Draft or stale | docs/features/review-session-model-redesign.md (implemented apart from the admin dashboard); signalr-active-reviewer-tracking.md:102-160 still describes the old ActiveReviewSession; the April overhaul, handover and copy documents |
Not a sound baseline for F1 sign-off. |
3. Findings¶
| ID | Sev | Location | Finding | Evidence | Recommended change |
|---|---|---|---|---|---|
| RT-01 | Blocker | PKG/integrated-plan.md:606, :741, :905; PKG/contracts.md:330; PKG/open-questions-and-assumptions.md:108 (E6); PKG/acceptance-criteria.md:262-278 |
Turning tracking on gives reconciliation no protection. So X-TRACK's first route ("tracking enabled") cannot satisfy RA1, which says pool reconciliation "uses existing active-work tracking to prevent two reconcilers simultaneously editing the same study". It would still let R4a pass its production gate. No R4a criterion tests editor exclusion. This shows round-1 B-29's resolution falls short. | Reconciliation skips tracking: StageReviewService.cs:67-70, :153; ReviewController.cs:1449-1452 ("Reconciliation flows do not use slot reservations"); stage-review-presence-policy.ts:17 (&& !reconciliationMode); stage-reconcile.component.ts makes no presence calls; reconciliation "Next" picks unclaimed (StageReviewService.cs:259-262). LEDGER:255-258. |
Replace X-TRACK with a reconciliation-task editor claim that works with tracking off ("X-RECLAIM"), owned by L6 with the presence owner and frozen in C9/C7 at F4. Reword E6 from "if tracking stays off" to "always". Add AC-R4a-14 and -15 (§5.3 below). Remove "tracking enabled" as a sufficient R4a prerequisite. |
| RT-02 | Major | Q-25 (PKG/open-questions-and-assumptions.md:52; decided as recommended at :38 and in decision-register §1.11 :195); C7 :327; R2b :452-457; R3a :510-514 |
Q-25's tracking line rests on a false premise. It says tracking is "not needed for slot-reservation claims". In fact claims, capacity guards and typed admission exist only when tracking is on. With it off (every deployed environment): no claim is created, saves are unguarded, EnforceAnnotationTarget has no effect and typed admission never runs. So R2b's claim re-keying, R3a's "reservation" admission action and every capacity promise do nothing in production, and the plan has no production route for them. This is not a request to reopen per-flag routes, only to fix the tracking line. |
StageReviewService.cs:220-221, :331-335, :631-634; NotificationHub.cs:498-517 (join granted with no reservation); SubmitAnnotationSessionService.cs:321-337 ("Without tracking there is no capacity guard and no presence write"); ReviewController.cs:986; StudyRepository.cs:2056-2073, :2523-2525; FeatureFlags.cs:24-36; flag unset in GITOPS (row 1 of §2). #2446 open. | Correct Q-25's tracking cell. Add a production prerequisite, "X-CLAIMS", for R2b claim behaviour, R3a's reservation column and any capacity promise, with the activation route from RT-03. Put the corrected route to Chris (Q-RT1). |
| RT-03 | Major | integrated-plan.md:741 (X-TRACK owner "Presence owner / L6", evidence "Claim tests"); §8 :980 |
Turning tracking on is a fleet-wide change to capacity and statistics semantics, not a presence-owner flag flip. Once FEAT-024's global control row exists, a process whose flag disagrees with it fails capacity writes with a typed 503. The code that changes the durable mode has no caller. Staging runs FEAT-024 writes and serving, so it can't even rehearse activation; that the control row exists there is inferred, not read (UNVERIFIED). The mode can't be scoped to pilot projects, and the PM host ignores runtime toggles. | materialized-stats rule; ProjectStatisticsWriteEpochLifecycleService.cs:115-118 (refuses a disagreeing mode), :370-399 (AdvanceModeEpochAsync, no caller by grep); matrix M15; MAIN/docs/features/materialized-project-statistics/async-point-fold-design.md:377-392 and invariant 13 at :1682-1684; GITOPS staging values (row 9); #3876; #3360. |
Make X-CLAIMS jointly owned by the presence owner and the FEAT-024 owner. Evidence: (a) the M15 transition wired and rehearsed on staging, or #3876 delivered in a form that works for shared forms (RT-12); (b) API and PM switched together in one static configuration change; © the load and failover runs in §5.3. Add FEAT-024 as an input to X-CLAIMS in §6.3. |
| RT-04 | Major | PKG/acceptance-criteria.md:49-50 (AC-ALL-01/02); plan §9 item 4 |
E2E runs with tracking on; every deployed environment runs with it off. "Existing specs pass with flags off" therefore proves the tracked behaviour and never production's untracked behaviour. Once R2a lands, its E2E journeys will also exercise claim paths that A-21 says R2a doesn't touch (RT-05). | appsettings.e2etest.json (API:129-131, PM:82-84); WEB/../assets/data/appConfig.e2e.json:29; production defaults false (API/appsettings.json:33, PM appsettings.json:35); MAIN/e2e/tests/allocation-capacity-integration.spec.ts:38. |
State the tracking mode in AC-ALL-01/02. Run the affected review-flow specs in both modes (a Playwright project matrix, not a second stack) for R2a, R2b, R3a and R4a. |
| RT-05 | Major | PKG/open-questions-and-assumptions.md:203 (A-21: R2a stays "free of claim, tally and allocation joins"); R2a criteria :150-172 |
R2a is not free of claim joins. Every capacity path decides "this reviewer already owns a place" from embedded ExtractionInfo.Sessions; canonical sessions live outside Study. A returning reviewer with a canonical session is then treated as new: a claim attempt, a possible AtCapacity refusal against their own work, or their study offered back to them as new work. Releasing the claim on first save must move to the first explicit Save or Complete. Presence links point at embedded sessions. Dirty and idle state follow AF2's dirty flag, which autosave will change. |
Own-place checks: StudyRepository.cs:2082-2087; Study.cs:427-430; NotificationHub.cs:641-642; StageReviewService.cs:676-681. Per-reviewer pool filters: PMM/Filters.cs:201, 227, 256-260, 273-275, 305, 398-402. Claim release on save: Study.cs:268-308; SubmitAnnotationSessionService.cs:416-424, :653-671. Presence link: ReviewerPresence.cs:87. Dirty wiring: WEB/stage/stage-review/stage-review.component.ts:1543-1568. |
Rewrite A-21's basis: R2a keeps claims stage-keyed (one stage per form) but must change own-place detection, claim release on save, presence linkage and dirty semantics. Add AC-R2a-20 to -23. Presence owner signs the R2a adapter at F1. |
| RT-06 | Major | C1 :108; C7 :325; domain-model :159; E20 :122; AC-R2a-12 :165 |
The Study summary projection is specified as "bounded per-bound-stage tallies and flags". Two problems. (1) Counts can't support the per-reviewer checks that capacity guards and pool filters run: "already owns a place", "not started by this reviewer" and reconciler self-exclusion. (2) A copy per bound stage turns every binding change in R2b into a rewrite of every Study in the project, and Study can't compute the copies because the binding isn't stored on it. AC-R2a-12 promises correct capacity guards and can't pass as specified. | SessionTallies is computed from embedded data and grouped by stage (PMC/Model/StudyAggregate/ExtractionInfo.cs:31-80); per-reviewer predicates (Filters.cs above; StudyRepository.cs:2076-2087); bindings live in StageSettings (domain-model §3.1). |
In E20, at F1: key the projection by form and by profile. Per form: allocated count, qualifying count, and the IDs of reviewers holding a current session (bounded by target plus extras). Canonical capacity guards and pool filters map the route stage to its bound form and query by form. Legacy per-stage SessionTallies stay legacy-only. |
| RT-07 | Major | R0 integrated-plan.md:275-301; C16 contracts.md:548-566 |
R0's floor promises old binaries tolerate new fields, but capacity correctness needs reader logic in the floor too. SessionTallies is recomputed on every load and stored values are ignored; the claim pipeline recomputes the allocated total as candidates plus reservations. A binary that doesn't merge canonical counts writes them away on any whole-Study save or claim, and capacity guards then read the wrong number. That includes any rollback image that lacks the merge. This shows round-1 B-02's resolution falls short. |
ExtractionInfo.cs:31-80; class-map comment at StudyRepository.cs:3217-3227 ("stored values are ignored"); pipeline formula at StudyRepository.cs:2913-2916; whole-Study saves in the hub (NotificationHub.cs:295, :665). | Add an R0 MVP item: the SessionTallies getter and claim pipeline already merge the form-keyed counts and per-reviewer markers, inert until a canonical writer exists. Add AC-R0-06: R0 and R2a binaries alternate writes on one Study and the tallies stay correct. |
| RT-08 | Major | R0 item 2 integrated-plan.md:283-288; PKG/migration-adoption-rollback.md:27-45 |
The writer and reader inventory leaves out tracking's Study writers and readers. Writers: the hub's join, leave, dirty/clean, disconnect and prior-study release; the PM idle, suspension and liveness consumers; the claim pipelines and typed admission; the direct-navigation claim; the screened-reservation release; the guarded settings save's "Apply anyway" revocation; the reservation restore on session deletion. Readers: the presence snapshot, and FEAT-024's availability calculators. Without them a stage-keyed claim can be made against a canonical shared form (double counting), and snapshots report the wrong allocation. | NotificationHub.cs:143-321, 413-457, 478-995; StageReviewService.cs:308-320, 509-541, 608-673; StudyRepository.ActivityReservations.cs:18-115; ReviewController.cs:205-208; API/SignalR/Dtos/StudyReviewPresenceSnapshot.cs:66-104; #3730. |
Add these rows with a route, refuse or adapt decision for each. AC-R0-02 gets one test per writer, including a stage-keyed claim on a canonical form being refused or translated. |
| RT-09 | Major | C5 drafts contracts.md:250-256; E21; R2a user value :379-380 and pilot exit "zero lost work" :445 |
Server drafts conflict with tracking's release rules. A clean tab close or leave removes the reservation even when the form is dirty. Idle and suspension expiry then free the place for someone else. With EnforceAnnotationTarget on, the hard lock has "no dirty form exception". A reviewer with a saved server draft can therefore be refused AtCapacity and be unable to submit it. That rule was decided when unsaved work lived only in the browser. |
NotificationHub.cs:218-231, :823-826; MAIN/docs/features/signalr-active-reviewer-tracking.md:345-355; session-copy-review.md:84-88 ("Until you save, your spot is temporary"). |
Ask Chris (Q-RT2). Then write into E21 and E18: release paths read whether a draft exists, in the same snapshot, and keep a draft-backed claim as decided; autosave never writes Study; the hub's "dirty" means C5's draft-changes flag, not AF2's pristine state. |
| RT-10 | Major | domain-model :91 ("tab lease"); E21 :123; AC-R2a-06 :159 |
Extends V2-18. The tab lease has no holder identity, liveness or takeover rule, and it must work with tracking off. Tracking already tells tabs and devices apart (one current presence, many connections, dirty state per connection, 2-minute heartbeat liveness). But a reconnect arrives with a new connection ID, and all of it sits behind the tracking flag. | ReviewSessionConnection.cs:55-90; NotificationHub.cs:323-345, :463-475, :533-538; FeatureFlags.cs:35-36. | Hold the lease by a stable client tab ID (from sessionStorage), recorded on both the draft and ReviewSessionConnection. Renew it by a REST heartbeat, following the existing lease pattern at API/Controllers/BulkPdfUploadController.cs:368-375, or by the hub heartbeat when available. Other tabs are read-only, with an explicit "take over" that ends the old lease. Replace AC-R2a-06 with lease criteria. |
| RT-11 | Major | C7 :327; E18 :120; domain-model :159, :164; R2b, R3a |
"Re-key claims to study + form + reviewer (+ profile)" understates the change. A v1 typed page holds exactly one screening and one annotation claim, but R3a stages can hold several form and profile steps (the "facts → screening → outcomes" scene). A claim shared by two stage tabs must stay alive until the last one closes. Every surface is keyed by stage: hub signatures, snapshot DTOs, connection and presence keys (including a partial unique index), scheduled commands held in Quartz for up to 2 h, and claim-revocation intents. | SlotReservation.cs:67-163; NotificationHub.cs:478, :782-787; StudyReviewPresenceSnapshot.cs:21-158; ReviewerPresenceRepository.cs:46-60; IRemoveSuspendedSessionCommand, IMarkSessionIdleCommand, IRemoveIdleSessionCommand, ICheckConnectionLivenessCommand (all carry StageId); StudyRepository.ActivityReservations.cs:93-97. |
Freeze a claim contract at F1. A claim is {kind (form slot, profile slot, requested review, task editor, query editor), scope ID, route stage, route step, reserved at, allocation regime}, unique per (study, kind, scope, reviewer), and released when the last page using it ends. Capacity claims (form slot, profile slot, requested review) live on Study so the atomic guard still works; editor claims live on their own aggregates. Claims key on form identity, not form version. Version the surfaces: add new hub methods rather than parameters until MinUiVersion moves (NotificationHub.cs:109); additive DTO fields; new command contracts, with the old handlers kept for at least the longer of the suspension grace and the idle timeout; migrate the presence index by creating the new one, reading both, then dropping the old. Legacy v0/v1 pages stay for legacy scopes. |
| RT-12 | Major | Q-28 :67; C6 :291 |
Tracking's per-stage settings have no rule for shared forms: EnforceAnnotationTarget, IdleSessionTimeoutMinutes, MaxInProgress, and the per-stage tracking proposed in #3876. Nor does the plan say whether a shared incomplete session counts once or once per bound stage against MaxInProgress. |
PMC/Model/ProjectAggregate/StageEntity/Stage.cs:18, 76-81, 290-306; StageReviewService.cs:40-52; ReviewController.cs:625, :752, :789. |
Extend Q-28 (Q-RT4). Shape #3876 as a binding-scope setting, not a per-stage one. Add AC-R2b-07 and -08. |
| RT-13 | Major | C7, C9; domain-model :106 (AdditionalReviewRequest); AC-R4a-06 :271 |
SF4 makes the form target a minimum, but today one number (SessionCountTarget) is both minimum and cap. The plan never separates capacity from target, and never says which sessions hold a place: a draft with a claim, saved incomplete, completed, withdrawn, or an older version that no longer qualifies. RA5's requested extra reviewer is never offered the study by "Next" (the pool filter drops studies at target) and is refused by the capacity guard when enforcement is on. |
LEDGER:414-422; Filters.cs:355-373; StudyRepository.cs:2110-2119; StageReviewService.cs:650-652. | Add to C7: an optional capacity cap (a stage or route policy) separate from the form's minimum target; the set of sessions that hold a place (draft-backed claim, saved incomplete, completed, Needs updating; withdrawal frees the place); a requested-review claim written to Study by the request command, which lets exactly that reviewer past the pool and capacity filters. Add AC-R4a-16. Q-RT3. |
| RT-14 | Major | C10 disclosure contracts.md:424-428; BL1, VS1 |
Presence snapshots are a disclosure channel the policy doesn't list. They carry the investigator ID of every reviewer holding a reservation and go to every member who can view studies, whatever the stage's blinding. If reconciliation task claims are later added to presence, candidates would also learn who is reconciling. Reservation timing could also link stable aliases to real reviewers. The UI shows no names; the payload does. | StudyReviewPresenceSnapshot.cs:86-96; NotificationHub.cs:1044-1074; #3892 (narrowed to members, still all members). | Add realtime presence to C10's channel list: people who aren't holders get counts and their own claim only; identities need a capability and never cross BL1. Add AC-T-08. Q-RT6. |
| RT-15 | Major | domain-model §5 :172-180; C1 :116-122 |
Separate from V2-19. The transaction table omits claims and presence. The first explicit Save or Complete must also release the claim and replace the presence record. Next, direct access and admission are claim transactions that carry FEAT-024 statistics (#3738) or fold entries (#3926). Task claims and assignment release need rows. FEAT-024's pinned command-count tests already count today's presence write. | SubmitAnnotationSessionService.cs:400-470, :653-671; StudyRepository.ActivityReservations.cs:36-105; materialized-stats rule, "Command budgets are pinned". | Add rows: first explicit Save/Complete also releases the claim and closes and opens presence; claim, release and expiry write the Study claim set plus the FEAT-024 part or fold entry; task claim and release write the claim plus assignment state. Name the budget tests to update. |
| RT-16 | Minor | AC-R2b-03 :180 |
"Releasing it frees both" tests the wrong behaviour. Closing one stage tab must not release the claim the other tab still uses. | NotificationHub.cs:167-179 (the remaining-connection check is per study and stage). | "Two tabs through stages A and B hold one claim; closing either keeps it; closing both releases it once; a third reviewer is admitted only then." |
| RT-17 | Minor | Q-20 :61; U5 |
The publish-pause "active-reviewer warning" needs a form-scoped answer to "who has form F open now". Presence is per study, keyed by stage, never aggregated, and off in production. | signalr-active-reviewer-tracking.md:27; PKG/ui-coverage-comparison.md:89. |
Record the dependency in Q-20 (presence and connections carry the form ID, tracking on), or drop the warning from the minimal version. |
| RT-18 | Minor | R3c :551-563; AC-R3c |
Completion ignores outstanding claims. A stage can complete while a reviewer holds a claim but has not autosaved yet; their first autosave then hits a Completed stage. A shared form may still be reachable through another stage. | LEDGER:737-760; claim-revocation outbox (#3720). | Completion withdraws that stage's references through the outbox; the claim survives if another bound stage still uses it; the client keeps its unsaved changes. Add AC-R3c-08. |
| RT-19 | Minor | R2c :463-479 |
Publishing a version that lowers the target, or switching enforcement on, leaves extra claims. D6's conflict report and "Apply anyway" exist only for stage-settings saves. | review-eligibility-policy.md:1085; #3579. |
Send target reductions through D6's flow, revoking the most recently acquired claims first. Add AC-R2c-10. |
| RT-20 | Minor | C10 revocation rule :413-415; R1c |
Reviewers whose access is revoked keep their claims until liveness or the 2-hour grace releases them, so a bulk group change keeps holding capacity. | signalr-active-reviewer-tracking.md:29-35. | Revoking a grant releases temporary claims through the outbox; drafts are kept. Add a test in R1c. |
| RT-21 | Minor | E32 :134; migration principle 8 :73-77 |
Engagement history is personal data kept indefinitely, and it is missing from the erasure and retention plan: pmReviewerPresence (documented as "preserved indefinitely") and pmReviewSessionConnection. |
ReviewerPresence.cs:6-23; nothing in account-deletion code references them (grep). | Add both to E32 with retention rules, and to adoption manifests if they become evidence of exposure. |
| RT-22 | Minor | C17 copy contract :591-593; plan §9 item 9; U13 |
The copy contract leaves out the tracking vocabulary ("review slot", "released", "offline", "enough reviewers"). Today's copy says "Saving secures your slot", which becomes ambiguous once autosave and explicit Save coexist. | session-copy-review.md:84-98, :311-323. | Add the slot terms and the draft/claim rule (RT-09) to the copy contract; add slot states to U13. |
| RT-23 | Minor | Q-24(d) :65; AC-R3a; X-ELIG :740 |
D6's screening-capacity defect is still on main, and R3a has no criterion for capacity under a profile's collective rule. Separately: because claims exist only when tracked, and the eligibility flag sends every claim through typed admission, production probably holds no legacy untyped reservations, and enabling eligibility before tracking would keep it that way (count UNVERIFIED). |
review-eligibility-policy.md:704-718; StudyRepository.cs:2502-2526, :1857-1861. |
Add AC-R3a-11 (screening capacity follows the profile rule). Replace X-ELIG's migration rehearsal with an authorised count-only check per environment, and switch eligibility on before tracking. |
| RT-24 | Minor | (no plan text) | Nothing cleans up an orphaned claim if a scheduled removal is lost; this has happened before (Quartz lost its RabbitMQ connection after a pod restart). Claims made dirty and then left when the flag is turned off persist and count again when it is turned back on. With claims becoming central (draft-backed, editor exclusion), a leak blocks capacity indefinitely. | RemoveSuspendedSessionConsumer.cs; docs/planning/active-reviewer-tracking-overhaul.md:87; hub returns early when the flag is off (NotificationHub.cs:145-146). |
Before X-CLAIMS: either claims carry an absolute lease expiry that guards treat as free once passed, or a bounded backstop sweep runs behind its own flag. Add AC-T-06. |
| RT-25 | Note | C15 :528-546; notifications §3 |
Notification rules near tracking aren't stated. Routine claims never notify (#3941 already ignores "individual study claims"). #3941's workloadChanged is per stage and would fire twice for a shared form. Claim-revoked events are keyed by stage. |
#3941 description; StudyRepository.ActivityReservations.cs:93-97. | In C15: no notice per claim; one workload notice per reviewer per plan change; revocation events keyed by claim scope. |
| RT-26 | Note | C3 exposure; C9 unseen-control warning | Exposure reports must not travel over the presence hub, which is off in production. | Every presence hub method checks the tracking flag (NotificationHub.cs). | Carry exposure in the draft, Save and Complete REST payloads (idempotent), never in hub calls. |
| RT-27 | Note | inventory :410, :412 |
The tracking documents the plan cites as background are stale (see §2, last row). The presence owner's F1 sign-off needs a current baseline. | signalr-active-reviewer-tracking.md:102-160 versus Study.cs:215; redesign document still Draft. | Before F1, a docs-only PR from the presence owner (T1 below). |
4. Changes to the existing implementation¶
"Presence owner" means the role named at G0; the tracking PRs so far (#2467, #3008, #3014, #3719) came from Chris's sessions.
| Change | Rationale | Timing | Compatibility and migration | Risk | PR slicing | Owner |
|---|---|---|---|---|---|---|
| T1. Update the delivered contract: claims exist only when tracked, typed claims, reconciliation excluded, FEAT-024 mode coupling, stage-keyed surfaces; mark the redesign Implemented; retire the April planning documents | The Q-25 error came from stale documents; F1 sign-off needs truth | Now, before G0 | None | Low | One docs PR | Presence owner |
T2. Give M15 an owner: expose the reviewer-mode transition (AdvanceModeEpochAsync plus the epoch batch and the 5-minute grace) through an admin route |
Without it, no environment with FEAT-024 writes can turn tracking on | Before any tracked pilot on staging; before X-CLAIMS | Fleet-wide invalidation, then family rebuild; static configuration on both hosts (#3360) | Medium | (a) route and batch wiring; (b) runbook and staging rehearsal | FEAT-024 owner with presence owner |
| T3. Reshape #3876 into a binding-scope tracking setting: effective per stage-settings binding; a shared form is tracked if any bound stage is | Per-project pilots, no fleet-wide transition, works for shared forms | Design at F1 with E18; build before X-CLAIMS | Default keeps today's behaviour; a change becomes an ordinary scoped statistics invalidation | Medium | (a) setting and read paths; (b) scoped invalidation; © settings UI | Presence + FEAT-024 owners |
| T4. R0 floor: tally getter and claim pipeline merge form-keyed canonical counts and per-reviewer markers; claim and presence writers check canonical ownership | Capacity guards stay correct during rolling deploys and rollback (RT-07, RT-08) | R0, before R2a writes | Inert until a canonical writer exists; tests with R0 and R2a writes interleaved | Medium (hot path) | (a) getter merge; (b) pipeline; © ownership checks in tracking writers | L0/L1 with presence owner |
T5. R2a adapter: canonical own-place detection; claim release on first explicit Save inside the canonical transaction; presence FormSessionId (additive, legacy field kept); dirty means the draft-changes flag; release paths aware of drafts per Q-RT2 |
RT-05, RT-09 | R2a | Behind R2a flags; additive presence field | Medium | (a) server release and own-place; (b) presence link; © client dirty semantics, in the L5 order | Presence owner with L1/L5 |
| T6. Claim contract v2 with versioned hub methods, DTOs and commands | RT-11 | Freeze at F1; build against fakes in W1; ship with R2b | New hub method (e.g. JoinReviewContext) beside the old ones; additive DTOs; old command handlers kept ≥ 2 h + idle timeout; presence index migration |
High | (a) domain claim set and Study class map; (b) canonical claim pipeline and guard; © hub and DTOs; (d) commands and consumers; (e) presence and connection keys plus index; (f) web store | Presence owner |
| T7. Draft lease built on connections (stable tab ID, REST heartbeat, takeover) | RT-10; must work untracked | F1 contract; R2a build | New fields on ReviewSessionConnection, already tolerant of extra elements (Entity) |
Medium | (a) tab-ID plumbing; (b) REST lease endpoints; © takeover UI | L1/L5 with presence owner |
| T8. Reconciliation task editor claim: compare-and-set plus lease; "Start reconciling" claims atomically; hooks for assignment and release (RA3, RA4); requested-review claim on Study | RT-01, RT-13 | F4 contract; R4a build | New aggregate fields; the reconcile host joins presence through a new method | Medium | (a) task claim and lease; (b) atomic start; © assignment interplay; (d) requested-review claim; (e) host states | L6 with presence owner |
| T9. Backstop for orphaned claims, plus a load and failover run on Bramble | RT-24; scale unknown | Before X-CLAIMS | None | Low–medium | (a) lease expiry or sweep behind its own flag; (b) benchmark | Presence owner |
| T10. Shape presence payloads by disclosure rule (counts plus the recipient's own claim) | RT-14 | Before any tracked pilot with blinding, and before task claims reach presence | Additive; the client still finds its own claim | Low | One PR | Presence owner with authorization owner |
| T11. E2E in both tracking modes | RT-04 | Before R2a acceptance (W2) | None | Low | Playwright project matrix | L17 |
Hold until contracts freeze: #3876 as a per-stage setting; any new ReviewActivity value on v1 pages; the admin presence dashboard (#2470's admin-visibility item, redesign phase 6) on stage-keyed presence; analytics on ReviewerPresence.AnnotationSessionId; in-place hub signature changes; #3742's migration of legacy untyped reservations until a count shows any exist.
The plan should adopt from this programme: ADR-008's absolute server timestamps and shared REST/hub access result, for all new timers (RA3 expiry, draft retention, leases); generation tokens and baselines that reject stale scheduled deliveries; AccessDenialReason extended with append-only values (step locked, stage completed, task held, place held by draft) rather than a parallel denial enum; the claim-revocation outbox (#3720) for every revocation; the version-guarded reservation save with its statistics part (ReservationChangeSave, #3738) for every claim write; the fail-closed pattern for fleet-wide modes; the handover's "no false positives" criteria (docs/planning/session-capacity-suspended-sessions-handover.md:291-299).
5. Changes to the plan¶
5.1 Section edits¶
- integrated-plan.md
- §3, after line 175, add: "Claims, capacity guards and typed admission exist only when tracking is on. It is off in every deployed environment and on in E2E. Reconciliation is excluded at every layer. Turning it on is a FEAT-024 durable mode transition with no owner (M15)."
- §5.2 R0: add the tracking writers and readers (RT-08) and MVP item 5 (RT-07).
- §5.4 R2a: add a bullet "Tracking adapter (presence owner)" (RT-05, RT-09, RT-10).
- §5.4 R2b: replace "Claims are re-keyed…" with "Claims follow the F1 claim contract: released when the last page closes, versioned hub, DTOs and commands, shared-form policies per Q-28".
- §5.5 R3a: add "per-step scope claims; screening capacity by profile rule; the dependent-step claim taken at Include; denial reasons extend
AccessDenialReason". - §5.5 R3c: add RT-18.
- §5.6 R4a: entry becomes "the task editor claim in every environment"; MVP adds atomic "Start reconciling" and the requested-review claim.
- §5.6 R4b: "query review uses the same editor claim (E4)".
- §5.11: replace the X-TRACK row with X-RECLAIM (internal to L6) and X-CLAIMS (presence + FEAT-024 owners; evidence as in RT-03).
- §6.1: F1 exit evidence adds the claim contract, the projection shape, the draft lease and versioning, signed by the presence and FEAT-024 owners; F4 adds the task claim.
- §6.3: update the graph to match.
- §8, tracking row: "merged; dark when deployed; on in E2E; M15 unowned; #3876 deferred", with joins F1, R0, R2a, R2b, F4 and X-CLAIMS.
- §10: add the risks "claims do nothing in production" and "switching tracking on breaks saves under FEAT-024 writes".
- contracts.md: C5 (drafts and claims, lease, dirty definition); C7 (claim contract, capacity versus target, tracking mode, FEAT-024 reservation derivations, replacement reconciliation row); C9 (editor claim, "started" defined as the reconciler's session having a draft or explicit version, requested-review claim); C10 (presence as a channel); C15 (RT-25); C16 (inventory, floor reader logic); C17 (slot copy).
- domain-model.md: §2 (presence row corrected,
ReviewSessionConnectionrow added); §3.3 (task editor claim; requested-review claim); §4 (Study claims per contract, form-keyed projection with markers; presence keys andFormSessionId); §5 (RT-15 rows). - open-questions-and-assumptions.md: correct Q-25; extend Q-28; add Q-RT1 to Q-RT7; reword E6, E18, E20, E21, E30 (stage completion and permission revocation) and E32 (presence); rewrite A-21.
- source-status-inventory.md: §2 presence row; §5 adds an E2E column and per-service values for
maxInProgressSessions(API true, PM false, preview false); §7 adds #2446, "no claims or guards in production", "no reconciliation editor exclusion" and "presence names reviewers". - migration-adoption-rollback.md: §1 principle 4 (RT-08); the R6 fence drains or converts claims, presence records, connections and scheduled messages for the adopted scope.
5.2 Acceptance criteria to add (verification method in brackets)¶
Concurrency
- AC-R2a-20 (I, both tracking modes): A reviewer whose only session is canonical (draft with claim, saved incomplete, or completed) is always admitted to it by Next, direct access and join, even at target with enforcement on. Another reviewer is refused AtCapacity.
- AC-R2a-21 (I/C): The first explicit Save or Complete releases the claim and replaces presence exactly once. Twelve concurrent first saves from two tabs leave one session with no claim and one post-save presence. Autosave never writes Study (pinned command counts).
- AC-R2a-22 (I/E): A draft-backed claim is never released by idle expiry, leave, clean close or suspension expiry (per Q-RT2). A claim without a draft keeps today's timers.
- AC-R2a-23 (I/E): A second tab or device is read-only until it takes over; take-over ends the first lease; the lease works with tracking off.
- AC-R2b-03: rewritten as in RT-16.
- AC-R2b-07 (C): Twelve concurrent first joins through stages A and B create one claim; the tallies both stages read are equal.
- AC-R2b-08 (I): A shared incomplete session counts once against MaxInProgress.
- AC-R3a-11 (C): Screening capacity follows the profile's collective rule (D6).
- AC-R3a-12 (I): Opening a study claims only steps currently available. An own Include tries the dependent form claim atomically; if enforcement refuses it, the reviewer sees a typed "enough reviewers" state.
- AC-R4a-14 (I, untracked and tracked): When two reconcilers open the same study and form task through any stage, exactly one holds the editor claim and the other is refused with a typed reason.
- AC-R4a-15 (I): With N reconcilers pressing "Start reconciling" at once, no task goes to two reconcilers.
- AC-R4a-16 (I): The requested extra reviewer is admitted past the pool and capacity filters; nobody else is.
- AC-R4a-17 (I): Only the assignee can claim an assigned task. Admin release revokes the editor claim through the outbox. Editor-lease expiry never expires a started assignment.
- AC-R4b-07 (I): Query review uses the same editor claim.
Reconnect - AC-T-01 (E): A healthy reviewer working for 30+ minutes sees no expiry UI. A Wi-Fi drop under 10 s changes nothing. Reconnecting within the grace period, even through another API pod, cancels the scheduled removal. Reconnecting after grace with the study full shows the refusal and keeps the draft recoverable. - AC-T-02 (I): Stale scheduled deliveries (old suspension baseline, old idle generation, old stage-keyed command shape) do nothing after a reconnect or a deploy.
Failover and rolling deploys
- AC-T-03 (R): A rolling restart of every API pod with 50 active reviewers (PROPOSAL) loses and duplicates no claim, clears orphaned connections within 2 minutes, and produces no burst of 503s.
- AC-T-04 (R): The previous web bundle keeps working through the declared window, then MinUiVersion forces a reload.
- AC-T-05 (R): After rolling the image back to the recorded minimum, with form claims and canonical sessions present, capacity guards and pool filters are still correct.
- AC-T-06 (I): A lost scheduled message cannot hold a place beyond the backstop horizon.
Scale
- AC-T-07 (B, Bramble): 300 connections across 2 replicas, with 30 s heartbeats (each is a Mongo update plus a Quartz schedule and a cancel, NotificationHub.cs:1568-1606) and a burst of 20 joins per second: join and save p95 within budget, flat Quartz backlog, zero duplicate claims (PROPOSAL numbers).
Mode parity and disclosure - AC-ALL-13 (E): The affected flows pass with tracking on and off. - AC-T-08 (I): Presence sent to someone who isn't a holder carries counts plus their own claim only, and never crosses BL1.
6. Questions for Chris¶
- Q-RT1. How should capacity claims work in production? Claims and guards exist only with tracking on, so Q-25's tracking line needs correcting. The options:
- (a) Reshape #3876 into a binding-scoped setting and switch it on first for admitted pilot projects, and for legacy projects that opt in to address #2446.
- (b) Switch it on fleet-wide once FEAT-024's M15 transition has an owner and the load test passes.
- © No production claims at all: counts are advisory, and RA1 is met only by the task claim.
Recommendation: (a). It scopes risk to one project at a time, avoids a fleet-wide statistics transition, and matches R0's per-project admission.
2. Q-RT2. Does a saved server draft hold the reviewer's place? Recommendation: yes, until Save or Complete, discard, admin release, or 14 days without activity (PROPOSAL). The hard lock never stops someone submitting a draft that held its place. Claims without drafts keep today's idle and 2-hour disconnect rules.
3. Q-RT3. Is "required reviewers per study" also a maximum? SF4 makes it a minimum. Recommendation: an optional per-stage or per-route cap, off by default as today; when on it defaults to the form target, never limits requested extra reviews (RA5), and never evicts existing work.
4. Q-RT4 (extends Q-28). A form bound to stages with different tracking settings. Recommendation: the most restrictive bound stage sets the cap and the idle timeout; the stage being used sets the in-progress limit, counting a shared session once; the form is tracked if any bound stage is.
5. Q-RT5. Should SyRF hold a place on a dependent form while the reviewer is still screening? Recommendation: no. Claim it at Include; if refused, show "enough reviewers" for that step and keep the screening decision. This matches D6's "acquire only currently eligible activities".
6. Q-RT6. Who may see who is reviewing a study right now? Recommendation: reviewers see counts and their own place; names only for holders of the Monitor capability; never across reconciliation blinding.
7. Q-RT7. Two tabs or devices on one session. Recommendation: single writer with an explicit "take over"; the displaced tab keeps a local copy until it reloads and is told why. The alternative, concurrent drafts merged on conflict, is harder to explain.
7. Coverage gaps¶
- No database reads. These are therefore UNVERIFIED: counts of non-empty
SlotReservationsper environment; whether the FEAT-024 global control row exists in staging (inferred from serving being on) or production; the sizes of the presence and connection collections. - Runtime flag overrides in staging and preview were not inspected.
- No tests, E2E runs or benchmarks were executed. The Quartz load figures come from code paths, not measurements.
- Not inspected in detail: #3939's claim-time batch admission recheck; how AL1's allocation regime interacts with form claims;
StudyPresenceStore's reconnect-generation logic; where the redesigned shell and Dockview place presence UI. - The detached source window does not join presence. I did not check whether it opens an extra hub connection per reviewer.
- SignalR session affinity at the ingress across replicas was not verified.
- How #3944/#3965 conversations interact with a reconciler editor claim was reviewed only from plan text.
- FEAT-024, eligibility, allocation and notifications belong to other reviewers. My statements about them are limited to where they touch tracking.
Critical Files for Implementation¶
- /home/chris/workspace/syrf/main/src/services/api/SyRF.API.Endpoint/SignalR/NotificationHub.cs
- /home/chris/workspace/syrf/main/src/libs/project-management/SyRF.ProjectManagement.Core/Model/StudyAggregate/SlotReservation.cs
- /home/chris/workspace/syrf/main/src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Repositories/StudyRepository.cs
- /home/chris/workspace/syrf/main/src/libs/project-management/SyRF.ProjectManagement.Core/Services/StageReviewService.cs
- /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10/contracts.md