Sleek Coach Docs

AI Conventions

The rulebook for the coach AI layer (apps/api/app/coach_ai/): how providers, tools, safety policies, prompts, and the streaming contract stay safe, auditable, and provider-agnostic. Treat this file as the source of truth for AI review: the ai-review skill walks it, and the pre-commit AI agent checks coach-touching changes against it. Every rule below was earned by a shipped defect or a deliberate architecture decision (the streamed-session continuity bug, the lb/kg unit bug, the stream path that skipped output policies).

Prime directive

The coach is safe, auditable, and provider-agnostic. Every model interaction goes through the orchestrator and the SafetyPolicyEngine, every tool call is logged, and no code outside the providers package knows which LLM vendor is serving the request.

Boundaries: docs/DESIGN_CONVENTIONS.md owns how the chat looks and feels on mobile; docs/AI_COACH.md is the descriptive architecture reference; this doc is normative. Cross-reference, never duplicate.

1. Providers are constructed only in the factory

LLM providers subclass LLMProvider (providers/base.py) and are constructed only by create_provider() in providers/factory.py, selected by settings.llm_provider. No other module imports a concrete provider class. Provider-specific request and response shapes (OpenAI tool dicts, Anthropic content blocks) stay inside the provider that owns them; the shared currency is the internal Message, ToolDefinition, and LLMResponse types plus the streaming contract in rule 5.

Why: the orchestrator once hard-imported OpenAIProvider, which made the documented Anthropic support fictional. The factory is the single seam where vendors plug in.

2. Model names and token budgets live only in model_config.py

Model IDs, max_tokens, and temperature live in providers/model_config.py, keyed by provider and tier. No inline model strings, no inline max_tokens= or temperature= outside the providers package. Anthropic tiers keep temperature=None: Claude Sonnet 5 rejects non-default sampling parameters, so the Anthropic provider omits the field entirely.

Why: cost is controlled per subscription tier from one file, and provider quirks (like the sampling restriction) are encoded once instead of rediscovered per call site.

3. Tools are registered, cached, and logged

Coach tools subclass BaseTool and are registered in tools/registry.py. The registry owns caching (Redis, per-tool TTL) and execution error handling; callers never invoke a tool object directly. Every execution is logged through _log_tool_call to ai_tool_call_log and surfaced to the user as a tool trace.

Why: “no black box” is a product principle. The trace the user sees and the audit row the operator sees both come for free only if execution has exactly one path.

4. Every input and output passes the policy engine

User input goes through SafetyPolicyEngine.check_input before the model sees it; model output goes through check_output before (non-streaming) or immediately after (streaming) it reaches the user. Policy actions on input mean: BLOCK and FLAG short-circuit (the safety message replaces the model call); MODIFY allows the request through and surfaces its message as a structured disclaimer. A blocked output is replaced by the policy message in the response and in stored history; on the streaming path the correction is appended as a final token. The streaming path may not retract already-streamed text, but it still runs the check, logs violations to AIPolicyViolationLog, and attaches disclaimers to the done event. A new policy is a BasePolicy subclass added to default_policies() in severity order (eating disorder first), with a dedicated tests/unit/test_<name>_policy.py covering block, flag, and allow cases. Disclaimers are structured data (ChatResponse.disclaimers, done.disclaimers), never text appended into the message body. Shared text-extraction helpers live in policies/extraction.py; a policy must not re-implement number or unit parsing.

Why: the stream path shipped without output checks, and two policies shipped with a duplicated regex whose unit handling silently weakened the weight-loss rule for metric users. Central helpers and a mandatory test file are the countermeasures.

5. The streaming contract is a cross-app contract

StreamEvent.type is exactly token | tool_start | tool_end | done | error. token.data and error.data are plain strings. The terminal done event is emitted by the service (not the router) and carries {session_id, confidence, data_gaps, disclaimers, tool_trace}. error.data is a generic user-safe message, never str(e). The mobile SSE client (apps/mobile/src/services/api/coachService.ts, useCoach.ts) depends on every one of these facts; changing any of them requires a paired mobile change in the same PR.

