Skip to content

Extract git-repo ingestion into a standalone /ingest-course-repo skill - #13

Merged
Carr1005 merged 1 commit into
mainfrom
extract-ingest-course-repo
Jul 26, 2026
Merged

Extract git-repo ingestion into a standalone /ingest-course-repo skill#13
Carr1005 merged 1 commit into
mainfrom
extract-ingest-course-repo

Conversation

@Carr1005

Copy link
Copy Markdown
Collaborator

Reverses the coupling introduced in #2 by extracting it rather than reverting it. /generate-spec goes back to being purely materials → spec.md; repo ingestion becomes its own skill.

Not a git revert of d62df53#3/#6/#7 rewrote generate-spec/SKILL.md afterwards, and a revert would clobber the guide-deference wording. The script moves via git mv, so its history follows.

Why

/generate-spec had to carve out an exception in its own ## Don't list — "Don't write to materials/… except the Step 2 ingest script" — an invariant with an exception inside the skill meant to honour it. It also made a single-purpose skill do two jobs, which nothing else in the loop does (/new-course deliberately doesn't generate the spec).

After: /generate-spec has a flat Don't write to builds/, evals/, or materials/, and /ingest-course-repo is the sole writer into materials/.

Two bugs found and fixed

Silent clobber. The --out path #2 prescribed (materials/notebooks/<course>-context.md) is byte-identical to the name of a hand-downloaded context dump — confirmed against the eval fixtures — and out.write_text overwrites without asking. Output now goes to materials/notebooks/from-repo/, so both coexist.

The target repo couldn't be ingested at all. deeplearningai-eng/courses is a private 3.4 GB monorepo, 262 root entries, one course_<id>/ per course, git-LFS-backed. find_notebooks/find_helpers rglob from the clone root with no scoping, so pointing the skill at it would have cloned 3.4 GB and merged ~260 courses into one context file.

ingest_repo.py gains an optional --subdir that scopes discovery and switches the clone to --filter=blob:none --sparse with GIT_LFS_SKIP_SMUDGE=1:

before after
course_10 ingest 3.4 GB clone, ~260 courses merged 8.7 MB, ~8s, 5 notebooks

Omitted, behaviour is byte-identical to today — standalone single-course repos are unaffected.

Interface

/ingest-course-repo https://github.com/deeplearningai-eng/courses/tree/main/course_10

The skill parses the tree URL into repo + --ref + --subdir. A bare monorepo root is refused with a pointer to browse for the course folder; any other bare root proceeds normally.

Framing correction

#2's copy sold this as a way to "skip the manual notebook download". It isn't a replacement — it's an optional supplement that adds the course's real code and helper.py on top of the site download, which stays the main path. Rewritten in new-course/SKILL.md, README.md, and the new skill; /new-course's item 4 handoff is untouched, with the ingest trailing as a clearly subordinate item 5.

Verified

  • Backward compatibility — pre-change script vs new, same fixture, no --subdir: byte-identical output
  • Scoping — unscoped fixture gives 2 notebooks + 1 cross-course helper.py conflict; --subdir gives 1 + 0
  • Sparse clone size against the real monorepo (8.7 MB of 3.4 GB; only course_10 checked out)
  • Guards: bad --subdir → exit 1, no partial file; --out into transcripts/ → exit 1
  • No clobber — hand-downloaded file's sha unchanged across an ingest
  • Tree-URL parse produces the same output as hand-typed flags
  • Both refusal paths (cwd guard, monorepo root) fire without invoking git

Not included: the untracked .agents/ Codex mirror was updated locally for parity but is deliberately out of this PR.

🤖 Generated with Claude Code

PR #2 bolted an optional repo-ingestion step onto /generate-spec. The
capability is useful; the placement was not. It forced generate-spec to
carve out an exception in its own "Don't write to materials/" rule, and
made a single-purpose skill do two jobs. Every other skill in the loop is
single-purpose and hands off.

This extracts it rather than reverting it: the script moves with its
history (git mv), the prose moves largely intact, and the shortcut keeps
working behind its own front door. /generate-spec is materials -> spec.md
again, with a flat write invariant; /ingest-course-repo is the sole
writer into materials/.

Two bugs fixed on the way:

- Silent clobber. The prescribed --out was byte-identical to the name of
  a hand-downloaded context dump (<course>-context.md), and the script
  overwrites without asking. Output now lands in
  materials/notebooks/from-repo/, so the two coexist.
- The target repo was un-ingestible. deeplearningai-eng/courses is a
  3.4 GB monorepo holding ~260 courses with no path scoping in the
  script, so ingesting it would have cloned everything and merged every
  course into one context file. ingest_repo.py gains an optional
  --subdir that scopes discovery and switches the clone to
  --filter=blob:none --sparse with GIT_LFS_SKIP_SMUDGE=1. Pulling
  course_10 now transfers 8.7 MB in ~8s. Omitted, behaviour is
  byte-identical to before, so standalone course repos are unaffected.

The skill parses a GitHub tree URL into repo + --ref + --subdir, and
refuses the bare monorepo root with a pointer to browse for the course
folder. Framing throughout is corrected from PR #2's "skip the manual
notebook download" to what it actually is: an optional supplement that
adds the course's real code and helper.py on top of the site download,
which remains the main path.

Verified: backward compatibility against the pre-change script on a local
fixture (byte-identical); subdir scoping; sparse clone size; bad-subdir
and transcripts-path guards; no clobber of a hand-downloaded file;
tree-URL parse producing the same output as hand-typed flags; both
refusal paths firing without invoking git.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Carr1005
Carr1005 merged commit 871a06b into main Jul 26, 2026
@Carr1005
Carr1005 deleted the extract-ingest-course-repo branch July 26, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant