# Architect Review — Dual-coach model rerun

Reviewed immutable Planner artifact: `/home/cube/projects/richard/traning coach/.gjc/_session-019fabe7-62cd-7000-b761-e040602b488c/plans/ralplan/dual-coach-model-rerun/stage-01-planner.md`

Planner SHA-256: `88a5f24a882329852fff159644ae1a5abd096a6ae0e184bd20bc99d4a01dc968`

## Summary
The plan preserves customer isolation, append-only correction, safety overrides, explicit operator delivery, and the no-calendar boundary, but it is not implementation-ready. Four high-severity contract gaps leave legacy history silently unprojected, strategy ownership split across two ledgers, the required automatic reminder contradicted by the plan's own no-send rule, and operator schedule confirmation without a defined authority transition.

## Claims
- `checkin_cli.models.Event` and `CanonicalEventTransaction` are the registered customer-local canonical source-fact boundary; the model is frozen/extra-forbid and the transaction binds `wizard/events.jsonl` to the customer's `nutrition-plans/canonical-sequence.jsonl`.
- `checkin_cli.adaptive_nutrition.AdaptiveEventStore` already owns plan proposal/revision/approval/activation/delivery lifecycle events, so persisting baseline/confirmed strategy lifecycle again as canonical `Event` payloads creates two authorities unless the plan assigns ownership explicitly.
- `history_imports._baseline_event()` persists `history_imported` with `status=archived`, `occurred_at_kst`, and no `import_manifest`; `project_canonical_events()` first excludes statuses other than `accepted`/`unsafe` and later separately requires `import_manifest.observation_kst_day`. The reported mismatch therefore has two independent causes.
- `customer_admin._reconciliation_event_plan()` already derives and persists `observation_kst_day` from `occurred_at_kst`, providing a compatible immutable date source and cross-check boundary without rewriting archived events.
- The originating interview requires one automatic reminder before non-response review and prohibits automatic delivery of AI results, not every bounded operational reminder. Planner acceptance criterion 8 broadens this into a blanket scheduler no-send rule.
- The canonical runbook separates the configured Topic-59 review triple from the refreshed canonical-owner triple and permits only typed, persisted, capability-bound mutations. The existing action set has no schedule-confirm action.

## Analysis
### Spec compliance
The proposed schedule reference, restrictive unconfirmed baseline, six morning fields, safety overrides, last-change note, customer isolation, and no trainer-calendar scope match the locked interview intent. The non-response branch does not: recording a reminder fact without a proven bounded reminder cannot satisfy “one automatic reminder and still no response,” while prohibiting scheduler sends makes that requirement impossible.

### Architecture
The strongest antithesis to the selected design is that adding every derived state to canonical `Event` appears auditable but actually duplicates the mature adaptive plan lifecycle. Canonical events should remain source facts; adaptive strategy revisions should remain in the adaptive journal. A one-way projection can preserve auditability if its exact source event/revision digest, partial-append recovery, lock order, and disagreement behavior are specified.

The import gate is correctly sequenced first but is not decided. Merely adding `import_manifest` would not recover archived history because status filtering occurs earlier. The least invasive compatible design is to preserve old rows, explicitly admit only validated archived `history_imported` observations, use their existing KST occurrence date as the canonical observation day, and require the reconciled source-day mapping to agree. Any new manifest may be additive evidence, not a prerequisite that invalidates legacy rows.

Operator confirmation is a separate domain transition from adaptive plan approval. It must consume a typed persisted capability for an exact schedule revision, retain authenticated-review and canonical-owner snapshots/pins, and be invalidated by schedule supersession. Reusing `approve` without this distinction would bind the wrong aggregate and authority.

Risk routing is directionally sound but the repository has no identified “common threshold” contract. The plan must pin an approved versioned risk-policy owner, deterministic formula, missing-input behavior, and dedupe preimage rather than letting tests canonize an arbitrary implementation.

