Skip to content

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 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 VB: versioning model, implementation, data and migration

Reviewer: independent adversarial reviewer (VB dimension), read-only, 3 October 2026. I created, edited and moved nothing, ran no git write commands and queried no database.

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/ (the ledger is PLN/review-form-owner-decisions-2026-10-02.md) - MAIN = /home/chris/workspace/syrf/main/ at 0f5c61073. Since the package's c59d9d0f1 check, only FEAT-024 and allocation-performance files have changed, so the cited files match the package's baseline. - CORE = MAIN/src/libs/project-management/SyRF.ProjectManagement.Core/; MONGO = MAIN/src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/; API = MAIN/src/services/api/SyRF.API.Endpoint/; AF2 = MAIN/src/services/web/src/app/shared/annotation/annotation-form-v2/; STATS = MAIN/docs/features/materialized-project-statistics/; RULES = MAIN/.claude/rules/ - QM v2 heads, read with git show <sha>:<path> in MAIN: QMA = af5136696 (#2572), QMB = 271290115 (#2573), QMC = 1b93cbbc4 (#2574)

The round-2 reviews DD and VA ran at the same time as this one. Where a finding overlaps one of theirs, I cite their ID and add only the implementation detail they do not cover.

1. Verdict

The versioning model can be built on SyRF's MongoDB stack, but not as the package currently specifies it. Several persistence promises are stated as requirements with no mechanism behind them: a rollback-safe floor; "never edited"; "carries FEAT-024 pending entries"; "one receipt authority"; "a concurrent Save lands in the manifest or under the new version"; and unique natural keys. In four places the obvious mechanism is unsafe against code already on main. The round-1 resolutions for B-01, B-02, B-05 and B-07 are inadequate in those respects. The five most important changes:

  1. Make R0 a floor for meaning, not just for parsing. Every embedded type the programme extends must capture unknown fields; ignoring them is not enough. R0 itself must also ship the canonical-aware derivation of every persisted computed field. Today, ExtractionInfo.SessionTallies and ScreeningInfo.InclusionInfo/AgreementMeasure/... are recomputed from legacy collections whenever any writer saves the whole Study. The alternative is a projection in the legacy shape that existing getters, indexes, eligibility, allocation and FEAT-024 already read.
  2. Treat FEAT-024 as a compatibility consumer, not a free carrier. Existing pending-entry kinds derive only from embedded Study data. Canonical work must either be projected into that shape, or get new kinds through a fold protocol bump and stamp advance, scheduled as floor steps. Separately, make each canonical command's own immutable output record its receipt (unique on project + command ID). FEAT-024 receipts exist only when statistics are on, carry no result, have capacity limits, and in fold mode are written late.
  3. Make publication safe by construction.
  4. Phase 1 does constant work: a compare-and-set on the form head, plus the policy and operation records.
  5. The affected set is a predicate, swept by an ADR-020-style operation with a lease and generation fencing, a stable cursor and a final pass that runs until nothing matches.
  6. The manifest becomes an audit snapshot built after commit.
  7. Only one publication per form runs at a time.
  8. Admission and readiness for the form pause while the Study projection lags behind.
  9. Specify the physical rules that otherwise break on first implementation:
  10. a scalar hash of the context key for unique indexes, because a unique index over entityPath[] is multikey;
  11. question identity scoped to the project, because system question IDs repeat across projects;
  12. append-only repositories enforced by an architecture test, because the generic save is an upsert ReplaceOne and collection names come from class names;
  13. entity-instance identity and withdrawal;
  14. ownership enforced through the existing IAggregateWriteGuard (the write-filter hook the bulk-update locks use), with ADR-020's lock, apply, release protocol for adoption cutover.
  15. Size M0, E28 and the AF2 contract from real data and real code seams.
  16. Benchmark with ADR-019's concurrency arms and command budgets, against the largest real project (2,023 questions).
  17. Express E28 in pins and bytes per commit, not questions.
  18. Add an AF2 extension point that loads questions from the session's pinned form version, and check at publication that AF2 can render the form, because canonical forms have no AF1 fallback.

2. Findings

ID Severity Location Finding Evidence Recommended change
VB-01 Blocker PKG/integrated-plan.md R0 l.279-299; PKG/contracts.md C1 l.108, l.118-121, C16 l.550-556; PKG/domain-model.md l.159, l.229; PKG/acceptance-criteria.md AC-R0-01..05 l.98-102, AC-R2a-12 l.165 R0 makes old binaries parse new fields but not compute legacy fields correctly. Study's legacy read surfaces are computed getters, and their results are persisted. Every whole-Study save by any writer recomputes them from embedded legacy collections: ExtractionInfo.SessionTallies (from Sessions + SlotReservations), the SessionTally totals, and ScreeningInfo.InclusionInfo/AgreementMeasure/IncludedCount/Inclusion/NumberOfScreenings. Pool filters, pmStudy indexes, eligibility, allocation and the FEAT-024 classifiers read exactly these. Failure scenario: in an admitted R2a project, legacy screening stays live (plan l.414-415). A legacy screening save, a fold-worker removal, a claim write, or an R0 pod during a rolling deploy or rollback recomputes SessionTallies without the canonical sessions. Pool and capacity reads then undercount, which leads to allocation beyond target and wrong readiness, silently. AC-R0-01 ("nothing throws, no field dropped") still passes. FEAT-024's own code states the rule: capture "is [not] the compatibility mechanism for new meaning". This makes the B-01/B-02 resolutions inadequate. CORE/Model/StudyAggregate/ExtractionInfo.cs:31-79; SessionTally.cs:52-65; ScreeningInfo.cs:52-74; MONGO/Repositories/StudyRepository.cs:3176-3186, 3209-3228 (getters mapped for persistence), :3008-3050 (indexes on SessionTallies.*); MONGO/Filters.cs:245, 334-370, 432-439, 494-547, 583-590; CORE/Services/ReviewEligibility/ReviewEligibilityFactsBuilder.cs:64-121; CORE/Services/ProjectStatistics/Families/Annotation/AnnotationStudyProfile.cs:53-79; CORE/Model/StudyAggregate/StudyPendingStatistics.cs:15-20 The storage ADR (E15/E20) must choose the projection's physical shape at F1, and its readers move into R0. Option (a), a projection in the legacy shape: each canonical commit writes a stub AnnotationSession per form session and bound stage into ExtractionInfo.Sessions, flagged Canonical, with FormId and SessionVersionId and no annotations. The existing getter, indexes, eligibility, allocation and FEAT-024 AnnotationStage/AnnotationMultiStage derivations then stay correct. R0 ships the stub-aware code: legacy writers refuse stubs, and AF1, legacy reconcile and legacy export skip them. Option (b), separate fields: R0 ships merged getters and every legacy reader change (19 non-test files read SessionTallies). Do the same for ScreeningInfo before R3a. Add AC-R0-06: in a mixed fleet (R0 + R2a pods) with interleaved legacy screening and canonical saves, every persisted computed field equals an authoritative recount.
VB-02 Major PKG/integrated-plan.md l.280-282 ("Tolerant class maps (or extra-element capture)"); PKG/contracts.md l.94-95, l.550-552; PKG/source-status-inventory.md fact 21 l.176-182; PKG/migration-adoption-rollback.md l.137 "Tolerant class maps" (ignore extra elements) strip unknown fields on write. The Study is replaced as a whole document, so an older binary with ignore-only maps drops canonical fields on its next save. AC-R0-01 cannot pass with that option. The package also misdescribes the FEAT-024 precedent: its pending classes capture unknown elements ([BsonExtraElements] UnknownElements) as well as ignoring them. Two further gaps: (1) serialisation that depends on schema version drops known fields regardless of capture, which is ADR-011's failure mode (RandomId and OutcomeData.GraphId serialise only when SchemaVersion > 0); (2) computed collections are rebuilt on save, so capturing extra fields on SessionTally elements cannot preserve new element fields. CORE/Model/StudyAggregate/StudyPendingStatistics.cs:15-20, 40-42; MONGO/Repositories/StudyPendingStatisticsClassMaps.cs:9-18, 46; MONGO/Repositories/StudyRepository.cs:3146-3147, 3159-3161; MAIN/docs/decisions/ADR-011-...md ("An older writer could deserialize the known subdocument but suppress it…"); MAIN/src/libs/mongo/SyRF.Mongo.Common/MongoExtensions.cs:262-299 Replace the option with "capture extra elements on every embedded type the programme may extend (ScreeningInfo, ExtractionInfo, SessionTally, and any Project, Stage or SystematicSearch value object); never ignore only". Forbid new fields inside persisted computed collections and put them on the source entities instead. Audit every SetShouldSerializeMethod and schema-version branch on extended types. Add a per-type round-trip test to AC-R0-01: the older map reads the document, mutates an unrelated field and replaces it, and the unknown nested fields survive. Correct inventory fact 21 and C1 l.94-95.
VB-03 Major PKG/contracts.md C1 l.118-121, C7 l.326, C8 l.336-347; PKG/domain-model.md l.174; PKG/integrated-plan.md l.406-408, l.473; PKG/open-questions-and-assumptions.md E3 l.105, E20 l.122 "Each canonical commit carries FEAT-024 pending entries" cannot be met with today's kinds. Every annotation kind derives its moves from the Study's before and after images: embedded Sessions, SessionTallies and Annotations, through one deriver. A canonical commit that changes nothing embedded produces no delta, and the QuestionAnswers family counts ExtractionInfo.Annotations. New kinds (form-version usage, question-version usage, canonical contribution) need all of the following before any writer emits them: a fold protocol bump with IntroducedAt, deployment to every host, and a stamp advance (a start-up config setting). Unknown kinds are quarantined and mark their families Stale. None of this appears in R0's floor steps or the F1/F2 exit evidence. Two further dependencies: (1) PS2's "wait for catch-up" in fold mode needs the operator-built IX_Study_PendingStatistics index, which exists in no environment; (2) phase-2 sweeps append entries under the 32-entry / 48 KiB cap per Study, so a lagging fold turns a large publication into overflow, then epoch fallback, then Stale families, which blocks the next PS3 check. Changing targets also changes the inputs to the configuration digest. CORE/.../Annotation/AnnotationStudyProfile.cs:53-79; RULES/materialized-stats.md l.34-46 (digest), l.116-122, l.129-134, l.149-151; CORE/Services/ProjectStatistics/Fold/ProjectStatisticsFoldProtocol.cs:30-34, 66, 95; CORE/Model/StudyAggregate/StudyPendingStatistics.cs:198-205 (closed set of kinds); STATS/STATUS.md l.353 (pending index not built) Add to F1/F2 exit evidence a FEAT-024 compatibility plan, signed by its owner, naming for each canonical write: the kind it emits (an existing kind via the legacy-shaped projection, or a new kind with IntroducedAt); the protocol bump and stamp advance as explicit floor steps before R2a (contributions), R2c (usage family) and R3a (decisions); the digest reconciliation; and the derivation tests. Make the operator-built pending index an R2c prerequisite in fold mode. Add AC-R2c-10: a 10,000-session sweep with fold on leaves no family Stale or quarantined after the fold drains, and overflow is reported.
VB-04 Major PKG/contracts.md C1 l.113-117; PKG/open-questions-and-assumptions.md E35 l.137; PKG/domain-model.md l.147, l.174; PKG/acceptance-criteria.md AC-R2a-07 l.160 Reusing FEAT-024 receipts as the canonical idempotency authority fails AC-R2a-07 in four ways. (1) Receipts are minted only when materialized writes, all annotation families and the project allowlist are on, which means never in production. (2) A receipt records source revisions and a digest, not the command's result (session version and revision IDs), so a retry cannot "return the original receipt". (3) Receipts have row and byte ceilings. A breach fails the source mutation with StatisticsIdempotencyCapacityExceeded, and an idempotency floor rejects older retries as terminal-too-old. (4) In fold mode the fold worker writes receipts later, and saves that overflow or are quarantined get none. An immediate retry before the fold runs finds nothing and applies again. The B-07 resolution's preferred option is unsafe. CORE/.../Annotation/ProjectAnnotationStatisticsWriter.cs:76-79; API/Services/SubmitAnnotationSessionService.cs:152-176; CORE/Model/ProjectStatisticsAggregate/ProjectStatisticsSourceOperationReceipt.cs:7-17 and RecordCommit; STATS/technical-plan.md l.400-412; RULES/materialized-stats.md l.146-148, l.297-303 Make the canonical receipt the immutable output record itself. Each command writes exactly one command-bearing record (session version, decision revision batch, publish operation) with a unique index on (ProjectId, CommandId), storing the result IDs and a request digest. Retries and unknown-commit resolution read it. A conflicting digest returns a typed 409. When statistics are on, use the same command ID as the FEAT-024 OperationId, so both resolve to one identity. Add an AC: a retry after commit, with fold on and before the fold runs, returns the original result and writes nothing.
VB-05 Major PKG/contracts.md C2 l.138-143; PKG/domain-model.md §7 l.217-222, l.89 The C2 natural key includes entityPath[], which breaks a unique index. In MongoDB a unique compound index over an array is multikey, and uniqueness is enforced per array element. Heads for (Q, [cohortA, tp1]) and (Q, [cohortA, tp2]) collide on (Q, cohortA), so the second Save fails with a duplicate key. (This is documented MongoDB behaviour; it is not exercised anywhere in this repository.) Keys also differ by kind: decision heads use a profile and have no question or entity path; decision-owned answers key under the decision; reconciled heads use an authority scope. Nullable key parts break uniqueness unless the indexes are partial. PKG/contracts.md l.138-143; PLN/screening-specialised-annotation-research.md:675-682 (different natural keys per kind) Store contextKey as an ordered value object plus contextKeyHash (SHA-256 over a versioned canonical serialisation), and put unique indexes on scalars only: {ProjectId, KeyHash} unique; partial unique indexes per kind (for example {ProjectId, StudyId, AuthorScope, ProfileId} where kind = ScreeningDecision); and a non-unique {ProjectId, StudyId, AuthorScope, QuestionId} for ancestor and SF5 reads. Add a conformance test: two heads that differ only in the second path element coexist, and an exact duplicate is refused.
VB-06 Major PKG/contracts.md C4 l.217-223; PKG/domain-model.md l.77, l.177; PKG/integrated-plan.md R2c l.466-479; PKG/acceptance-criteria.md AC-R2c-05/06 l.193-194; PKG/open-questions-and-assumptions.md E1 l.103, E22 l.124 Publication is not concurrency-safe as specified. (a) Phase 1 runs "under the definition-rewrite fence", but that FEAT-024 fence only makes statistics bundles return a typed 503; it does not serialise reviewer writes. (b) Under snapshot isolation, a Save that reads form head v1 and inserts a version pinned to v1 does not conflict with phase 1 writing the head and enumerating sessions (write skew). A session can then land in neither the manifest nor v2, which is exactly what AC-R2c-05 forbids. © Enumerating the manifest inside phase 1 grows with session count against the 60-second transaction lifetime, and AC-R2c-06 tests only 10,000. (d) Overlapping publications of one form, and FV4 revisions during phase 2, are unspecified. (e) Legacy readers see the Study projection change only when the sweep reaches each Study, so C4 l.221 ("follow the policy immediately") is false for them. DD-10's per-Study "publication pending" marking at phase 1 would recreate the large transaction. (f) The QM v2 worker that Q-08 would harvest pages with Id > last over random GUIDs and filters Status == Incomplete. Late sessions that sort below the cursor are skipped, and completed sessions are never transitioned (contrary to FV2/FV3). Overlaps DD-10 and VA-03. STATS/technical-plan.md l.991 (token makes bundles return a typed 503); CORE/.../QuestionAnswers/IProjectStatisticsDefinitionRewriteFence.cs:36-51; QMC:src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Readers/StageTransitionBatchReader.cs:31-40; QMC:.../Core/Services/StageTransitionWorker.cs:44, 121, 130, 145; QMB:.../StageTransitionJobAggregate/StageTransitionJob.cs:40, 90-91; ledger PS3 l.53 (1) Phase 1 is O(1): a CAS on the AnnotationForm head (current published version, publication sequence), plus the policy and operation records. Every Save writes its FormSession head with pinnedFormVersion and is accepted as the version it declares. (2) The affected set is a predicate: currentFormVersionSeq < N, plus drafts with baseFormVersionSeq < N. Late saves are covered by construction. CAS on the session head resolves races between the sweep and the reviewer: if the reviewer wins, the final pass re-sweeps; if the sweep wins, the reviewer gets a stale-base conflict that keeps the draft. (3) Phase 2 reuses ADR-020's operation pattern: lease plus generation fencing; chunks keyed by (formVersionSeq, sessionId); short transactions per session or small batch; skip and retry on conflict; a final pass by predicate until nothing matches. (4) The manifest is an audit snapshot built after commit, at the operation's stamp, for the dialog and notices only. (5) One active publication per form, through a unique partial index, as pmRobRunOperation.ActiveSearch does. FV4 revisions CAS the policy generation, and each batch asserts it. (6) During phase 2, new admissions and reconciliation-readiness transitions for the form are paused. (7) Delete "or evaluate lazily on read" for anything legacy readers consume. Replace AC-R2c-06 with: phase-1 time is flat across 1k, 10k and 100k sessions; a Save racing phase 1 (forced interleaving) is swept; a late session whose ID sorts below the cursor is swept; reviewer save p95 during the sweep stays within budget.
VB-07 Major PKG/domain-model.md l.142; PKG/contracts.md C16 l.553-556; PKG/integrated-plan.md l.283-288; PKG/migration-adoption-rollback.md §1.4 l.32-52, §4 step 5 l.126-129; PKG/acceptance-criteria.md AC-R0-02 l.99, AC-R6-05 l.395 The ownership enforcement has three implementation gaps beyond DD-13 (which proposes a marker on documents legacy writers already compare-and-set). (a) The marker must be scope-aware. R2a admitted projects keep legacy screening live on the same Study documents as canonical annotation (plan l.414-415), so a single "canonical" flag would refuse it. (b) Direct pipeline writers bypass generic save filters and are missing from the inventory. The project-wide inclusion recalculation runs three UpdateManyAsync calls over ScreeningInfo.InclusionInfo computed from embedded screenings, which would overwrite an R3a canonical projection. © AC-R6-05 wants the record to "switch atomically", which no single transaction can do for a project with tens of thousands of Studies once each Study carries the marker. MONGO/Repositories/StudyBulkUpdateLockGuard.cs:18-60 (IAggregateWriteGuard<Study>, Writable, ExplainMissAsync); MAIN/src/libs/mongo/SyRF.Mongo.Common/MongoExtensions.cs:262-299 (guard applied inside generic save); RULES/bulk-study-locks.md l.14-31; MONGO.Tests/StudyWriteLockArchitectureTests.cs:16, 60, 72; MONGO/Repositories/StudyRepository.cs:1619-1651; CORE/Services/ProjectManagementService.cs:226-229 (project-wide pre-check pattern); MAIN/docs/decisions/ADR-020-...md l.113-130, l.253-256 Implement ownership as a registered IAggregateWriteGuard on Study and Project, over a CanonicalScopes marker (annotation-form F, screening-profile P, and so on), with each writer declaring its scope. Extend StudyWriteLockArchitectureTests so every direct write adds the ownership filter, as it already adds Unlocked. Add project-wide pre-checks of the same kind as ThrowIfBulkUpdateInProgressAsync. Add the inclusion-recalculation writer, and every UpdateMany on pmStudy, to the inventory with a refuse or route decision. For cutover, reuse ADR-020's protocol: plan, lock each Study, verify and refresh the delta, stamp the marker, then release with an Audit.Version bump. Restate AC-R6-05 as all-or-nothing via locks, with legacy writes refused and retried during the window.
VB-08 Major PKG/contracts.md C17 l.586-587; PKG/integrated-plan.md §7 l.947-953, R2a l.391-398, R2c l.471-474; PKG/open-questions-and-assumptions.md A-02 l.184 The F1 AF2 extension points omit the two that versioning needs first. (1) A question source pinned to a version. AF2's data source reads the current stage's questions from ngrx, and its eligibility guards read projectDetails.annotationQuestions and stage.annotationQuestions. A session pinned to F v1 must still render v1 after v2 publishes, and two reviewers in one project may be on different versions. (2) Needs-updating presentation. VU1 keeps the invalidated answer visible, but an option removed or renamed in v2 cannot be shown by v2's control. AF2 also fails closed by falling back to v1 (AF1), which reads legacy questions and posts legacy submissions that R0 refuses for canonical scopes. A published form that trips any AF2 structural guard (supported categories, one label per mutable-unit category, parent-closed hierarchies, roots without targets) cannot be reviewed at all. AF2/annotation-form-data-source.ts:25, 748; AF2/annotation-form-v2-eligibility.ts:51-63; MAIN/src/services/web/CLAUDE.md:343, 353-354, 357 Add to F1 a VersionedAnnotationFormDataSource (loads the pinned form version and its question versions by ID; immutable and cacheable) and a Needs-updating presenter contract (fromVersion, toVersion, treatment, reason, guidance, and the prior value rendered with fromVersion's options and labels). Make AF2's structural guards part of C4 publication validation, server-side with shared fixtures, so every published canonical form version can be rendered by AF2. Canonical routes never fall back to v1; they show a typed error visible to admins. Keep presentation state (entity order) in a separate per-session document, so order-only edits never CAS the session head or create versions.
VB-09 Major PKG/contracts.md C2 l.138-143, C13 l.497-499; PKG/domain-model.md l.116; PKG/open-questions-and-assumptions.md E2 l.104, E27 l.129 Entity instances are undefined. The C2 key relies on "repeated-entity and branch instance identities", but nothing defines what an instance is, who mints it, or how add, rename, duplicate and delete work once revisions are immutable. Today a unit's identity is its label root annotation, AF2 creates units inline with client IDs, and delete prunes the unit's answers and outcome cells. SF3 needs the same instance identity in two forms, and StudyPopulation needs membership per instance from R2a. MAIN/src/services/web/CLAUDE.md:347, 349, 354 (inline creation, identity preserved on save, delete prunes cells); CORE/Model/StudyAggregate/AnnotationSession.cs:51-68 (roots are the units); ExtractionInfo.cs:146-183 (children merged per root) Define at F1: instance identity is the label AnnotationHead ID in the author's scope, minted once (VB-16). Rename is a new label revision. Delete is a set of withdrawal revisions on the label head and its descendants in one commit; it is append-only and flags other forms' sessions as outdated. Duplicate mints new instance IDs, with copied revisions carrying copiedFrom provenance. Population membership is an attribute of the instance; the default population is derived (DD-24). Outcome cells key on instance IDs. Add a conformance test per operation.
VB-10 Major PKG/domain-model.md principle 3 l.37-39, l.17-19, l.252-253; PKG/contracts.md C1 l.113 "Never edited" has no persistence mechanism. The generic save is an upsert ReplaceOne filtered on Audit.Version, so saving a loaded immutable record again silently replaces it. Collections are reachable only as IAggregateRoot and are named from the C# class name, so the planned renaming at F1 ("reconciled with the QM v2 harvest") would orphan any data written earlier. QM v2's own documents name pmAnnotationQuestion while its class AnnotationQuestionV2 maps to pmAnnotationQuestionV2. Nothing makes an export's reproducibility claim verifiable (no tamper evidence). TTL deletion, used on other collections, would breach "nothing published is deleted". MAIN/src/libs/mongo/SyRF.Mongo.Common/MongoExtensions.cs:262-299; MAIN/src/libs/mongo/SyRF.Mongo.Common/MongoContext.cs:155-163; QMA:.../QuestionVersioning/AnnotationQuestionV2.cs:15; MAIN/docs/architecture/mongodb-reference.md l.106-111 (TTL precedents) Add an AppendOnlyRecord base and an IAppendOnlyRepository<T> exposing only Insert, InsertMany and Find; on duplicate key, compare the digest and return idempotently or a typed conflict. Add an architecture test, following StudyWriteLockArchitectureTests, that fails the build on ReplaceOne, Update*, Delete*, FindOneAnd* or update models in bulk writes against registered immutable collections. Put a ContentDigest on every version, revision and snapshot, and record digests in export manifests. Freeze explicit collection names at F1, decoupled from class names, with a test asserting the map. No TTL index on any canonical collection.
VB-11 Major PKG/domain-model.md l.75, principle 5 l.43-45; PKG/migration-adoption-rollback.md l.94-95 ("keeping original IDs"); PKG/contracts.md C4 l.199, l.225-227; PKG/open-questions-and-assumptions.md E24 l.126, E31 l.133 Identity and referential integrity are unspecified in storage. (a) System question IDs are constants shared by every project, so a project-scoped pmQuestionDefinition keyed by _id = questionId cannot hold them, and adoption that "keeps original IDs" collides the same way. (b) There is no list of invariants and no checker for dangling references: revision to question version; session version to revisions and form version; gold to reconciled revisions; stage settings to form and profile versions; task to candidate session versions; and references across projects. © QD1's enforcement points are not listed: the legacy delete cascade must refuse canonical questions, deleting a draft must check draft-form references, and no path may delete a published definition. (d) New collections must be registered with the deletion lifecycle and with restore checks. CORE/Model/ProjectAggregate/AnnotationQuestion.cs:520-592 (system questions built per project from constant GUIDs, with structure varying by SystemQuestionVersion); CORE/Services/ProjectManagementService.cs:201-229 (delete cascade); MONGO/Repositories/StudyRepository.cs:1122-1141 (deletion scheduler pending) Use a record GUID as _id, with a unique {ProjectId, QuestionId}. Keep system versions in a global pmSystemQuestionVersion keyed by (QuestionId, SystemQuestionVersion, StructuralDigest), referenced from project definitions (consistent with VA-09). Write referential invariants I1–I6 into C1/C4 and deliver a read-only integrity checker in R2a, used by E31 rehearsals and R6 verification. Add an architecture test that lists every pm* collection with a ProjectId field against the deletion-lifecycle and restore registries.
VB-12 Major PKG/acceptance-criteria.md AC-M0-02 l.90, AC-R2a-19 l.172, AC-R2c-06 l.194, AC-R4a-13 l.278; PKG/open-questions-and-assumptions.md E28 l.130, E25 l.127; PKG/domain-model.md l.143, l.174 Benchmarks and limits are not grounded in SyRF data. The largest project has 2,023 questions (a 1.45 MB Project document, 6× the next), and an ordinary project holds about 270 annotations per reviewer per study; extraction multiplies that by entity instances. FEAT-001's premise that "N is tens, not thousands" is false. M0 benchmarks 200 questions with sequential writes. E28 limits "form size" in questions, but the real risk is pins, changed revisions and bytes per commit. No concurrency arms are required, although ADR-019 measured two things: (1) any per-project document written by every save serialises the write path (399 of 1,000 submissions exhausted their retries at 10 writers on different studies); (2) 25 commands per transaction already broke the p95 gate. The plan adds ProjectCommitSequence to every commit (see DD-02), on top of heads, revisions, a session version, Study, receipts and inbox rows. Overlaps DD-01 and DD-02. MAIN/docs/platform/saga-duplicate-event-handling/README.md:36-39, 325-335; PLN/little-doms-db-findings.md:128; MAIN/docs/features/annotation-versioning/README.md:476-489; MAIN/docs/decisions/ADR-019-...md l.21-27; STATS/technical-plan.md l.1022-1038; STATS/phase0-benchmark-and-capacity-baseline.md l.360-408; RULES/materialized-stats.md l.139-145 Use three fixture tiers: typical (50 questions, 100 pins), p99 (340 questions, 1,000 pins), and max (2,023 questions, 5,000 pins, 50 instances per category). Express E28 as maximum pins per session version, maximum changed revisions per commit and maximum BSON bytes per commit, and refuse above it before any write. Require ADR-019's B1/B2 arms (1, 2, 5 and 10 writers; same and different studies), with a write-conflict rate no higher than the source-only baseline. Issue one bulk command per collection, so the command count does not grow with the number of answers, pinned by a CanonicalCommitCommandBudgetTests like the fold budget tests. Drop the per-project counter inside transactions. F6a's watermark (at least one transaction lifetime plus a clock-skew bound in the past) over server-assigned commit stamps already makes as-of reads reproducible, and E20's Study version bump already orders commits within a study.
VB-13 Major PKG/migration-adoption-rollback.md §3 l.94-98, §4 l.112-131; PKG/contracts.md C2 l.146-147, C3 l.164; PKG/open-questions-and-assumptions.md E10 l.112 The adoption mapping cannot be applied as written. (a) Conflicting legacy duplicates for one context (acknowledged under SF5) map to one natural key. The unique index rejects the second, and a head with a single current pointer cannot represent "conflict; the latest is never chosen". (b) Adopted revisions need a typed AuthoredUnder = Unknown value, but C1 declares questionVersionRef as a plain reference that every consumer will dereference. © "Ancestors included automatically" adds questions legacy reviewers never saw; if they are required, legacy completed sessions fail validation. (d) Options and conditions exist in two representations: schema-v0 OptionInfo entities with IDs, and schema-v1 values plus ADR-011's hybrid ConditionalParentAnswers. (e) Merging two stage sessions into one FormSession drops a legacy session ID that #3944 threads and exports reference. Ledger SF5 l.457-458; CORE/Model/ProjectAggregate/OptionInfo.cs:67-130; ADR-011 (representation section); MAIN/src/services/web/CLAUDE.md:357; PKG/notifications-integration.md §1.3 item 10 Add to E10/C2: a Conflicted head state holding at least two unordered legacy-snapshot revisions and no current pointer until the reviewer resolves it with Fix or Save, excluded from prefill and agreement in the meantime; a typed AuthoredUnder union; ancestors added at adoption are display-only and never required; a deterministic v0/v1 mapping to canonical options and conditions (reuse OptionInfo.Id for v0, mint IDs for v1 values, list unmatched values in the manifest); and a LegacyIdAlias table used by #3944/#3945 remapping and by exports. Use Annotation.Question as evidence of wording (VA-18).
VB-14 Minor PKG/contracts.md C3 l.165; PKG/integrated-plan.md l.414-415; PKG/acceptance-criteria.md AC-R2a-17 l.170; PKG/open-questions-and-assumptions.md E26 l.128 History capture is underspecified. Capture must happen in the same transaction as each legacy screening write, but those writers are mostly not transactional and FEAT-024 tests pin their command counts. AC-R2a-17 does not name the writers: interactive submit (two paths), administrative screening, reference-file screening columns (two parsers), the bulk-update planner and seeding. API/Controllers/ReviewController.cs:998; API/Services/ReviewScreeningFoldTarget.cs:63; CORE/Services/ReviewEligibility/AdministrativeScreeningPolicy.cs:81; CORE/Services/ParserImplementations/NewParser/ScreeningColumnHandler.cs:157; CORE/Services/StudyReferenceFileParser.cs:239; CORE/Services/BulkStudyUpdate/Atomic/BulkStudyUpdatePlanner.cs:177; RULES/materialized-stats.md l.139-145 For admitted projects only, either wrap capture and source write in one transaction, or capture into a bounded log embedded in the Study that a worker moves out (the pending-entry pattern, which needs no transaction). Name every writer in AC-R2a-17 with one test each, and record the command-budget change with the FEAT-024 owner.
VB-15 Minor PKG/contracts.md C5 l.250-256; PKG/open-questions-and-assumptions.md E21 l.123; PKG/domain-model.md l.91 Draft retention and size are open. The existing precedent for retention on SyRF collections is TTL indexes, which delete silently; that contradicts audited discard and LC1 readiness. Draft size and write granularity are undefined, although AF2 autosave can rewrite a large normalised draft often. Drafts in two forms on one shared head (R2d) have no conflict criterion. Cardinality is VA-12. MAIN/docs/architecture/mongodb-reference.md l.106-111; MAIN/src/services/web/CLAUDE.md:344 (the store owns normalised drafts) No TTL on drafts; retention only through an audited discard (by the owner, an admin, or the system with an audit record). Store drafts as patches against the base version, with a size cap tied to E28. Add AC-R2d-09: saving form G with a draft based on an older revision of a head changed through form F returns a typed stale-base conflict that shows both values and keeps G's draft.
VB-16 Minor PKG/domain-model.md §7 l.217-218; PKG/contracts.md C1 l.104; PKG/open-questions-and-assumptions.md E27 l.129 ID minting is contradictory. The package says both "clients no longer choose IDs" and "client-proposed and validated". AF2 mints session and outcome-data IDs on the client today. Server-only minting would force remapping temporary IDs to server IDs across the AF2 store, comments, entity order and outcome cells. Random client session IDs also let two tabs create two FormSessions for one natural key. AF2/annotation-form-data-source.ts:103-104; AF2/annotation-form-outcome-topology.ts:31-33; deterministic SHA-256 IDs for StudyConversation (PKG/notifications-integration.md §1.3 item 2) Use deterministic name-based IDs (SHA-256 over a versioned canonical key, stored as CSUUID) for aggregates with natural keys: FormSession, AnnotationHead (via its key hash), ScreeningOutcome, StudyGold, ReconciliationTask and CanonicalOwnership. Use client-proposed, server-validated IDs for revisions and entity instances (unused, same project). Keep legacy IDs at adoption, with the natural-key unique index as the real guard.
VB-17 Minor PKG/open-questions-and-assumptions.md Q-08 l.48; PKG/acceptance-criteria.md AC-M0-04 l.92; PKG/integrated-plan.md l.992 The harvest decision has no "avoid" list, and several QM v2 parts would breach confirmed rules if ported. ReconstructiveRollbackService rebuilds embedded state and then deletes the extracted documents, a destructive down-migration. ADR-012's draft RevertToEmbeddedQuestionModel hands a scope back to legacy. Drafts and question-set versions live inside the Project document. AQVersion carries PublishDecisions, Optional and Multiple, although policy and requiredness belong to the form and C4 treats multiplicity as structural. AnswerOptionFilters is an untyped object?. Version arrays embedded in annotation and session documents are unbounded. Annotation identity has no entity path, owner scope or population. Export selectors are keyed by stage, and AsOfDate is a raw DateTime. QMC:.../Services/ReconstructiveRollbackService.cs:13-16; QMC:.../DataExportJobAggregate/ExportSpec.cs:6-20, 100; QMA:.../QuestionVersioning/AQVersion.cs:56-68; QMA:.../AnnotationQuestionV2.cs:113, 119; QMA:.../AnnotationAggregate/Annotation.cs:34-44; QMA:.../AnnotationSessionAggregate/AnnotationSession.cs:32-38; QMA:docs/decisions/ADR-012-DRAFT-...md l.31, 51, 150-154 Add a harvest and avoid table to the Q-08 execution record (§3 item 8) and make AC-M0-04 require it.
VB-18 Minor PKG/integrated-plan.md R0, R2a; PKG/domain-model.md l.159 Index builds on pmStudy are not planned. Repositories create indexes at start-up, but FEAT-024 refused that for pmStudy: IX_Study_PendingStatistics is built only through an admin route with commit quorum. Any new pmStudy index (projection, ownership marker, stubs by FormId) needs the same treatment, and the plan says nothing about it. MONGO/Repositories/StudyRepository.cs:3063-3100; STATS/STATUS.md l.353; RULES/materialized-stats.md l.149-150 List each new pmStudy index in the storage ADR with its build route (operator, commit quorum, staged per environment) as an R0/R2a deployment step. Prefer partial indexes on the marker, which cost nothing until used, as the bulk-lock index does.
VB-19 Note PKG/open-questions-and-assumptions.md E32 l.134; PKG/migration-adoption-rollback.md l.65-68 Account erasure can be handled without editing immutable records, provided revisions, receipts, exposure events and command records store only opaque investigator GUIDs (never names, emails or display labels) and account deletion anonymises the Investigator record. Free-text notes remain the residual risk. MAIN/docs/architecture/mongodb-reference.md l.132-136 (Identity tombstones survive deletion) State this storage rule in E32 and add a schema check for it.
VB-20 Note PKG/open-questions-and-assumptions.md E31 l.133; PKG/migration-adoption-rollback.md l.178-179 Consistency across collections is guaranteed only by a whole-database point-in-time restore, and ADR-011's recovery playbook also allows restoring selected collections. ADR-011 "Rollback and restore"; references to selected-collection restore Canonical scopes are restored only by a point-in-time restore into an isolated database, followed by selective recovery driven by a manifest. Run the VB-11 checker after every rehearsal.

