# Critic pass 1 — Dual-coach connection flow

## Artifact binding
- Planner path: `/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`
- Planner stage: `planner`
- Planner stage_n: `1`
- Review lane: `critic`
- Architect output consumed: no

## Verdict
**ITERATE**

## Summary
The plan preserves the locked non-goals, selects the right broad direction over a registration-only schedule or parallel calendar service, and gives unusually strong isolation, fail-closed delivery, migration, and verification constraints. It is not yet implementation-ready because it postpones the importer/projection contract decision, leaves derived strategy authority ambiguous between canonical and adaptive ledgers, has no producer for the required durable reminder fact, and omits an executable risk-policy contract plus exact host anchors.

## Claims checked
1. **Principle/option consistency:** Principles 2–5 and Option A are directionally consistent with the minimal, append-only, customer-isolated handoff. Options B and C are fairly rejected: `AdaptiveRegistrationInputs.training_schedule` is mandatory and dated (`customer_coaching.py:143-226`), while a new schedule store would duplicate existing identity and lifecycle boundaries.
2. **Importer/projection gate:** The defect is real. `history_imports._baseline_event()` writes archived history with `occurred_at_kst` and `check_in` but no manifest (`history_imports.py:83-104`); `project_canonical_events()` silently skips historical rows without `import_manifest.observation_kst_day` (`adaptive_nutrition.py:2585-2590`). `models.Event` and the external schema currently forbid that property (`models.py:759-785`; `contracts/checkin-event.schema.json:1-270`).
3. **Existing lifecycle authority:** `AdaptiveEventStore` already persists plan proposal/revision/approval states (`adaptive_nutrition.py:4579-4647`, `4991-5016`). The proposed new canonical strategy/review event types therefore need an explicit ownership rule, not a later implementation-time choice.
4. **Isolation and delivery:** The plan correctly preserves exact triples, customer-local state, no fallthrough, unknown-receipt terminality, and explicit latest-revision approval. These match the canonical runbook's Topic-59 and host lifecycle constraints (`PILOT_RUNBOOK.md:24-49`, `116-145`).
5. **Exact anchors:** The plan's `canonical_snapshot()` anchor does not exist; the inspected projection entry point is `project_canonical_events()`. The canonical runbook names the omitted host repository and anchors: `/home/cube/projects/richard/hermes-agent`, `gateway/platforms/nutrition_coaching.py`, `gateway/platforms/telegram.py`, `gateway/platforms/nutrition_coaching_config.py`, and focused gateway tests (`PILOT_RUNBOOK.md:99-113`).

## Analysis

### What is sound
- The chosen option reuses the typed canonical transaction and avoids a trainer calendar, booking workflow, or generic route.
- The baseline is explicitly prohibited from workout-day targeting, while confirmed strategy requires an operator-confirmed schedule and a new immutable revision.
- Safety overrides are independent of score; pain and inability to train fail closed.
- Acceptance criteria cover malformed/stale/duplicate input, append-only correction, A/B isolation, restart/migration, unknown delivery, and provider call count zero before explicit send.
- Focused and full-suite verification is sequenced after the schema gate, and automated evidence is not confused with Gate-D or rollout approval.
- Synthetic history, production activation, direct storage edits, raw conversations, automatic customer delivery, recurring calendars, and detailed scheduling remain explicit non-goals.

### Blocking inconsistencies and omissions

#### 1. The first gate diagnoses the defect but does not decide the contract
“Select one canonical representation” is still the central migration design decision. An executor cannot safely implement downstream projection or prove preservation until the plan chooses a model/schema version and legacy-read rule. A viable bounded design is: make the observation day explicit and typed for all new schema-v2 history imports; recognize existing schema-v1 `historical_markdown` rows through one narrow, tested adapter that derives the day from their exact KST `occurred_at_kst`; append source-day mappings without rewriting archived events; reject ambiguous/malformed legacy evidence rather than skipping it. Another viable design is to make the canonical event's KST occurrence date authoritative for all history and remove the manifest-only projector condition. The revised plan must select one, not carry both.

#### 2. Strategy and review ownership can become parallel truth
The plan says baseline/confirmed/superseded strategy and review item are new canonical `Event` payloads, then builds them around APIs whose plan revision and approval lifecycle already lives in `AdaptiveEventStore`. Its mitigation says only to “define one projection authority.” This conflicts with the single-source principle and leaves crash recovery, latest revision, supersession, and dedupe undefined across two append-only ledgers.

