Uh oh!
There was an error while loading. Please reload this page.
refactor(documents): retire the legacy pipelines and delete gemini_service (#151b, 2/2) - #473
Conversation
…rvice — 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>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 4b61aec | Commit Preview URL Branch Preview URL | Jul 30 2026, 02:44 PM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:27 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)
📝 WalkthroughWalkthroughThe PR retires the legacy Gemini service and document-upload fallbacks, standardizes agent failure behavior, adds deterministic concept-scan E2E handlers, introduces a benchmark-only raw Gemini helper, and updates tests, frontend handling, architecture documentation, and ADRs. ChangesAgent migration and failure semantics
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/DocumentUploadModal.tsx (1)
175-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTerminal
error:failednow double-toasts.The backend's failure tail is
error:failed→status:donewith noresult, souploadDocumentStreamalso rejects ("stream ended without a result event") and the catch block at Line 213 fires a secondUpload failed: …toast. Withstep="fallback"gone this is the only failure path, so every failed upload shows two toasts.🐛 Suggested guard
try { + let toastedInBand = false; const fd = new FormData(); @@ if (ev.step === "failed") { toast.error(`Upload failed: ${ev.message}`); + toastedInBand = true; } @@ - if (!aborted) toast.error(`Upload failed: ${errorMsg}`);+ if (!aborted && !toastedInBand) toast.error(`Upload failed: ${errorMsg}`);🤖 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 `@frontend/src/components/DocumentUploadModal.tsx` around lines 175 - 185, Update the error handling in the upload flow around uploadDocumentStream so terminal step === "failed" records the failure state without showing a toast there, since the rejected stream is already handled by the catch-block toast. Preserve the existing progress update and ensure non-terminal error events remain informational.
🧹 Nitpick comments (1)
backend/routes/documents.py (1)
895-927: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the terminal
error:failed+status:donetail into a helper.The same two-event tail is now emitted in five places in
event_stream(extraction failure, unusable text, both agent-failure branches, post-roll failure). A small helper keeps the wire contract in one place.♻️ Sketch
+def _terminal_failure_sse(message: str, request_id: str | None):+ """Terminal SSE tail for any in-stream failure: error:failed + status:done."""+ yield sapling_event_to_sse(SaplingEvent(+ type="error", step="failed", message=message,+ data={"request_id": request_id} if request_id else None,+ ))+ yield sapling_event_to_sse(SaplingEvent(+ type="status", step="done", message="Failed.",+ ))Then each branch becomes:
- yield sapling_event_to_sse(SaplingEvent(- type="error", step="failed",- message="Document processing failed. Please try again.",- data={"request_id": request_id} if request_id else None,- ))- yield sapling_event_to_sse(SaplingEvent(- type="status", step="done",- message="Failed.",- ))- return+ for ev in _terminal_failure_sse(+ "Document processing failed. Please try again.", request_id,+ ):+ yield ev+ return🤖 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/documents.py` around lines 895 - 927, Extract the repeated terminal error:failed and status:done SSE emission from event_stream into a small helper, preserving the existing messages and request_id data. Replace all five duplicated terminal branches, including the shown guardrail and unexpected-exception handlers, with calls to that helper followed by their existing return behavior.
🤖 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/scripts/_raw_gemini.py`:
- Around line 21-24: Defer construction of the module-level _client in
_raw_gemini until after agents._providers.model_mode() confirms "real", so
importing _raw_gemini or calling _generate() in non-real modes cannot initialize
google-genai. Preserve the existing client configuration and ensure the guarded
path still provides the client for real-mode generation.
In `@backend/tests/test_hermetic_llm_guard.py`:
- Around line 69-71: Update the docstring in the hermetic LLM guard test to
qualify the “one remaining module-level client” statement, specifying that it
refers to production clients or that benchmark-only clients are outside this
test’s scope; keep the existing coverage description unchanged.
In `@docs/architecture.md`:
- Around line 17-19: Update the “LLM seam (current)” paragraph to make the
raw-client exception list consistent: explicitly identify
services/rag_service.py as the sanctioned model_mode()-gated raw embedding
client, and revise the statement about scripts/_raw_gemini.py to distinguish the
only ungated benchmark caller. Preserve the requirement that any raw client
below agents/_providers.py uses a model_mode() gate.
In `@docs/decisions/0001-adopt-pydantic-ai.md`:
- Around line 6-9: Update ADR 0001’s passages around the references to
gemini_service.py, including the sections at lines 13, 17, and 26, to remove
claims that it is currently used, remains during migration, or serves as the
legacy fallback. Rephrase them as historical context or remove them, while
preserving the framework-adoption decision and consistency with ADR 0024.
In `@docs/decisions/0020-streaming-tutor-interrupt-retry.md`:
- Around line 51-52: Update the retry-guarantee statement in ADR 0020 to remove
the claim that no data is persisted on stop or failure. State instead that
transcript persistence occurs only on completion, while failures after graph or
mastery tool writes remain non-retryable, consistent with the canonical behavior
referenced by ADR 0024.
In `@docs/decisions/0024-retire-legacy-gemini-seam.md`:
- Around line 90-94: Clarify the `/upload/sync` retry-safety statement around
`process_document` and `apply_concepts_to_graph`: do not claim that nothing was
persisted unless graph merging and document insertion are atomic or the merge is
idempotent. Update the ADR to describe the actual side-effect boundary and retry
behavior, including the conditions required for a fresh `X-Request-ID` retry to
be safe.
---
Outside diff comments:
In `@frontend/src/components/DocumentUploadModal.tsx`:
- Around line 175-185: Update the error handling in the upload flow around
uploadDocumentStream so terminal step === "failed" records the failure state
without showing a toast there, since the rejected stream is already handled by
the catch-block toast. Preserve the existing progress update and ensure
non-terminal error events remain informational.
---
Nitpick comments:
In `@backend/routes/documents.py`:
- Around line 895-927: Extract the repeated terminal error:failed and
status:done SSE emission from event_stream into a small helper, preserving the
existing messages and request_id data. Replace all five duplicated terminal
branches, including the shown guardrail and unexpected-exception handlers, with
calls to that helper followed by their existing return behavior.
🪄 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: c3c12d80-9acf-4cd1-9e5a-dd2fa006acb2
📒 Files selected for processing (28)
CLAUDE.mdREADME.mdSECURITY.mdbackend/agents/document.pybackend/agents/function_handlers_e2e.pybackend/routes/documents.pybackend/scripts/_raw_gemini.pybackend/scripts/benchmark_quiz.pybackend/scripts/benchmark_rag.pybackend/services/cache.pybackend/services/gemini_service.pybackend/tests/README.mdbackend/tests/conftest.pybackend/tests/test_concept_scan.pybackend/tests/test_documents_routes.pybackend/tests/test_e2e_function_handlers.pybackend/tests/test_event_capture_seams.pybackend/tests/test_gemini_service.pybackend/tests/test_gemini_usage_logging.pybackend/tests/test_hermetic_llm_guard.pybackend/tests/test_usage_instrumentation_coverage.pydocs/architecture.mddocs/decisions/0001-adopt-pydantic-ai.mddocs/decisions/0020-streaming-tutor-interrupt-retry.mddocs/decisions/0024-retire-legacy-gemini-seam.mddocs/frontend-audit/07-integrations.mdfrontend/src/components/DocumentUploadModal.test.tsxfrontend/src/components/DocumentUploadModal.tsx
💤 Files with no reviewable changes (4)
- backend/services/gemini_service.py
- backend/tests/test_gemini_usage_logging.py
- backend/tests/README.md
- backend/tests/test_gemini_service.py
| _client = genai.Client( | ||
| api_key=GEMINI_API_KEY or "dummy-key-for-import", | ||
| http_options=types.HttpOptions(timeout=180_000), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A8 -B4 '\bdef model_mode\b|\bmodel_mode\(' backend/agents/_providers.pyRepository: SaplingLearn/Sapling
Length of output: 1173
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'--- backend/scripts/_raw_gemini.py first 90 lines ---\n'
sed -n '1,90p' backend/scripts/_raw_gemini.py
printf'\n--- imports/usages of _raw_gemini and google.genai (excluding cache/tools) ---\n'
rg -n "backend/scripts/_raw_gemini|from .*_raw_gemini|import .*_raw_gemini|google\.genai|genai\.Client|from agents\._providers import model_mode|model_mode\(\) ==" backend -g '*.py'Repository: SaplingLearn/Sapling
Length of output: 249
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- backend/scripts/_raw_gemini.py first 90 lines ---'
sed -n '1,90p' backend/scripts/_raw_gemini.py
printf'%s\n'''printf'%s\n''--- imports/usages of _raw_gemini and google.genai ---'
rg -n "backend/scripts/_raw_gemini|from .*_raw_gemini|import .*_raw_gemini|google\.genai|genai\.Client|from agents\._providers import model_mode|model_mode\(\) ==" backend -g '*.py'||trueRepository: SaplingLearn/Sapling
Length of output: 8453
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- backend/scripts/ingest_catalog.py relevant section ---'
sed -n '1,80p' backend/scripts/ingest_catalog.py
printf'%s\n'''printf'%s\n''--- backend/routes/documents.py model_mode construction guarded section ---'
sed -n '1068,1105p' backend/routes/documents.py
printf'%s\n'''printf'%s\n''--- backend/services/rag_service.py model_mode construction guarded section ---'
sed -n '20,45p' backend/services/rag_service.pyRepository: SaplingLearn/Sapling
Length of output: 6624
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- backend/scripts/benchmark_quiz.py relevant imports/call site ---'
sed -n '1,80p' backend/scripts/benchmark_quiz.py
printf'%s\n'''printf'%s\n''--- backend/scripts/benchmark_rag.py relevant imports/call site ---'
sed -n '1,120p' backend/scripts/benchmark_rag.py
printf'%s\n'''printf'%s\n''--- backend/scripts/_providers-related imports/usages in backend/scripts ---'
rg -n "agents\._providers|agents/_providers|from agents\._providers import model_mode|model_mode\(\)" backend/scripts -g '*.py'||trueRepository: SaplingLearn/Sapling
Length of output: 8351
Gate the raw Gemini client before importing _raw_gemini.
This is the only sanctioned raw google-genai path, but the module-level client is still constructed at import time and _generate() can fail before model_mode() == "real" is checked. Move/defer the client construction behind the same agents._providers.model_mode() guard, or import through a gated helper.
🤖 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/scripts/_raw_gemini.py` around lines 21 - 24, Defer construction of
the module-level _client in _raw_gemini until after
agents._providers.model_mode() confirms "real", so importing _raw_gemini or
calling _generate() in non-real modes cannot initialize google-genai. Preserve
the existing client configuration and ensure the guarded path still provides the
client for real-mode generation.
Source: Coding guidelines
| exact objects a forgotten `patch(...)` would leave live. (The | ||
| gemini_service client this class also covered was deleted in #151b / | ||
| ADR 0024; rag_service holds the one remaining module-level client.)""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify the “one remaining module-level client” statement.
backend/scripts/_raw_gemini.py now also creates a module-level genai.Client. Either clarify this as the one remaining production client or explicitly state that benchmark-only clients are outside this test’s scope.
🤖 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_hermetic_llm_guard.py` around lines 69 - 71, Update the
docstring in the hermetic LLM guard test to qualify the “one remaining
module-level client” statement, specifying that it refers to production clients
or that benchmark-only clients are outside this test’s scope; keep the existing
coverage description unchanged.
Uh oh!
There was an error while loading. Please reload this page.
| - Superseded by: ADR 0024 — the legacy-fallback clause ONLY (the | ||
| "`gemini_service.py` stays as the fallback during migration" posture). | ||
| The framework adoption itself stands; `services/gemini_service.py` was | ||
| deleted in #151. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the stale migration-era claims from this ADR.
The new metadata says the fallback clause is superseded and services/gemini_service.py was deleted, but Lines 13, 17, and 26 still say current calls use that module, that it remains during migration, and that it is the legacy fallback. Rewrite those passages as historical context or remove them so ADR 0001 does not contradict ADR 0024.
Suggested direction
-Today every LLM call in the backend goes through `services/gemini_service.py`.+During the original migration, LLM calls went through `services/gemini_service.py`.-The existing `services/gemini_service.py` stays as-is during migration.+The migration-era fallback was removed by `#151`; current calls use agents.🤖 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 `@docs/decisions/0001-adopt-pydantic-ai.md` around lines 6 - 9, Update ADR
0001’s passages around the references to gemini_service.py, including the
sections at lines 13, 17, and 26, to remove claims that it is currently used,
remains during migration, or serves as the legacy fallback. Rephrase them as
historical context or remove them, while preserving the framework-adoption
decision and consistency with ADR 0024.
| unchanged. ADR 0024 records the full server+client rung ladder as the | ||
| canonical description post-#151.)* (`CancelledError` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the retry guarantee after adding the tool-write caveat.
This ADR now points to ADR 0024 as canonical, but Lines 82-84 still claim that nothing is persisted on stop/failure, contradicting Lines 46-50 where graph/mastery tool writes may already have landed. Replace the old statement with the narrower guarantee that transcript persistence is completion-only; side-effecting failures remain non-retryable.
Suggested wording
-No backend change ... nothing persisted on stop/failure ...+Transcript persistence remains completion-only, but graph/mastery tool writes+may persist before a failure; those errors are marked non-retryable.🤖 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 `@docs/decisions/0020-streaming-tutor-interrupt-retry.md` around lines 51 - 52,
Update the retry-guarantee statement in ADR 0020 to remove the claim that no
data is persisted on stop or failure. State instead that transcript persistence
occurs only on completion, while failures after graph or mastery tool writes
remain non-retryable, consistent with the canonical behavior referenced by ADR
0024.
| - **Document upload `/upload/sync`** — both guardrail exceptions and bare | ||
| exceptions map to a retry-friendly **502** (`UPLOAD_FAILED_DETAIL`): | ||
| nothing was persisted and the client mints a fresh X-Request-ID per | ||
| attempt, so retrying re-runs the pipeline. Guardrails log WARNING; bare | ||
| exceptions log the full traceback. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== repo files =="
git ls-files | rg '(^|/)(docs/decisions/0024-retire-legacy-gemini-seam.md|backend/agents/document.py|.*upload.*|.*sync.*)'||trueechoecho"== ADR excerpt =="
sed -n '1,150p' docs/decisions/0024-retire-legacy-gemini-seam.md | cat -n
echoecho"== backend/agents/document.py outline =="
ast-grep outline backend/agents/document.py --view compact ||trueechoecho"== backend/agents/document.py relevant excerpt =="
sed -n '1,260p' backend/agents/document.py | cat -n
echoecho"== search for process_document and upload/sync references =="
rg -n "def process_document|process_document|/upload/sync|upload/sync|UPLOAD_FAILED_DETAIL|X-Request-ID|request-id" -S backend docs ||trueRepository: SaplingLearn/Sapling
Length of output: 35123
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== documents route sync handler =="
sed -n '90,125p' backend/routes/documents.py | cat -n
sed -n '521,625p' backend/routes/documents.py | cat -n
echoecho"== documents route save/persistence code =="
sed -n '250,340p' backend/routes/documents.py | cat -n
rg -n "def _save|save_assignments_to_db|insert_documents_row|upload_document|raise HTTPException|exposed_http_exception|guardrail" -S backend/routes/documents.py backend/agents -A 8 -B 4
echoecho"== graph apply implementation =="# Inspect the graph tool directly and related call sites
fd -a "graph.py|graph\.py" backend | sed 's#^\./##'
sed -n '1,300p' backend/agents/tools/graph.py 2>/dev/null | cat -n ||trueechoecho"== dbos/exposed_http_exception implementation =="
sed -n '1,240p' backend/services/durable.py | cat -nRepository: SaplingLearn/Sapling
Length of output: 49320
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== graph service implementation =="
sed -n '1,220p' backend/services/graph_service.py | cat -n
echoecho"== sync rest of upload_document_sync =="
sed -n '610,650p' backend/routes/documents.py | cat -n
echoecho"== request_id idempotency tests =="
sed -n '920,970p' backend/tests/test_documents_routes.py | cat -n
sed -n '1116,1165p' backend/tests/test_documents_routes.py | cat -n
sed -n '1230,1325p' backend/tests/test_documents_routes.py | cat -nRepository: SaplingLearn/Sapling
Length of output: 21834
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"== graph_service apply_graph_update and insert helpers =="
rg -n "def apply_graph_update|def .*graph.*|insert\(|graph_nodes|new_nodes|graph_ids|rowids" backend/services/graph_service.py -S -A 25 -B 8
echoecho"== graph service read-only static coverage =="
python3 - <<'PY'from pathlib import Pathp = Path("backend/services/graph_service.py")s = p.read_text()checks = { "contains apply_graph_update": "def apply_graph_update" in s, "contains graph_nodes table select": 'graph_nodes"' in s, "contains graph_ids table select": '"graph_ids"' in s, "contains insert": "insert(" in s, "contains idempotency/upsert on request_id": any(tok in s for tok in ('request_id"', 'X-Request-ID', 'on_conflict', 'is.null', 'is_not.null')),}for k,v in checks.items(): print(k, v)if "def apply_graph_update" in s: lines=s.splitlines() for i,l in enumerate(lines,1): if "def apply_graph_update" in l: for j in range(max(1,i-3), min(len(lines), i+65)+1): print(f"{j:04d} {lines[j-1]}") breakPYRepository: SaplingLearn/Sapling
Length of output: 21312
Clarify the retry-safe boundary for upload failures.
process_document merges concepts via apply_concepts_to_graph before returning, and sync’s document insert happens after the same call. A fresh X-Request-ID retry is safe only if no upload side effects persist after an exception, or the graph merge is idempotent/transactional with doc persistence; otherwise the ADR’s “nothing was persisted” claim is inaccurate.
🤖 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 `@docs/decisions/0024-retire-legacy-gemini-seam.md` around lines 90 - 94,
Clarify the `/upload/sync` retry-safety statement around `process_document` and
`apply_concepts_to_graph`: do not claim that nothing was persisted unless graph
merging and document insertion are atomic or the merge is idempotent. Update the
ADR to describe the actual side-effect boundary and retry behavior, including
the conditions required for a fresh `X-Request-ID` retry to be safe.
…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>
AndresL230
commented
Jul 30, 2026
Code reviewFound 1 issue:
Lines 85 to 87 in eb7cbfd Lines 18 to 20 in eb7cbfd Fixed in 4b61aec — both docs now enumerate the two raw-client sites (gated 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
What
Part 2 of 2 of the final
gemini_servicecutover — and the deletion itself:services/gemini_service.pyis gone (zero production references; the benchmark scripts' baseline arms move to a benchmark-onlyscripts/_raw_gemini.py). Full detail in the commit message:/upload/syncmaps agent failures to a retry-friendly 502; the streaming route emits the terminalerror:failed+donepair (step=fallbackleaves the SSE vocabulary, frontend dead branch removed with it);/scan-conceptsdegrades to the empty shape.concept_scanregistered in the e2e function handlers (the one unregistered request-path task the scoping pass found).retryable, thesapling_wrotestamp, 413-vs-502), the/start-sessionconvergence, the pre-beta rationale in place of a reachability query, and the revert path (refactor(learn): agent-only rung ladder — retire the legacy chat paths (#151a, 1/2) #472 + this PR). ADR 0001's fallback clause superseded; architecture.md / CLAUDE.md / README / SECURITY swept to the agents-only reality.Verification
Backend 1468 passed + ruff clean; 148 passed under lock-pinned pydantic-ai 1.107; evals replay green ×6 (untouched); frontend 349 + tsc clean. 12 red-first tests; ~50 legacy tests deleted/ported per the scoping brief's disposition table. Full local e2e cycle pre-merge; results below.
Closes#151.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation