Skip to content

Profile Personas (RAG half)

Status: implemented (this PR) Repos: rag (this repo) + gateway (persona/profile CRUD, image upload flow — out of scope here)

Problem

  1. Personas were inert. teacher_agent.load_context selected response_style_json,reasoning_rules_json from personas — columns that don't exist (the table is id, name, system_prompt, is_default). The query always raised, the except swallowed it, and persona_rules was silently None for every request. persona_id was accepted end-to-end but never did anything.
  2. No image moderation. The gateway needed a way to screen a student-uploaded image before it reaches chat/vision flows, and RAG already owns the OpenAI client/settings this needs.

Plan

Persona loader

  • Query id, name, system_prompt by persona_id; when absent or unknown, fall back to the is_default = TRUE row.
  • Cache the resolved row (CacheService.persona_cache, mirroring the existing rerank/chunk cache idiom) so a chat turn doesn't pay a DB round-trip every time.
  • Truncate system_prompt defensively (2000 chars) before it ever enters the prompt.
  • Injection point and precedence were already correct in app/services/llm.py (_build_system_prompt): the persona block sits after the core grounding/citation contract but before the exercise contract, the answer-format contract, and the final language/script directive — the parts of the prompt that speak last win, so persona tone guidance cannot override them. This PR only clarified that block (heading + explicit precedence comment); no reordering was needed or made, since existing pinned tests (tests/services/test_llm.py) already encode that ordering.

Image moderation

  • POST /moderate-image, {image_base64, mime_type} → {flagged, categories}.
  • OpenAI Moderation API, omni-moderation-latest, image input via a data: URL built from mime_type + image_base64 — no new API-key plumbing (reuses settings.OPENAI_API_KEY).
  • Validates mime_type (image/jpeg|png|webp), base64 decodability, and a 5 MB decoded-size cap before calling out; 400 on any of those, 502 on an OpenAI failure. Image bytes are never logged.
  • Auth mirrors /chat (get_current_user — any authenticated caller, not admin-gated), since the gateway calls this service directly.

Behavior change flagged for downstream review

Falling back to the is_default persona when persona_id is absent means every /chat request without an explicit persona now gets the default persona's system_prompt injected (previously: no persona_id → no persona block at all). This is what the spec for this work explicitly asked for, and it only changes tone/style guidance — it cannot override grounding, citations, format, or language per the precedence above — but it is a live behavior change for the default case. No promptfoo/golden fixtures live in this repo (the live-eval config lives in the gateway repo), so nothing here needed updating; flag this for whoever owns the gateway-side eval fixtures in case a personas.is_default row exists in production.

Review round 1 (quality review: 2 CRITICAL, 1 HIGH, 3 MEDIUM, 1 LOW)

CRITICAL — guardrails never ran on the streaming path

generate_answer's grounding (apply_grounding_guardrails) and language (ensure_answer_language) checks only ran in its non-streaming branch — /chat always streams (stream=True), so a persona-influenced streamed answer had nothing checking it for missing citations or wrong-language output. This PR is what first makes persona text reach that prompt in practice, so it's also what first activates the gap.

Decision — post-stream guardrail, not inline gating. generate_answer's streaming branch stays untouched: tokens are forwarded to the client the moment they're generated, which is the entire point of streaming (first-token latency). Instead, app/api/routers/chat.py's agent_streaming_generator — which already accumulates full_answer for billing — runs the SAME two checks, in the SAME order, against the fully -streamed answer once it's complete, and emits a new notice SSE event (added to ChatChunk's event Literal) carrying {grounding, language, corrected_answer} when either check would have altered the answer. It never rewrites what was already streamed; it flags it, and logs an error-level line with the failure kind.

Latency tradeoff (explicit, not free): zero added latency before the first token — nothing changes about when streaming starts or how fast tokens arrive. A small amount of latency is added before the done event: apply_grounding_guardrails is synchronous/local (negligible), but ensure_answer_language makes a real OpenAI call when it detects a language mismatch, so a language-flagged turn pays one extra API round-trip in the tail of the stream, after all tokens have already reached the client. Accepted as a fixed cost of having any backstop on the streaming path — the alternative (buffering the whole answer before streaming) would reintroduce exactly the first-byte-timeout failure class the stream-first design (app/api/routers/chat.py's stream_with_graph) exists to avoid.

apply_grounding_guardrails was renamed from _apply_grounding_guardrails (dropped the leading underscore) since it's now a shared cross-module function, not an llm.py-private helper.

CRITICAL — moderation call unbounded + shared thread pool

Three independent fixes, all in app/services/moderation.py: - client.moderations.create(..., timeout=MODERATION_TIMEOUT_SECONDS) (10s) — an unbounded call could hang a request indefinitely. - A dedicated ThreadPoolExecutor(max_workers=4) (_MODERATION_EXECUTOR) plus moderate_image_async(), which the router now calls instead of asyncio.to_thread. asyncio.to_thread uses the event loop's shared default executor — the same one /chat's synchronous LangGraph invocation runs on — so a moderation burst could have starved /chat, or vice versa. - /moderate-image added to RateLimitMiddleware's /wallet//upload bucket (30 req/min) — a media-handling endpoint, the closest existing fit — so the burst is also bounded at the edge, not only inside the executor.

HIGH — no size bound before body parse

ModerateImageRequest.image_base64 gets max_length=7_000_000 (ceil(5 MB / 3) * 4 + slack, matching MAX_IMAGE_BYTES) so Pydantic rejects an oversized payload at validation, before any base64 decode. The service's own decoded-size check stays as defense in depth for direct (non-HTTP) callers.

MEDIUM — negative persona-cache TTL

load_persona_rules now caches a "nothing found" result (no persona_id match AND no is_default row) too, under a short, distinct TTL (_NEGATIVE_PERSONA_CACHE_TTL_SECONDS = 60, vs. the 5-minute positive TTL) — a personas-less deployment no longer pays a DB read on every single chat turn, while still recovering within a minute of a default persona being added.

MEDIUM — cache keying / deduplication

A persona resolved via the is_default fallback (whether persona_id was absent or simply unknown) is now always cached under the shared default-persona sentinel key, never under the raw requested id — a client cycling through bogus persona_ids no longer stores N full duplicate copies of the (identical) default persona, which would otherwise evict legitimate cache entries under the 200-item LRU cap. A distinct raw id that falls back to default gets a tiny {"alias_of": <sentinel>} marker instead, so a repeat lookup of that same bogus id follows the alias and skips the DB without storing another duplicate.

LOW — mime_type as a closed schema type

ModerateImageRequest.mime_type is now Literal["image/jpeg", "image/png", "image/webp"] instead of a bare str. An unsupported value now 422s at the schema layer instead of reaching the service's InvalidImageInputError → 400 path. Noted for the gateway team: the gateway treats any 4xx from this endpoint as a moderation failure either way, so this is not a cross-repo contract break — but the specific status code for a bad mime_type changed from 400 to 422.