Uh oh!
There was an error while loading. Please reload this page.
feat(rag): Layer 0 RAG pipeline - BU catalog scrape, embedding, and per-message retrieval - #282
Merged
Conversation
…etrieval - Scrapes all 8,720 BU courses across 22 schools into bu_catalog_fall_2026.json - Ingests catalog into course_chunks table with gemini-embedding-001 (768-dim) - Adds match_course_chunks RPC wiring via db/connection.py rpc() helper - Wires per-session catalog lookup and per-message semantic retrieval into build_system_prompt and both chat paths (agent + legacy) - Indexes student-uploaded documents into course_chunks after upload, with relevance gate (cosine similarity >= 0.35 against catalog chunk) using the AI-generated summary as the document representative
📝 WalkthroughWalkthroughAdds a BU catalog RAG pipeline: catalog data is scraped and ingested into ChangesRAG Pipeline
UI Text Updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 496582b | Commit Preview URL Branch Preview URL | Jul 03 2026, 03:41 AM |
| import asyncio | ||
| import json | ||
| import re | ||
| import sys |
| for school in SCHOOLS: | ||
| print(f"\n>> {school}", flush=True) | ||
| before = len(all_courses) | ||
| school_courses = await scrape_school(client, school, seen_urls, on_batch=save_checkpoint) |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/data/scrape_summary.json`:
- Line 209: The committed scrape_summary artifact currently hardcodes a
machine-local absolute path in the output_file field, which leaks
workstation-specific details and breaks portability. Update the value in
scrape_summary.json to use a repo-relative path or remove the field entirely,
and ensure any code that writes this summary uses a stable path source so future
runs do not reintroduce local identifiers; check the summary generation logic
and the output_file serialization path.
- Around line 2-205: The scrape summary currently includes unresolved failures,
so do not keep this snapshot as-is. Update the ingestion flow that reads
scrape_summary.json to either rerun scraping until the errors array is empty or
make the ingest step fail fast when total_errors is nonzero, using the summary
fields total_errors and errors as the check. Locate the validation in the
scrape-summary ingestion path and ensure it blocks indexing when any failed
course URLs remain.
In `@backend/routes/documents.py`:
- Around line 994-1014: The batch embedding flow in the document ingest path
still upserts every record at the end, including chunks whose embedding stayed
None after _embed_texts failed. Update the course_chunks ingest logic to mirror
the catalog ingest pattern by upserting only the successfully embedded records
inside each batch loop, and skip failed batch records instead of carrying them
forward. Keep the batching behavior in place around the existing record
processing in the documents route so large uploads do not get sent as one final
upsert.
- Around line 960-975: The relevance check currently uses only the first
returned catalog chunk from table("course_chunks"), which is arbitrary because
the query has limit=1 without ordering. Update the logic around the catalog_rows
lookup to evaluate all catalog embeddings for the course and compare the
document sample against the best cosine match, then apply the
MIN_COURSE_RELEVANCE gate to that maximum score. Keep the change localized near
the existing sample_text, doc_sample_vec, and dot calculation so the
representative catalog selection is no longer dependent on Supabase row order.
In `@backend/routes/learn.py`:
- Around line 504-505: The course-scoped retrieval in learn.py is broadening to
global RAG when _get_course_info() does not return a BU code, because bu_code or
None passes None into retrieve_chunks() and drops the course filter. Update the
logic around _get_course_info, bu_code, and retrieve_chunks so that when
course_id is present but no valid course_code is found, retrieval does not fall
back to all course_chunks; instead, skip or short-circuit the course-scoped RAG
path and keep the query scoped to the requested course only. Apply the same fix
in both places that build rag_chunks from retrieve_chunks.
- Around line 300-304: The retrieved catalog/chunk content is being appended
directly into the active prompt, so malicious uploads can inject instructions
into both the legacy system prompt path and the agent user-turn path. Update
build_system_prompt and the code paths that add catalog_text/chunk_text to treat
retrieved text as untrusted data: wrap it in clear delimiters or a context
block, add an explicit rule to ignore any instructions inside retrieved content,
and avoid merging raw retrieved text into the prompt where possible.
In `@backend/scripts/ingest_catalog.py`:
- Line 41: The Gemini client setup and ingest retry path are allowing missing or
invalid credentials to continue until rows are written with empty embeddings.
Update the `_gemini` initialization and the ingest flow in `ingest_catalog.py`
so a missing `GEMINI_API_KEY` or a failed embedding request stops processing
immediately instead of upserting `embedding=None` into `course_chunks`. Make the
failure explicit in the `genai.Client`/embedding generation path and propagate
it through the affected ingest logic around the referenced retry/upsert section.
In `@backend/scripts/scrape_bu_catalog.py`:
- Around line 26-47: The SCHOOLS list in scrape_bu_catalog.py is missing two BU
school slugs, so the crawl only covers 20 schools instead of the full 22-school
contract. Update the SCHOOLS constant in scrape_bu_catalog.py to include the
missing school slugs, or replace the hardcoded list with a source that derives
all schools dynamically so the crawler and downstream index always stay
complete. Use the existing SCHOOLS configuration in scrape_bu_catalog.py as the
place to fix this.
- Around line 210-232: The second _extract_schedule() implementation overwrites
the earlier heading-based semester parser, so pages with only an h4 term heading
lose semester detection. Update _extract_schedule() to preserve the existing
<h4>FALL 2025Schedule</h4> parsing logic while still collecting semesters and
instructors from table headers, and ensure semester_offered remains populated
when the term appears only in the heading.
In `@backend/services/rag_service.py`:
- Around line 21-48: `retrieve_chunks()` in `rag_service.py` is doing blocking
Gemini and PostgREST I/O on the async chat path, which can stall the event loop.
Update the RAG flow so `retrieve_chunks` and its helper `_embed` are async
end-to-end, or run the sync work in a threadpool, and then adjust the async chat
handlers that call `retrieve_chunks()` to await/use the non-blocking version.
Keep the existing `retrieve_chunks`, `_embed`, and chat handler call sites as
the main places to update.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42df138f-228e-4ce0-a192-993b5752c4b0
📒 Files selected for processing (8)
.gitignorebackend/data/scrape_summary.jsonbackend/db/connection.pybackend/routes/documents.pybackend/routes/learn.pybackend/scripts/ingest_catalog.pybackend/scripts/scrape_bu_catalog.pybackend/services/rag_service.py
Comment on lines
+2
to
+205
| "total_courses": 8720, | ||
| "total_errors": 50, | ||
| "errors": [ | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-650a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-512a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pe-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-en-522a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-or-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-529a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-640a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-512a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-os-520a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-641a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-525a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pe-521a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pd-640a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pd-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pe-640a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-od-642a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-520a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-os-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-511a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-od-522a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-522a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-541a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-os-532a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-660a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-521a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-od-531a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-523a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-en-521a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-od-644a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-581a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pa-530a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-534a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-md-531a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-os-521a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-546a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-542a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-en-640a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-642a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-519a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-pe-520a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-527a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-521a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-642a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-os-640a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-544a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-gd-540a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-524a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-ph-524a/", | ||
| "error": "empty response" | ||
| }, | ||
| { | ||
| "url": "https://www.bu.edu/academics/sdm/courses/sdm-rs-532a/", | ||
| "error": "empty response" | ||
| } | ||
| ], |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't commit a scrape summary that already records 50 failed course pages.
Ingestion uses this scrape output as the source corpus, so these unrecovered SDM URLs mean the initial RAG index is knowingly incomplete. Please either rerun until this is clean or make ingestion fail fast when the scrape summary reports unresolved errors.
🤖 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/data/scrape_summary.json` around lines 2 - 205, The scrape summary
currently includes unresolved failures, so do not keep this snapshot as-is.
Update the ingestion flow that reads scrape_summary.json to either rerun
scraping until the errors array is empty or make the ingest step fail fast when
total_errors is nonzero, using the summary fields total_errors and errors as the
check. Locate the validation in the scrape-summary ingestion path and ensure it
blocks indexing when any failed course URLs remain.
| "elapsed_seconds": 1890, | ||
| "completed_at": "2026-06-27T01:39:55.726124+00:00", | ||
| "semester_tag": "fall_2026", | ||
| "output_file": "C:\\Users\\Jack\\Desktop\\VS Code\\sapling\\backend\\data\\bu_catalog_fall_2026.json" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Remove the machine-local absolute path from this committed artifact.
C:\Users\Jack\... leaks workstation details and makes the summary non-portable. A repo-relative path or no path field at all would avoid noisy diffs and the local identifier exposure.
🤖 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/data/scrape_summary.json` at line 209, The committed scrape_summary
artifact currently hardcodes a machine-local absolute path in the output_file
field, which leaks workstation-specific details and breaks portability. Update
the value in scrape_summary.json to use a repo-relative path or remove the field
entirely, and ensure any code that writes this summary uses a stable path source
so future runs do not reintroduce local identifiers; check the summary
generation logic and the output_file serialization path.
Comment on lines
+960
to
+975
| catalog_rows = table("course_chunks").select( | ||
| "embedding", | ||
| filters={"course_id": f"eq.{bu_course_id}", "category": "eq.catalog"}, | ||
| limit=1, | ||
| ) | ||
| if catalog_rows and catalog_rows[0].get("embedding"): | ||
| catalog_vec = catalog_rows[0]["embedding"] | ||
| # Use the AI-generated summary as the document representative — | ||
| # it's more reliable than raw first-chunk text (avoids cover pages, | ||
| # tables of contents, and boilerplate skewing the score). | ||
| sample_text = doc_summary or chunks[0] | ||
| doc_sample_vec = _embed_texts([sample_text])[0] | ||
| time.sleep(1.5) | ||
| # cosine similarity (vectors are unit-norm from the model) | ||
| dot = sum(a * b for a, b in zip(doc_sample_vec, catalog_vec)) | ||
| if dot < MIN_COURSE_RELEVANCE: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Compare against the best catalog match, not an arbitrary catalog row.
Line 960 fetches limit=1 with no ordering, so the relevance gate can reject an on-topic upload if Supabase returns a non-representative catalog chunk. Fetch the course’s catalog embeddings and gate on the max cosine score instead.
Suggested adjustment
- catalog_rows = table("course_chunks").select(- "embedding",- filters={"course_id": f"eq.{bu_course_id}", "category": "eq.catalog"},- limit=1,- )- if catalog_rows and catalog_rows[0].get("embedding"):- catalog_vec = catalog_rows[0]["embedding"]+ catalog_rows = table("course_chunks").select(+ "embedding",+ filters={"course_id": f"eq.{bu_course_id}", "category": "eq.catalog"},+ )+ catalog_vecs = [r["embedding"] for r in catalog_rows if r.get("embedding")]+ if catalog_vecs:
# Use the AI-generated summary as the document representative —
# it's more reliable than raw first-chunk text (avoids cover pages,
# tables of contents, and boilerplate skewing the score).
sample_text = doc_summary or chunks[0]
doc_sample_vec = _embed_texts([sample_text])[0]
time.sleep(1.5)
- # cosine similarity (vectors are unit-norm from the model)- dot = sum(a * b for a, b in zip(doc_sample_vec, catalog_vec))+ dot = max(sum(a * b for a, b in zip(doc_sample_vec, catalog_vec)) for catalog_vec in catalog_vecs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| catalog_rows=table("course_chunks").select( | |
| "embedding", | |
| filters={"course_id": f"eq.{bu_course_id}", "category": "eq.catalog"}, | |
| limit=1, | |
| ) | |
| ifcatalog_rowsandcatalog_rows[0].get("embedding"): | |
| catalog_vec=catalog_rows[0]["embedding"] | |
| # Use the AI-generated summary as the document representative — | |
| # it's more reliable than raw first-chunk text (avoids cover pages, | |
| # tables of contents, and boilerplate skewing the score). | |
| sample_text=doc_summaryorchunks[0] | |
| doc_sample_vec=_embed_texts([sample_text])[0] | |
| time.sleep(1.5) | |
| # cosine similarity (vectors are unit-norm from the model) | |
| dot=sum(a*bfora, binzip(doc_sample_vec, catalog_vec)) | |
| ifdot<MIN_COURSE_RELEVANCE: | |
| catalog_rows=table("course_chunks").select( | |
| "embedding", | |
| filters={"course_id": f"eq.{bu_course_id}", "category": "eq.catalog"}, | |
| ) | |
| catalog_vecs= [r["embedding"] forrincatalog_rowsifr.get("embedding")] | |
| ifcatalog_vecs: | |
| # Use the AI-generated summary as the document representative — | |
| # it's more reliable than raw first-chunk text (avoids cover pages, | |
| # tables of contents, and boilerplate skewing the score). | |
| sample_text=doc_summaryorchunks[0] | |
| doc_sample_vec=_embed_texts([sample_text])[0] | |
| time.sleep(1.5) | |
| dot=max(sum(a*bfora, binzip(doc_sample_vec, catalog_vec)) forcatalog_vecincatalog_vecs) | |
| ifdot<MIN_COURSE_RELEVANCE: |
🤖 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 960 - 975, The relevance check
currently uses only the first returned catalog chunk from
table("course_chunks"), which is arbitrary because the query has limit=1 without
ordering. Update the logic around the catalog_rows lookup to evaluate all
catalog embeddings for the course and compare the document sample against the
best cosine match, then apply the MIN_COURSE_RELEVANCE gate to that maximum
score. Keep the change localized near the existing sample_text, doc_sample_vec,
and dot calculation so the representative catalog selection is no longer
dependent on Supabase row order.
Comment on lines
+994
to
+1014
| "embedding": None, | ||
| "category": category, | ||
| "semester": "current", | ||
| "section_id": None, | ||
| "school": "", | ||
| }) | ||
| # Embed in batches of 50 | ||
| BATCH = 50 | ||
| for i in range(0, len(records), BATCH): | ||
| batch = records[i : i + BATCH] | ||
| texts = [r["chunk_text"] for r in batch] | ||
| try: | ||
| vecs = _embed_texts(texts) | ||
| for rec, vec in zip(batch, vecs): | ||
| rec["embedding"] = vec | ||
| except Exception as e: | ||
| logger.warning("[RAG] embed failed for doc %s batch %d: %s", doc_id, i, e) | ||
| time.sleep(1.5) # stay under 3000 req/min quota | ||
| table("course_chunks").upsert(records, on_conflict="id") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only upsert successfully embedded chunks, and keep upserts batched.
If _embed_texts fails for a batch, those records keep embedding=None but are still sent to course_chunks; additionally, all chunks are posted in one large request after embedding. That can either pollute the retrieval index or make large uploads fail at the final Supabase call. Upsert only records with embeddings per batch, matching the catalog ingest pattern.
Suggested adjustment
for i in range(0, len(records), BATCH):
batch = records[i : i + BATCH]
texts = [r["chunk_text"] for r in batch]
try:
vecs = _embed_texts(texts)
+ if len(vecs) != len(batch):+ raise ValueError(f"embedding count mismatch: {len(vecs)} for {len(batch)} chunks")
for rec, vec in zip(batch, vecs):
rec["embedding"] = vec
+ table("course_chunks").upsert(batch, on_conflict="id")
except Exception as e:
logger.warning("[RAG] embed failed for doc %s batch %d: %s", doc_id, i, e)
time.sleep(1.5) # stay under 3000 req/min quota
- table("course_chunks").upsert(records, on_conflict="id")
logger.info("[RAG] indexed %d chunks for doc %s", len(records), doc_id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "embedding": None, | |
| "category": category, | |
| "semester": "current", | |
| "section_id": None, | |
| "school": "", | |
| }) | |
| # Embed in batches of 50 | |
| BATCH=50 | |
| foriinrange(0, len(records), BATCH): | |
| batch=records[i : i+BATCH] | |
| texts= [r["chunk_text"] forrinbatch] | |
| try: | |
| vecs=_embed_texts(texts) | |
| forrec, vecinzip(batch, vecs): | |
| rec["embedding"] =vec | |
| exceptExceptionase: | |
| logger.warning("[RAG] embed failed for doc %s batch %d: %s", doc_id, i, e) | |
| time.sleep(1.5) # stay under 3000 req/min quota | |
| table("course_chunks").upsert(records, on_conflict="id") | |
| "embedding": None, | |
| "category": category, | |
| "semester": "current", | |
| "section_id": None, | |
| "school": "", | |
| }) | |
| # Embed in batches of 50 | |
| BATCH=50 | |
| foriinrange(0, len(records), BATCH): | |
| batch=records[i : i+BATCH] | |
| texts= [r["chunk_text"] forrinbatch] | |
| try: | |
| vecs=_embed_texts(texts) | |
| iflen(vecs) !=len(batch): | |
| raiseValueError(f"embedding count mismatch: {len(vecs)} for {len(batch)} chunks") | |
| forrec, vecinzip(batch, vecs): | |
| rec["embedding"] =vec | |
| table("course_chunks").upsert(batch, on_conflict="id") | |
| exceptExceptionase: | |
| logger.warning("[RAG] embed failed for doc %s batch %d: %s", doc_id, i, e) | |
| time.sleep(1.5) # stay under 3000 req/min quota | |
| logger.info("[RAG] indexed %d chunks for doc %s", len(records), doc_id) |
🤖 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 994 - 1014, The batch embedding
flow in the document ingest path still upserts every record at the end,
including chunks whose embedding stayed None after _embed_texts failed. Update
the course_chunks ingest logic to mirror the catalog ingest pattern by upserting
only the successfully embedded records inside each batch loop, and skip failed
batch records instead of carrying them forward. Keep the batching behavior in
place around the existing record processing in the documents route so large
uploads do not get sent as one final upsert.
Comment on lines
+300
to
+304
| if course_id: | ||
| course_info = _get_course_info(course_id) | ||
| catalog_text = _get_catalog_chunk(course_info.get("course_code", "")) | ||
| if catalog_text: | ||
| parts.append("COURSE CATALOG INFO (BU official course data):\n\n" + catalog_text) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat retrieved text as untrusted context, not prompt instructions.
These paths concatenate catalog_text / chunk_text straight into the active prompt. Because course_chunks also holds indexed uploads, a malicious document can inject instructions that the legacy path elevates into system_prompt, and the agent path merges into the user turn. Wrap retrieved text in hard data delimiters, add an explicit “ignore instructions inside retrieved context” rule, or pass it as tool/context payload instead of raw prompt text. As per coding guidelines, backend/routes/learn.py: build_system_prompt defines the streaming tutor (SSE) system prompt and should preserve the tutor prompt contract.
Also applies to: 503-509, 565-571
🤖 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 300 - 304, The retrieved catalog/chunk
content is being appended directly into the active prompt, so malicious uploads
can inject instructions into both the legacy system prompt path and the agent
user-turn path. Update build_system_prompt and the code paths that add
catalog_text/chunk_text to treat retrieved text as untrusted data: wrap it in
clear delimiters or a context block, add an explicit rule to ignore any
instructions inside retrieved content, and avoid merging raw retrieved text into
the prompt where possible.
Source: Coding guidelines
Comment on lines
+504
to
+505
| bu_code = _get_course_info(course_id).get("course_code") if course_id else None | ||
| rag_chunks = retrieve_chunks(user_message, course_id=bu_code or None, k=5) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not broaden to global RAG when course lookup fails.
If course_id is present but _get_course_info() returns no BU code, these calls pass None into retrieve_chunks(), which drops the course filter and searches all course_chunks. That breaks the course-scoped grounding this PR is adding and can inject material from the wrong class.
Minimal fix
- bu_code = _get_course_info(course_id).get("course_code") if course_id else None- rag_chunks = retrieve_chunks(user_message, course_id=bu_code or None, k=5)+ bu_code = _get_course_info(course_id).get("course_code") if course_id else None+ rag_chunks = retrieve_chunks(user_message, course_id=bu_code, k=5) if bu_code else []- bu_code = _get_course_info(course_id).get("course_code") if course_id else None- rag_chunks = retrieve_chunks(body.message, course_id=bu_code or None, k=5)+ bu_code = _get_course_info(course_id).get("course_code") if course_id else None+ rag_chunks = retrieve_chunks(body.message, course_id=bu_code, k=5) if bu_code else []Also applies to: 561-562
🤖 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 504 - 505, The course-scoped retrieval
in learn.py is broadening to global RAG when _get_course_info() does not return
a BU code, because bu_code or None passes None into retrieve_chunks() and drops
the course filter. Update the logic around _get_course_info, bu_code, and
retrieve_chunks so that when course_id is present but no valid course_code is
found, retrieval does not fall back to all course_chunks; instead, skip or
short-circuit the course-scoped RAG path and keep the query scoped to the
requested course only. Apply the same fix in both places that build rag_chunks
from retrieve_chunks.
| EMBED_BATCH = 100 # texts per Gemini embed_content call | ||
| RATE_DELAY = 3.0 # 100 texts / 3s = ~2,000 texts/min (limit is 3,000/min) | ||
| _gemini = genai.Client(api_key=os.getenv("GEMINI_API_KEY", "")) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast instead of upserting rows with embedding=None.
Because the client is created with an empty default API key, a missing or invalid GEMINI_API_KEY falls into the retry path and then writes null embeddings to course_chunks anyway. That leaves the ingest looking successful while retrieval can never match those rows.
Suggested fix
-_gemini = genai.Client(api_key=os.getenv("GEMINI_API_KEY", ""))+_api_key = os.getenv("GEMINI_API_KEY")+if not _api_key:+ raise RuntimeError("GEMINI_API_KEY is required for catalog ingestion")+_gemini = genai.Client(api_key=_api_key)
@@
- except Exception as exc2:- print(f" FAILED [{i}:{i+EMBED_BATCH}]: {exc2} — inserting without embedding")- embeddings.extend([None] * len(batch))+ except Exception as exc2:+ raise RuntimeError(+ f"Embedding failed for batch [{i}:{i + EMBED_BATCH}]"+ ) from exc2Also applies to: 117-156
🤖 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/ingest_catalog.py` at line 41, The Gemini client setup and
ingest retry path are allowing missing or invalid credentials to continue until
rows are written with empty embeddings. Update the `_gemini` initialization and
the ingest flow in `ingest_catalog.py` so a missing `GEMINI_API_KEY` or a failed
embedding request stops processing immediately instead of upserting
`embedding=None` into `course_chunks`. Make the failure explicit in the
`genai.Client`/embedding generation path and propagate it through the affected
ingest logic around the referenced retry/upsert section.
Comment on lines
+26
to
+47
| SCHOOLS = [ | ||
| "cas", | ||
| "com", | ||
| "eng", | ||
| "cfa", | ||
| "cgs", | ||
| "cds", | ||
| "khc", | ||
| "gms", | ||
| "grs", | ||
| "sdm", | ||
| "met", | ||
| "questrom", | ||
| "sar", | ||
| "sha", | ||
| "law", | ||
| "sph", | ||
| "ssw", | ||
| "sth", | ||
| "wheelock", | ||
| "frederick-s-pardee-school-of-global-studies", | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only 20 school slugs are configured.
This list has 20 entries, so two BU schools never get crawled and the catalog/index is incomplete before ingestion even starts. Please add the missing slugs or derive the school list dynamically so it matches the 22-school scrape contract in this PR.
🤖 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/scrape_bu_catalog.py` around lines 26 - 47, The SCHOOLS list
in scrape_bu_catalog.py is missing two BU school slugs, so the crawl only covers
20 schools instead of the full 22-school contract. Update the SCHOOLS constant
in scrape_bu_catalog.py to include the missing school slugs, or replace the
hardcoded list with a source that derives all schools dynamically so the crawler
and downstream index always stay complete. Use the existing SCHOOLS
configuration in scrape_bu_catalog.py as the place to fix this.
Comment on lines
+210
to
+232
| def _extract_schedule(soup: BeautifulSoup) -> tuple[list[str], list[str]]: | ||
| semesters: list[str] = [] | ||
| instructors: list[str] = [] | ||
| sem_pattern = re.compile( | ||
| r'^(FALL|SPRING|SPRG|SUMMER|SUMM|SUM|WINTER|WINT)\s+\d{4}$', re.I | ||
| ) | ||
| for table in soup.find_all("table"): | ||
| headers = [th.get_text(strip=True) for th in table.find_all("th")] | ||
| for h in headers: | ||
| if sem_pattern.match(h.strip()) and h not in semesters: | ||
| semesters.append(h.strip().title()) | ||
| # Instructor column | ||
| lower_headers = [h.lower() for h in headers] | ||
| if "instructor" in lower_headers: | ||
| idx = lower_headers.index("instructor") | ||
| for row in table.find_all("tr")[1:]: | ||
| cells = row.find_all("td") | ||
| if len(cells) > idx: | ||
| name = cells[idx].get_text(strip=True) | ||
| if name and name not in ("TBA", "Staff", "") and name not in instructors: | ||
| instructors.append(name) | ||
| return semesters, instructors |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This second _extract_schedule() overrides the <h4> semester parser.
The earlier implementation already handled the documented <h4>FALL 2025Schedule</h4> structure. Redefining the function here drops that path and only looks at table headers, so semester_offered will be empty on pages where the term appears only in the heading.
Suggested fix
def _extract_schedule(soup: BeautifulSoup) -> tuple[list[str], list[str]]:
semesters: list[str] = []
instructors: list[str] = []
+ for h4 in soup.find_all("h4"):+ m = _SEM_RE.search(h4.get_text(strip=True))+ if m:+ label = m.group(0).strip().title()+ if label not in semesters:+ semesters.append(label)
sem_pattern = re.compile(
r'^(FALL|SPRING|SPRG|SUMMER|SUMM|SUM|WINTER|WINT)\s+\d{4}$', re.I
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def_extract_schedule(soup: BeautifulSoup) ->tuple[list[str], list[str]]: | |
| semesters: list[str] = [] | |
| instructors: list[str] = [] | |
| sem_pattern=re.compile( | |
| r'^(FALL|SPRING|SPRG|SUMMER|SUMM|SUM|WINTER|WINT)\s+\d{4}$', re.I | |
| ) | |
| fortableinsoup.find_all("table"): | |
| headers= [th.get_text(strip=True) forthintable.find_all("th")] | |
| forhinheaders: | |
| ifsem_pattern.match(h.strip()) andhnotinsemesters: | |
| semesters.append(h.strip().title()) | |
| # Instructor column | |
| lower_headers= [h.lower() forhinheaders] | |
| if"instructor"inlower_headers: | |
| idx=lower_headers.index("instructor") | |
| forrowintable.find_all("tr")[1:]: | |
| cells=row.find_all("td") | |
| iflen(cells) >idx: | |
| name=cells[idx].get_text(strip=True) | |
| ifnameandnamenotin ("TBA", "Staff", "") andnamenotininstructors: | |
| instructors.append(name) | |
| returnsemesters, instructors | |
| def_extract_schedule(soup: BeautifulSoup) ->tuple[list[str], list[str]]: | |
| semesters: list[str] = [] | |
| instructors: list[str] = [] | |
| forh4insoup.find_all("h4"): | |
| m=_SEM_RE.search(h4.get_text(strip=True)) | |
| ifm: | |
| label=m.group(0).strip().title() | |
| iflabelnotinsemesters: | |
| semesters.append(label) | |
| sem_pattern=re.compile( | |
| r'^(FALL|SPRING|SPRG|SUMMER|SUMM|SUM|WINTER|WINT)\s+\d{4}$', re.I | |
| ) | |
| fortableinsoup.find_all("table"): | |
| headers= [th.get_text(strip=True) forthintable.find_all("th")] | |
| forhinheaders: | |
| ifsem_pattern.match(h.strip()) andhnotinsemesters: | |
| semesters.append(h.strip().title()) | |
| # Instructor column | |
| lower_headers= [h.lower() forhinheaders] | |
| if"instructor"inlower_headers: | |
| idx=lower_headers.index("instructor") | |
| forrowintable.find_all("tr")[1:]: | |
| cells=row.find_all("td") | |
| iflen(cells) >idx: | |
| name=cells[idx].get_text(strip=True) | |
| ifnameandnamenotin ("TBA", "Staff", "") andnamenotininstructors: | |
| instructors.append(name) | |
| returnsemesters, instructors |
🤖 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/scrape_bu_catalog.py` around lines 210 - 232, The second
_extract_schedule() implementation overwrites the earlier heading-based semester
parser, so pages with only an h4 term heading lose semester detection. Update
_extract_schedule() to preserve the existing <h4>FALL 2025Schedule</h4> parsing
logic while still collecting semesters and instructors from table headers, and
ensure semester_offered remains populated when the term appears only in the
heading.
Comment on lines
+21
to
+48
| def _embed(text: str) -> list[float]: | ||
| resp = _client.models.embed_content( | ||
| model=_EMBED_MODEL, | ||
| contents=[text], | ||
| config=genai_types.EmbedContentConfig(output_dimensionality=_OUTPUT_DIM), | ||
| ) | ||
| return list(resp.embeddings[0].values) | ||
| def retrieve_chunks( | ||
| query: str, | ||
| course_id: str | None = None, | ||
| k: int = 5, | ||
| min_similarity: float = 0.55, | ||
| ) -> list[dict]: | ||
| """Return up to k chunks similar to query, optionally filtered by course_id. | ||
| Each result: {"course_id": str, "chunk_text": str, "similarity": float} | ||
| """ | ||
| try: | ||
| embedding = _embed(query) | ||
| params: dict = { | ||
| "query_embedding": embedding, | ||
| "match_count": k, | ||
| "filter_course_id": course_id, | ||
| } | ||
| rows = rpc("match_course_chunks", params) | ||
| return [r for r in rows if r.get("similarity", 0) >= min_similarity] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
echo"Async chat handlers calling retrieve_chunks:"
rg -n -C2 'async def (_chat_via_agent|_legacy_chat)|retrieve_chunks\(' backend/routes/learn.py
echoecho"Blocking calls inside the RAG path:"
rg -n -C2 '_client\.models\.embed_content|rpc\("match_course_chunks"|_client\.post\(' \
backend/services/rag_service.py backend/db/connection.pyRepository: SaplingLearn/Sapling
Length of output: 2952
Move RAG I/O off the async request path.
retrieve_chunks() makes synchronous Gemini and PostgREST calls, and both async chat handlers invoke it directly. That blocks the event loop on every chat request and will hurt concurrency under load; make this path async end-to-end or wrap it in a threadpool.
🤖 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/rag_service.py` around lines 21 - 48, `retrieve_chunks()` in
`rag_service.py` is doing blocking Gemini and PostgREST I/O on the async chat
path, which can stall the event loop. Update the RAG flow so `retrieve_chunks`
and its helper `_embed` are async end-to-end, or run the sync work in a
threadpool, and then adjust the async chat handlers that call
`retrieve_chunks()` to await/use the non-blocking version. Keep the existing
`retrieve_chunks`, `_embed`, and chat handler call sites as the main places to
update.
…legend - Remove r > 10 gate in KnowledgeGraph2D so every concept node shows its name regardless of mastery score (unexplored nodes have r=8, below old threshold) - Capitalize legend labels: mastered/learning/struggling/unexplored → title case - Add react-force-graph-3d (missing from node_modules, caused build error) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/KnowledgeGraph2D.tsx (1)
518-518: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLabels now render for all non-root nodes regardless of size.
Dropping
r > 10means even the smallest (e.g., unexplored/low-mastery) nodes get text labels. In dense graphs this can cause overlapping/cluttered labels since node radii can be as small as 8px and collision radius as low as 18px (Line 202, 233). If this is the intended UX, LGTM; otherwise consider re-adding a minimum-size or zoom-level gate.🤖 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/KnowledgeGraph2D.tsx` at line 518, The label rendering in KnowledgeGraph2D now applies to every non-root node, which can make tiny nodes clutter the graph. In the node label rendering block near the n.is_subject_root check, add back a guard based on node size or zoom level so labels only appear when the node radius is large enough or the graph is sufficiently zoomed in. Use the existing node radius/collision logic in KnowledgeGraph2D as the reference point for choosing the threshold.
🤖 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.
Nitpick comments:
In `@frontend/src/components/KnowledgeGraph2D.tsx`:
- Line 518: The label rendering in KnowledgeGraph2D now applies to every
non-root node, which can make tiny nodes clutter the graph. In the node label
rendering block near the n.is_subject_root check, add back a guard based on node
size or zoom level so labels only appear when the node radius is large enough or
the graph is sufficiently zoomed in. Use the existing node radius/collision
logic in KnowledgeGraph2D as the reference point for choosing the threshold.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5568e158-ca74-488f-bf4b-67843028f7b5
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/src/components/KnowledgeGraph2D.tsxfrontend/src/components/screens/Dashboard.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/screens/Dashboard.tsx
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
using an async httpx + BeautifulSoup scraper (
scrape_bu_catalog.py). Capturescourse code, title, description, credits, prerequisites, instructors, and semester offerings.
ingest_catalog.pyembeds each course withgemini-embedding-001(768-dim) and upserts intocourse_chunkswith an HNSW indexfor cosine similarity search.
build_system_promptinjects the enrolled course'scatalog entry at session start; both chat paths run per-message semantic search
(top-5 chunks, similarity ≥ 0.55) to ground the tutor in the student's actual course.
_index_document_chunksruns in the background via
_spawn_post_roll, chunks the text into 800-char windows,and embeds + upserts into
course_chunks. A relevance gate (similarity ≥ 0.35 againstthe course catalog using the AI-generated summary) blocks off-topic uploads.
rpc()todb/connection.py— thin wrapper around Supabase/rest/v1/rpc/used to call
match_course_chunksfor vector similarity queries.Test plan
includes
COURSE CATALOG INFOblock[RAG]lines appear in backend logs[RAG] indexed N chunksin logs[RAG] doc skipped — relevance < 0.35in logsSummary by CodeRabbit
New Features
Bug Fixes
Chores