Why: the backend once emitted done without session_id, and every streamed message silently created a new server session. The contract is written down so it breaks loudly instead.

6. Sessions expire and history is capped

Session IDs are UUIDv4, created and resumed only through CoachService._get_or_create_session, which expires sessions idle longer than settings.coach_session_idle_hours. Stored conversation history is capped by settings.coach_max_conversation_history via _record_exchange; the prompt window uses the PROMPT_HISTORY_MESSAGES constant. Datetimes are naive UTC throughout (regression history: PRs #36, #42, #43).

Why: unbounded sessions grow unbounded token bills, and mixed-timezone datetimes have already caused three fix PRs.

7. Prompts live in prompts/

System prompts live in prompts/system_prompts.py; disclaimer text lives in prompts/disclaimer_templates.py and is fetched with get_disclaimer(). Context reaches the model through ContextBuilder and its compact summary, not through ad hoc string building. ContextBuilder loads sequentially on one shared AsyncSession; do not parallelize its loads with asyncio.gather.

Why: prompt text is product behavior and gets reviewed like code, which only works when it has one home. The gather ban is a correctness rule: concurrent queries on a single SQLAlchemy AsyncSession are illegal.

8. AI changes ship with their tests

A new tool ships with cases in tests/unit/test_coach_tools.py plus registry coverage. A new policy ships with its dedicated policy test file. A new provider ships with conversion round-trip and streaming-contract tests against a mocked SDK (see test_anthropic_provider.py for the pattern). Orchestrator or service stream changes extend test_coach_orchestrator.py / test_coach_service.py, and endpoint-shape changes extend tests/api/test_coach.py. The LLM is always mocked; tests never hit a vendor API.

Why: the coach is the highest-risk surface in the product. Untested safety code is untested safety.

Review checklist

Walk this in order against the diff. Run the mechanical greps as written; the judgment items require reading the full post-change files.

Mechanical:

  1. Concrete provider classes or vendor SDKs outside the providers package (expect no hits): grep -rn 'OpenAIProvider\|AnthropicProvider\|AsyncOpenAI\|AsyncAnthropic\|import openai\|import anthropic' apps/api/app --include='*.py' | grep -v 'coach_ai/providers/'
  2. Model names outside model_config (expect no hits; the pattern matches either quote style): grep -rEn '["'"'"'](gpt|claude)-[0-9a-z.-]+["'"'"']' apps/api/app --include='*.py' | grep -v model_config.py
  3. Inline sampling or budget params outside the providers package (expect no hits): grep -rn 'max_tokens=\|temperature=' apps/api/app/coach_ai --include='*.py' | grep -v providers/
  4. Stream event types stay in the allowed set (constructors often span lines, so this grep is a floor, not a census; read every emission site in service.py and router.py as well): grep -rn -A2 'StreamEvent(' apps/api/app --include='*.py'
  5. Every BaseTool subclass is registered, and every policy in default_policies() has a matching test file: grep -rn 'class .*(BaseTool)' apps/api/app/coach_ai/tools/ cross-checked against registry.py; grep -rn 'class .*(BasePolicy)' apps/api/app/coach_ai/policies/ cross-checked against engine.py and tests/unit/.

Judgment:

  1. Input and output both pass the policy engine on every new model-facing path; disclaimers stay structured.
  2. The done event still carries session_id, confidence, data_gaps, disclaimers, tool_trace; any contract change has a paired mobile change in the same PR.
  3. Error events and HTTP error bodies leak no internals (no str(e) toward the client).
  4. Sessions and history respect the settings-driven caps; datetimes stay naive UTC.
  5. New AI behavior arrives with the tests rule 8 demands, and the LLM stays mocked.