Repository navigation
feat(study_material): add the sources list and coach setup to download study material - #92
Merged
Merged
Conversation
sources.yaml names the System Design Primer text, worked solutions and Anki decks and the chosen Tech Interview Handbook pages, each pinned to a commit with its licence and whether it is local-only, validated by Pydantic models. coach setup downloads them into the git-ignored data/study-material/ folder, skipping sources already at their pinned commit. THIRD_PARTY_NOTICES.md credits both sources. Closes #15
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
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.
Intent
Add a committed sources list naming each piece of study material the coach teaches from, and a
coach setupcommand that downloads it into the git-ignoreddata/study-material/folder, so third-party material is downloaded at setup rather than committed (ADR-0002).The sources list covers the System Design Primer README text (no images), its worked system design solutions and its Anki decks, all under CC BY 4.0, and the Tech Interview Handbook's six behavioural pages plus its system design, engineering levels and coding rubric pages, under MIT and fetched from the repository rather than the website. Each entry records its download URL, a pinned commit SHA, its licence and whether it may be used only locally, and the list is validated against a schema.
coach setupmust be safe to run twice and must never write outside the git-ignored folder. A third-party notices file credits Donne Martin for the Primer and carries the Handbook's MIT notice. Tests validate the sources list against its schema and exercise the downloader against a local fake source, the project's single check command passes locally and in CI, and code comments link the technical references used so the change can be studied while reviewed.Closes #15
What Changed
sources.yaml, a committed manifest pinning each study source (System Design Primer text, worked solutions and Anki decks; Tech Interview Handbook pages) to a full commit SHA with its SPDX licence,local_onlyflag and download URL template, validated by new PydanticSource/SourcesListmodels insrc/interview_coach/study_material.pythat reject bad commits, escaping paths, images, malformed URL templates and duplicate names.coach setupsubcommand that downloads every source into the git-ignoreddata/study-material/folder and is safe to run twice: files are written to a temporary name and renamed into place, and a.commitstamp makes already-downloaded sources skip fetching; failures print a readablecoach: error:message and exit 1.THIRD_PARTY_NOTICES.mdcrediting Donne Martin (CC BY 4.0) and the Tech Interview Handbook (MIT), documentcoach setupin the README, add thepydanticandpyyamldependencies, and add tests covering the sources-list schema and the downloader against a localfile://fake source.Risk Assessment
✅ Low: The change is well-bounded: all acceptance criteria are source-verified (pinned URLs resolve, coverage matches the report, notices credit both sources, behavioral tests for the schema and downloader), and both fix-round changes were verified by execution to do exactly what was prescribed with no new defects or unrequired components.
Testing
Drove the real
coachCLI live: clean-clone setup downloaded every pinned source into the git-ignored folder with a clean git status, the second run was a no-op, a local-only fake source stayed inside the data folder and was labelled, and five corrupted/hand-edited sources lists all failed with a readable one-line error and exit 1; all targeted tests pass and every scenario passed with no findings.coach setup(no API key) downloads every source at its pinned commit into data/study-materialcoach setuptwice is safe: the second run skips every source without refetchingcoach setup,git statusshows nothing new to commitgit status --porcelainreturned empty after both runsfindof the scenario dir showing zero files outside data/study-materialcoach: error: …and exits 1 instead of a traceback{oops}placeholder, space in download_url,../escape.mdpath — all exit 1 with one-line errors##section placed above## Licensegrep -n '^## ' README.mdshows the section at line 64, before## Licenseat line 75Evidence: First `coach setup` run on clean data folder (no API key)
system-design-primer-text: downloaded system-design-primer-solutions: downloaded system-design-primer-anki: downloaded tech-interview-handbook: downloaded exit=0Evidence: Second `coach setup` run (idempotent)
system-design-primer-text: already up to date system-design-primer-solutions: already up to date system-design-primer-anki: already up to date tech-interview-handbook: already up to date exit=0Evidence: Full listing of downloaded study material (30 files, no images)
Evidence: Local-only fake source via real CLI (second run shows skip)
fake-deck (local only): already up to datePipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
src/interview_coach/cli.py:46-coach setup's readable-error handler is incomplete, so some bad sources lists produce a raw traceback instead ofcoach: error: ...and exit 1. Verified by execution: (1) a malformed sources.yaml (e.g. tab indentation) raisesyaml.scanner.ScannerError, whose MRO isYAMLError -> Exception— neither OSError nor ValueError — so it escapesexcept (OSError, ValueError)at cli.py:46, contradicting the comment at cli.py:47-48 that 'a bad sources list is a ValueError'; the sibling is theyaml.safe_loadcall in load_sources (src/interview_coach/study_material.py:80). (2) Adownload_urlwith a stray placeholder (e.g.https://x/{commit}/{path}/{oops}) passes_url_template(src/interview_coach/study_material.py:26-29) and raisesKeyError('oops')inSource.url_for'sstr.format(src/interview_coach/study_material.py:61) — also uncaught. Remedy (non-functional, error path only): wrapyaml.YAMLErrorintoValueErrorinsideload_sources, and reject unknown{...}placeholders in_url_template. Reachable only via a hand-edited or corrupted manifest — the exact input class this handler exists for — so warning, not error.🔧 Fix applied.
2 issues (1 warning, 1 info) still open:
src/interview_coach/study_material.py:38- Residual sibling of finding R1 that fix round d436932 left behind: a hand-edited sources list can still produce a raw traceback instead ofcoach: error: ...and exit 1. Verified by execution: adownload_urlwhose literal text contains a raw space or control character (e.g.https://example.com/my folder/{commit}/{path}) passes_url_template(it inspects only{...}placeholders, not the literal template text), reachesurllib.request.urlopen, and raiseshttp.client.InvalidURL— anHTTPException, neitherOSErrornorValueError— which escapesexcept (OSError, ValueError)at src/interview_coach/cli.py:46 and makes the comment at cli.py:47-49 ('a bad sources list is a ValueError') inaccurate again. Probe output:urlopen('https://example.com/my folder/x')→InvalidURL -> HTTPException -> Exception. Remedy at the earliest shared boundary every download_url passes through: reject charactershttp.clientrefuses ([\x00-\x20\x7f]) in the literal template inside_url_template(src/interview_coach/study_material.py:38), with one CLI test mirroring the two R1 tests (tests/test_study_material.py:175,184); catchinghttp.client.HTTPExceptionin_setup()additionally covers server-borne mid-connection failures but alone would be a symptom patch for the manifest-borne class. This is the only manifest field that can still raise outside (OSError, ValueError): name/commit/licence/local_only are constrained by patterns, andfilespaths are percent-encoded inurl_for(study_material.py:58) and re-checked with resolve/is_relative_to at download time (study_material.py:102-105). Reachable only via a hand-edited or corrupted manifest — the exact input class this handler exists for — so warning, not error; the remedy is a one-condition error-path fix, not an extension of intent.README.md:68- The new usage section### Downloading the study material(README.md:68) is placed after and nested under## License(README.md:64), so the setup instructions read as a subsection of the license. Remedy: move the block above## Licenseand promote the heading to##. Non-functional documentation-structure fix; introduced with this change.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
coach setup(no API key) downloads every source at its pinned commit into data/study-materialcoach setuptwice is safe: the second run skips every source without refetchingcoach setup,git statusshows nothing new to commitgit status --porcelainreturned empty after both runsfindof the scenario dir showing zero files outside data/study-materialcoach: error: …and exits 1 instead of a traceback{oops}placeholder, space in download_url,../escape.mdpath — all exit 1 with one-line errors##section placed above## Licensegrep -n '^## ' README.mdshows the section at line 64, before## Licenseat line 75env -u OPENAI_API_KEY .venv/bin/coach setupin the worktree on a clean data/study-material (first run: 4 sources downloaded, exit 0)env -u OPENAI_API_KEY .venv/bin/coach setupsecond run (4 sources 'already up to date', exit 0)git status --porcelainafter both setup runs (empty — nothing new to commit)find data/study-material -type f+ stamp files +fileon .apkg decks + zero image/.part files (content integrity at pinned SHAs ae9bbd7… and e1d28e8…)Bad-manifest drives of the realcoach setupin disposable dirs: missing sources.yaml, tab-indented malformed YAML,{oops}unknown placeholder, download_url with a space,../escape.mdpath — all exit 1 withcoach: error:and no tracebackLocal-only drive: fakefile://source withlocal_only: truethrough the real CLI — outputfake-deck (local only): downloaded,findproves zero files outside data/study-material.venv/bin/python -m pytest tests/test_study_material.py -v(21 passed).venv/bin/python -m pytest tests/test_cli.py tests/test_study_material.py tests/test_smoke.py -q(24 passed)Verified THIRD_PARTY_NOTICES.md credits Donne Martin (CC BY 4.0) and carries the Handbook's full MIT notice; README's## Downloading the study materialsits above## Licenseat##level (R3)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.