Profile Personas (RAG half) — Implementation Record

1. Issue reference

• GitHub issue: n/a (task-driven, no filed issue at implementation time)
• Issue title: Profile personas — make persona injection work + add image moderation
• Issue type: bug fix (persona loader) + feature (moderation endpoint)
• Milestone: n/a

2. Summary

• What this issue changed: fixed the personas table loader in app/agents/teacher_agent.py (was querying nonexistent columns, silently no-opping), added persona caching, clarified the persona block's precedence in app/services/llm.py, and added POST /moderate-image (OpenAI omni-moderation, image input).
• Why the change was needed: persona_id was accepted by /chat end-to-end but never had any effect (dead feature). Separately, the gateway needed an image-moderation endpoint before it can accept student-uploaded images.

3. Initial repo state

• Relevant behavior before implementation: load_context selected response_style_json,reasoning_rules_json from personas (columns that don't exist); the query always raised and the broad except swallowed it, so persona_rules was always None. No /moderate-image endpoint existed.
• Known constraints or gaps at start: personas table only has id, name, system_prompt, is_default; no read-model/event-fed persona cache existed yet (target-state architecture in definition/phase3-services.md moves this to an Identity-owned read-model — not built yet, so RAG still reads personas directly via the service-role Supabase client).

4. Plan doc referenced

• Plan doc path: docs/95_plans/profile-personas.md
• Plan status at implementation start: written alongside implementation (retrospective)
• Was the plan updated during implementation?: no
• If yes, what changed in the plan?: n/a

5. Decisions taken

Decision	Reason	Alternative rejected
Reuse CacheService's LRU idiom for a new persona_cache	Matches the existing rerank/chunk cache pattern; no new caching library	A bespoke module-level dict cache in teacher_agent.py
Keep the persona block's existing position in _build_system_prompt (after core contract, before exercise/format/language)	Already correct and already pinned by tests/services/test_llm.py; instructions later in the prompt carry more weight, so this ordering already prevents persona override	Moving the persona block to the very end of the prompt (would contradict pinned tests asserting the language line is always last)
/moderate-image uses get_current_user (same as /chat), not require_admin	Gateway calls this service directly as an internal, non-admin-gated call, mirroring /chat's auth	Gating behind require_admin like the /admin/* ingestion surface
[Review round 1] Run grounding + language guardrails on the streamed answer AFTER the token loop completes (chat.py's _post_stream_guardrail_notice, emitted as a new notice SSE event), instead of inside generate_answer's streaming branch	generate_answer's streaming branch has no interception point before tokens reach the client — that's the point of streaming. Post-stream checking is the only way to add a backstop without reintroducing the buffering/first-byte-timeout problem stream-first was built to avoid. Costs zero latency before the first token; a small amount before done only when the language check actually trips (extra OpenAI call)	Gating tokens inside generate_answer's stream (breaks stream-first); silently dropping the guardrail on the streaming path (status quo bug)
[Review round 1] Rename _apply_grounding_guardrails → apply_grounding_guardrails in llm.py	It's now called from chat.py too — a private, underscore-prefixed helper shouldn't be imported cross-module	Importing the private name directly from chat.py
[Review round 1] Dedicated ThreadPoolExecutor(max_workers=4) for moderation (moderate_image_async), not asyncio.to_thread	asyncio.to_thread uses the loop's shared default executor — the same one /chat's synchronous LangGraph invocation runs on. A moderation burst could starve /chat's thread pool (or vice versa) without isolation	Keeping asyncio.to_thread (original round's choice — flagged CRITICAL in review)
[Review round 1] Explicit timeout=10 on the moderations.create call	An unbounded call could hang a request (and hold an executor thread) indefinitely	Relying on the OpenAI SDK's default client-level timeout only
[Review round 1] /moderate-image added to RateLimitMiddleware's /wallet//upload bucket (30 req/min)	Closest existing bucket for a media-handling endpoint; bounds a moderation burst at the edge, in addition to the dedicated executor	A new dedicated bucket (unnecessary — no distinct limit requirement given)
[Review round 1] image_base64 gets max_length=7_000_000 at the Pydantic layer	Rejects an oversized payload before any base64 decode runs, not just after (defense in depth alongside the service's own decoded-size check)	Relying solely on the service-layer decoded-size check
[Review round 1] Negative persona lookups (nothing found at all) cached with a short, distinct TTL (60s) via CacheService.set_persona(..., ttl=...)	A personas-less deployment shouldn't pay a DB read every chat turn; short TTL still recovers quickly once a default persona is added	Never caching negative results (original round's choice — flagged MEDIUM)
[Review round 1] A persona resolved via the is_default fallback is always cached under the shared default sentinel key, not the raw requested persona_id; a distinct raw id gets a tiny {"alias_of": ...} marker instead of a full duplicate	A client cycling bogus persona_ids was storing N full copies of the identical default persona, evicting legitimate entries under the 200-item LRU cap	Caching each raw id's resolution independently (original round's choice — flagged MEDIUM)
[Review round 1] mime_type is Literal["image/jpeg", "image/png", "image/webp"] at the schema layer	Rejects an unsupported value at validation (422) instead of reaching the service's InvalidImageInputError → 400 path; matches repo idiom of closed literals at the schema boundary	Leaving mime_type: str and relying solely on the service-layer check (original round's choice — flagged LOW)
[Review round 2] Explicit timeout=8.0 on _translate_answer's chat.completions.create call	Round 1's post-stream guardrail made this call reachable synchronously from the chat request path with no timeout (SDK default: 600s) — reproduced hanging the event loop	Relying on the OpenAI SDK's default client-level timeout only
[Review round 2] Run the entire post-stream guardrail check (_post_stream_guardrail_notice) via loop.run_in_executor on a new dedicated _POST_STREAM_GUARDRAIL_EXECUTOR (ThreadPoolExecutor(max_workers=4)), not inline on the event loop	Even with a timeout, a synchronous OpenAI call inline on the loop still blocks all concurrent asyncio work for up to that timeout. Mirrors _MODERATION_EXECUTOR; a separate executor keeps it isolated from /chat's own asyncio.to_thread (teacher_graph.invoke) default-pool usage	Calling it inline (round 1's choice — flagged CRITICAL); sharing asyncio.to_thread's default executor (would reintroduce the moderation-vs-chat starvation risk fixed in round 1's CRITICAL #2)
[Review round 2] Wrap the guardrail check in its own try/except, log and continue as notice = None on any failure	The answer was already streamed successfully by this point — a guardrail bug/timeout must not skip billing (finalize_and_publish), skip usage/done, or leak exception text via an error event	Letting it fall through to the generator's generic except Exception handler (round 1's status quo — flagged HIGH)

