Uh oh!
There was an error while loading. Please reload this page.
refactor(learn): agent-only rung ladder — retire the legacy chat paths (#151a, 1/2) - #472
Conversation
…s (#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>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 99a40ed | Commit Preview URL Branch Preview URL | Jul 30 2026, 01:32 PM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ 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 |
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/services/chat_stream.py (1)
374-383: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake streaming message persistence atomic/idempotent.
_persistwrites the user message, then the assistant reply independently, so a failure after the first insert will not be retried as an atomic turn and a transparent client retry can create duplicate rows for the samesession_id/content. Run the two inserts in one transaction, and either make them idempotent for the same turn payload or persist before yielding the completion event.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/chat_stream.py` around lines 374 - 383, Update the streaming persistence flow around _persist so the user message and assistant reply are written in a single transaction and retried atomically. Ensure repeated retries for the same session_id and turn payload do not create duplicate rows, and complete this persistence before yielding the completion event.
🧹 Nitpick comments (3)
backend/tests/test_streaming_rung1_live.py (1)
65-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded fast-tier model name will drift from the production fallback.
The test's stated purpose is to exercise "the same second chance
_chat_turn_json(model_pref="fast")takes live", but it pins"gemini-2.5-flash-lite"literally. If the fast-tier default moves, this lane keeps passing against a model production no longer uses. Resolving the name through the same fast-tier helper_prepare_chat_run/model_name_foruses would keep the two in step.#!/bin/bash# How does the route resolve the "fast" tier model name? rg -nP -C5 'model_pref|fast' backend/agents/_providers.py | head -60🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_streaming_rung1_live.py` around lines 65 - 75, The fallback test’s Agent configuration hardcodes a fast-tier model that can diverge from production. Update fallback() to resolve the model through the same fast-tier model-name helper used by _prepare_chat_run/model_name_for, while preserving the existing Agent.run invocation and response shape.backend/tests/test_learn_routes.py (1)
421-430: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale section header. The header still advertises a "legacy fallback" that the class docstring right below declares gone.
♻️ Proposed fix
-# ── POST /api/learn/chat (agent path + legacy fallback) ──────────────────────+# ── POST /api/learn/chat (agent path) ────────────────────────────────────────🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_learn_routes.py` around lines 421 - 430, Remove the “legacy fallback” wording from the section header above TestChatViaAgent so it describes only the current POST /api/learn/chat agent path, matching the class docstring.backend/routes/learn.py (1)
462-473: 🩺 Stability & Availability | 🔵 TrivialLazy-session state stays process-local and unbounded.
PENDING_SESSIONSentries are only removed by_consume_pending, so abandoned openers accumulate for the process lifetime, and with more than one worker a start-session served by worker A followed by a chat on worker B silently skips thesessionsinsert. Pre-existing, but this PR makes the agent path the only writer — worth a TTL sweep or moving the stash to Redis (services/cache.py) / asessionsrow with a pending flag before scaling out.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/routes/learn.py` around lines 462 - 473, The lazy-session stash in PENDING_SESSIONS is process-local and unbounded, and cannot support requests routed across workers. Update the pending-session flow around PENDING_SESSIONS and _consume_pending to add expiry cleanup for abandoned entries and use shared storage such as services/cache.py (or persist a pending sessions row) so a later chat request on another worker can consume the state and create the sessions record.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/routes/learn.py`:
- Around line 1184-1189: Validate body.action_type against the supported keys in
action_prompts before constructing action_message or running the action turn.
Return an HTTP 422 response for unknown values, while preserving the existing
hint, confused, and skip handling.
- Around line 1220-1229: Update the action-turn persistence flow around
_action_turn and the shown save_message call so the original “[ACTION: ...]”
request is stored as the user message before saving the assistant reply. Ensure
action turns preserve alternating ModelRequest/ModelResponse history while
retaining the existing whitespace-only reply guard and graph update handling.
In `@backend/services/chat_stream.py`:
- Around line 126-136: Update the fallback flow around _rung1_fallback_events
and the nonstream fallback routes (_chat_turn_json and _start_session_agent) to
track or propagate dependency write state from the fresh fallback run. If the
fallback performs any graph or mastery tool writes before failing, surface that
state and emit the error event with retryable: False; only retain retryable:
True when no writes occurred.
---
Outside diff comments:
In `@backend/services/chat_stream.py`:
- Around line 374-383: Update the streaming persistence flow around _persist so
the user message and assistant reply are written in a single transaction and
retried atomically. Ensure repeated retries for the same session_id and turn
payload do not create duplicate rows, and complete this persistence before
yielding the completion event.
---
Nitpick comments:
In `@backend/routes/learn.py`:
- Around line 462-473: The lazy-session stash in PENDING_SESSIONS is
process-local and unbounded, and cannot support requests routed across workers.
Update the pending-session flow around PENDING_SESSIONS and _consume_pending to
add expiry cleanup for abandoned entries and use shared storage such as
services/cache.py (or persist a pending sessions row) so a later chat request on
another worker can consume the state and create the sessions record.
In `@backend/tests/test_learn_routes.py`:
- Around line 421-430: Remove the “legacy fallback” wording from the section
header above TestChatViaAgent so it describes only the current POST
/api/learn/chat agent path, matching the class docstring.
In `@backend/tests/test_streaming_rung1_live.py`:
- Around line 65-75: The fallback test’s Agent configuration hardcodes a
fast-tier model that can diverge from production. Update fallback() to resolve
the model through the same fast-tier model-name helper used by
_prepare_chat_run/model_name_for, while preserving the existing Agent.run
invocation and response shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 67800a0b-b0c5-4d36-ab77-6e5ccaa7d977
📒 Files selected for processing (25)
backend/prompts/expository.txtbackend/prompts/preamble.txtbackend/prompts/shared_context.txtbackend/prompts/socratic.txtbackend/prompts/teachback.txtbackend/routes/learn.pybackend/services/chat_stream.pybackend/services/graph_context.pybackend/services/prompt_safety.pybackend/tests/test_chat_stream.pybackend/tests/test_e2e_function_handlers.pybackend/tests/test_event_capture_seams.pybackend/tests/test_graph_context_block.pybackend/tests/test_learn_routes.pybackend/tests/test_learn_stream_routes.pybackend/tests/test_prompt_injection.pybackend/tests/test_shared_course_context.pybackend/tests/test_streaming_rung1_live.pydocs/frontend-testids.mdfrontend/e2e/tutor.spec.tsfrontend/src/components/screens/Learn.tsxfrontend/src/lib/api.stream.test.tsfrontend/src/lib/api.tsfrontend/src/lib/sse.test.tsfrontend/src/lib/sse.ts
💤 Files with no reviewable changes (5)
- backend/prompts/teachback.txt
- backend/prompts/socratic.txt
- backend/prompts/expository.txt
- backend/prompts/shared_context.txt
- backend/prompts/preamble.txt
| action_prompts = { | ||
| "hint": "The student asked for a hint. Give a small scaffold or clue without giving away the answer.", | ||
| "confused": "The student said they are confused. Identify the likely point of confusion and re-explain with a different analogy.", | ||
| "skip": "The student wants to skip this concept. Acknowledge and transition to the next recommended concept.", | ||
| } | ||
| action_message = f"[ACTION: {action_prompts.get(body.action_type, '')}]" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Is action_type constrained at the schema level?
rg -nP -C6 'class ActionBody' backendRepository: SaplingLearn/Sapling
Length of output: 918
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== backend/routes/learn.py outline around route/action =="
rg -n "_action_turn|ACTION:|ActionBody|action_type|action_prompts" backend/routes/learn.py backend/models/__init__.py
echoecho"== relevant backend/routes/learn.py lines 1150-1215 =="
sed -n '1150,1215p' backend/routes/learn.py | nl -ba -v1150
echoecho"== relevant backend/models/__init__.py lines 20-45 =="
sed -n '20,45p' backend/models/__init__.py | nl -ba -v20Repository: SaplingLearn/Sapling
Length of output: 1095
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== relevant backend/routes/learn.py lines 1150-1260 =="
sed -n '1150,1260p' backend/routes/learn.py
echoecho"== relevant backend/models/__init__.py lines 1-60 =="
sed -n '1,60p' backend/models/__init__.py
echoecho"== pydantic imports in backend =="
rg -n 'from pydantic|import pydantic|Field|constr|Literal|Enum' backend -g '*.py'| head -80Repository: SaplingLearn/Sapling
Length of output: 13614
Reject unsupported action_type before running an action turn.
ActionBody.action_type is an unconstrained str, so values outside hint, confused, and skip fall through to "[ACTION: ]" and consume a full agent run. Return 422 for unknown action types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/learn.py` around lines 1184 - 1189, Validate body.action_type
against the supported keys in action_prompts before constructing action_message
or running the action turn. Return an HTTP 422 response for unknown values,
while preserving the existing hint, confused, and skip handling.
| if not reply.strip(): | ||
| # #153: degenerate whitespace-only output — surface through the | ||
| # guardrail mapping rather than persisting an empty assistant row. | ||
| raise UnexpectedModelBehavior( | ||
| "chat_tutor produced a whitespace-only action reply" | ||
| ) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=502, detail=f"Gemini error: {e}") | ||
| reply, graph_update = extract_graph_update(raw) | ||
| save_message(body.session_id, "assistant", reply, graph_update) | ||
| apply_graph_update(body.user_id, graph_update, course_id=course_id) | ||
| graph_update = merge_graph_updates(deps.graph_updates) | ||
| save_message(body.session_id, "assistant", reply, graph_update or None) | ||
| return {"reply": reply, "graph_update": graph_update} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Pydantic AI Google/Gemini model require alternating user and model roles in message_history?
💡 Result:
Yes, Pydantic AI does require that message history adheres to the structural requirements of the underlying LLM provider, which typically mandates alternating user and model roles [1][2]. While Pydantic AI simplifies interaction by providing automatic internal repair mechanisms to handle common issues like interrupted tool calls or invalid sequences [2], it enforces baseline validation to ensure compatibility with model APIs. For example, Pydantic AI validates that the message history cannot start with a ModelResponse, as most providers (including Gemini via Google's APIs) require conversations to begin with a user message [1]. Key points regarding message history and role requirements: - Provider Compliance: Underlying models like Gemini, Anthropic, and OpenAI have strict structural rules regarding role order [1][2]. If a provided message history violates these rules, the provider will reject the request [1][2]. - Automatic Repair: Pydantic AI includes an internal pipeline that runs before each model request to "repair" histories. This ensures that hand-built, truncated, or context-evicted histories are formatted in a way that is provider-valid, such as fixing mismatched tool call/tool return pairings [2]. - Developer Responsibility: While the framework handles much of the complexity, you must still provide a logical sequence of messages that aligns with the model's expected conversation flow [1][2]. Providing a series of back-to-back user messages or model responses without the appropriate alternating structure may still lead to errors or unexpected behavior if they fall outside the scope of what the automatic repair logic can resolve [3][2]. If you are encountering specific issues with Gemini where the model misinterprets the role of a message (e.g., treating a user message as coming from the model due to content structure), it is often because of how specific parts (like function responses) are bundled within the message objects [3]. Pydantic AI's internal mechanisms generally manage this, but ensure your message history implementation is consistent with the standard expected by the API [3][2].
Citations:
- 1: feat: enforce message history starts with user message pydantic/pydantic-ai#3440
- 2: Make
message_historyprovider-valid out of the box (repair tool-call/result pairing) pydantic/pydantic-ai#6319 - 3: Continuing conversation with
message_historyafter structured output fails on Gemini pydantic/pydantic-ai#3763
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== locate relevant files =="
git ls-files | rg '(^backend/routes/learn\.py$|services/gemini_service\.py$|models|schema|messages)'| sed -n '1,120p'echoecho"== outline backend/routes/learn.py around functions =="
ast-grep outline backend/routes/learn.py --match _action_turn --view expanded ||true
ast-grep outline backend/routes/learn.py --match _load_message_history --view expanded ||trueechoecho"== relevant learn.py sections =="
sed -n '1150,1265p' backend/routes/learn.py
echo"---"
sed -n '1020,1155p' backend/routes/learn.py
echoecho"== search message_history load/save/action related =="
rg -n "def (save_message|_load_message_history|_action_turn)|message_history|ModelResponse|ModelRequest|chat_tutor|guardrail|UnexpectedModelBehavior" backend -S | sed -n '1,220p'Repository: SaplingLearn/Sapling
Length of output: 35534
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== learn.py helper implementation =="
sed -n '220,290p' backend/routes/learn.py
echo"---"
sed -n '380,440p' backend/routes/learn.py
echoecho"== Pydantic AI MessageHistory role validation references in deps/source if present =="
python3 - <<'PY'import ast, pathlibfor p in pathlib.Path('.').rglob('*.py'): if any(part.startswith('.') or part in {'build','dist'} for part in p.parts): continue try: text = p.read_text(errors='ignore') except Exception: continue for name in ['message_history', 'ModelResponse', 'ModelRequest', 'alternating', 'role order']: if name in text: try: tree = ast.parse(text) for node in ast.walk(tree): if isinstance(node, ast.FunctionDef) and 'message' in node.name.lower(): print(f"{p}:func {node.name}:{node.lineno}-{node.end_lineno}") break except Exception: print(f"{p}:text has {name!r}")PYechoecho"== Gemini provider role order docs/imports in backend =="
rg -n "google|gemini|Alternating|role order|require alternating|last.*model|last_message" backend -S | sed -n '1,180p'Repository: SaplingLearn/Sapling
Length of output: 22554
🌐 Web query:
Pydantic AI ModelMessage history role order Gemini Google model ModelResponse ModelRequest alternating validation
💡 Result:
In Pydantic AI, communication is structured around alternating ModelRequest and ModelResponse objects [1]. This alternation is a fundamental requirement for most LLM providers, including Gemini, which expects a strict sequence of turns [2][1]. Key aspects of message history, role order, and validation for Gemini in Pydantic AI include: 1. Alternating Turns: Pydantic AI manages the conversation as a sequence where each ModelRequest (containing system prompts, user prompts, or tool returns) is followed by a ModelResponse (containing the model's text, tool calls, or thinking output) [3][1][4]. 2. Gemini-Specific Handling: - Role Mapping: Internally, Pydantic AI maps messages to roles recognizable by Gemini (user or model) [2][5][6]. - Tool Return Grouping: Gemini requires that function responses follow a user turn (or appear in a way that respects the provider's turn structure) [2]. Pydantic AI includes logic to group multiple ToolReturnPart objects into a single user message to satisfy Gemini API requirements [7]. If the history ends incorrectly or violates turn order (e.g., consecutive model responses without a user request), the integration may inject dummy user messages (e.g., with empty text) to maintain the required alternating structure [2]. - Structural Constraints: Gemini APIs generally reject multiple consecutive model turns or incorrect role ordering [2][7]. Pydantic AI's Gemini model implementation handles these constraints by mapping internal message types into valid Gemini content structures, ensuring tool calls and text responses are properly formatted [5]. 3. Message History and Validation: - When passing a message_history to Agent.run, Pydantic AI prepends this history to the current interaction [8][9]. - History processors can be used to modify or filter messages before they are sent to the model [9]. However, users must be cautious when reordering or removing messages, as this can break the alternating turn structure required by providers like Gemini, potentially leading to API validation errors [9][10]. - If an Agent run fails due to structured output or sequence errors, manual intervention via history processors to correct the message roles or turn sequences is sometimes required [10]. In summary, while Pydantic AI automates most of the turn-alternation logic, integrating with Gemini requires strict adherence to the expected sequence of roles, which the library enforces by grouping tool results and potentially injecting necessary filler messages to keep the turn order valid [2][7].
Citations:
- 1: https://www.wasilzafar.com/pages/series/ai-app-dev-xtreme/ai-app-dev-sdk-pydantic-part03-messages-history.html
- 2: https://github.com/pydantic/pydantic-ai/blob/main/pydantic_ai_slim/pydantic_ai/models/google.py
- 3: https://pydantic.dev/docs/ai/api/pydantic-ai/messages/
- 4: https://deepwiki.com/pydantic/pydantic-ai/2.3-message-system-and-types
- 5: https://github.com/pydantic/pydantic-ai/blob/7f5214c6/pydantic_ai_slim/pydantic_ai/models/gemini.py
- 6: Change: Internal Discriminator Modification pydantic/pydantic-ai#247
- 7: Fix: group multiple tool responses into a single user message pydantic/pydantic-ai#587
- 8: https://pydantic.dev/docs/ai/core-concepts/message-history/
- 9: https://github.com/pydantic/pydantic-ai/blob/1344df70/docs/message-history.md
- 10: Continuing conversation with
message_historyafter structured output fails on Gemini pydantic/pydantic-ai#3763
🌐 Web query:
Google Generative AI Gemini conversation history last message user model role order documentation
💡 Result:
In the Gemini API, conversation history must be structured as an alternating sequence of user and model roles to ensure valid interaction [1][2][3]. Key requirements for the conversation history structure: 1. Role Alternation: The history must strictly alternate between the 'user' and 'model' roles [1][2][3]. You cannot have two consecutive messages from the same role [2]. 2. Starting Role: The history array should generally begin with a message from the 'user' role [1][4]. 3. Message Format: Each message in the history is a 'Content' object consisting of a 'role' (either 'user' or 'model') and a 'parts' array (containing the actual text, tool calls, or other content) [2][5][3]. 4. Tool Calls: When using tools, the sequence follows a specific pattern to maintain the conversation flow: a 'user' message prompts a 'model' message containing a 'functionCall', which is then followed by a 'user' message containing the corresponding 'functionResponse' [1]. Managing Conversation History: - When using SDKs like the Python or JavaScript ChatSession, the library automatically appends new user messages and model responses to the internal history list [5][6][7]. This history is then sent in its entirety with each subsequent request to maintain context [5]. - If you are managing history manually (e.g., in a stateless environment), you must ensure the full, ordered sequence of messages is included in every API request [8][9]. - Because the entire history is sent with each turn, be mindful of token limits as the conversation grows [5][10]. You can prune or truncate the history as needed to manage these constraints [5]. If you are using the OpenAI-compatible endpoint, the API expects the standard 'messages' array format (e.g., 'system', 'user', 'assistant'), which is mapped to Gemini's internal 'contents' structure (using 'systemInstruction' for system prompts) [3][11].
Citations:
- 1: Ensure Valid Chat Message History Structure google-gemini/gemini-cli#7003
- 2: https://medium.com/google-cloud/gemini-has-entered-the-chat-crowd-conversations-made-easy-f7a8d419ac50
- 3: https://chuizi.ai/docs/protocols/gemini-api
- 4: https://github.com/googleapis/js-genai/blob/89400098/src/chats.ts
- 5: https://adhdecode.com/articles/gemini-api/gemini-api-chat-history-conversation/
- 6: https://github.com/google-gemini/generative-ai-python/blob/main/docs/api/google/generativeai/ChatSession.md
- 7: https://theneuralbase.com/gemini-api/learn/beginner/streaming-with-chat-sessions/
- 8: https://ai.google.dev/gemini-api/docs/interactions-overview
- 9: https://ai.google.dev/gemini-api/docs/text-generation
- 10: https://googleapis-python-genai-70.mintlify.app/guides/chats
- 11: https://ai.google.dev/gemini-api/docs/openai
🌐 Web query:
Pydantic AI Gemini model alternates invalid_messages model responses after function result ModelRequest ModelResponse
💡 Result:
The issue you are encountering with Pydantic AI and Gemini models—where the model alternates between invalid messages or fails after a function result—is primarily due to a known bug in how the Google Gemini API handles the functionResponse role [1][2]. Specifically, the Gemini API treats any content object containing a functionResponse part as having role=model, even if role=user is explicitly specified [1][2]. This causes the model to ignore any subsequent text parts (such as user follow-up questions) within the same content object, as it interprets the entire block as a model response [2]. Pydantic AI has implemented a workaround to mitigate this, which involves splitting functionResponse parts from user text into separate content objects [2]. If you are still observing issues, it may be due to the following reasons: 1. API Behavioral Bug: The core issue resides in the Gemini API's interpretation of the functionResponse role [1][2]. While Pydantic AI's internal logic attempts to repair history and format messages correctly [3], it remains dependent on the underlying provider's adherence to these message roles [2]. 2. Versioning: Ensure you are using the latest version of pydantic-ai and the google-genai SDK, as fixes for message handling and malformed response retries are frequently updated [2][4]. 3. Missing Thought Signatures: If you are using newer Gemini models (e.g., Gemini 3/Flash-thinking models), ensure that thoughtSignature fields are preserved [5]. Older implementations or deprecated model classes may drop these required fields, leading to 400 INVALID_ARGUMENT errors [5]. To troubleshoot or debug: - Check your message history for any functionResponse parts bundled with text in a user role content object [2]. - Ensure your Agent is using the GoogleModel (which utilizes the google-genai SDK) rather than older, deprecated interfaces [5][6]. - Monitor for MALFORMED_FUNCTION_CALL finish reasons, which Pydantic AI is configured to retry automatically in recent versions [4]. If you are a developer investigating this, you can verify if the bug is present by crafting a minimal reproduction script that sends a functionResponse followed by text within a single user content object to the Gemini API [2]. If the model ignores the follow-up text, the issue is currently active at the API level [2].
Citations:
- 1: Continuing conversation with
message_historyafter structured output fails on Gemini pydantic/pydantic-ai#3763 - 2: Track Google Gemini fix for
functionResponserole handling pydantic/pydantic-ai#4210 - 3: https://pydantic.dev/docs/ai/core-concepts/message-history/
- 4: Retry instead of error when Google response is empty with
MALFORMED_FUNCTION_CALLor other recoverable finish reason pydantic/pydantic-ai#3300 - 5: Error because
thoughtSignaturemissing from Gemini 3 function call pydantic/pydantic-ai#3539 - 6: https://pydantic.dev/docs/ai/api/models/google/
Persist the action request as the user turn.
_action_turn posts [ACTION: ...] as the current turn, then stores only the assistant reply. The next load reconstructs that prior assistant as the last ModelResponse, so the preserved history can end with two assistant messages in a row. Store this [ACTION: ...] text as the user row for action turns so conversation history remains alternating ModelRequest/ModelResponse.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/learn.py` around lines 1220 - 1229, Update the action-turn
persistence flow around _action_turn and the shown save_message call so the
original “[ACTION: ...]” request is stored as the user message before saving the
assistant reply. Ensure action turns preserve alternating
ModelRequest/ModelResponse history while retaining the existing whitespace-only
reply guard and graph update handling.
Uh oh!
There was an error while loading. Please reload this page.
…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>
AndresL230
commented
Jul 30, 2026
Review pass complete: one confirmed finding (both deep reviewers converged) — the writes-guard protected entry to the fallback but the fallback is itself a tool-calling run, and its own write-then-fail path hardcoded retryable:true, letting the client re-run a turn whose writes had landed. Fixed by stamping write-state on the raised exception at both route helpers and reading it in the fallback's terminal error; red-first tests at both layers. ADR 0020's stale 'wrote nothing' safety argument amended. Backend 1515 + lockvenv green. e2e cycle next. |
| return_value=(agent, "msg", {}, deps)), | ||
| patch("routes.learn.record_agent_usage", side_effect=lambda r, **k: r), | ||
| ): | ||
| import routes.learn as learn_routes |
| return_value=(agent, "msg", {}, deps)), | ||
| patch("routes.learn.record_agent_usage", side_effect=lambda r, **k: r), | ||
| ): | ||
| import routes.learn as learn_routes |
AndresL230
commented
Jul 30, 2026
Pre-merge gate: full lane green (incl. the FIRST live run of the new greeting-turn journey) + oracles clean. Merging — #151 stays open for part 2. |
Uh oh!
There was an error while loading. Please reload this page.
…rvice (#151b, 2/2) (#473) * refactor(documents): retire the legacy pipelines and delete gemini_service — the cutover completes (#151) Part 2 of 2. services/gemini_service.py is DELETED — zero production references remain; the benchmark scripts' baseline arms move to a benchmark-only scripts/_raw_gemini.py helper. - documents.py: _process_document, _extend_course_concepts, _legacy_upload_pipeline, _stream_legacy_fallback and the three dead coercion helpers deleted. /upload/sync maps agent failures to a retry-friendly 502; the streaming route emits the terminal error:failed + done pair (step=fallback leaves the SSE vocabulary, and the frontend's dead toast branch goes with it); /scan-concepts degrades to the empty shape (best-effort enrichment). The #154 preconditions are preserved untouched: the X-Request-ID idempotency short-circuit, the three separately-to_thread'd persistence helpers, and the post-roll try/except (comment strengthened — never a second result; the fallback it guarded against no longer exists). - concept_scan registered in the e2e function handlers (it was the one unregistered request-path task) with the constants-sync test. - ADR 0024 records the retirement: the canonical rung ladder (server + client, retryable/sapling_wrote/413-vs-502), the /start-session convergence, the pre-beta rationale (prod carries no user traffic — catalog-only — so legacy-reachability measurement is moot; #117's events make post-beta rates observable from day one), and the revert path (git history, the #472 + this PR pair). ADR 0001's fallback clause superseded; architecture.md/CLAUDE.md/README/SECURITY docs swept to the agents-only reality. - 12 red-first tests (502 mapping, terminal-pair, scan degrades, seam handler); ~50 legacy tests deleted/ported per the scoping brief's disposition table. Gates: backend 1468 passed + ruff clean; lockvenv 148 passed; evals replay green ×6; frontend 349 + tsc clean. Closes#151. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review: fix raw-client doc contradiction + stale spec header + close test/ADR gaps - CLAUDE.md/architecture.md claimed scripts/_raw_gemini.py was the ONLY raw google-genai caller while rag_service.py's gated embedding client exists, and contradicted the #439 gate rule as worded — both now enumerate the two sites and scope the rule. - frontend/e2e/streaming.spec.ts item-3 header described the deleted gemini_service seam in the present tense; rewritten for the post-#151 agent-based Rung-1. - ADR 0024 now cross-references #154 (the post-roll structure it preserves). - New events-sink test: streaming /upload agent failure emits document.upload but never document.processed (sync twin already existed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
Part 1 of 2 of the final
gemini_servicecutover (#151) — the learn.py/streaming half, i.e. the rung-ladder redesign the batch plan flagged as the reason #151 is not a deletion chore. Part 2 (documents.py legacy pipelines, deletingservices/gemini_service.py, ADR 0024) follows; the issue closes with part 2.Highlights (full detail in the commit message):
nonstream_fallback) — but the fallback is now a fresh non-streaming agent turn on the fast tier (a different, faster model = a genuinely better second chance), via the extracted_chat_turn_json/ new_start_session_agent.retryable: false, which the client now honors (closing the pre-existing hole where Learn's ladder re-ran blank turns over/chatand double-applied mastery, defeating feat(agents): structured-output retry + validation hardening (#153) #470's server-side guard)./chat,/start-session,/action./start-sessionJSON gets its first agent implementation (legacy was its primary, not a fallback) and/action— which had no agent path at all — is agent-ified in place. Both covered by the existingchat_tutorfunction-mode handler via task dispatch, pinned by a new route test./start-session) exercising the entry screen, the lazy-session contract, and the DB-polled transcript.Verification
Part of #151 — do not auto-close.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes