# Critic review — Guarded Korean expression-only postprocessor plan

**Reviewed artifact:** `/home/cube/projects/richard/traning coach/.gjc/_session-019fa7fb-3be3-7000-b23f-03f6c05d81ff/plans/ralplan/019fa7fb-3be3-7000-b23f-03f6c05d81ff/stage-01-planner.md`  
**Planner binding:** stage 1, SHA-256 `57e9aaac51a12d7e93f13bfd3bb5b9d7703f844bbad9ba2f0f992edc579e1da1`  
**Review mode:** plan-only and read-only; Architect output was not consumed.

## Verdict
**ITERATE**

The finite, code-owned variant-selection direction is appropriate, and the plan correctly blocks the existing free-form daily interpretation from remaining authoritative. Execution is not yet approved because the plan does not actually guarantee one transport attempt, does not cover the repository's full Kimi/Moonshot family, leaves the daily/weekly slot contract and shadow mode undecided, and points integration tests at a different profile-local package than the proposed implementation.

## Claim Checks

- **Artifact identity verified.** The ralplan `index.jsonl` binds planner stage 1 to the supplied immutable path and exact SHA-256.
- **The unsafe daily path is real and correctly prioritized.** `telegram.py:_nutrition_daily_interpretation` accepts up to two arbitrary model lines after checking only five token fragments. `_saved_physique_coaching_feedback` calls `_generate_physique_coaching_feedback` after Save, `_render_physique_callback_prompt` injects the result, and `_render_physique_feedback_replay` repeats the same route. This is the existing unvalidated daily LLM interpretation path the implementation must eliminate, not merely hide behind a stronger prompt.
- **The weekly ordering claim is correct.** `_send_nutrition_coaching_tick` renders `_nutrition_report_text`, computes `template_digest`, calls `reserve_customer_task_delivery`, obtains durable provider authority, and then sends once. A cosmetic selection must be finalized before the digest/reservation, while the current unknown/reconciliation states must remain terminal.
- **The adaptive immutability claim is correct.** `render_operator_card` contributes to `NutritionProposal.operator_body` and its digest; callback and recovery paths publish service/durable text. A zero-slot pass-through is the safe v1 decision, and proposal/event/feature-epoch schemas should remain unchanged.
- **The selected option is semantically strong.** A strict response containing only local `variant_id` selections avoids the polarity, omission, modality, and fabricated-fact failures that protected-token or whole-body rewriting cannot exclude. Exact JSON validation plus a local fixed table is a better fit than a second probabilistic critic.
- **The proposed no-retry claim is false under the actual client path.** `agent.auxiliary_client.resolve_provider_client` constructs OpenAI clients without `max_retries=0`; the repository pins `openai==2.24.0`, whose documented default is two automatic retries, including on timeouts, 408/409/429, connection errors, and 5xx. One call to `client.chat.completions.create` can therefore make three wire attempts. Provider wrappers also do not expose one uniform retry-control surface. An outer `asyncio` deadline bounds the Telegram coroutine but does not prevent retries or cost in the still-running worker.
- **The Kimi guard is too narrow.** Rejecting only normalized labels containing `kimi` misses repository-recognized family identifiers such as provider alias `moonshot`, model slugs `moonshot-v1-8k`, `k1.*`, `k2-*`, and Kimi routed through an aggregator or a custom `api.moonshot.ai`/`api.kimi.com` endpoint. The repository already documents broader Kimi/Moonshot family detection in `agent/anthropic_adapter.py` and `agent/moonshot_schema.py`.
- **The daily canonical baseline is unresolved.** With the raw feedback removed, the current `_nutrition_daily_interpretation(None)` always says some fields are missing, even when every fact is present. Conversely, the current complete-data fixture is populated by free-form feedback. Thus “remove free-form feedback,” “keep the interpretation fallback,” and “disabled output is byte-for-byte the current expected daily fixture” cannot all hold without defining a new deterministic complete-data canonical line and updating the fixture/guide.
- **Profile/package references do not line up for integration.** The implementation target is `dualcoachtest/workspace/checkin_cli/checkin_cli/adaptive_nutrition.py`, but `tests/gateway/test_telegram_physique_checkin.py` inserts and imports `/home/cube/.hermes/profiles/physique-coach/workspace/checkin_cli`; that module is materially different. Because `checkin_cli` is process-global in `sys.modules`, merely inserting the dualcoachtest path later will not make those integration tests exercise the new contract.
- **Shadow rollout is not implementable from the stated configuration.** The local mapping has only `enabled`, per-surface booleans, and timeout; it has no shadow/apply mode. The plan also correctly forbids adding this cosmetic flag to adaptive `_FEATURE_FLAG_NAMES`. “analytics_shadow-style validated but discarded” therefore has no specified switch, outcome, callsite, or test.
- **Attribution is grounded.** The Kiro port's MIT license on its `master` branch contains `Copyright (c) 2026 Taehan Kim` and `Copyright (c) 2026 epoko77-ai (Original work - https://github.com/epoko77-ai/im-not-ai)` plus the full MIT permission/warranty text. The plan's proposed credit, source repository, selected-principle attribution, and exclusion of the source workflow/artifact logging are appropriate.
- **Privacy and rollout boundaries are directionally correct.** The selection document need not contain customer copy, identifiers, facts, or digests; enum-only observability and default-disabled isolated-profile rollout fit the stated privacy and Gate-D limits. The remaining legacy daily completion call must be removed, however, or the snapshot is still sent to a model despite the new non-observation principle.

