Temporary planning review record. The report below is reproduced verbatim as returned by the independent read-only reviewer (Plan agent, Fable model, launched 3 October 2026 about 14:35 BST). 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.
Review D — domain-driven design (DD-)¶
Reviewer: independent adversarial reviewer D, read-only, 3 October 2026. Nothing was created, edited, committed or sent.
Path legend (absolute roots):
- PKG = /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10/
- PLN = /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/
- MAIN = /home/chris/workspace/syrf/main/ (read at the checked-out head; the files cited are unchanged in the areas cited by the package's own re-checks)
- CORE = MAIN/src/libs/project-management/SyRF.ProjectManagement.Core/
- API = MAIN/src/services/api/SyRF.API.Endpoint/
- LEDGER = PLN/review-form-owner-decisions-2026-10-02.md
- AUTHZ = /home/chris/workspace/syrf/handover/2026-09-08-authorization-3335/PLAN.md
1. Verdict¶
The plan is strong where DDD is usually weak: invariants are stated once and traced, history is immutable with mutable pointers, transactions are enumerated, and the research that feeds it kept scientific meaning out of a generic graph. It is weak where DDD is usually strong. First, two logical choices contradict measured evidence already in the repository: making every answer (AnnotationHead) an aggregate root turns each Save into a transaction over hundreds of documents, and ProjectCommitSequence puts a per-project hot document into every canonical transaction, which is exactly the contention ADR-019 removed after 399 of 1,000 submissions exhausted their retries at ten reviewers. Second, the model has no event or command layer: no named domain events, no decision between the four post-commit mechanisms that already coexist (in-process IDomainEvent, FEAT-024's outbox and fold command, the inbox rows plus change streams, the claim-revocation outbox), and no statement of which service hosts the canonical commands, even though today's write engine lives in the API, not in the "core domain" service the architecture document describes. Third, there is no context map: lanes are delivery roles, the legacy model has no named anti-corruption layer, and relationship types with FEAT-024, authorization, notifications, AF2 and eligibility are implied rather than declared. Fourth, the ubiquitous language collides with Approved specifications and shipped code on ten load-bearing words (publication, canonical, profile, outcome, admission, correction, population, report, session, release). The five changes that matter most, all before F1: (1) make the Evidence aggregate the study × author-scope boundary and drop the per-project sequence in favour of the per-study version plus the as-of watermark; (2) add an event-taxonomy and command-catalogue ADR with a hosting rule; (3) publish a context map with relationship types and a glossary that resolves the collisions; (4) give ScreeningOutcome separate candidate and final facets and simplify the reconciliation task key to study × form; (5) model duplicate merges as aliases rather than re-keying immutable revisions, and put the canonical-ownership marker on the documents legacy writers already replace.
2. Findings¶
| ID | Severity | Location | Finding | Evidence | Recommended change |
|---|---|---|---|---|---|
| DD-01 | Major (before F1) | PKG/domain-model.md:29-31 (principle 1), :89 (AnnotationHead root), :174 (Save transaction row); PKG/contracts.md:104-105; PKG/acceptance-criteria.md:172 (AC-R2a-19), :90 (AC-M0-02) |
Aggregate granularity contradicts measured evidence. The plan makes each answer head an aggregate root "written through one command path with compare-and-set", so a Save of a 200-question form is a transaction over up to 200 heads, 200 revisions, the session version, Study, the sequence, the receipt and inbox rows, each with its own CAS. ADR-019 measured 25 commands in one snapshot transaction at 46–1,283% over the p95 gate; AC-R2a-19 demands ≤ 1.2× today's p95. The research deliberately kept partitioning a physical validation choice; the plan has promoted it to a logical aggregate boundary, while deferring the aggregate decision to a "storage ADR" (domain-model.md:17-19), which conflates the two. |
MAIN/docs/decisions/ADR-019-materialized-statistics-async-point-fold.md:15-27; PLN/screening-specialised-annotation-research.md:949-954 ("an implementation can change physical partitioning while preserving these logical contracts"); CORE/Model/StudyAggregate/Study.cs:151-170 (the fold precedent: one document carries pending work) |
Define the logical consistency boundary as ReviewerStudyEvidence = (project, study, author-or-authority scope): it owns that author's FormSessions, heads and revisions for all forms on the study. SF3/SF5 cross-form sharing and SF1 cross-stage reuse become intra-aggregate; "one head per author scope per context" becomes a local invariant; a Save writes one evidence aggregate + Study projection + receipt + inbox rows. Keep heads/revisions as entities; let the F1 storage ADR decide whether the aggregate is one document, one document plus a revision archive, or rows under a manifest (the research's chunking). Reconciled scope maps to StudyGold (DD-07). Update principle 1, §3.2, §5 and §7 accordingly. |
| DD-02 | Major (before F1) | PKG/domain-model.md:143, :174-176; PKG/contracts.md:444-446 (C11 ordering); PKG/open-questions-and-assumptions.md:127 (E25) |
ProjectCommitSequence reintroduces the per-project hot document ADR-019 just removed. "A monotonic counter allocated inside canonical transactions" means every Save by every reviewer on every study in a project updates one document under snapshot isolation. ADR-019's benchmark: "Every save writes shared per-project statistics documents… At ten reviewers on different Studies, 399 of 1,000 materialized submissions exhausted their retries." The plan freezes this at F1 as the as-of ordering contract that L11 builds against. |
MAIN/docs/decisions/ADR-019-…:21-27; CORE/Model/StudyAggregate/Study.cs:151-152 (StatisticsFoldSequence is already a per-study monotonic sequence); PKG/contracts.md:118-121 (E20 already bumps Study per commit) |
Order history per study: the Study version bump E20 already mandates is the per-study commit sequence; as-of reproducibility uses (watermark per F6a, studyId, studySequence). Reserve a project-level sequence for rare definition-level operations (publication phase 1, binding changes, gold-ownership moves), which already run under the definition-rewrite fence. Record the rejected alternative in the C11 ADR with the ADR-019 numbers. |
| DD-03 | Major (before F1) | PKG/domain-model.md:40-42, :170-180 ("Afterwards (eventual, idempotent)"); PKG/open-questions-and-assumptions.md:131-132 (E29, E30), :124 (E22); PKG/contracts.md:534-536 (C15 "emit domain events") |
No domain-event or integration-event design. The package names no domain events, no publisher/consumer pairs and no delivery guarantee for its own eventual work (outdated-flag fan-out, publication phase 2, LC1 evaluation, readiness re-evaluation, statistics folds). Today's IDomainEvent mechanism is in-process, held until commit, then dispatched without awaiting (fire-and-forget, at-most-once, lost on crash). Three durable mechanisms also exist: FEAT-024's outbox + IFoldProjectStatisticsCommand, the notification stack's in-transaction inbox rows plus change-stream hint, and the eligibility claim-revocation outbox. The research had an outbox in the commit transaction; the plan dropped it without saying what replaces it. |
MAIN/src/libs/kernel/SyRF.SharedKernel/BaseClasses/Entity.cs:61-63; MAIN/src/libs/mongo/SyRF.Mongo.Common/MongoUnitOfWorkBase.cs:342, :731-739 (_eventManager.DispatchAsync(...) not awaited); MAIN/src/libs/kernel/SyRF.SharedKernel/EventManager.cs:21-32; MAIN/src/libs/project-management/SyRF.ProjectManagement.Messages/ (27 command/event contracts, none for this programme); PLN/screening-specialised-annotation-research.md:929, :943-947; PKG/notifications-integration.md:60-62 (no new outbox for notifications) |
Add an F1 ADR (call it E36): four classes with guarantees — (a) domain facts persisted append-only in the transaction (pool entry, exposure, lifecycle, status history, capture; these are the model's event store); (b) in-process domain events (IDomainEvent) only for loss-tolerant same-process effects; © durable post-commit work through the ADR-019 pending-entry/fold pattern or a receipt-keyed outbox plus a PM.Messages command consumed in the PM service, for anything that must happen (phase-2 publication, outdated-flag fan-out, LC1 evaluation, folds); (d) change-stream invalidation for UI freshness only. Publish an event catalogue (aggregate → event → consumers → class) alongside the aggregate table. |
| DD-04 | Major (before F1) | PKG/integrated-plan.md:228-247 (lanes), :47-55 (design stance "the server is authoritative"); PKG/contracts.md:77-129 (C1 says nothing about hosting) |
Where business rules live is undecided, and the architecture document is wrong about today. The canonical write engine today is API-hosted (SubmitAnnotationSessionService, 600+ lines orchestrating unit of work, fold, receipts, eligibility and flags; ReviewController 1,707 lines), calling Core domain services. ADR-009's Core/Application/host split exists but ProjectManagement.Application holds one service. The PM service cannot capture inbox rows (no notification flags; email throws) and Quartz holds no domain code, so phase-2 batches, ASySD matching, adoption backfills, PRISMA snapshots and exports have no stated host. |
API/Services/SubmitAnnotationSessionService.cs:25-57, :79, :125, :216; API/Controllers/ReviewController.cs:519; CORE/Services/ReviewSubmissionService.cs:11-45; CORE/Services/ReviewEligibility/ (facts → policy → decision); MAIN/docs/decisions/ADR-009-…:24-58; MAIN/docs/architecture/dependency-map.yaml:87-108, :252-268; MAIN/docs/architecture/platform-architecture.md:125, :134 (claims API is thin and PM holds all logic); PKG/notifications-integration.md:84-86, :171-174 |
Add a command catalogue (command, aggregates touched, transaction class, receipt namespace, events raised, capability, host) and one hosting rule: domain policies and aggregates in Core, command handlers (application services) in ProjectManagement.Application, API and PM are hosts that invoke the same handler (interactive via HTTP, batch via PM.Messages consumer). Make "PM gets the notification flags or the admission-record pattern" an R0 item, not a C15 footnote. Correct platform-architecture.md §2 in the F1 documentation PR. |
| DD-05 | Major (before F1) | PKG/contracts.md:53-73 (catalogue has owner/consumers only); PKG/open-questions-and-assumptions.md:192 (A-10 lanes are not contexts); PKG/domain-model.md:50-51, :168 (adapters mentioned, no boundary) |
No bounded-context map; relationship types with existing programmes are implied, not declared. "Extend existing machinery rather than clone it" (contracts.md:42-46) is a conformist stance toward six upstream programmes, but the plan never says which are conformist, customer–supplier, shared kernel or anti-corruption. The legacy model (embedded ExtractionInfo/ScreeningInfo, overwritten reconciled answers, LegacyAuthorityUnknown) is handled by "read adapters" and a "compatibility profile" but is not named as an ACL with an owner and an exit. |
PKG/contracts.md:316-330 (C7 "not a takeover"), :391-409 (C10 conforms to #3335), :528-546 (C15 conforms), :586-589 (AF2 extension points); PKG/migration-adoption-rollback.md:90-110 (the ACL's mapping rules, unnamed as such) |
Add a context map (§3 below) to domain-model.md and freeze it at F1 as the first contract ADR. Name the legacy ACL (LegacyReviewDataAdapter: readers only, refuses writes via ownership, retired at R7) and the shared kernels (AnswerContextKey, provenance, receipt, claim identity, ReviewEligibilityPolicy). |
| DD-06 | Major (F1 shape, F3 freeze) | PKG/domain-model.md:92 (ScreeningOutcome "result, authority"), :108 (ProfileAdjudication "only writer"), :176 (submit "recomputed ScreeningOutcome"); PKG/contracts.md:294-307 (C6 "Collective state"); PKG/prisma-amendments.md:139-153 (H) |
ScreeningOutcome collapses candidate and final results, and has two writers. The research requires candidateResult (Pending/Conflict/Included/Excluded from votes) kept separate from finalResult + finalSource (CandidateAgreement/Adjudication) + adjudicationFreshness, and requires every dependency to say whether it gates on OwnDecision(profile), the candidate collective, or the authoritative final outcome. The plan's C6 table gates on an unqualified "Collective state"; amendment H adds authority values but not the two facets; the submit transaction recomputes the outcome while adjudication is "the only writer", so precedence after a DP2 correction (Chris, 25 September: the old adjudication becomes inapplicable to the new input vector) is not encoded anywhere. |
PLN/screening-specialised-annotation-research.md:763-770, :809-816, :818-823, :743-749; LEDGER:915-923 (PR1 reports the collective authoritative outcome) |
Make ScreeningOutcome a per-(study, profile) aggregate whose current value is a VO {candidateResult, voteCounts, ruleVersion, finalResult, finalSource, adjudicationRef?, freshness}; one domain service CollectiveOutcomePolicy(profileVersion, decisions, adjudication?) → ScreeningOutcomeValue is the single writer, invoked by both the submit and the adjudication commands. C6 dependency edges and PRISMA (C12) reference a named facet (own, candidateCollective, final). Record the 25 September precedence rule as a recovered baseline, not a question. |
| DD-07 | Major (F4, key shape at F1) | PKG/domain-model.md:104, :220, :255; PKG/contracts.md:371-372; PKG/domain-model.md:90 (FormSession keyed by reviewer) |
Reconciliation task identity contains an undefined component and the reconciler's session is mis-keyed. The task key is study × form × "version-compatibility class", undefined until F4. If a publication makes versions incompatible, the key rule creates a new task while the drift rule puts the existing task into "inputs changed"; the two rules conflict. RE4 says one task per study and form, with versions recorded, not keyed. Separately, "the reconciler's session" is modelled as a FormSession whose natural key is (study, form, reviewer), but RA4 release/reacquire and Q-10 handover change the holder, so the session belongs to the task, not to a reviewer. |
LEDGER:655-669 (RE4); LEDGER:253-258 (RA4); PKG/notifications-integration.md:106 (handover) |
Key ReconciliationTask by (study, form). Pin form version and candidate versions as state; let PublicationImpactPolicy decide which candidates still qualify, and the drift state carry the rest. Model ReconciliationSession as an entity of the task with authority scope reconciled and a current holder; its drafts use SessionDraft keyed by the task. Remove the compatibility-class decision from F4. |
| DD-08 | Major (F1 design, P2) | PKG/domain-model.md:180 (merge row), :109 (C1 merge operation); PKG/prisma-amendments.md:252-256 (L item 4); PKG/open-questions-and-assumptions.md:135 (E33); PKG/contracts.md:139 (studyId in the context key), :113 ("a revision is never edited") |
Merge by re-keying contradicts immutability. Every natural key (context key, FormSession, ScreeningOutcome, StudyGold, task) contains studyId; "remaps sessions and gold with provenance" therefore rewrites immutable revisions and session versions, which C1 forbids, and makes "split undoes the merge" a second rewrite. FEAT-012's own term for the surviving record, "canonical Study", also collides with the plan's "canonical" (DD-11). |
MAIN/docs/features/deduplication/service-specification.md:433-473, :543; PKG/acceptance-criteria.md:356 (AC-P2-05) |
Model merge as an alias: Study.mergedInto plus a StudyAlias set on the surviving study; nothing under the secondary study is re-keyed. Reads, reconciliation candidate selection, statistics and PRISMA resolve aliases, and ContributionQualificationPolicy counts a reviewer once across aliased studies (SF2). Split removes the alias and re-derives. Amend L accordingly and rename FEAT-012's "canonical Study" to "primary Study" in the same amendment. |
| DD-09 | Major (F3) | PKG/domain-model.md:81-82, :158 (Stage keeps "identity, name, order and the Active switch"), :179 (change approval transaction) |
One concept, three aggregates. Stage (embedded in Project, with its question set, security settings and Active switch), StageSettings (versions) and StageLifecycle (status, change requests) share invariants that now span aggregates: "completed bindings are frozen" needs lifecycle status and settings publication in one boundary; "Active" (Project) versus lifecycle status (StageLifecycle) is a dual-write with no stated precedence. |
CORE/Model/ProjectAggregate/StageEntity/Stage.cs:16-37 (identity, AnnotationQuestions, SecuritySettings inside Project); LEDGER:737-760 (RX2: frozen bindings on completion) |
For canonical projects, one Stage aggregate (identity, settings versions, lifecycle status, change requests, status history), referenced by Project by ID for ordering and navigation; Active is derived from status. Stage security grants stay with the Membership context (Project) and are referenced by stage ID. State the invariant "a Completed stage refuses settings publication except through an approved change request" as local to this aggregate. |
| DD-10 | Major (F2) | PKG/contracts.md:219-221 ("apply transitions idempotently in batches, or evaluate the recorded policy lazily on read"); PKG/domain-model.md:77, :90 ("qualification is derived, never stored as a flag"), :177; PKG/open-questions-and-assumptions.md:124 (E22) |
Phase 2 left as two incompatible designs (B-05 resolution inadequate). Lazy evaluation means a session's current version is not the truth (truth = version ⊕ applicable policies), which conflicts with SF6 ("a recorded action/policy creates a new incomplete session version"), with the in-transaction Study summary projection (stale for every affected study until something materialises it), and with LC1 readiness. Batched application needs a durable carrier (DD-03). | LEDGER:534-537 (SF6); PKG/contracts.md:118-121 (E20) |
Decide at F2: batched materialisation that writes real policy-created versions with provenance = publication policy, carried by the DD-03 class © mechanism; mark the Study projection "publication pending" per affected study at phase 1 so readers never see a stale count as fresh. Allow derived-at-read only for display before the batch reaches a session. Delete "or evaluate lazily on read" from C4. |
| DD-11 | Major (F1 glossary) | Throughout; examples: PKG/domain-model.md:77 (FormPublishOperation), :126 (Publication), :49-51 (canonical), :92 (ScreeningOutcome), :116 (StudyPopulation), :131 (PrismaReportSnapshot), :141 (ProjectAdmission), :148 (concern "outcomes"); PKG/contracts.md:430 ("History & corrections"), :476 (search-population family), :500 (classification "claims") |
Ubiquitous-language collisions with Approved specs and shipped code. Publication: FEAT-011 bibliographic Publication; FEAT-024 ProjectStatisticsPublicationOperation/Guard/Manifest/Marker/Phase; the plan's form/profile *PublishOperation; the matrix's "Publish review definitions". Canonical: FEAT-012 "canonical Study" vs the plan's canonical path/scope/engine. Profile: ScreeningProfile vs FEAT-024 AnnotationStudyProfile vs #2987 AnnotationProfile (the research itself warns they are distinct) vs investigator profile. Outcome: screening outcome vs outcome measure/schema/data vs query "concerns and outcomes" vs SubmitAnnotationSessionOutcome. Admission: R0 project admission vs C6 work admission (ActivityReviewAdmission) vs FEAT-024 CheckpointBuildAdmission vs AF2/upload admission. Correction: existing StudyCorrectionAggregate/StudyPdfCorrection vs DP2/LC1/query corrections. Population: StudyPopulation vs FEAT-024 search population vs box-1 review population vs UI "study/form/profile populations". Report: PRISMA reports (boxes 6/8/10/16) vs PrismaReportSnapshot vs #3945 study reports vs amendment K reported counts. Session: FormSession vs legacy AnnotationSession vs presence ReviewSessionConnection/SessionTimeoutConsumer vs maxInProgressSessions. Release: R-releases vs RA4 release vs batch release vs PoolEntryEvent "release" vs BulkPdfUploadReleaseRecord. |
CORE/Model/ProjectStatisticsAggregate/ProjectStatisticsPublication*.cs; CORE/Services/ProjectStatistics/Families/Annotation/AnnotationStudyProfile.cs; PLN/unified-annotation-classification-research.md:210; CORE/Model/StudyCorrectionAggregate/StudyPdfCorrection.cs; CORE/Services/ReviewEligibility/ActivityReviewAdmission.cs; MAIN/docs/features/deduplication/service-specification.md:433-473; MAIN/docs/features/prisma-specification/prisma-flow-diagram-mapping.md:122-138; CORE/Model/ReviewSessionConnection.cs; PLN/review-permission-matrix-proposal-2026-10-03.md:71 |
Adopt the glossary in §3.6 below as part of the C4 naming ADR at F1 (the ADR the plan already schedules, domain-model.md:252-253). Minimum code-level renames: FormPublishOperation → FormVersionRelease… no — "release" is also taken; use FormVersionIssue/ProfileVersionIssue internally and keep "publish" only as the user-facing verb; ProjectAdmission → CanonicalEnrolment; ScreeningOutcome kept (Approved) but query outcomes → ConcernResolution (the ledger's own word); StudyPopulation → AnimalPopulation; PrismaReportSnapshot → PrismaFlowSnapshot; HistoryCapture → LegacyWriteLedger; FEAT-012 "canonical Study" → "primary Study" (amendment L). Namespace each context so Publication (bibliographic) and FormVersionIssue never share one. |
| DD-12 | Major (F1) | PKG/domain-model.md:75 (QuestionDefinition identity includes "entity category or type"), :115 (EntityType at C1); PKG/contracts.md:191-193 (D38: structural properties fixed), :498 (legacy categories become system entity types), :201 (extraction types join form content only at O1); PKG/integrated-plan.md:693 (O1 row has no C1/EntityType dependency), :875-877 (graph) |
Entity-type identity is frozen too late, and O1 silently depends on it. A question's structural identity (fixed per D38 from R2a) includes its entity category; the identity that category becomes (EntityType) is minted at C1 under F-C. Forms published in R2a will pin a category string that later needs an identity rewrite, which D38 forbids. O1's cohort/outcome-measure/experiment "system types" (TC1) are EntityTypes, yet the graph makes O1 depend only on R2a and F-O. |
LEDGER:826-845 (TC1); PKG/open-questions-and-assumptions.md:116 (E14 at F-C/F-O) |
Mint stable system EntityType identities for the seven legacy categories plus cohort, outcome measure and experiment at F1 as a shared-kernel VO of C4/C13 (E14 moves to F1); C1 later adds capabilities and project-defined types without re-identifying. Add the edge "EntityType identity (F1) → O1" to §6.3. |
| DD-13 | Major (R0) | PKG/domain-model.md:142 ("read inside every legacy writer's transaction"); PKG/contracts.md:553-556; PKG/integrated-plan.md:283-288 |
The ownership check assumes transactional legacy writers; they are not all transactional. The API engine still has an "untracked whole-document replace" branch chosen per attempt, and RemoveSession, question delete and import paths are plain saves. Reading a separate CanonicalOwnership document outside a transaction cannot exclude a concurrent adoption cutover; the "typed conflict" is then a race. |
API/Services/SubmitAnnotationSessionService.cs:79, :149, :216 (branch selection); PKG/migration-adoption-rollback.md:32-52 (writer list) |
Keep CanonicalOwnership as the audited source, but have the cutover stamp an ownership marker on the Project document and on every Study of the adopted scope; legacy writers include ownership != canonical in their existing version-filtered replace, so refusal is a filter miss on the document they already CAS, with no extra read and no transaction requirement. R0's writer inventory then checks "filter includes ownership" per writer. |
| DD-14 | Minor (F1) | PKG/contracts.md:107 (kind policies), :290-292 (admission decision), :291 (shared-evidence policies), :424-428 (disclosure), :163 (exposure), :201 (gold-completeness), :376 (gold ownership); PKG/open-questions-and-assumptions.md:81 (Q-36 authority policy) |
Policy objects are referred to but never named. Twelve policies are described in prose; none has a name, inputs, output type or location, although the eligibility programme already established the pattern (ReviewEligibilityFacts → ReviewEligibilityPolicy → typed decision with reasons). Parallel agents cannot build against prose. |
CORE/Services/ReviewEligibility/ReviewEligibilityFacts.cs, ReviewEligibilityFactsBuilder.cs, ReviewEligibilityPolicy.cs; MAIN/docs/decisions/ADR-009-…:26-31 (domain-service criteria) |
Add the policy catalogue in §3.4 below to domain-model.md; each is a pure Core domain service with a fixture suite as its conformance test. |
| DD-15 | Minor (F1) | PKG/domain-model.md:90-91 (FormSession draft_only; SessionDraft keyed by SessionId), :175 ("Autosave: SessionDraft only") |
Creation point of FormSession is contradictory. A draft_only status implies a FormSession exists before any explicit version, but autosave writes "SessionDraft only"; either autosave creates the FormSession (contradicting §5) or SessionId must exist before the document does. The unique (study, form, reviewer) index cannot protect two tabs if the row doesn't exist yet. |
PKG/notifications-integration.md:67-69 (deterministic SHA-256 IDs are the stack's precedent) |
Mint SessionId deterministically from (project, study, form, reviewer) (and (task) for reconciliation sessions); create the FormSession document on first autosave as an explicit, Study-free write; keep "never bumps Study". Say so in C5 and §5. |
| DD-16 | Minor | PKG/domain-model.md:33-36 (keep Project small), :158 (security settings grow); AUTHZ:119 (WP-M2 renames Registrations→Memberships, CustomProjectRoles→CustomProjectGroups inside the Project document) |
Project remains a god aggregate and the Membership context has no root of its own. Memberships, groups, grants, delegation envelope, invitations, join requests, stage identities, legacy questions and four embedded job types (BulkPdfUploadJob, BulkStudyUpdateJob, RiskOfBiasJob, SearchImportJob) all CAS the same document. Every grant change contends with every running job. The plan conforms to #3335 without saying it. |
CORE/Model/ProjectAggregate/ listing; CORE/Model/ProjectAggregate/Security/ProjectSecuritySettings.cs |
State explicitly: Project is the root of the Membership & Permissions context (conformist to #3335 schema 1); list the embedded jobs as an R7-scope extraction or an explicit non-goal; add a Project-contention line to AC-M0-02 (grant change racing a bulk job). |
| DD-17 | Minor | PKG/domain-model.md:159 (summary projection "for legacy readers"), :174; PKG/migration-adoption-rollback.md:51-52 |
A read model lives inside a write aggregate with no exit. The Study summary projection is a necessary coexistence adapter, but it is never marked temporary and has no R7 retirement line; after R7 it would remain a second source of truth for tallies. | PKG/open-questions-and-assumptions.md:122 (E20 inventory of readers) |
Label it "coexistence adapter, retired at R7 with the legacy readers in E20's inventory"; after R7 the only Study-held facts are lifecycle, identity and the bulk-lock/version coupling. |
| DD-18 | Minor | PKG/domain-model.md:93-94 (PoolEntryEvent, ExposureEvent "aggregates"), :127-130, :144 |
Append-only "aggregates" are ledgers. PoolEntryEvent, ExposureEvent, StudyLifecycleEvent, ExternalStepRecord, DedupAuditEntry and HistoryCapture have no invariants beyond append-only; they are DD-03 class (a) facts. Calling each a one-row aggregate hides that they need an ordering key and one writer. | — | Rename to ledgers (StudyPoolLedger, ExposureLedger, StudyLifecycleLedger, ExternalStepLedger, DedupAuditLedger, LegacyWriteLedger), each ordered by the DD-02 per-study sequence, written only by the command that produces the fact. |
| DD-19 | Minor (F1) | PKG/domain-model.md §7 (:215-223) lists keys only; PKG/contracts.md:138-141 (context key), :159-164 (provenance, exposure), :468-470 (structured reason) |
No value-object classification. Context key, entity path, provenance, exposure state, structured reason with coverage, authority value, compatibility relation, route, alias, coverage label and population reference are all VOs with equality semantics that cross contexts; the plan never says so, so each lane may model them differently. | — | Add a VO list (§3.5) with equality rules; make AnswerContextKey, Provenance, AuthorityValue and Route shared-kernel types frozen at F1. |
| DD-20 | Minor (F3) | PKG/domain-model.md:83 (project-level PrismaPhaseMapping) vs PKG/contracts.md:482-486 ("set in profile settings"); PKG/integrated-plan.md:690 (Included transition) |
Two homes for the phase mapping, and the lifecycle "Included" rule has no owner. The transition needs every required profile's final outcome, the mapping and the Study status: a cross-aggregate rule with no named domain service. | MAIN/docs/architecture/platform-architecture.md:334-342 |
Keep the mapping project-level (it defines the set of required profiles); the profile editor is only its UI. Add StudyLifecyclePolicy(outcomesByProfile, mapping) → lifecycle transition, invoked in the same transaction that writes the screening outcome (which already touches Study). |
| DD-21 | Minor | PKG/domain-model.md:147-152 (queues are read models), :174 (in-transaction projection); PKG/integrated-plan.md:557 (readiness from authoritative records while FEAT-024 is dark); PKG/contracts.md:543-546 |
Three read-model consistency regimes, no rule for which applies. In-transaction projections (Study summary), eventual materialised rows (FEAT-024), and computed-at-read views (queues, agreement, "Who is offered what", readiness) coexist; new read models have no placement rule or freshness label. | — | Add to C17 DTO rules: every read model declares its regime and carries a freshness marker (asOfSequence or "authoritative"); gates (admission, LC1, publication) read only authoritative or fence-verified sources (the plan already says this for publication, contracts.md:339-343; generalise it). |
| DD-22 | Minor (F1) | PKG/integrated-plan.md:956-966 (coordination rules); MAIN/docs/architecture/dependency-map.yaml:87-108 |
Modularity for many parallel agents is contractual only. All new aggregates land in one Core library consumed by API and PM; there are no architecture-fitness tests in src (search for NetArchTest/ArchUnitNET found none), so a lane can reference another context's internals and nothing fails. |
rg over MAIN/src for NetArchTest|ArchUnitNET returns nothing |
One folder/namespace per bounded context under Core/Model/<Context>/ and Core/Services/<Context>/, an internal default with explicit public contracts, and a fitness-test project in the F1 conformance suite enforcing the context map's allowed dependencies. |
| DD-23 | Note | MAIN/docs/architecture/platform-architecture.md:125, :134, :139 |
The Approved architecture reference says the API is "intentionally thin" and PM holds "all business logic… receives commands from API"; the verified reality (DD-04) is the reverse for review writes. Builders will design against the document. | DD-04 evidence | Correct in the F1 docs PR; cite ADR-009 and the command catalogue. |
| DD-24 | Minor | PKG/domain-model.md:116 ("a default whole-study population exists from the first canonical write"); PKG/contracts.md:140-143 |
A sentinel population needs no document: creating a StudyPopulation row per study at R2a adds a write and a collection two releases before C1 uses it. |
— | Derive the default population ID deterministically from the study ID; create the aggregate only when C1 enables classification. |
| DD-25 | Minor | PKG/domain-model.md:43-45 (principle 5: everything carries a project ID except Publication) vs :80 (DefinitionTemplate has no project in its root key); PKG/integrated-plan.md:305-318 (R1a) |
DefinitionTemplate scope is undefined and contradicts principle 5 (see question Q-D1). |
LEDGER:926-933 (SET1) |
Decide the scope at G0 (question below); then fix principle 5 or the root key. |
| DD-26 | Minor | PKG/contracts.md:104-105 ("decision-owned answer"; "owned-children edges"), :138-141 (ownerScope) |
Two ownership notions are used with one word: definition ownership (profile owns eligibility questions, DP4) and answer-tree ownership (a decision owns its reason revisions). C2's ownerScope covers the first; the owning-parent edge is only in C1's revision row. |
PLN/screening-specialised-annotation-research.md:726-733 |
Name them definitionOwner (project |
3. Improvements¶
3.1 Context map (proposed for the F1 ADR)¶
| Bounded context | Root aggregates | Owns the language of | Relationship to others |
|---|---|---|---|
| Review Design (Definitions) | QuestionDefinition, AnnotationForm, ScreeningProfile (definition side), DefinitionTemplate, FormVersionIssue/ProfileVersionIssue, OutcomeSchema, EntityType (type side), SharedConcept, ProjectRule | question, form, profile, version, issue (publish), template, impact policy | Upstream to Evidence, Workflow, Reconciliation, Reporting (published language: version references). Customer of FEAT-024 for usage evidence (C8). |
| Review Workflow (Stages) | Stage (settings versions + lifecycle), admission decisions | stage, step, route, gate, admission, readiness, lifecycle | Shared kernel with the eligibility programme (ReviewEligibilityPolicy, facts/decision pattern). Customer of allocation, presence and batches (C7). Downstream of Design. |
| Evidence (engine) | ReviewerStudyEvidence (per study × author scope), SessionDraft, ledgers (exposure, legacy writes) | answer, revision, session, save, complete, draft, provenance, context | Shared kernel exports: AnswerContextKey, Provenance, receipt (with FEAT-024, E35), claim identity (with presence, E18). Conformist to AF2 (host) via extension points. |
| Eligibility Results (screening outcomes) | ScreeningOutcome, ProfileAdjudication, StudyPoolLedger | decision, candidate result, final result, adjudication, pool entry | Downstream of Evidence (decisions) and Design (profile rules); upstream to Workflow gates and Reporting. |
| Reconciliation & Accepted Answers | ReconciliationTask (+ ReconciliationSession, assignments, additional-review requests), StudyGold, QueryWorkItem | task, candidate, match, accepted (gold), snapshot, concern, resolution | Downstream of Evidence; upstream to Reporting/Export; conformist to the notification stack (C15) and to #3944 conversations (clarification channel only). |
| Classification & Populations | AnimalPopulation, EntityType instances, inference results | population, cohort, instance, concept, rule, assertion, inference | Shares EntityType identity with Design (minted F1); downstream of Evidence. |
| Identification & Deduplication | Publication (system), Citation (VO on Study), DuplicateReviewItem, dedup/lifecycle/external-step ledgers | citation, record, publication, duplicate, primary/secondary study, source | Conformist to FEAT-011/FEAT-012 with the amendment change policy; supplier to Reporting; customer of PDF/deletion-lifecycle programmes (retrieval status, withdrawal). |
| Reporting & History | PrismaFlowSnapshot, PrismaPhaseMapping, ExportManifest, DataExportJob, agreement computations | report (PRISMA), flow, manifest, coverage, as-of, agreement | Downstream of everything; never writes review data. |
| Membership & Permissions | Project (root), groups, grants, delegation envelope, disclosure policy | member, group, grant, capability, delegation, disclosure | Conformist to authorization #3335 (evaluator, audit, catalogue). Supplies DisclosurePolicy to every other context. |
| Platform / Coexistence | CanonicalEnrolment, CanonicalOwnership, compatibility floor | enrolment, ownership, scope, floor | Anti-corruption layer LegacyReviewDataAdapter toward the legacy embedded model (readers only; writes refused by ownership; retired at R7). |
| Upstream programmes | FEAT-024, allocation, presence, batches, notifications, AF2/layouts, authorization, identity, PDF, deletion lifecycle | — | Customer–supplier (FEAT-024, allocation, batches, PDF, deletion); conformist (authorization, notifications, AF2/layouts, identity). |
3.2 Evidence aggregate refinement (DD-01, DD-07)¶
ReviewerStudyEvidence {projectId, studyId, authorScope}→FormSession[](per form; status derived from the latest explicit version),AnnotationHead[](entities keyed byAnswerContextKeyminus the aggregate's own components),AnnotationRevision[](immutable, sequence-ordered), presentation state. Invariants local: one head per context; current pointers by CAS on the aggregate version; versions pinned by gold never deleted (checked against StudyGold references at withdraw time, the one cross-aggregate read).StudyGold {studyId}→ reconciled-scope heads and revisions,GoldSnapshot[], current pointer, shared-question ownership (first publisher wins is then a local rule).ReconciliationTask {studyId, formId}→ pinned candidate session versions, match set,ReconciliationSession(holder, drafts via SessionDraft), assignments, additional-review requests, drift state.- Trade-off to record in the storage ADR: document growth per reviewer-study is bounded by forms × questions × revisions and is strictly smaller than today's whole-Study document; if the M0 benchmark shows revision growth, archive old revisions to a side collection under the aggregate's manifest without changing the logical boundary.
3.3 Event taxonomy and catalogue skeleton (DD-03)¶
| Event (aggregate) | Class | Consumers |
|---|---|---|
SessionVersionRecorded (Evidence; Save/Complete/Fix/Withdraw) |
(a) fact in Study projection + © durable work | outdated-flag fan-out (R2d), readiness (R3c), statistics fold, inbox rows (C15) written in-transaction |
ScreeningDecisionSubmitted → ScreeningOutcomeChanged (Eligibility Results) |
(a) ledger + © | pool recomputation, lifecycle policy, PRISMA groundwork |
FormVersionIssued / ProfileVersionIssued phase 1 (Design) |
(a) + © ApplyIssuePolicyCommand (PM consumer, batched, receipt-keyed) |
phase 2 transitions, notices once per recipient |
GoldSnapshotPublished, ConcernResolved (Reconciliation) |
(a) + © | exports, statistics, inbox rows |
StageCompleted / StageReopened / ChangeRequested (Workflow) |
(a) status history + © | queues, notices |
StudyEnrolledInPool, StudyMerged, CitationRecorded (Identification) |
(a) ledgers | PRISMA, dedup audit |
| UI invalidation | (d) change streams | SignalR only; never a correctness path |
3.4 Policy catalogue (DD-14), all pure Core domain services with fixture suites¶
ApplicabilityEvaluator (E23) · ContributionQualificationPolicy (C5/SF2/SF6) · StepRoutingPolicy (C6 table, facets per DD-06) · WorkAdmissionPolicy (extends ReviewEligibilityPolicy; selection/reservation/access/submit) · CollectiveOutcomePolicy (profile rules → outcome VO) · StudyLifecyclePolicy (DD-20) · IssueImpactPolicy (requireReanswer/autoUpdate/doNothing per category, E1) · UsageEvidenceGate (C8 fence) · ReconciliationReadinessPolicy (candidate selection, drift) · PrefillPolicy (RE2/RE5 exact match) · GoldOwnershipPolicy (shared question gold) · ExposurePolicy (three states, fail-safe) · DisclosurePolicy (C10: reads, exports, statistics, notifications) · BlindingPolicy (BL1 most restrictive, Q-28) · StageCompletionPolicy (LC1/E29) · CanonicalOwnershipGuard (C16) · AliasResolutionPolicy (DD-08).
3.5 Value objects to freeze at F1 (DD-19)¶
AnswerContextKey (project, study, authorScope, definitionOwner, questionId, entityPath, populationRef; value equality), EntityPath (ordered instance IDs), Provenance (stage, step, settings version, question version, shown revision, real actor, on-behalf-of), ExposureState (enum + evidence), AuthorityValue (candidateAgreement | reconciled | adjudicated | admin-override | legacyUnknown), StructuredReason (primary + counted reasons + coverage), CompatibilityRelation (same | compatible | incompatible, with reason), Route (stage, step), ReviewerAlias (stage-owned), CoverageLabel, Watermark.
3.6 Glossary: user-facing term → internal name (resolving DD-11)¶
| User-facing | Internal (code/ADR) | Never use for this |
|---|---|---|
| Publish a form/profile version | FormVersionIssue, ProfileVersionIssue |
FEAT-011 Publication, FEAT-024 statistics publication |
| Publication (bibliographic) | Publication (Identification namespace only) |
the act of publishing a definition |
| Primary / secondary study (dedup) | primaryStudyId, StudyAlias |
"canonical" |
| Canonical path / scope (new model) | CanonicalEnrolment, CanonicalOwnership |
dedup primaries, FEAT-024 "canonical tuple" |
| Screening result (UI) / screening outcome (spec) | ScreeningOutcome {candidate, final} |
measured outcomes, concern resolutions |
| Outcome measure / outcome data | OutcomeMeasure, OutcomeSchema, Observation |
screening |
| Concern resolution | ConcernResolution |
"outcome" |
| Screening profile | ScreeningProfile |
FEAT-024 AnnotationStudyProfile (rename there is a FEAT-024 request), #2987 AnnotationProfile (do not harvest the name) |
| Work admission (can this reviewer do this now) | WorkAdmissionDecision |
project enrolment, upload admission |
| Correction (DP2, Fix, LC1) | OwnDecisionCorrection, FixTransition, PendingChange |
StudyPdfCorrection |
| Population (animals) | AnimalPopulation |
search population (statistics), review population (box 1) |
| PRISMA flow report | PrismaFlowSnapshot |
PRISMA "reports" (bibliographic unit), study issue reports, reported counts (ExternalStepRecord) |
| Session (reviewer's work on a form) | FormSession; presence stays ReviewSessionConnection |
legacy AnnotationSession once adopted |
| Release (assignment) | AssignmentRelease |
R-releases, batch release, pool availability (PoolEntry) |
| Accepted answers (gold) | StudyGold, GoldSnapshot |
"reconciled" as a status word (use AuthorityValue) |
3.7 ADRs needed before F1 (in addition to E1–E35)¶
ADR-021 Bounded contexts and context map (§3.1, DD-05, DD-22 fitness tests) · ADR-022 Evidence aggregate boundary and physical storage (DD-01, E15, E28) · ADR-023 Event taxonomy, post-commit guarantees and carriers (DD-03, E29, E30, E22 phase 2) · ADR-024 Command catalogue, hosting rule and single receipt authority (DD-04, E35) · ADR-025 Ubiquitous-language glossary and naming (DD-11; merges the C4 naming ADR and the Q-08 harvest naming) · ADR-026 Compatibility floor with document-level ownership marker (DD-13, E16) · ADR-027 Answer context key, system EntityType identities and default population (DD-12, DD-19, DD-24, E27) · ADR-028 Screening-outcome facets and collective policy (DD-06; shape at F1, freeze with amendment H at F3) · ADR-029 History ordering per study and as-of watermark (DD-02, E25).
3.8 Command catalogue template (DD-04)¶
Columns: command · intent (ledger IDs) · aggregates written · aggregates read · transaction class (interactive / fenced definition / batch) · receipt namespace · events raised (class) · capability (Q-03 name) · host (API / PM consumer) · typed refusals. Seed rows: SaveSession, CompleteSession, FixSession, WithdrawSession, SubmitScreeningDecision, CorrectOwnDecision, IssueFormVersion (phase 1), ApplyIssuePolicy (phase 2, PM), BindStageSettings, RequestStageChange, ApproveStageChange, StartReconciliation, PublishGold, RaiseConcern, ResolveConcern, RequestAdditionalReview, MergeStudies, SplitStudies, RecordExternalStep, FreezePrismaFlow, EnrolProject, AdoptScope.
4. Questions for Chris (product decisions only)¶
| ID | Question | Recommendation |
|---|---|---|
| Q-D1 | Who owns question/profile/form templates? A CAMARADES-curated system catalogue, per-project collections, or per-user? Principle 5 (every aggregate carries a project ID) and DefinitionTemplate (no project) currently disagree; R1a's "cross-project references are refused" implies copies from somewhere outside the project. | A system-scoped curated catalogue (editable by application administrators) plus "copy from a project I administer"; both produce copies (DP4/SET1), never links. Templates then become the second system-wide aggregate, and principle 5 is amended. |
| Q-D2 | Can a study ever have two concurrent reconciliation tasks for one form (for example, candidates under incompatible form versions)? | No: one task per study and form (RE4). Incompatible candidates are simply non-qualifying until re-completed under the issue policy; the task shows "inputs changed". This removes the "compatibility class" from the task key. |
| Q-D3 | After a duplicate merge, should the secondary study's sessions and decisions remain visible under their own study with a "merged into" lineage (alias), or be shown as if they had always belonged to the primary study? This is what reviewers and reconcilers see, and it decides whether merge rewrites history. | Alias: records stay attributed to the study they were made on, the primary study's workspace shows them as candidates with lineage, and a split is a clean reversal. |
| Q-D4 | User-facing name for the authoritative answer set: "Gold standard", "Accepted answers" or "Reconciled answers"? The ledger uses "accepted" for queries and "gold" for snapshots; v10 uses gold. | "Accepted answers" in the UI with "(gold standard)" in help text on first use; StudyGold stays the internal name. |
| Q-D5 | User-facing name for a screening result: keep "screening outcome" (FEAT-011's stored name) or use "screening result"/"eligibility decision" so that "outcome" means a measured outcome throughout extraction and reporting? | "Screening result" in the UI and user guide; screeningOutcomes[] stays in storage and the Approved spec. |
5. Coverage gaps¶
- I did not read the QM v2 PR code (#2572–#2575), the notification PR code, the AF2/Dockview web code or the v10 prototype pack; claims about them come from the package and the three first-round reviews, which cite them.
- MongoDB transaction size and duration limits were not benchmarked; DD-01 and DD-02 rest on ADR-019's recorded benchmark, not a new one.
CORE/Services/Category.csdid not expose the seven category constants to a grep; the "seven hard-coded categories" fact is taken from the inventory (PKG/source-status-inventory.md:205) and review C.- The deterministic SHA-256 identifiers in
StudyConversation.cswere not re-read in the PR worktree; the citation is viaPKG/notifications-integration.md:67-69. AnnotationStudyProfile's semantics (FEAT-024 before/after profile of a study) were confirmed by file path and its use inSubmitAnnotationSessionService.cs:88-97, not by reading the class.- FEAT-024's README was not re-read beyond the lines the package cites; the "search-population family" name is taken from
PKG/contracts.md:476and review B. - The eligibility policy document was read only at its decision table (
PLN/review-eligibility-policy.md:985-992), not in full. - No runtime, database or environment checks were made.
Critical Files for Implementation¶
- /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10/domain-model.md
- /home/chris/workspace/syrf/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10/contracts.md
- /home/chris/workspace/syrf/main/docs/decisions/ADR-019-materialized-statistics-async-point-fold.md
- /home/chris/workspace/syrf/main/src/services/api/SyRF.API.Endpoint/Services/SubmitAnnotationSessionService.cs
- /home/chris/workspace/syrf/main/src/libs/mongo/SyRF.Mongo.Common/MongoUnitOfWorkBase.cs