refactor(calendar): unify syllabus extraction onto agent (refactor #4)#79
Merged
Merged
Conversation
Closes the four-refactor migration plan from ADR 0001. The agent (syllabus_extraction_agent) already shipped with refactor #1 and is used by routes/documents.py's agentic upload. This PR migrates the second consumer — services/calendar_service.py::parse_syllabus — onto the same agent, removing the duplicated parsing path that ADR 0005 called out as refactor #4 ("low-effort cleanup that fits anywhere"). What shipped: - agents/tools/syllabus_adapter.py — syllabus_to_wire_dict adapter. Maps the agent's date|None to ISO-8601 strings, defaults assignment_type to "other", passes course_title and grading_categories through as additive keys. Pure logic, 10 tests. - services/calendar_service.py — _extract_via_agent (new), parse_syllabus (preserved as legacy fallback per ADR 0001), extract_assignments_from_file rewritten agent-first with fallback on UsageLimitExceeded / UnexpectedModelBehavior / Exception. Pure dedup helpers (save_assignments_to_db, insert_new_assignments, load_existing_assignment_keys, assignment_dedupe_key) untouched. - tests/test_syllabus_adapter.py — 10 new tests. Pins legacy-shape contract, "other" default, ISO-8601 date serialization, mutable-default isolation, dedupe-key integration. - tests/test_ocr_pipeline.py — TestExtractAssignmentsViaAgent class with 4 new tests covering agent-success + 3 fallback triggers + the empty-text short-circuit. Plus test_agent_and_legacy_paths_share_required_keys pinning the superset-invariant on both paths. - tests/test_calendar_routes.py — TestImportExtractWireFormat with 3 regression tests confirming routes/calendar.py::extract tolerates both agent-path and legacy-path dict shapes. - tests/evals/syllabus_extraction.py — three new evaluators: WireFormatRequiredKeysEvaluator, AssignmentTypeNonNullEvaluator, DueDateIsoStringEvaluator. Run on existing recorded cassettes; no new cases needed. - prompts/refactor-4-syllabus-unification/ — sub-agent playbook (8 files) used to dispatch this refactor. Committed for archive. - docs/decisions/0016-refactor-4-syllabus-unification-shipped.md — ADR with surprises, consequences, rollback. Notes that this is the LAST named refactor in ADR 0001's migration plan. No prompt-hash bump (97946a2b84b2 unchanged from refactor #1) — the adapter handled the contract translation entirely. Existing eval cassettes stay valid. 596 tests pass on this branch; 3 pre-existing live-Supabase failures unchanged (test_skips_self_edges, test_save_to_db, test_full_pipeline). No regressions. Migration plan from ADR 0001 is COMPLETE after this PR merges. The remaining cleanup is three small PRs: gemini_service.py deletion, start_session/action migration on chat (PR #78 follow-up), and sub-agent E's deletion of legacy parse_syllabus + the prompt file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements Refactor ChangesSyllabus Extraction Unification
Sequence DiagramsequenceDiagram
participant CalendarRoute as /extract Route
participant CalendarSvc as calendar_service
participant Agent as syllabus_extraction_agent
participant Adapter as syllabus_to_wire_dict
participant LegacyParse as parse_syllabus<br/>(Gemini)
participant TextExtract as extract_text()
CalendarRoute->>CalendarSvc: extract_assignments_from_file()<br/>(file_bytes, filename, content_type,<br/>user_id, request_id)
CalendarSvc->>TextExtract: Extract text from file
TextExtract-->>CalendarSvc: extracted_text
alt Empty text
CalendarSvc-->>CalendarRoute: {assignments: [],<br/>warnings: [],<br/>raw_text: ""}
else Non-empty text
CalendarSvc->>Agent: _extract_via_agent()<br/>(extracted_text, user_id, request_id)
alt Agent succeeds
Agent-->>Adapter: SyllabusAssignments
Adapter-->>CalendarSvc: wire_dict<br/>(assignments, warnings,<br/>raw_text, course_title,<br/>grading_categories)
CalendarSvc-->>CalendarRoute: wire_dict
else UsageLimitExceeded /<br/>UnexpectedModelBehavior /<br/>Exception
CalendarSvc->>LegacyParse: parse_syllabus()<br/>(extracted_text)
LegacyParse-->>CalendarSvc: legacy_dict<br/>(assignments, warnings, raw_text)
CalendarSvc->>CalendarSvc: Ensure raw_text,<br/>course_title,<br/>grading_categories present
CalendarSvc-->>CalendarRoute: enriched_legacy_dict
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| from datetime import date | ||
|
|
||
| import pytest |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
frontend | 7ca027a | Commit Preview URL Branch Preview URL |
May 05 2026, 04:39 AM |
…symmetry Five issues from the self-review on PR #79. 1. CRITICAL: asyncio.run inside async route would 500 in production. `routes/calendar.py::extract` is `async def`. The sync `extract_assignments_from_file` was calling `asyncio.run(_extract_via_agent(...))` from inside the running event loop, which CPython rejects with "cannot be called from a running event loop". Tests missed it because direct unit tests called from sync test bodies (no loop) and the route tests mocked the helper entirely. - services/calendar_service.py: extract_assignments_from_file and process_and_save_syllabus are now `async def`. asyncio.run replaced with `await _extract_via_agent(...)`. `import asyncio` removed. Stale TODO removed. - routes/calendar.py: route now does `await extract_assignments_from_file(...)`. 2. user_id and request_id plumbed end-to-end. Logfire spans for syllabus extractions are now correlatable to a user + a request. routes/calendar.py reads request_id from request.state with current_request_id() fallback (mirrors quiz/learn pattern). process_and_save_syllabus passes user_id through. 3. Wire-format symmetry on the legacy fallback path. Both fallback branches now setdefault course_title=None and grading_categories=[] on the dict returned by parse_syllabus, so consumers see the same shape whether the agent fired or fell back. 4. Tests updated for the async API. 7 asyncio.run wrappings added across test_ocr_pipeline.py — TestExtractAssignmentsViaAgent (4), test_agent_and_legacy_paths_share_required_keys (2), and the live-DB test_full_pipeline (1). 5. Regression test that catches the asyncio.run bug. New test_import_extract_real_async_chain_works mocks only at the AGENT layer (syllabus_extraction_agent.run + extract_text_from_file), then POSTs to the real /api/calendar/extract route. The full chain — route -> service -> _extract_via_agent -> adapter — actually executes. If the awaits get unwired again, this test fails immediately. 597 tests pass on this branch (up from 596 by the +1 integration test). 3 pre-existing live-Supabase failures unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Optional kwarg so existing callers (the live-DB tests in test_ocr_pipeline.py) keep working without change. When a route ever wires this function, it should pass request.state.request_id through so the syllabus run's Logfire spans correlate with the user-facing request — same plumbing the /extract route does. Closes the last in-scope minor issue from PR #79's review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/tests/evals/syllabus_extraction.py (1)
147-165: ⚡ Quick winMake
REQUIREDimmutable.Line 155 is a mutable class attribute on a dataclass, which Ruff is already flagging as
RUF012. Afrozenset/ClassVarkeeps the intent the same and avoids shared mutable state.Suggested change
+from typing import ClassVar + `@dataclass` class WireFormatRequiredKeysEvaluator(Evaluator[str, SyllabusAssignments]): @@ - REQUIRED = {"assignments", "warnings", "raw_text"} + REQUIRED: ClassVar[frozenset[str]] = frozenset( + {"assignments", "warnings", "raw_text"} + )🤖 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/evals/syllabus_extraction.py` around lines 147 - 165, The class attribute REQUIRED on WireFormatRequiredKeysEvaluator is mutable; change it to an immutable class-level constant by declaring it as a frozenset (or annotate with typing.ClassVar[frozenset]) so it is not shared mutable state on the dataclass; update the REQUIRED definition in the WireFormatRequiredKeysEvaluator class accordingly and ensure any uses (e.g., self.REQUIRED.issubset(...)) continue to work.backend/tests/test_calendar_routes.py (1)
283-297: ⚡ Quick winUse factories instead of mutable class-level payloads.
AGENT_RESULTandLEGACY_RESULTare shared mutable dicts, and the currentdict(...)copies are only shallow. Ruff is already flagging this asRUF012; returning a fresh payload per test avoids cross-test leakage.Suggested change
class TestImportExtractWireFormat: """Pins the wire format of POST /api/calendar/extract.""" - AGENT_RESULT = { - "assignments": [ - { - "title": "Lab 7: Recursion", - "due_date": "2026-03-15", - "assignment_type": "other", - "notes": "Hands-on recursion lab.", - "weight_pct": 10.0, - } - ], - "warnings": [], - "raw_text": "syllabus body", - "course_title": "CS 101", - "grading_categories": [{"name": "Labs", "weight": 0.4}], - } - - LEGACY_RESULT = { - "assignments": [ - { - "title": "HW1", - "due_date": "2026-03-01", - "assignment_type": "homework", - "notes": None, - } - ], - "warnings": [], - "raw_text": "fallback text", - } + `@staticmethod` + def _agent_result(): + return { + "assignments": [ + { + "title": "Lab 7: Recursion", + "due_date": "2026-03-15", + "assignment_type": "other", + "notes": "Hands-on recursion lab.", + "weight_pct": 10.0, + } + ], + "warnings": [], + "raw_text": "syllabus body", + "course_title": "CS 101", + "grading_categories": [{"name": "Labs", "weight": 0.4}], + } + + `@staticmethod` + def _legacy_result(): + return { + "assignments": [ + { + "title": "HW1", + "due_date": "2026-03-01", + "assignment_type": "homework", + "notes": None, + } + ], + "warnings": [], + "raw_text": "fallback text", + }Also applies to: 299-310
🤖 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_calendar_routes.py` around lines 283 - 297, AGENT_RESULT and LEGACY_RESULT are module-level mutable dicts causing cross-test state; replace them with factory functions (e.g., make_agent_result() and make_legacy_result()) that return a fresh dict each call, update tests in test_calendar_routes.py to call these factories instead of referencing the module constants, and if any test needs a modified version use copy.deepcopy(...) or construct the dict from the factory to avoid shallow-copy pitfalls.
🤖 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/prompts/refactor-4-syllabus-unification/00-orchestrator-overview.md`:
- Around line 61-66: The documented wire-format example incorrectly lists
course_id as part of each assignment; update the example in
00-orchestrator-overview.md so the adapter output matches the actual contract
(as produced by extract_assignments_from_file and consumed by
routes/calendar.py) by removing course_id from the assignment shape and keeping
the rule that missing fields (e.g., assignment_type) get a sensible default
("other"); ensure the text references the adapter/consumer symbols
(extract_assignments_from_file, routes/calendar.py) and the assignment field
names ({title, due_date, assignment_type, notes}) to avoid future drift.
In `@backend/prompts/refactor-4-syllabus-unification/05-sub-agent-E-cleanup.md`:
- Around line 83-86: The grep command is searching "backend/" after you already
cd'd into the backend root, which can cause false negatives; update the command
string "grep -rn
\"parse_syllabus\\|process_and_save_syllabus\\|prompts/syllabus_extraction.txt\"
backend/ --include=\"*.py\" --include=\"*.txt\"" to search the current directory
(e.g. replace backend/ with . or remove the explicit path) so it reliably
inspects files from the backend root; keep the same include patterns and flags.
In `@backend/services/calendar_service.py`:
- Around line 141-157: The fallback except branches that call
parse_syllabus(text) must also ensure the returned result always contains a
warnings key; update both exception handlers (the block catching
UsageLimitExceeded/UnexpectedModelBehavior and the generic Exception block where
parse_syllabus is called and result is assigned) to call
result.setdefault("warnings", []) in addition to the existing setdefault calls
for "raw_text", "course_title", and "grading_categories" so the API shape is
consistent regardless of which branch runs.
---
Nitpick comments:
In `@backend/tests/evals/syllabus_extraction.py`:
- Around line 147-165: The class attribute REQUIRED on
WireFormatRequiredKeysEvaluator is mutable; change it to an immutable
class-level constant by declaring it as a frozenset (or annotate with
typing.ClassVar[frozenset]) so it is not shared mutable state on the dataclass;
update the REQUIRED definition in the WireFormatRequiredKeysEvaluator class
accordingly and ensure any uses (e.g., self.REQUIRED.issubset(...)) continue to
work.
In `@backend/tests/test_calendar_routes.py`:
- Around line 283-297: AGENT_RESULT and LEGACY_RESULT are module-level mutable
dicts causing cross-test state; replace them with factory functions (e.g.,
make_agent_result() and make_legacy_result()) that return a fresh dict each
call, update tests in test_calendar_routes.py to call these factories instead of
referencing the module constants, and if any test needs a modified version use
copy.deepcopy(...) or construct the dict from the factory to avoid shallow-copy
pitfalls.
🪄 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: d9cc9142-3cbf-4ad2-a039-9eb70e1a9322
📒 Files selected for processing (16)
backend/agents/tools/syllabus_adapter.pybackend/prompts/refactor-4-syllabus-unification/00-orchestrator-overview.mdbackend/prompts/refactor-4-syllabus-unification/01-sub-agent-A-adapter.mdbackend/prompts/refactor-4-syllabus-unification/02-sub-agent-B-service.mdbackend/prompts/refactor-4-syllabus-unification/03-sub-agent-C-routes.mdbackend/prompts/refactor-4-syllabus-unification/04-sub-agent-D-evals.mdbackend/prompts/refactor-4-syllabus-unification/05-sub-agent-E-cleanup.mdbackend/prompts/refactor-4-syllabus-unification/06-adr-template.mdbackend/prompts/refactor-4-syllabus-unification/README.mdbackend/routes/calendar.pybackend/services/calendar_service.pybackend/tests/evals/syllabus_extraction.pybackend/tests/test_calendar_routes.pybackend/tests/test_ocr_pipeline.pybackend/tests/test_syllabus_adapter.pydocs/decisions/0016-refactor-4-syllabus-unification-shipped.md
Comment on lines
+61
to
+66
| - **Wire format unchanged.** `routes/calendar.py` reads | ||
| `{"assignments": [{title, due_date, assignment_type, notes, course_id}], "warnings": [...], "raw_text": ...}` | ||
| from `extract_assignments_from_file`. The adapter must produce | ||
| exactly that shape; if a field is genuinely not in the agent's | ||
| schema (`assignment_type` is the only one to watch), the adapter | ||
| fills a sensible default (`"other"`) rather than dropping the field. |
There was a problem hiding this comment.
Correct the wire-format contract example (course_id is not part of adapter output)
At Line 62, the documented assignment shape includes course_id, but the implemented adapter/tested contract does not emit that key. This mismatch can misdirect future refactors and tests.
Suggested fix
-- **Wire format unchanged.** `routes/calendar.py` reads
- `{"assignments": [{title, due_date, assignment_type, notes, course_id}], "warnings": [...], "raw_text": ...}`
+- **Wire format unchanged.** `routes/calendar.py` reads
+ `{"assignments": [{title, due_date, assignment_type, notes, weight_pct}], "warnings": [...], "raw_text": ...}`🤖 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/prompts/refactor-4-syllabus-unification/00-orchestrator-overview.md`
around lines 61 - 66, The documented wire-format example incorrectly lists
course_id as part of each assignment; update the example in
00-orchestrator-overview.md so the adapter output matches the actual contract
(as produced by extract_assignments_from_file and consumed by
routes/calendar.py) by removing course_id from the assignment shape and keeping
the rule that missing fields (e.g., assignment_type) get a sensible default
("other"); ensure the text references the adapter/consumer symbols
(extract_assignments_from_file, routes/calendar.py) and the assignment field
names ({title, due_date, assignment_type, notes}) to avoid future drift.
Comment on lines
+83
to
+86
| cd "/Users/josegaelcruzlopez/Documents/Startup_Projects /Sapling/backend" | ||
| python -m pytest tests/ -q --no-header --ignore=tests/evals | ||
| grep -rn "parse_syllabus\|process_and_save_syllabus\|prompts/syllabus_extraction.txt" backend/ --include="*.py" --include="*.txt" | ||
| ``` |
There was a problem hiding this comment.
Fix the verification grep path to avoid false negatives
At Line 83 you already cd into the backend root, so Line 85 should not search backend/ again. As written, it can silently miss matches and incorrectly validate cleanup completion.
Suggested fix
cd "/Users/josegaelcruzlopez/Documents/Startup_Projects /Sapling/backend"
python -m pytest tests/ -q --no-header --ignore=tests/evals
-grep -rn "parse_syllabus\|process_and_save_syllabus\|prompts/syllabus_extraction.txt" backend/ --include="*.py" --include="*.txt"
+grep -rn "parse_syllabus\|process_and_save_syllabus\|prompts/syllabus_extraction.txt" . --include="*.py" --include="*.txt"📝 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
| cd "/Users/josegaelcruzlopez/Documents/Startup_Projects /Sapling/backend" | |
| python -m pytest tests/ -q --no-header --ignore=tests/evals | |
| grep -rn "parse_syllabus\|process_and_save_syllabus\|prompts/syllabus_extraction.txt" backend/ --include="*.py" --include="*.txt" | |
| ``` | |
| cd "/Users/josegaelcruzlopez/Documents/Startup_Projects /Sapling/backend" | |
| python -m pytest tests/ -q --no-header --ignore=tests/evals | |
| grep -rn "parse_syllabus\|process_and_save_syllabus\|prompts/syllabus_extraction.txt" . --include="*.py" --include="*.txt" |
🤖 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/prompts/refactor-4-syllabus-unification/05-sub-agent-E-cleanup.md`
around lines 83 - 86, The grep command is searching "backend/" after you already
cd'd into the backend root, which can cause false negatives; update the command
string "grep -rn
\"parse_syllabus\\|process_and_save_syllabus\\|prompts/syllabus_extraction.txt\"
backend/ --include=\"*.py\" --include=\"*.txt\"" to search the current directory
(e.g. replace backend/ with . or remove the explicit path) so it reliably
inspects files from the backend root; keep the same include patterns and flags.
Comment on lines
+141
to
+157
| except (UsageLimitExceeded, UnexpectedModelBehavior) as e: | ||
| logger.warning( | ||
| "Syllabus agent guardrails tripped; falling back to legacy", | ||
| exc_info=e, | ||
| ) | ||
| result = parse_syllabus(text) | ||
| result.setdefault("raw_text", text) | ||
| result.setdefault("course_title", None) | ||
| result.setdefault("grading_categories", []) | ||
| except Exception: | ||
| logger.exception( | ||
| "Unexpected syllabus-agent failure; falling back to legacy" | ||
| ) | ||
| result = parse_syllabus(text) | ||
| result.setdefault("raw_text", text) | ||
| result.setdefault("course_title", None) | ||
| result.setdefault("grading_categories", []) |
There was a problem hiding this comment.
Backfill warnings in the fallback branches too.
Both fallback paths normalize raw_text, course_title, and grading_categories, but they still return a shape without warnings if parse_syllabus() omits it. That makes the API contract depend on which path ran.
Suggested change
except (UsageLimitExceeded, UnexpectedModelBehavior) as e:
logger.warning(
"Syllabus agent guardrails tripped; falling back to legacy",
exc_info=e,
)
result = parse_syllabus(text)
+ result.setdefault("warnings", [])
result.setdefault("raw_text", text)
result.setdefault("course_title", None)
result.setdefault("grading_categories", [])
except Exception:
logger.exception(
"Unexpected syllabus-agent failure; falling back to legacy"
)
result = parse_syllabus(text)
+ result.setdefault("warnings", [])
result.setdefault("raw_text", text)
result.setdefault("course_title", None)
result.setdefault("grading_categories", [])🤖 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/calendar_service.py` around lines 141 - 157, The fallback
except branches that call parse_syllabus(text) must also ensure the returned
result always contains a warnings key; update both exception handlers (the block
catching UsageLimitExceeded/UnexpectedModelBehavior and the generic Exception
block where parse_syllabus is called and result is assigned) to call
result.setdefault("warnings", []) in addition to the existing setdefault calls
for "raw_text", "course_title", and "grading_categories" so the API shape is
consistent regardless of which branch runs.
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 free
to 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
Closes the four-refactor migration plan from ADR 0001. The agent (
syllabus_extraction_agent) already shipped with refactor #1 and is used byroutes/documents.py's agentic upload. This PR migrates the second consumer —services/calendar_service.py::parse_syllabus— onto the same agent, removing the duplicated parsing path that ADR 0005 called out as refactor #4 ("low-effort cleanup that fits anywhere").This is the last named refactor in the migration plan.
What shipped
backend/agents/tools/syllabus_adapter.py—syllabus_to_wire_dictadapter. Maps the agent'sdate | None→ ISO-8601 strings, defaultsassignment_type="other", passescourse_titleandgrading_categoriesthrough as additive keys. Pure logic, 10 tests.backend/services/calendar_service.py—_extract_via_agent(new),parse_syllabus(preserved as legacy fallback per ADR 0001),extract_assignments_from_filerewritten agent-first with fallback onUsageLimitExceeded/UnexpectedModelBehavior/Exception. Pure dedup helpers untouched.test_syllabus_adapter.py(10),TestExtractAssignmentsViaAgent(4) + invariant test intest_ocr_pipeline.py,TestImportExtractWireFormat(3) intest_calendar_routes.py. 596 pass on this branch; 3 pre-existing live-Supabase failures unchanged from PR re-architecture: agentic document upload + AES-256-GCM column encryption + dev-context vault #67/refactor(quiz): convert generate_quiz to quiz_agent (refactor #2) #71/refactor(learn): convert chat tutor to chat_tutor_agent (refactor #3) #78. No regressions.backend/tests/evals/syllabus_extraction.py— three new evaluators (WireFormatRequiredKeysEvaluator,AssignmentTypeNonNullEvaluator,DueDateIsoStringEvaluator) wired intomake_dataset(). Run on existing recorded cassettes; no re-record needed.docs/decisions/0016-refactor-4-syllabus-unification-shipped.md— full ADR with surprises, consequences, and explicit rollback path.backend/prompts/refactor-4-syllabus-unification/— sub-agent playbook (8 files) used to dispatch this refactor. Committed for archive.Prompt hash
97946a2b84b2— unchanged from refactor #1. The adapter handled the contract translation entirely; no schema move means existing eval cassettes stay valid.What's NOT in this PR (per ADR 0001 migration contract)
parse_syllabusandprompts/syllabus_extraction.txtare NOT deleted. They stay as the fallback target. Sub-agent E's cleanup PR removes them after this PR is onmainfor ~1 week. Same playbook PR refactor(quiz): convert generate_quiz to quiz_agent (refactor #2) #71 and PR refactor(learn): convert chat tutor to chat_tutor_agent (refactor #3) #78 followed.process_and_save_syllabusis NOT deleted. Architecture.md flagged it as "exists for direct OCR→Gemini→DB use but is not currently wired"; onlytests/test_ocr_pipeline.py's pre-existing-failing tests call it. Cleanup PR handles deletion if no consumer surfaces.services/gemini_service.pyis NOT deleted. Still alive as the chat fallback target (PR refactor(learn): convert chat tutor to chat_tutor_agent (refactor #3) #78) and the quiz fallback target (PR refactor(quiz): convert generate_quiz to quiz_agent (refactor #2) #71). After this PR plus Sub-agent E's cleanup,call_gemini_jsonhas just one remaining caller (the quiz fallback). The eventual one-linegemini_service.pydeletion PR ships once both fallbacks retire.Migration plan complete
After this PR merges, all four named refactors from ADR 0001 are on
main:The remaining migration cleanup is three small PRs: Sub-agent E's syllabus deletion, PR #78's
start_session/actionfollow-up, and the finalservices/gemini_service.pydeletion once both fallbacks retire.References
Test plan
pytest tests/test_syllabus_adapter.py -q→ 10 passedpytest tests/test_ocr_pipeline.py -q→ all green except the 3 pre-existing live-DB failures (unchanged)pytest tests/test_calendar_routes.py -q→ all greenpytest tests/ -q --ignore=tests/evals→ 596 passed, 3 pre-existing failures (unchanged)SAPLING_EVAL_MODE=live pytest tests/evals/syllabus_extraction.py -q) — recommended; the three new evaluators run on existing cassettes so replay-mode passes already/api/calendar/import-extractand confirm assignments come back with the expected shape (assignments + warnings + raw_text + the new course_title/grading_categories extras)Rollback
Single revert of the merge commit drops the adapter + the new helper, reverts
extract_assignments_from_fileto its pre-refactor body, and the prompt + agent + the documents-upload path are unchanged (refactor #1 already shipped them). The new test files are pure additions — harmless if rolled back partially.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor