Update homework ingestion and topic card curation for stronger study data coverage - #16
Conversation
There was a problem hiding this comment.
Pull request overview
Expands the study dataset + topic-cards pipeline to ingest and surface Homework solutions alongside lecture, notebook, and exam content, improving coverage and enabling homework-driven curation in the UI.
Changes:
- Add homework ingestion into
study_db.json(week-levelhomework_cells) and pipeline materialization (homeworks). - Extend topic-card generation/curation to attach
homework_snippets+homework_recommended_ids, and update UI selection/preview behavior accordingly. - Add/adjust integrity tests and introduce a new homework integration script + dry-run test.
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| topic_cards.json | Adds homework snippet sections and normalizes typography (arrows/dashes/apostrophes). |
| data/study_db.json | Materializes homework sources + homework_cells per week; updates topic analysis metadata. |
| scripts/integrate_homework_material.py | New CLI to ingest homework solutions and sync topic cards; emits a curation report. |
| pipelines/shared/study_database.py | Adds homework_cells to week record shape and homeworks to flattened pipeline payload. |
| pipelines/topic_cards/assembly.py | Attaches homework content to topic cards and adds recommended homework IDs. |
| pipelines/topic_cards/pipeline.py | Wires homework attachment into main topic-cards pipeline and updates metadata notes. |
| pipelines/study_database/{curation.py,validators.py} | Normalizes and validates homework_cells; updates “non-empty week” rules. |
| app/* | Adds homework snippet rendering, selection splits, preview support, and state migration. |
| tests/* | Updates integrity expectations and adds dry-run test for homework integration script. |
| docs/* | Documents new homework_cells field and topic merging guidance for homework sections. |
| Makefile | Adds py_compile coverage for the new integration script. |
| materials/homework/* | Adds homework prompt files and solutions text used for ingestion. |
Comments suppressed due to low confidence (4)
pipelines/topic_cards/assembly.py:1
out.strip()assumes every homework output is a string. If outputs come from notebook-style cells (e.g., dicts or other non-string payloads), this will raise an AttributeError and break topic-card generation. Safer pattern is to normalize withstr(out).strip()(or reuse the same output-compaction approach used elsewhere in the repo) before filtering.
scripts/integrate_homework_material.py:1- Dry-run currently writes
topic_cards.json(insidesync_topic_cards_homework) and then restores the previous content. Even if the digest stays the same, this still mutates file mtime and can trigger watchers, CI cache invalidation, or local tooling that reacts to writes. Consider adding adry_run/write=Falseoption tosync_topic_cards_homework(or writing to a temp path) so dry-run produces the same report without touching tracked files.
scripts/integrate_homework_material.py:1 - The solution-block cleaner replaces NBSP with a normal space but does not normalize indentation. In the generated homework snippets (e.g., Week 2 Exercise 2.9), this can yield 5-space indents (NBSP + 4 spaces), producing syntactically invalid Python when users copy/paste. A robust fix is to normalize indentation after NBSP replacement (e.g., dedent to the minimum common indent, and/or convert leading indentation to consistent 4-space groups) before storing snippets.
pipelines/study_database/validators.py:1 - The duplicate index error message is a bit misleading: it produces strings like
Duplicate notebook cell_index found: 3via...{bucket_label(bucket)}_index.... Since the field name is specificallycell_index, consider emitting a clearer and more directly searchable message likeDuplicate {bucket}.cell_index found: {cell_index}(orDuplicate {bucket} cell_index found: ...) and removingbucket_labelentirely.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b06800d1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| canonical = str(card.get("canonical_topic") or "").strip() | ||
| if not canonical: | ||
| canonical = topic_key(str(card.get("topic") or "")) |
There was a problem hiding this comment.
canonical_topic before matching homework snippets
sync_topic_cards_homework() is comparing the raw card['canonical_topic'] against normalized homework topic keys, which silently drops matches for curated cards whose canonical topic is still human-facing text. In the committed data, topic-args-star keeps canonical_topic: "*arg", so is_relevant() never matches the week 3 args_star homework cells and that card ends up with zero homework snippets even though exercises 3 and 5 are present in study_db. Re-running this integrator will keep under-populating homework coverage for any card whose canonical topic was not already topic_key()-normalized.
Useful? React with 👍 / 👎.
| valid_ids = { | ||
| item.get("id") | ||
| for bucket in ["lecture_snippets", "exam_questions", "notebook_snippets"] | ||
| for bucket in ["lecture_snippets", "exam_questions", "notebook_snippets", "homework_snippets"] | ||
| for item in sections.get(bucket, []) |
There was a problem hiding this comment.
recommended_ids scoped to buckets the UI can resolve
This change makes homework snippet IDs count as valid recommended_ids, but the app still resolves recommended_ids through buildSourceItems()/getSourceSplit() in app/view-and-data.js, which only materialize exam, lecture, and notebook items. If a curator follows the updated contract and adds a homework ID to recommended_ids, the integrity test will pass here and the snippet will still disappear from both the swipe card and preview at runtime instead of landing in the new homework sections.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 252 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
tmp/exam_coverage_audit/manifest.json:1
- This committed audit artifact contains absolute local file paths (including a username and home directory layout). That’s sensitive and non-portable, and it will break other environments. Recommendation: do not commit generated
tmp/exam_coverage_audit/*outputs (add to.gitignore), and ensure any tracked audit fixtures use repo-relative paths only (or sanitized placeholders) so CI and other developers can run the workflow deterministically.
tmp/exam_coverage_audit/manifest.json:1 - This committed audit artifact contains absolute local file paths (including a username and home directory layout). That’s sensitive and non-portable, and it will break other environments. Recommendation: do not commit generated
tmp/exam_coverage_audit/*outputs (add to.gitignore), and ensure any tracked audit fixtures use repo-relative paths only (or sanitized placeholders) so CI and other developers can run the workflow deterministically.
scripts/exam_coverage_audit.py:1 - The audit script’s exam source root points outside the repo (
ROOT.parent / 'course_files_after_midterm'). But the repo itself (e.g.,data/study_db.json) references exam PDFs undermaterials/..., implying these files are expected to live inside the repository layout. As-is,preparewill fail for anyone without that adjacent directory. Recommendation: switchCOURSE_DIR/PRACTICE_EXAMS_DIRto repo-internal locations (e.g., underROOT / 'materials' / ...) or make them configurable CLI args with clear defaults, so the documented commands indocs/TESTING.mdwork reliably in CI and on other machines.
scripts/integrate_homework_material.py:1 - In
--dry-run, the script still writes totopic_cards.jsonand then restores it. If the process is interrupted (crash/kill) between those operations, the working tree can be left modified—violating the dry-run contract and causing confusing diffs. Recommendation: refactorsync_topic_cards_homeworkto support a pure in-memory mode (return updated payload + report without writing), or write to a temp file path during dry-run instead of touching the realtopic_cards.json.
pipelines/study_database/validators.py:1 - The duplicate-index error message is currently rendered as
Duplicate notebook cell_index found: ...(note the awkwardcell_indexconcatenation via_index). This is confusing and inconsistent with the rest of the validation messages. Recommendation: either havebucket_label()return something that composes cleanly (e.g., include a trailing space) or format the message directly asDuplicate {bucket}.cell_index found: {cell_index}so it’s unambiguous and matches other field-scoped errors.
pipelines/study_database/validators.py:1 - The duplicate-index error message is currently rendered as
Duplicate notebook cell_index found: ...(note the awkwardcell_indexconcatenation via_index). This is confusing and inconsistent with the rest of the validation messages. Recommendation: either havebucket_label()return something that composes cleanly (e.g., include a trailing space) or format the message directly asDuplicate {bucket}.cell_index found: {cell_index}so it’s unambiguous and matches other field-scoped errors.
tmp/exam_coverage_audit/seed_exact_matches.json:1 - This audit fixture contains the same
question_numbertwice within a single exam (question_number: 4appears twice with different evidence/topic routing). If any downstream tooling assumes question numbers are unique per exam (common for reports, validation, or UI linking), this will cause collisions and misleading summaries. Recommendation: de-duplicate question numbers within an exam (or represent multi-evidence as a single question entry with multiple evidence candidates). Also, if this is generated output, prefer not committing it and instead regenerate deterministically.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17d9819061
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sections["homework_snippets"] = deduped | ||
| recommended_count = min(4, max(1, (len(deduped) + 1) // 2)) if deduped else 0 | ||
| sections["homework_recommended_ids"] = [snippet.get("id") for snippet in deduped[:recommended_count] if snippet.get("id")] |
There was a problem hiding this comment.
This writes homework_snippets and homework_recommended_ids into every card, but the runtime selection model still only reads exam_questions, lecture_snippets, notebook_snippets, and recommended_ids (checked app/topic-selection.js:114-180). In practice the 28 new hw-* items added by this commit are unreachable in the Topic Explorer/preview, so the advertised homework coverage never becomes user-visible.
Useful? React with 👍 / 👎.
| for item in useful_lecture_snippets(card): | ||
| items.append({"id": item.get("id"), "source_type": "lecture", "priority": 1, "item": item}) | ||
| for item in useful_notebook_snippets(card): | ||
| items.append({"id": item.get("id"), "source_type": "notebook", "priority": 2, "item": item}) |
There was a problem hiding this comment.
The new audit packet only enumerates exam/lecture/notebook sources here, even though this commit also adds homework_snippets and homework_recommended_ids to topic_cards.json. As a result prepare underreports selectable evidence and validate-findings rejects any homework-backed evidence ID (hw-* IDs currently fail validation), which makes the new homework corpus invisible to the exam-coverage workflow.
Useful? React with 👍 / 👎.
Summary
topic_cards.jsonand related study data to improve coverage, routing, and selectable content qualityTesting
make leave-betterpython3 scripts/exam_coverage_audit.py prepare