3. Improvements

  1. Storage blueprint to freeze at F1, which also settles E15:
Collection (proposed) Identity and unique keys Other indexes Serves
pmQuestionDefinition (+ versions) _id = record GUID; {ProjectId, QuestionId} unique; versions {DefinitionId, Seq} unique {ProjectId, Status} Resolving pinned versions by ID; designer lists
pmSystemQuestionVersion {QuestionId, SystemQuestionVersion, StructuralDigest} unique — System pins across projects
pmAnnotationForm (+ versions) {ProjectId, FormId}; versions {FormId, Seq} unique; head holds CurrentPublishedSeq, PublicationSeq — Phase-1 CAS; pinned form resolution
pmFormSession Deterministic _id; {ProjectId, StudyId, FormId, AuthorId} unique {ProjectId, FormId, CurrentFormVersionSeq, State}; {ProjectId, StudyId, AuthorId} Publication predicate, usage fallback (Q-31), SF5 and outdated reads, reconciliation candidates
pmFormSessionVersion {SessionId, Seq} unique; {ProjectId, CommandId} unique (the receipt) {ProjectId, FormId, CommittedAt} History panel; previous-version and as-of exports
pmSessionDraft {SessionId} unique (or per workspace, VA-12) {ProjectId, FormId, BaseFormVersionSeq} draft_only category; LC1 readiness
pmAnnotationHead {ProjectId, KeyHash} unique; partial unique per kind {ProjectId, StudyId, AuthorScope, QuestionId}; {ProjectId, QuestionId, CurrentQuestionVersionId} Current state (head carries a copy of the current payload); ancestors; usage per question version
pmAnnotationRevision _id; {AnnotationId, Seq} unique {ProjectId, StudyId, CommittedAt} As-of reconstruction; gold and task references
pmFormPublishOperation (+ chunks) One active per form (unique partial index); chunks {OperationId, Chunk} unique — Phase-2 sweep; audit manifest
pmStudyGold (+ snapshots) {StudyId} unique; snapshots {StudyId, Seq} unique — Pointer CAS; gold history