## Missing Evidence

Definitely missing or too thin for execution:

1. A transport-level single-attempt mechanism covering every supported resolved client/wrapper; the current plan only prevents an explicit application loop.
2. A complete Kimi/Moonshot disallow predicate covering aliases, family model names, aggregator slugs, and configured custom endpoints before any model request.
3. Literal v1 slot definitions: exact canonical and warm strings, exact locations/anchors, predicates, offset invariants, maximum response size, and a final decision—not “may” or “if”—for whether daily and weekly have a slot.
4. A deterministic, truthful complete-data daily canonical interpretation and an explicit migration rule for the guide/current fixture after raw feedback is removed.
5. A zero-call acceptance criterion for the legacy `_generate_physique_coaching_feedback`/`_request_physique_coach_completion` route on saved daily and replay rendering. “Cannot be appended” alone still permits a wasteful, privacy-violating call whose result is discarded.
6. A concrete shadow-mode configuration and telemetry contract. The current schema cannot select validated-but-discarded behavior, and `success` does not distinguish shadow from applied selection.
7. A test/package isolation strategy that makes gateway integration load the modified dualcoachtest contract rather than the older physique-coach package.
8. A definition of “one enhancement maximum” under concurrent weekly ticks. Because enhancement is before durable reservation, two contenders can both call the model before only one gains delivery authority. The plan must either coordinate selection per schedule task or explicitly scope the guarantee per render invocation and set matching acceptance.
9. A concrete production telemetry sink and failure policy. “Event/counter” and “injectable seam” do not say whether the default is a structured log or counter, what the latency-bucket boundaries are, how invalid configuration is distinguished, whether a sink exception is swallowed, or where adaptive `no_eligible_slot` is classified without touching durable callback/recovery text.
10. Adversarial coverage for SDK automatic retries, Kimi aliases/family endpoints, bool-as-int/unknown config keys, overlapping/out-of-bounds/stale slot anchors, response-shape failures, late worker completion after the outer deadline, concurrent weekly ticks, shadow discard, and telemetry-sink failure.

## Approval Boundary

The architectural choice of finite local variants, exact canonical fallback, pre-reservation weekly finalization, zero-slot adaptive pass-through, profile-only default-disabled rollout, enum-only telemetry, and full MIT attribution may carry into a planner revision. No product/profile implementation or rollout is approved until the single-attempt, Kimi-family, exact-slot/canonical, shadow, package-loading, observability, and concurrency contracts below are resolved. Existing non-daily conversational/active-turn model features remain outside this approval.

## Summary

- **Clarity:** Strong high-level boundary; ambiguous daily canonical copy, slot literals, shadow mode, and adaptive classification callsite.
- **Verifiability:** Good fallback/digest/delivery themes; no proof of wire-attempt count, correct profile module, or shadow behavior.
- **Completeness:** Covers semantic immutability, privacy intent, weekly ledger ordering, adaptive pass-through, docs, and MIT; misses actual SDK retry controls and several exact APIs/acceptance cases.
- **Big Picture:** Finite selection is the right safety tradeoff and directly addresses the unsafe daily interpretation path, provided the old request itself is removed.
- **Principle/Option Consistency:** Mostly consistent; current fixture preservation conflicts with removing free-form daily content, and “no retry” conflicts with default SDK behavior.
- **Alternatives Depth:** Good for free-form versus finite variants; absent for shadow-mode encoding, package placement/loading, and per-render versus per-task concurrency semantics.
- **Risk/Verification Rigor:** Useful pre-mortem and delivery tests, but retry amplification, Kimi aliases, module contamination, concurrent enhancement, and observability failure are untested.

