Uh oh!
There was an error while loading. Please reload this page.
feat(agents): structured-output retry + validation hardening (#153) - #470
Conversation
Second link of the seam endgame. The issue predates ten merges — the
failure-mapping half was already true on this base (verified per route);
what was missing:
- The schema budget is now CODIFIED and enforced: agents/__init__.py
states the orchestrator-schema-complexity rules (≤8 props/object,
root→list nesting ceiling, no optional nested models, string enums,
≤20 total props) and test_agent_output_schemas.py auto-discovers all
20 registered agents, freezes the roster, walks every structured
output's JSON schema against the budget — with the exact 2026-05-03
Gemini-rejected DocumentProcessingResult kept as a negative control.
- Output-retry budget of 2 on all 14 structured-output agents.
Dep-universe catch: pydantic-ai 1.107's recommended retries={'output':2}
dict form SILENTLY breaks 1.89 (the dict lands in the retry counter →
TypeError at retry time, reproduced empirically) — quiz keeps the
deprecated-but-correct output_retries kwarg with the rationale, and the
structural test rejects any future dict-form config. WORKER_LIMITS
request_limit 2→3 so the final retry surfaces as UnexpectedModelBehavior
(persistent garbage) rather than UsageLimitExceeded (misfiled as
note-too-long at notes' 413/500 split). Free-text agents deliberately
untouched — the streaming tutor's failures belong to the rung ladder.
Recovered-after-retry runs get a warning log via record_agent_usage
(no new #117 event; taxonomy untouched).
- The ADR-0023 bare-newline tutor reply, fixed at the stream layer (a
prompt fix would force billable cassette re-records): a completed run
with a whitespace-only reply prefers the joined streamed chunks, else
takes the Rung-1 ladder via a shared _rung1_fallback_events helper —
never persists an empty assistant row; on_usage still fires (the run
billed). At-most-one-of on_complete/legacy_fallback preserved (13
prior invariant tests untouched-green). The JSON path raises
UnexpectedModelBehavior on blank → existing fallback.
- The two surviving legacy call_gemini_json sites catch non-JSON
ValueErrors and degrade (concept-scan → [], _process_document → safe
minimal shape) instead of 500ing the last-resort path.
Gates: backend 1490 passed + ruff clean (1.89 venv); 354 passed across
all affected files under the lock-pinned 1.107 venv; evals replay green
across all 6 datasets under BOTH venvs (cassettes untouched — no prompt
changes by design); frontend + e2e handlers untouched.
Closes#153.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 4293d06 | Commit Preview URL Branch Preview URL | Jul 30 2026, 11:19 AM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (25)
📝 WalkthroughWalkthroughStructured-output agents now use bounded validation retries with schema and usage-limit checks. Malformed Gemini responses degrade safely, whitespace-only chat output enters fallback handling, streaming turns avoid duplicate writes, and frontend token detection ignores whitespace-only deltas. ChangesAgent output contracts and retry budgets
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentRun
participant stream_agent_turn
participant LegacyFallback
participant LearnClient
AgentRun->>stream_agent_turn: final output and streamed chunks
stream_agent_turn->>stream_agent_turn: classify visible text and prior writes
stream_agent_turn->>LegacyFallback: use fallback when blank output has no writes
LegacyFallback-->>LearnClient: token and done events
stream_agent_turn-->>LearnClient: terminal error when blank output follows writes
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| from pydantic import BaseModel | ||
| from pydantic_ai import Agent | ||
| import agents as agents_pkg |
…lback after real tool writes - Client ladder: whitespace-only deltas no longer flip sawToken (both onToken sites), so a degenerate blank stream takes the transparent Rung-3 retry instead of stranding the user on manual Retry. - chat_stream: a blank-reply turn whose tools ALREADY wrote (append-only mastery events / graph upserts) now ends in a terminal error instead of the legacy fallback — the fallback re-runs the turn and would apply mastery twice for one student turn. The no-writes blank turn still degrades to the fallback (re-running is safe with nothing to double-apply); tests rewritten to the corrected contract + a no-writes twin added. Backend 1491 + ruff green; lockvenv stream/seam files 42 green; frontend 343 + tsc green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 30, 2026
Review pass complete: two reviewers fully clean (one re-verified the retry semantics against both installed pydantic-ai sources); the history pass found two real gaps in the new blank-reply path, both fixed — whitespace deltas no longer count as 'content seen' for the client ladder, and a blank turn whose tools already wrote mastery ends in a terminal error rather than a fallback that would double-apply (the no-writes case keeps the fallback). Contract tests rewritten accordingly. All gates green incl. lockvenv. e2e cycle next. |
Uh oh!
There was an error while loading. Please reload this page.
…ero-width forgeries stripped, docs squared - read_session_history_tool neutralizes replayed content (a student could plant a literal envelope delimiter in one turn and have it handed back as a REAL byte-match later); read_concepts_for_user_tool and read_graph_neighborhood_tool neutralize student-derived concept names (the same data the seed block already defangs). Three red-style tests. - neutralize_delimiters strips zero-width/invisible Unicode before matching — a visually identical forged delimiter threaded with U+200B no longer dodges the regex (empirically demonstrated in review). - Docs squared: ADR 0023's follow-up note marked shipped (via #470+#471), prompt_safety's 'every agent' overclaim corrected (note workers carry their own one-line guards, deliberately), graph_context's budget docstring notes the envelope overhead. - FYI-class same-PR fix: {last_session_summary} (LLM-generated text of student content) now wrapped like the quiz digest. Backend 1521 + ruff green; lockvenv 78 green; evals replay green ×6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) * feat(agents): prompt-injection hardening on student content (#150) Third link of the seam endgame; gates the public beta. - One shared containment helper (services/prompt_safety.py): wrap_untrusted builds a delimited BEGIN/END UNTRUSTED CONTENT envelope with data-not-instructions framing; neutralize_delimiters defangs embedded marker forgeries (case/whitespace-insensitive, idempotent) so content can't fake an early END and escape. Applied at ASSEMBLY boundaries only — storage keeps raw text: RAG chunks (format_rag_context — covers agent chat, legacy chat, quiz context in one place), the graph seed block's student-derived concept names, the legacy prompt's COURSE MATERIALS + shared-context JSON, and the tool→ LLM boundary (search_course_materials, read_active_note, read_misconceptions, quiz-history; note-worker user prompts). - INJECTION_GUARD_PROMPT (single source) in the tutor's three preambles, note_chat, quiz, and the legacy preamble; the ACADEMIC INTEGRITY block deferred out of #149 lands in the agent preamble for parity. - Tool-use constraint: ConceptMasteryUpdate.mastery_delta schema clamped ±1.0 → the instructed [-0.1, +0.3] band — an injected 'set my mastery to 1.0' now fails validation into the #153 retry loop; plus a contract test freezing that no tutor/note tool signature exposes user_id/course_id/session_id/note_id to the model. - Documented scope calls: the student's own message channel and session history stay unwrapped (their instruction channel; wrapping the tool but not message_history would be theater); catalog chunks stay trusted (script-ingested official data); misconceptions tool stays unregistered on the tutor (consent enforcement still deferred — the data that DOES reach prompts is now contained); document-pipeline workers untouched (no tools to coerce, ~70 cassettes at stake) — noted as follow-up. - Evals: chat_tutor (16) + quiz_generation (10) honestly re-recorded (their prompts changed; replay keys on case names and would have stayed silently green). The re-record also landed ADR-0023's tracked prompt-shape fix (never end the turn on a tool call) after the quirk reproduced live. Scores: all evaluators 1.000 on both datasets (GroundedConcept ratcheted 0.875 → 1.0); other four datasets untouched. 27-case red-first injection suite (14 red at base) incl. an end-to-end FunctionModel test proving an injected tool return reaches the model enveloped. Gates: backend 1518 passed + ruff clean; lockvenv 278 passed; evals replay green ×6. Closes#150. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review(#471): fix all findings — sibling tool surfaces neutralized, zero-width forgeries stripped, docs squared - read_session_history_tool neutralizes replayed content (a student could plant a literal envelope delimiter in one turn and have it handed back as a REAL byte-match later); read_concepts_for_user_tool and read_graph_neighborhood_tool neutralize student-derived concept names (the same data the seed block already defangs). Three red-style tests. - neutralize_delimiters strips zero-width/invisible Unicode before matching — a visually identical forged delimiter threaded with U+200B no longer dodges the regex (empirically demonstrated in review). - Docs squared: ADR 0023's follow-up note marked shipped (via #470+#471), prompt_safety's 'every agent' overclaim corrected (note workers carry their own one-line guards, deliberately), graph_context's budget docstring notes the envelope overhead. - FYI-class same-PR fix: {last_session_summary} (LLM-generated text of student content) now wrapped like the quiz digest. Backend 1521 + ruff green; lockvenv 78 green; evals replay green ×6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tryable; ADR 0020 amended - The writes-guard now reaches INSIDE the fallback: _chat_via_agent and _start_session_agent stamp sapling_wrote (their own deps' write-state) on any post-run exception, and _rung1_fallback_events reads it — a fallback that wrote graph/mastery and then failed emits retryable:false so the client cannot re-run the turn a third time and re-apply the writes (the double-apply class, one level deeper than #470's guard). Red-first stream tests (wrote-then-failed → not retryable; clean failure → retryable) + stamp tests at the helper level. - ADR 0020's 'Retry is already safe' argument amended: transcript persistence is still exactly-once, but tool writes can land mid-turn — retryable:false / 413 gate the automatic re-runs now. Backend 1515 + ruff green; lockvenv 77 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s (#151a, 1/2) (#472) * refactor(learn): agent-only rung ladder — retire the legacy chat paths (#151a, part 1 of 2) Part one of the final gemini_service cutover (#151): everything learn.py/ streaming. Part two (documents.py legacy pipelines, the file deletion, ADR 0024) follows; the issue closes with it. - stream_agent_turn's seam renamed legacy_fallback → nonstream_fallback, SAME contract (fallback owns persistence + usage; at-most-one-of with on_complete; error rungs run neither). Rung 1 now degrades to a fresh NON-STREAMING agent turn on the fast tier (a different, faster model is a materially better second chance than the same one re-streamed), wired through the extracted _chat_turn_json / _start_session_agent. - The writes-guard generalized (#470's blank-reply rule → ALL fallback entries): if tools already wrote graph/mastery, no fallback ever runs — terminal error with the new additive retryable:false field. The client honors it (and 413s): ChatStreamError.retryable + shouldFallBackToJson(), so Learn's ladder can no longer silently re-run a turn whose side effects landed (the pre-existing hole that defeated #470's server guard from the client side). - Guardrail → status mapping on /chat, /start-session, /action (the notes precedent): UsageLimitExceeded → 413 naming the cause (deterministic — the client does NOT retry it), UnexpectedModelBehavior → 502 retry-friendly, bare Exception → 502 + exception log. - /start-session's JSON route gets its FIRST agent implementation (_start_session_agent; the legacy pipeline was its primary, not a fallback), converging the greeting prompt on what /start-session/stream already shipped. /action agent-ified in place (assistant-only persist preserved; task-dispatch means the existing chat_tutor handler covers both — pinned by a new function-mode route test). - Deleted: _legacy_chat, build_system_prompt, get_conversation_history, _get_course_documents, _resolve_legacy_model, the template loader, the five legacy prompt files (grep-verified single reader), and compact_graph_context. chat.message_sent now has exactly one JSON-path emission site (inside _chat_turn_json). - test_streaming_rung1_live.py redesigned: broken-model streaming agent + good fast-tier Agent.run() fallback — still proving the cross-version exception-wrapping seam (#459's failure class) live. - New greeting-turn journey in tutor.spec.ts (the scoping pass found ZERO journeys touched /start-session): entry screen → deterministic greeting → lazy-session contract (no row until the first follow-up) → DB-polled transcript. New testids registered in docs/frontend-testids.md. Gates: backend 1511 passed + ruff clean; lockvenv 192 passed across all touched stream/agent/route files; frontend 350 passed + tsc clean; evals replay green ×6 (prompts untouched by design). Part of #151 (do not auto-close). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review(#472): fix both findings — fallback write-state surfaces to retryable; ADR 0020 amended - The writes-guard now reaches INSIDE the fallback: _chat_via_agent and _start_session_agent stamp sapling_wrote (their own deps' write-state) on any post-run exception, and _rung1_fallback_events reads it — a fallback that wrote graph/mastery and then failed emits retryable:false so the client cannot re-run the turn a third time and re-apply the writes (the double-apply class, one level deeper than #470's guard). Red-first stream tests (wrote-then-failed → not retryable; clean failure → retryable) + stamp tests at the helper level. - ADR 0020's 'Retry is already safe' argument amended: transcript persistence is still exactly-once, but tool writes can land mid-turn — retryable:false / 413 gate the automatic re-runs now. Backend 1515 + ruff green; lockvenv 77 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
Second link of the B7 seam-endgame chain. The failure-mapping half of the issue was already true on this base (re-verified route by route); what landed:
retries={"output": 2}dict form silently breaks 1.89 at retry time (reproduced empirically); the correct-everywhere kwarg is used and the structural test rejects the dict form.WORKER_LIMITSbumped so the last retry surfaces asUnexpectedModelBehavior, not a misfiledUsageLimitExceeded. Free-text agents (the streaming tutor especially) deliberately untouched — that's the rung ladder's jurisdiction. Recovered retries get a warning log; the [P2] Observability: instrument capture seams (middleware, auth, feature routes) #117 taxonomy is untouched.on_usagestill fires, and the at-most-one-of persistence invariant holds (all 13 prior invariant tests untouched-green).call_gemini_jsonsites degrade cleanly on non-JSON instead of 500ing the last-resort path (both retire in [P1] Agent migration: retire call_gemini* + gemini_service.py (final cutover) #151).Verification
Closes#153.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Reliability