### Constructive synthesis
Use this ownership split:
1. Canonical EventStore: schedule-reference facts/corrections, morning check-in facts, typed deterministic review-trigger facts.
2. AdaptiveEventStore: baseline/confirmed nutrition proposal revisions, approval, activation, and delivery lifecycle.
3. Host AdaptiveOperatorService: persisted review sessions/capabilities, schedule-confirm mutation, and review-card publication; review identity and canonical owner remain distinct.
4. Customer schedule ledger: only actual bounded reminder delivery reservation/receipt evidence; non-response projection reads it but does not fabricate delivery.

Required cross-boundary identifiers are customer key, source schedule/check-in event ID, terminal schedule revision digest, policy version/digest, feature epoch, proposal parent/revision digest, and operator authority/config pins. Any disagreement or partial transition fails closed and is reconciled; no ledger silently wins by recency.

## Root Cause
The plan identifies the right subsystems but defers aggregate ownership and transition contracts to implementation. That leaves derived strategy state, imported observation dates, reminder delivery evidence, and operator authority without one authoritative lifecycle, making split-brain recovery and false evidence possible.

## Findings
1. **HIGH — Planner lines 44–45 — Legacy history contract remains unresolved and incomplete.** `import_manifest` is absent, and archived status is independently filtered out. Fix the plan by selecting the compatibility representation and explicitly supporting immutable archived legacy rows with source-day agreement.
2. **HIGH — Planner lines 41–50 — Strategy ownership is split between canonical and adaptive ledgers.** Keep source facts canonical and use the existing adaptive revision/approval journal for strategies; define one-way projection and crash reconciliation.
3. **HIGH — Planner line 52 and acceptance criterion 8 — Required reminder is contradicted by blanket no-send behavior.** Permit only a bounded static operational reminder through reservation/receipt fencing; create non-response review only after proven reminder delivery and elapsed response window. Unknown remains terminal and cannot count as reminded.
4. **HIGH — Planner lines 50–53 — Operator schedule confirmation lacks a typed authority transition.** Define an exact-revision schedule-confirm capability/action with review identity, refreshed canonical owner, config/registry/consent/activation/epoch pins, single consumption, and supersession invalidation.
5. **MEDIUM — Planner lines 69–80 — Composite risk policy is not pinned.** Name the policy artifact/owner, formula, threshold semantics, missing evidence behavior, version/digest, and deterministic review key.

## Recommendations
1. Revise the plan before implementation to settle the four HIGH findings; do not defer them to executor choice.
2. Amend the import gate to cover both `archived` status and observation-day representation, with legacy/new compatibility tests and no event rewrite.
3. Replace canonical nutrition-strategy event ownership with an explicit canonical-source → adaptive-revision projection and recovery contract.
4. Separate AI/coaching no-auto-send from the single allowed operational reminder; define template, due window, response correlation, delivered evidence, and unknown outcome semantics.
5. Add a typed schedule-confirm host action/capability and pin its authority/invalidation model.
6. Define the versioned risk policy before writing threshold tests.

## Architectural Status
`BLOCK`

## Code Review Recommendation
`REQUEST CHANGES`

## Tradeoffs
| Option | Benefit | Cost / risk | Recommendation |
|---|---|---|---|
| Put schedule and strategy lifecycle in canonical Event | One visible event stream | Duplicates adaptive plan revision/approval truth and complicates recovery | Reject for strategy lifecycle |
| Canonical source facts → adaptive strategy revisions | Reuses established approval/delivery state machine and keeps aggregates clear | Requires explicit projection/reconciliation pins | Preferred |
| Require new `import_manifest` on every history row | Strict new shape | Existing immutable rows remain invalid; archived status still skips them | Reject as sole fix |
| Use existing `occurred_at_kst` plus source-day agreement for legacy history | Preserves append-only data and matches current reconciler | Needs history-specific validation and disagreement stop | Preferred |
| Record reminder intent without sending | Zero provider calls | Falsely claims reminder prerequisite and violates interview acceptance | Reject |
| Send one fenced static reminder | Meets acceptance with exactly-once evidence | Requires a narrow delivery workflow and operator-approved template | Preferred |

Verification: planning review only; no tests, formatters, implementation, or provider actions were run.