## Required Changes

1. **Guarantee one wire attempt, not one Python call.** Pin a provider-neutral implementation path that sets SDK retries to zero for raw OpenAI clients and every adapter/wrapper that may be returned. If the current resolver cannot express this uniformly, extend it with an explicit single-attempt option forwarded to client construction/adapters or fail closed for unsupported client shapes. Preserve existing 30-second callers. Add a retryable 429/5xx/connection-timeout transport test proving exactly one underlying HTTP attempt, no credential/provider fallback, no late result application, and the stated wall-clock bound.
2. **Use a complete no-Kimi classifier.** Specify normalized provider aliases and family-model rules at least as broad as the repository's existing Kimi/Moonshot detection, including `moonshot`, `kimi-coding(-cn)`, `moonshot-*`, `k1.*`, `k2.*`, aggregator paths, and configured Kimi/Moonshot endpoint hosts. Check configured and resolved identifiers without making a model request; test every family plus near-miss non-Kimi names.
3. **Freeze the v1 copy contract in the plan.** Provide literal daily/weekly canonical and allowed variant strings, stable slot IDs, exact anchor/range construction, deterministic eligibility predicates, non-overlap/bounds/canonical-slice validation, and a numeric response-size limit. Decide now whether each surface has one slot or zero. Do not leave copy or slot existence to the executor.
4. **Resolve the daily baseline contradiction and eliminate the legacy call.** Define truthful canonical copy separately for complete and missing data, reconcile it with the guide, and state which old fixture intentionally changes. Saved-check-in and replay rendering must make zero calls to `_generate_physique_coaching_feedback` and `_request_physique_coach_completion`; only the new ID-selection request may occur. Update the current test that expects raw generated prose, while preserving unrelated conversation/active-turn callers.
5. **Specify shadow mode as local presentation configuration.** Add an exact `off|shadow|apply` mode (or equivalent strict field) to `korean_expression_postprocessor`, not adaptive feature-epoch flags. Define shadow outcomes/metrics and prove a valid selection is validated and counted but canonical bytes/digest/delivery remain unchanged. State rollback behavior from both shadow and apply.
6. **Fix profile-local package loading and tests.** Either place the pure contract in a genuinely shared hermes-agent module or add a separate process-isolated gateway test that installs dualcoachtest before importing gateway/checkin_cli. Do not duplicate divergent contracts into both profile packages. Name any required `__init__.py` export and prove the runtime profile loads the same module path tested.
7. **Define concurrency semantics.** State whether one attempt is per helper invocation or per durable weekly task. If per task, add selection authority/coordination before the provider call without compromising the final-body digest reservation; if per invocation, remove contradictory “one enhancement maximum” task claims and explicitly accept bounded duplicate cosmetic calls. Add concurrent-tick and restart tests with exact enhancement and Telegram call counts.
8. **Make observability executable and non-blocking.** Name the default structured sink, exact event schema, latency bucket boundaries, invalid-config and shadow labels, and the precise adaptive classification callsite. A sink exception must not alter canonical fallback, persistence, reservation, or send behavior. Capture all application logs/events in tests and assert an allowlist of keys/enum values only.
9. **Broaden structural and adversarial tests.** Add stale/overlapping/out-of-range slot documents, local canonical-anchor mismatch, duplicate selection keys, Unicode/control-character IDs, absent choices/content, oversized response before parse, bool timeout, unknown config keys, telemetry failure, outer-timeout late completion, and concurrent schedule cases. Assert canonical UTF-8 bytes, locked fragments, ledger digests, and provider/send counts—not only substring preservation.
10. **Retain the verified MIT boundary.** Include the complete license text and both copyright lines from the source license, link the Kiro port and original, identify only the selected principles, and keep the source's multi-agent/retry/filesystem artifact workflow out of code and operations documentation.