RAG Quality Monitor — Task 4 report (evaluator loop + degradation rules + main wiring)

Status: DONE. Commit 4674ee4 on branch feat/rag-quality:
feat(quality): evaluator loop + degradation rules + main wiring (dormant unless enabled)

What was built

(A) internal/ragquality/evaluator.go + evaluator_test.go
- scorer interface (Score(ctx, eventID, question, context, answer) (JudgeScores, error)), satisfied implicitly by *Judge.
- Evaluator{Cfg config.RAGQuality; Pool *pgxpool.Pool; Judge scorer; dec *zstd.Decoder; now func() time.Time} + NewEvaluator(cfg, pool, judge) *Evaluator (builds the zstd decoder exactly like internal/api.New, panics on the (unreachable) static-option error, defaults now to time.Now).
- Run(ctx): ticker at Cfg.EvalIntervalS, waits one interval before the first tick (mirrors notify.Evaluator), never returns a fatal error — a tick error is logged and the loop continues.
- tick(ctx) error: lists candidates via store.UngradedSynthesisCandidates, grades each via gradeOne (wrapped in recover), returns only on a list-candidates DB failure.
- gradeOne: GetBody → decompress req/resp → Extract with Markers built from Cfg. Non-synthesis → judge_status=skipped, parse_ok=false, upsert, no judge call. Synthesis: fills deterministic fields (question_preview via the existing truncateRunes helper from judge.go, 200 runes); calls Judge.Score only when HadContext (ok/scores on success, error/nil-scores on failure); !HadContext → skipped, judge never charged. Always UpsertQuality (idempotent via the DB's ON CONFLICT DO NOTHING).
- Tests (all against store.TestPool, real DB, no mocked store): one tick seeding 4 candidate shapes (ok/error/non-synthesis/no-context) in one batch, asserting exact row shape + that the fake judge is called only for the two HadContext candidates; a two-tick idempotency test (second tick makes zero additional judge calls, exactly 1 DB row); a direct gradeOne call against a nonexistent event_id (no panic, nothing upserted); a Run-cancels-promptly test mirroring notify's.

(B) internal/predict/quality.go + quality_test.go — pure, no store/config dependency. QualityStats{Graded, MeanFaithfulness, UngroundedShare}; EvaluateQuality(st, warn, crit, ungroundedShareWarn, minGraded) []Warning — quiet below minGraded; rag_low_faithfulness (critical below crit, warning below warn, metric faithfulness, subject ahu-chatbot/public); rag_ungrounded_spike (warning above ungroundedShareWarn, metric ungrounded_share, same subject); Indonesian detail strings, evidence-citing. 6 table-driven tests covering the min-graded guard, both bands, healthy-quiet, spike-only, and both-firing-together.

(C) Wiring
- internal/store/ragquality.go: added QualityUngroundedCount(ctx, pool, from, to, floor) (int64, error) — see "UngroundedShare" below for why this is a new helper rather than an extension of QualitySummary. Test added in ragquality_test.go (exact-count, using the same fixed-2019-date + defensive-DELETE isolation pattern as metricseries_test.go, since this function — like QualitySummary — has no engine/surface filter and can't isolate via a unique surface the way UngradedSynthesisCandidates tests do).
- internal/api/quality.go (new): EvaluateQualityWarnings(ctx, pool, cfg) ([]predict.Warning, error) — (nil, nil) when !cfg.Enabled; else builds predict.QualityStats from store.QualitySummary (mean faithfulness, graded) + store.QualityUngroundedCount (only queried when Graded > 0) over a fixed 24h window (matches EvaluateSecurityWarnings's lookback rationale — a behavioral/quality signal, not a 15-min ops alert), then calls predict.EvaluateQuality.
- internal/api/predict.go: EvaluateAllWarnings signature changed to EvaluateAllWarnings(ctx, pool, ragCfg config.RAGQuality); unions early + security + quality warnings; an error from any of the three propagates (no partial result).
- Callers updated: grepped for EvaluateAllWarnings( across the repo — the only production caller is cmd/observatory/main.go's notifier Eval closure (now passes cfg.RAGQuality); the only other caller is the direct test TestEvaluateAllWarningsIsUnionOfEarlyAndSecurity in internal/api/predict_test.go (now passes config.RAGQuality{}, i.e. disabled, so the union assertion is unchanged — quality contributes nothing). There is no separate HTTP handler that calls EvaluateAllWarnings directly today (Task 5's /api/quality/* endpoints don't exist yet); handleEarlyWarnings/handleSecurityWarnings call their own Evaluate*Warnings functions, not EvaluateAllWarnings, and were untouched — their JSON responses are unchanged.
- Added internal/api/quality_test.go: dormancy (Enabled:false → (nil,nil), no DB touch asserted implicitly) and an enabled+seeded test asserting both rules fire via EvaluateQualityWarnings AND that EvaluateAllWarnings folds them in.
- cmd/observatory/main.go: imports ragquality; when cfg.RAGQuality.Enabled, builds ragquality.NewJudge(cfg.RAGQuality) + ragquality.NewEvaluator(qc, pool, judge), logs one startup line, go qualityEvaluator.Run(ingestCtx) — placed directly below the existing notify.Evaluator block, same ingestCtx/shutdown lifecycle. Disabled → this whole block is skipped (no goroutine, no log line), and the notifier's Eval closure now threads cfg.RAGQuality through, so passing a disabled/absent block leaves EvaluateAllWarnings byte-identical to before this feature existed.

How UngroundedShare was computed: new store helper (not an extension of QualitySummary)

QualitySummary's existing groundedThreshold param is cfg.FaithfulnessWarn (a distinct, independently-tunable config value from cfg.UngroundedFloor); conflating the two into one param would silently couple thresholds the config block intentionally keeps separate. QualitySummary also already has a locked-in test asserting its exact 4-arg signature — Task 1 is "done," and changing that signature would mean editing already-committed/tested code. Adding a small dedicated QualityUngroundedCount(ctx, pool, from, to, floor) avoided both problems. EvaluateQualityWarnings computes UngroundedShare = ungroundedCount / graded (only queried when graded > 0, since EvaluateQuality already gates on minGraded and would divide by a nonzero denominator by construction in the normal case; guarding the query itself avoids a wasted query on a genuinely empty window).

Gates

• go build ./..., go vet ./..., gofmt -l . — all clean.
• go test -p 1 -count=1 ./... against the real test DB — all 8 packages green (cmd/observatory, internal/api, internal/config, internal/ingest, internal/notify, internal/predict, internal/ragquality, internal/store).

One important correction made mid-task: the test DB is NOT reachable at localhost:5434 from this session's shell — Docker here runs via a remote dind daemon (DOCKER_HOST=tcp://dind:2375), so the container's published port is only reachable at host dind, exactly as the task brief's TEST_PG_DSN said. Early runs against localhost:5434 silently skipped every DB-gated test (connection refused → t.Skip) while still reporting package-level ok, which would have been a false-green gate. Switched to TEST_PG_DSN='postgres://postgres:test@dind:5434/postgres' and reran — this surfaced real (fixable) test-isolation bugs in my first draft.

Flakiness found and fixed (shared-DB pollution, not a product bug): UngradedSynthesisCandidates and QualitySummary/the new QualityUngroundedCount have no test-specific engine/surface scoping — any other test or concurrent process writing ahu-chatbot/public (or any) rows within the same recent lookback window is picked up too. Fixed by:
- Evaluator tests now use a unique-per-test surface ("ragq-surf-" + ulid), the same isolation idiom store's own TestUngradedSynthesisCandidatesFiltering already uses — this was the actual cause of one observed failure (judge calls after first tick = 6, want 1).
- The new TestQualityUngroundedCount moved to a fixed historical date (2019, mirroring metricseries_test.go's established pattern) plus a defensive DELETE of that exact window, since this query has no surface filter to isolate by.
- The new TestEvaluateQualityWarnings_FiresOnLowFaithfulnessAndFoldsIntoAll deletes the [now-25h, now+1min) rag_quality window before seeding, since EvaluateQualityWarnings' lookback is hardcoded to [now-24h, now) and can't be moved to an ancient date like the above.

Pre-existing flake observed once, not mine: during repeated reruns, TestQualitySummaryAndRecentWorst and TestUngradedSynthesisCandidatesFiltering (both from Task 1, already committed, unmodified by me) failed once and passed on immediate rerun — consistent with the task brief's warning that a concurrent agent shares this DB. Not investigated further per that instruction.

Judgment calls

1. NewEvaluator panics on a zstd construction error rather than returning (*Evaluator, error) — mirrors internal/api.New's exact rationale (static valid option, unreachable in practice; panicking at construction time beats threading an impossible error through main.go).
2. Judge is called only when Extract reports HadContext; a no-context (already-refused) synthesis turn is judge_status=skipped and never charges the judge — matches the plan's stated default explicitly ("default: judge only when HadContext").
3. Added test coverage beyond the plan's explicit list where it meaningfully de-risked the new production code: a store-level test for QualityUngroundedCount, and API-level tests for EvaluateQualityWarnings (dormancy + fold-in). Kept minimal — no dashboard/API-route work (that's Task 5, untouched).

---

Adversarial-review fixes (2026-07-09)

STATUS: DONE. Commit 39efb94 on feat/rag-quality. Observatory-only. Gates green: go build ./..., go vet ./..., gofmt -l clean on all touched files, and full go test -p 1 ./... against the dind:5434 test DB — all 8 packages ok.

Per-finding

1. Judge backpressure must not create permanent grading holes — judge.go now classifies outcomes: HTTP 429/503, any transport error from client.Do (covers network failure + context deadline/cancel), all wrap the new sentinel ErrJudgeRetryable. evaluator.go gradeOne checks errors.Is(err, ErrJudgeRetryable) and on a retryable failure upserts NOTHING (returns early) so the candidate stays ungraded and UngradedSynthesisCandidates re-selects it next tick; terminal failures still upsert judge_status='error'. Deterministic Idempotency-Key=event_id unchanged. Tests: TestTick_JudgeBackpressureLeavesUngraded (429 → 0 rows, still re-selectable), TestTick_JudgeTerminalErrorWritesErrorRow (500 → error row), plus judge.go-level TestScore_RetryableVsTerminalClassification / TestScore_TransportErrorIsRetryable / TestScore_TerminalOn2xxUnparseable.
2. parse_ok_rate / refusal_rate over SYNTHESIS rows only — added is_synthesis boolean NOT NULL DEFAULT false to rag_quality (migration 004, in the CREATE + a defensive ALTER … ADD COLUMN IF NOT EXISTS). store.QualityRow carries IsSynthesis; UpsertQuality writes it ($18); evaluator sets row.IsSynthesis = ex.IsSynthesis (true for all synthesis turns incl. no-context, false for planning). QualitySummary denominators for both rates now FILTER (… AND is_synthesis) / NULLIF(count FILTER (is_synthesis), 0); count/graded/means untouched. New isolated TestQualitySummarySynthesisScopedRates proves the planning row is ignored (parse_ok_rate=1.0 not 0.667, refusal_rate=0.5 not 0.333); existing summary test rows marked IsSynthesis:true so its non-nil rate assertions stay robust on the shared DB.
3. Grader body reads recorded in body-access audit (PDP) — evaluator.go calls store.LogBodyAccess(ctx, pool, "rag-quality-evaluator", "system", eventID, "grade") once right after GetBody succeeds; failure is logged, never aborts grading. Test TestTick_RecordsBodyAccessAudit.
4. Config validation — validateRAGQuality now also rejects min_graded <= 0 and ungrounded_floor outside [0,1] (only when enabled). Tests TestRAGQualityBadMinGradedRejected (Load -1 + direct ==0) and TestRAGQualityBadUngroundedFloorRejected (1.5).
5. Truncate the QUESTION sent to the judge — judge.go wraps question in truncateRunes(question, maxJudgeInputRunes), same cap as CONTEXT/ANSWER.
6. X-Surface fix — judge.go now sets X-Surface: internal (was the invalid system); X-Priority: batch kept. Updated the happy-path header assertion in judge_test.go.
7. Hoist the score regexp — extract.go maxScore no longer compiles per candidate; a scoreRegexp(marker) memoizer (mutex-guarded map keyed by marker) compiles once per distinct score marker. Behavior identical (still keyed by the config-driven marker, not a hardcoded default).
8. Migration header — 004_rag_quality.sql stale -- 003: comment corrected to -- 004:.

Retryable-vs-terminal classification chosen

• Retryable (wrap ErrJudgeRetryable, leave candidate ungraded → retried next tick): HTTP 429, HTTP 503, any client.Do transport error, context deadline/timeout/cancel.
• Terminal (record judge_status='error'): every other non-2xx (400/404/500/502/504/…), and any 2xx whose body has no parseable JSON scores (decode failure, no choices, missing required score field).
• Rationale: 429/503 are exactly the gateway backpressure/queue signals CONVENTIONS §3 calls "graceful degradation, not an error"; a transport blip / timeout means the judge is momentarily down. A 400/404/500 or a malformed 2xx is a genuine defect retrying won't fix, so a permanent error row is correct there (and stops re-charging the judge forever).

Migration compatibility confirmed

Verified directly in psql: an old-shape rag_quality table (no is_synthesis) with a pre-existing row survives the migration — the CREATE TABLE IF NOT EXISTS is a no-op on the existing table, and ALTER TABLE … ADD COLUMN IF NOT EXISTS is_synthesis boolean NOT NULL DEFAULT false adds the column (pre-existing row reads back is_synthesis=false); a repeat ALTER is a NOTICE-only no-op. So the shared test DB (which may already have had the old table) and the never-yet-applied real observatory DB both end up identical. The full -p 1 suite passing against the shared dind DB is the end-to-end proof.

Judgment calls

• 429/503 only among status codes (not 500/502/504) as retryable — followed the task's explicit list and CONVENTIONS §3 wording literally; the terminal test uses 500 exactly as the brief specified. 502/504 being terminal is a deliberate consequence, not an oversight.
• All client.Do errors treated as retryable, including context-cancel during shutdown — safe: it just means "grade later," never a false error row.
• scoreRegexp uses a mutex+map memoizer rather than a package-init compile of the default marker, because the marker is config-driven; a package-init default would silently mis-scan a retuned marker. This preserves behavior exactly while removing the per-candidate compile.
• Added a new isolated TestQualitySummarySynthesisScopedRates (ancient window + defensive DELETE) for the exact-rate assertion rather than trying to assert exact rates in the existing shared-window summary test (which by design sees other tests' rows); the existing test was updated only enough to keep its non-nil assertions robust.
• LogBodyAccess role = "system" per the task's given signature/args; it fires for every candidate whose body is read (incl. non-synthesis/no-context/error), which is correct — the body genuinely was read in each of those cases.