6. Files changed

File	Change summary
app/agents/teacher_agent.py	Replaced the broken persona query with _fetch_persona_row (id → is_default fallback) and load_persona_rules (cached, 2000-char truncation, no-crash on DB error); load_context now calls it unconditionally. [Review round 1] Redesigned caching: _resolve_cached_persona follows one alias hop; negative results cached under _NEGATIVE_PERSONA_CACHE_TTL_SECONDS (60s); default-fallback resolutions always cache under _DEFAULT_PERSONA_CACHE_KEY with a lightweight alias for the raw requested id.
app/services/llm.py	Added an explicit precedence comment above the persona block and renamed its heading to ## Persona style (secondary — tone/approach only); no reordering. [Review round 1] Renamed _apply_grounding_guardrails → apply_grounding_guardrails (now called from chat.py). [Review round 2] _translate_answer's chat.completions.create call now passes timeout=_TRANSLATION_TIMEOUT_SECONDS (8.0s).
app/services/cache.py	Added persona_cache (LRUCache, 5 min TTL) + get_persona/set_persona to CacheService, and folded it into get_stats. [Review round 1] set_persona takes an optional ttl override for negative/short-lived entries.
app/services/moderation.py (new)	moderate_image(): validates mime_type/base64/size, calls omni-moderation-latest with an image data: URL, returns {flagged, categories}. Raises InvalidImageInputError (400) / ModerationServiceError (502). [Review round 1] Added timeout=MODERATION_TIMEOUT_SECONDS (10s) to the OpenAI call; added _MODERATION_EXECUTOR (dedicated ThreadPoolExecutor(max_workers=4)) + moderate_image_async().
app/schemas/moderation.py (new)	ModerateImageRequest / ModerateImageResponse. [Review round 1] image_base64 gets max_length=MAX_IMAGE_BASE64_CHARS (7,000,000); mime_type is now Literal["image/jpeg", "image/png", "image/webp"].
app/api/routers/moderation.py (new)	POST /moderate-image, get_current_user-gated. [Review round 1] Calls moderate_image_async instead of wrapping moderate_image in asyncio.to_thread.
app/api/router.py	Registered the moderation router.
app/api/routers/chat.py	[Review round 1] New _post_stream_guardrail_notice helper + wiring into agent_streaming_generator's non-clarification branch: runs apply_grounding_guardrails then ensure_answer_language against the fully-streamed answer, emits a notice SSE event when either would have altered it. [Review round 2] New _POST_STREAM_GUARDRAIL_EXECUTOR (dedicated ThreadPoolExecutor(max_workers=4)); the guardrail check now runs via loop.run_in_executor(...) instead of inline, wrapped in its own try/except (failure → log + notice = None, never re-raised).
app/schemas/chat.py	[Review round 1] Added "notice" to ChatChunk.event's Literal.
app/core/middleware.py	[Review round 1] /moderate-image added to the /wallet//upload rate-limit bucket (30 req/min); docstring updated.
tests/agents/test_teacher_agent.py	New TestLoadPersonaRules (found/unknown→default/absent→default/DB error/truncation/caching) + TestLoadContextPersonaResolution; updated the pre-existing test_load_context_uses_request_supplied_context_without_db assertion (persona lookup now legitimately touches personas, but not wallet/profiles). [Review round 1] New TestPersonaNegativeCacheTtl + TestPersonaCacheDeduplication.
tests/services/test_llm.py	New tests pinning persona block placement (after core contract, before format block) and its absence when no persona resolves. [Review round 1] New test_adversarial_persona_prompt_stays_subordinate. [Review round 2] New test_translation_call_has_explicit_timeout.
tests/services/test_cache.py	New get_persona/set_persona tests. [Review round 1] New test_cache_service_persona_ttl_override.
tests/services/test_moderation.py (new)	Unit tests for moderate_image (flagged/clean/bad mime/bad base64/oversized/OpenAI failure), OpenAI client mocked. [Review round 1] New test_moderation_call_has_explicit_timeout + TestModerateImageAsync (dedicated-executor + error-propagation).
tests/routers/test_moderation.py (new)	Router-level tests (200/400/422/502/401). [Review round 1] Updated to patch moderate_image_async; new mime_type-422, base64-too-long-422, and base64-at-boundary tests.
tests/routers/test_chat_streaming.py	[Review round 1] New post-stream guardrail tests: missing-citation flags, compliant answer doesn't flag, language-mismatch flags, adversarial-persona violation caught. [Review round 2] New test_post_stream_guard_runs_on_dedicated_executor_not_default_pool (executor-usage spy) + test_post_stream_guard_exception_does_not_break_billing_or_stream (billing/usage/done proceed, no error event, no leaked exception text).
tests/core/test_middleware.py	[Review round 1] New test_rate_limit_middleware_moderate_image_endpoint.
docs/95_plans/profile-personas.md, docs/96_implementation/profile-personas.md (new)	This plan + implementation record. [Review round 1] Added a "Review round 1" section / rows throughout.
mkdocs.yml	Added nav entries for both new docs.