The constraints that settle E15: - Never embed revisions in sessions. SF3 and SF5 share revisions across forms, and gold and tasks reference them. - Store the full pin map per session version in its own document (consistent with VA-22), bounded by E28. - Keep revisions in their own collection for the as-of index. - Keep only the projection in Study.

  1. Reuse reviewed patterns already on main instead of new ones:
  2. IAggregateWriteGuard with StudyWriteLockArchitectureTests for ownership;
  3. ADR-020's operation record, plan chunks (written because a plan can exceed 16 MB) and lock, apply, release for publication phase 2, adoption cutover and merging reviewed studies;
  4. pmRobRunOperation's unique partial index for "one active operation per scope";
  5. FEAT-024's UnknownElements capture for the floor, and its command-budget tests for the canonical commit;
  6. deterministic SHA-256 IDs, as #3944 uses;
  7. operator-route index builds;
  8. BeginIsolatedReads() in every canonical command handler, because the repository cache hands out the same mutable instance for two seconds (RULES/repository-cache.md).
  9. Cache immutable definitions forever. A process-wide cache keyed by version ID needs no invalidation, and version-by-ID endpoints can return Cache-Control: immutable. This keeps AF2 history and Needs-updating reads cheap and takes Project-document reads off the review hot path (118–465 KB typical, 1.45 MB maximum).
  10. Outdated flags are naturally small. Heads belong to one author's scope, so a shared-answer change can only make the same reviewer's sessions on the same study outdated, at most one per form sharing the question. Write the flag in the same transaction, or derive it on read (VA improvement 4), and drop E30's eventual fan-out and the inbox-flood risk for SF5. Reconciliation drift is likewise bounded per (study, form).
  11. Ship the integrity checker (VB-11) in R2a and run it in CI fixtures, rehearsals, R6 verification and after restores.
  12. Freeze a typed error catalogue with the F1 fakes: StaleBase, DraftConflict, OwnershipRefused, LockedByBulkUpdate (existing), PublicationInProgress, FormVersionNotRenderable, SizeLimitExceeded, ConflictedLegacyAnswer and CommandDigestMismatch. Generate them through NSwag (the client is pinned at 14.0.8 with checksums) and have AF2 handle each without losing drafts.
  13. Digest everything immutable (version content, pin maps, snapshots) and put the digests in export manifests. "Two exports at the same watermark are identical" then becomes a checksum comparison.
  14. QM v2 harvest and avoid table.