Prefer the existing boundary: canonical EventStore owns authenticated source facts (first-session reference, confirmations, check-ins, corrections, safety); the customer-local adaptive journal owns deterministic derived baseline/strategy revisions, review lifecycle, approval, and delivery state. Adaptive rows must pin canonical source event IDs/digests, policy versions, customer key, and authority/epoch pins. If the reverse is chosen, the plan must retire or migrate overlapping adaptive event types and define an atomic reconciliation protocol. Do not write the same lifecycle fact to both and choose the winner during reads.

#### 3. The reminder requirement has a consumer but no producer
The proposed `customer_schedule.py` evaluator can look for a durable reminder fact, but the plan defines no reminder payload/event, actor, send/receipt transition, check-in window, or deadline. The current schedule ledger records delivery reservations for daily/weekly/monthly tasks; it is not proof that a reminder was delivered. Counting a reservation, a scheduled task, or an unknown receipt would violate the no-inferred-delivery rule. Creating an automatic reminder would violate the no-auto-send non-goal.

Use the existing explicit operator approval/send path: define a typed, customer-scoped reminder intent and append the reminder-sent fact only after the existing delivery lifecycle reaches its audited success state. Its deterministic key should bind customer, expected check-in/window, reminder policy version, and ordinal `1`; unknown or audit-pending delivery does not count. Define the no-response evaluation instant and create one review item from `(customer, window, reminder_fact_id, reason_version)`. This adds evidence, not a second provider path.

#### 4. Risk behavior and host integration are not executable specifications
“Documented common threshold” is not a test oracle. The revised plan must name the six normalized inputs, enum/range mapping, score formula, inclusive threshold value, policy/version identifier, reason codes, and review dedupe preimage. Pain and exercise-impossible remain hard overrides outside the numeric score.

Replace `canonical_snapshot()` with `project_canonical_events()` and name the exact gateway files/symbol-discovery tests from the canonical runbook. The adapter verification should run the focused gateway tests in addition to the package tests and prove all update kinds in Topic 59 reject/no-fallthrough, registered trainer input maps to only one customer, and provider calls stay zero before explicit operator send.

## Findings

### HIGH — Decide the history observation-day representation
- Reference: planner lines 41–43 and sequencing step 1.
- Impact: existing archived rows may continue to be silently omitted or a migration may rewrite evidence.
- Required revision: select the representation, schema/version rule, legacy read behavior, no-rewrite migration, and fail-closed concurrency/rebuild proof.

### HIGH — Select one strategy/review lifecycle authority
- Reference: planner lines 38–50 and risk “Parallel truth.”
- Impact: two ledgers can disagree about latest strategy, review, approval, supersession, or recovery.
- Required revision: assign source facts to the canonical ledger and derived lifecycle to the adaptive journal (recommended), or fully specify the reverse migration and cross-ledger transaction.

### HIGH — Define creation of the one durable reminder fact
- Reference: planner lines 52–55, sequencing step 5, acceptance criterion 5.
- Impact: non-response review cannot be produced without inventing delivery evidence or adding an automatic send path.
- Required revision: specify operator-approved reminder intent, audited receipt condition, one-per-window key, deadline, and idempotent review key.

### MEDIUM — Pin risk policy and correct exact code anchors
- Reference: planner summary, adaptive changes, acceptance criterion 5, gateway integration.
- Impact: threshold tests have no expected value and the host slice cannot be handed off without renewed discovery.
- Required revision: provide formula/value/version/reason/dedupe contract; use `project_canonical_events()`; name the gateway repository files and focused tests.

## Required planner revision
1. Choose and document the historical observation-day representation and legacy migration contract.
2. Declare a single authoritative ledger for source facts versus derived strategy/review lifecycle, including crash/idempotency boundaries.
3. Define the explicit, non-automatic reminder lifecycle and exact no-response timing/dedupe contract.
4. Specify the numeric risk policy and correct all projection/gateway anchors.
5. Preserve the current alternatives, isolation matrix, no-send behavior, safety overrides, non-goals, and verification structure; these do not need expansion.

## Routing
Return to Planner for one bounded revision. Re-run Critic against the revised immutable Planner artifact; no implementation, tests, formatter, fixture generation, migration, activation, provider call, or Architect-output consumption is authorized in this lane.