7. Migrations / schema changes

• Migration files: none (schema is owned by the migration service; personas already has id, name, system_prompt, is_default).
• Schema changes: none.
• Data backfill or manual steps: none required by this change. Whether a personas.is_default = TRUE row exists in each environment is an operational question outside this repo.
• Rollback notes: reverting is a pure code revert; no data changes to unwind.

8. API changes

Surface	Change	Compatibility impact
POST /chat	persona_id now actually resolves and shapes tone; absent/unknown persona_id now falls back to the default persona (previously a no-op either way). [Review round 1] Streamed responses may now include a notice SSE event ({grounding, language, corrected_answer}) after sources and before usage/done.	Behavioral: default-persona text now appears in the system prompt for every chat turn once a personas.is_default row exists. Response schema unchanged except the additive notice event, which unknown-event-tolerant clients already ignore per ChatChunk's existing contract.
POST /moderate-image (new)	{image_base64, mime_type} → {flagged, categories}. [Review round 1] mime_type is a closed Literal (422 instead of 400 for an unsupported value); image_base64 capped at 7,000,000 chars (422 if exceeded); rate-limited at 30 req/min.	Additive; new endpoint. The mime_type 400→422 change is a status-code detail within the 4xx family the gateway already treats uniformly as failure (see plan doc, LOW #6).

9. Tests added or updated

Test file or suite	Change
tests/agents/test_teacher_agent.py	Added persona loader + load_context integration tests; fixed one pre-existing assertion (see Decisions). [Review round 1] TestPersonaNegativeCacheTtl, TestPersonaCacheDeduplication.
tests/services/test_llm.py	Added persona block placement/absence tests. [Review round 1] test_adversarial_persona_prompt_stays_subordinate.
tests/services/test_cache.py	Added persona cache get/set tests. [Review round 1] test_cache_service_persona_ttl_override.
tests/services/test_moderation.py	New — service-level moderation tests. [Review round 1] Timeout assertion, dedicated-executor tests.
tests/routers/test_moderation.py	New — router-level moderation tests. [Review round 1] mime_type-422, base64 length boundary tests; updated to the async wrapper.
tests/routers/test_chat_streaming.py	[Review round 1] Post-stream guardrail tests: missing-citation, compliant-answer, language-mismatch, adversarial-persona.
tests/core/test_middleware.py	[Review round 1] /moderate-image rate-limit routing test.

10. Risks / caveats

• The default-persona fallback is a genuine behavior change for every /chat call with no persona_id (see the plan doc's flagged section). Nothing in this repo pins that behavior in a golden/eval fixture, but the gateway-side promptfoo suite might.
• personas.is_default is trusted as-is (no uniqueness enforcement checked here); if multiple rows have is_default = TRUE, .limit(1) picks whichever Postgres returns first — acceptable for now, but worth a follow-up constraint if this becomes an issue.
• [Review round 1] The post-stream notice event is informational only — it never retracts or replaces tokens already streamed to the client. A UI that doesn't handle notice will simply show the (possibly non-compliant) answer with no visible flag; this is a client-side follow-up, not something RAG can fix from the server side.
• [Review round 2] The post-stream guardrail check is bounded, not latency-free — and round 1's framing ("small amount before done, worth watching") undersold the risk: round 2 confirmed the check ran the language guard's translation call (ensure_answer_language → _translate_answer) synchronously and inline on the event loop, with no timeout (the OpenAI SDK default is 600s). A single slow translation call froze all concurrent asyncio work on the process for its duration — reproduced at 0.507s of frozen event-loop time for a 0.5s mocked call. Two independent fixes close this, both scoped to the guardrail path only:
• app/services/llm.py's _translate_answer now passes timeout=_TRANSLATION_TIMEOUT_SECONDS (8.0s) to chat.completions.create — bounds the worst case regardless of caller.
• app/api/routers/chat.py's agent_streaming_generator now runs _post_stream_guardrail_notice via loop.run_in_executor(_POST_STREAM_GUARDRAIL_EXECUTOR, ...) instead of calling it inline — a dedicated ThreadPoolExecutor(max_workers=4), mirroring app.services.moderation._MODERATION_EXECUTOR, so the check (translation call included) never blocks the event loop and never shares /chat's own default-pool usage (teacher_graph.invoke via asyncio.to_thread).

Net effect: the check still adds a bounded amount of wall-clock time to the stream's tail (up to ~8s, only when the language check trips) before usage/done — that part of round 1's tradeoff was correct — but it can no longer freeze the whole single-worker process while doing so. This is the corrected framing; do not restate the round 1 "worth watching latency" note as if the blocking risk were still open. - [Review round 2] A failure inside the post-stream guardrail check (bug, OpenAI error, or the new timeout firing) is caught by its own try/except in agent_streaming_generator and treated as "no notice" — logged at error level, never re-raised. This matters because the answer was already delivered to the client by this point: without this, an exception would have fallen through to the generator's generic handler, which (a) skips finalize_and_publish() entirely (an already-billable turn goes unbilled), (b) never emits usage/done (the client's stream hangs waiting for events that never arrive), and (c) leaks the raw exception string to the client via an error event. All three are now prevented; see test_post_stream_guard_exception_does_not_break_billing_or_stream.

11. Follow-up work

• None required by this change; the target-state Identity-owned persona read-model (definition/phase3-services.md) is a separate, larger migration out of scope here.
• [Review round 1] Whether/how the gateway and Flutter client surface the notice event to users (e.g. a "this answer may be incomplete" banner) is a downstream, cross-repo follow-up — RAG's contract obligation ends at emitting the event.

12. Final repo state

• Relevant behavior after implementation: persona_id shapes the chat system prompt as intended (found id, or default fallback); POST /moderate-image is live and gated the same way as /chat.
• Remaining limitations: persona resolution is still a direct Supabase read from RAG (pre-read-model target state); no admin surface in this repo to manage personas rows.

13. Docs updated

Doc path	Update summary
docs/95_plans/profile-personas.md	New plan doc.
docs/96_implementation/profile-personas.md	This implementation record.
mkdocs.yml	Nav entries for both.