Category Items
Harvest VersionHistory<T>; the typed AnnotationAnswer payload with EnsureCompatible; ChildQuestionScope (lazily committed, shared versus per-answer children) as a structural property in C4 and the C2 key; ReplacementDraftLineagePlanner, if D38 stands; CandidateProjectQuestionSetValidator, CrossQuestionValidationService and AnnotationValidationState as inputs to E23; SystemQuestionFactory as the E24 snapshot builder; AnnotationMutationMapper, ExtractedAnnotationLegacyMapper, MigratedStudyReadModelAssembler and MigrationValidationService as R6 adapters and parity checks; the StageTransitionWorker pattern of lease plus batch writes plus a job CAS in the same transaction (with the VB-06 fixes); the ExportSpec mode reservation, renamed to form-version selectors
Avoid Everything listed in VB-17
  1. Store kind and status enums as strings in canonical records, as FEAT-024 does for its pending kinds, parsing a closed set. This removes the ordinal-append failure mode across mixed fleets and keeps exports readable.

Missing acceptance criteria (proposed IDs):

ID Criterion From
AC-R0-06 In a mixed fleet with interleaved legacy and canonical writes, persisted computed fields equal an authoritative recount VB-01
AC-R0-07 Per-type round trip of unknown nested fields through the older class map, including schema-conditional serialisation VB-02
AC-R2a-20 A retry after commit, before the fold runs, returns the original result and writes nothing VB-04
AC-R2a-21 The multikey context-key fixture passes VB-05
AC-R2a-22 The immutability architecture test is green, and digests verify on read-back VB-10
AC-R2a-23 The integrity checker reports zero findings on seeds and detects an injected dangling reference VB-11
AC-R2a-24 Concurrency arms and command budget at the three fixture tiers VB-12
AC-R2a-25 A session pinned to v1 renders v1 after v2 publishes; a form AF2 cannot render is refused at publication; no AF1 fallback VB-08
AC-R2a-26 Unit delete is a withdrawal that flags the same reviewer's other forms; rename keeps identity; duplicate mints new IDs with provenance VB-09
AC-R2a-27 One capture test per legacy screening writer VB-14
AC-R2c-10 to 13 Fold drain after a sweep; flat phase-1 time; swept late saves, including those below the cursor; one active publication per form, with FV4 generation checks VB-03, VB-06
AC-R2d-09 Cross-form draft conflict on a shared head VB-15
AC-R6-05 (restated) All-or-nothing via locks, not atomic VB-07
AC-R6-07 Conflicting legacy duplicates adopt as a Conflicted head VB-13
AC-R6-08 The legacy ID alias table remaps #3944 threads and exports VB-13

4. Questions for Chris

  1. Scoped pause while a publication is applied. While a new form version is applied to existing sessions, may SyRF pause new study admission and reconciliation-readiness changes for that form only? Reviewers would keep saving, and the admin would see progress. Recommendation: yes. Usually this means minutes; if it runs past a set limit (for example 30 minutes), it stops and shows as pending to the admin.
  2. One publication at a time per form. Admins would wait for v2 to finish applying before publishing v3. Recommendation: yes. It removes a class of policy-composition errors at little cost, because publications are rare.
  3. Initial size ceiling. Until benchmarks prove larger forms safe, canonical form versions above a measured ceiling would be refused at publication. The largest existing project (2,023 questions) would stay on the legacy path even after adoption opens. Recommendation: yes, and revisit after M0 with the tiered benchmarks.
  4. Account deletion and immutable answers. Should a deleted reviewer's answers stay in history, attributed to an anonymised identity, never deleted or rewritten? Recommendation: yes; this answers E32's escalation.
  5. Abandoned drafts. Drafts would never be deleted automatically. Stale drafts would be shown to admins, because they block automatic stage completion under LC1, and removed only by an audited discard. Recommendation: yes. No automatic expiry.

5. Coverage gaps

  • No data was read from production, staging or preview databases. Sizes come from repository documents: the 2,023-question project is from a January 2026 analysis, and the roughly 270 annotations per study from the little-DOMS findings. The frequency of legacy duplicates and the session counts per form are unknown.
  • Atlas server settings are UNVERIFIED: the server version, transactionLifetimeLimitSeconds and minSnapshotHistoryWindowInSeconds. I therefore did not rely on atClusterTime snapshot reads; the commit-stamp predicate in VB-06 works without them.
  • QM v2 coverage is partial. I read #2572, #2573 and #2574 selectively; I did not review #2575 (web scaffolds) or #2461.
  • Storage outside my core dimension was not analysed: P1/P2 (Citations embedded or separate), O1 outcome schemas, C1 populations, gold-snapshot sizing at scale and ExposureEvent write volume.
  • Not tested: NSwag generation of polymorphic revision payloads, and AF2 rendering performance with a version-pinned data source.
  • Round-2 overlaps. I read the DD and VA reviews only to avoid duplication. Overlaps are cross-referenced: DD-01, DD-02, DD-10, DD-13, DD-24, VA-03, VA-05, VA-09, VA-12, VA-18 and VA-22.

Critical files for implementation

  • /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/pr/pr3617.research-screening-as-specialised-annotation-gxgahs/docs/planning/integrated-review-plan-2026-10/domain-model.md
  • /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/Model/StudyAggregate/ExtractionInfo.cs
  • /home/chris/workspace/syrf/main/src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Repositories/StudyBulkUpdateLockGuard.cs