Skip to content

Lead onboarding docs with ContentNode(uri=) over legacy File/Node classes#683

Open
rtibblesbot wants to merge 5 commits into
learningequality:mainfrom
rtibblesbot:issue-681-6b907b
Open

Lead onboarding docs with ContentNode(uri=) over legacy File/Node classes#683
rtibblesbot wants to merge 5 commits into
learningequality:mainfrom
rtibblesbot:issue-681-6b907b

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

The onboarding docs and example chef scripts taught only the legacy Node(files=[FileSubclass(path=...)]) pattern; no doc showed the canonical ContentNode(uri=...) form that the file pipeline actually infers kind, preset, and files from. This rewrites the getting-started/tutorial examples, notebooks, and reference docs (nodes.md, files.md, pdfutils.md, htmlapps.md, concepts/introduction.md) to lead with ContentNode(uri=...), demotes the legacy *Node/*File subclasses to a labelled "legacy / advanced" section, and keeps exercises and video subtitles (SubtitleFile) as the two documented exceptions.

It also lands the remaining reference-doc fixes from the issue: installation.md's uv/pyproject.toml rewrite, chefops.md's CLI-flag drift, the video_compression.md legacy ffmpeg_settings mention, and downloader.md's import-path fix. Dead links (stale master-branch le-utils links, an archive.org tutorial video, a dead poppler-windows binary link) are fixed, files.md now documents getting YouTube subtitles via YouTubeSubtitleFile and recommends ContentNode's thumbnail attribute over a standalone ThumbnailFile.

A round of reviewer questions on the docs surfaced two real gaps in ContentNode itself, fixed alongside the docs that now describe them: _process_uri didn't propagate the node's language to the file(s) the pipeline builds from uri, and there was no way to pass subtitle_languages (or other per-node pipeline options) through to the transfer stage's YoutubeDownloadHandler. ContentNode now accepts a context dict that's threaded into pipeline.execute(), and _process_uri defaults each produced file's language to the node's own unless the pipeline inferred a different one. See ricecooker/classes/nodes.py and the new tests in tests/test_tree.py.

References

Closes #681.

Reviewer guidance

  • This now covers all four plan phases (examples/notebooks, tutorial walkthrough, reference docs, and the installation/chefops/video_compression/downloader fixes) plus the review feedback from PR Lead onboarding docs with ContentNode(uri=) over legacy File/Node classes #683.
  • The full doc set was re-verified to build cleanly under Sphinx (no new warnings vs. baseline), so the files.md#subtitle-files anchor referenced from docs/nodes.md and docs/tutorial/explanations.md resolves.
  • docs/examples/languages.ipynb, exercises.ipynb, and document_conversion.ipynb were re-executed rather than hand-edited, so stored outputs reflect a real run — spot-check the rendered notebook output cells match the new ContentNode(uri=) code.

AI usage

Used Claude Code to audit the docs against ricecooker/classes/nodes.py and the file pipeline, then rewrite the examples and reference docs. Verified by re-executing the affected notebooks, running the example chef scripts' construct_channel() directly against the rewritten code, and building the docs under Sphinx.


@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?
  • Ran pre-flight CI checks (lint, format, tests) and verified all pass
  • Rebased onto the target branch and resolved any conflicts
  • Reorganized commit history into clean, logical commits
  • Audited the diff to ensure only issue-relevant files are changed
  • Built PR body from the repository's PR template with evidence blocks
@rtibblesbot

🟡 Waiting for feedback

Last updated: 2026-07-05 21:48 UTC

…sses

Rewrites the getting-started/tutorial examples and reference docs (nodes.md,
files.md, pdfutils.md, htmlapps.md) to teach ContentNode(uri=...) as the
canonical construction pattern, demote VideoNode/AudioNode/DocumentNode/
HTML5AppNode to a legacy/advanced section, and keep exercises and video
subtitles (SubtitleFile) as the two documented exceptions. Fixes the
nonexistent HTML5ZipFile reference and re-executes the affected notebooks.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rtibblesbot rtibblesbot marked this pull request as ready for review July 5, 2026 02:50

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good start, some cleanup needed on what has been implemented, and what hasn't been implemented needs to be!

Phase 4 (installation.md uv/pyproject rewrite, chefops.md CLI-flag drift, video_compression.md, downloader.md import-path fix) and the final Sphinx clean-build check are not yet in this PR.

Comment thread docs/tutorial/explanations.md Outdated
the content node is in English. The Python package `le-utils` defines the internal
language codes used throughout the Kolibri platform (e.g. `en`, `es-MX`, and `zul`).
To find the internal language code for a given language, you can locate it in the
[lookup table](https://github.com/learningequality/le-utils/blob/master/le_utils/resources/languagelookup.json),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a dead link. Default branch on this repo is main. Please verify all links in the documentation are still live.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — the repo's default branch is main, and le-utils uses the same. Fixed this link and every other le-utils/blob/master/... reference across the docs (7 total: nodes.md, files.md, languages.md, csv_metadata/csv_exercises.md, concepts/developer_workflows.md, tutorial/explanations.md) to point at main. Also re-ran the full Sphinx build to confirm no other broken cross-references were introduced.

4. Repeat steps 1-3 for the video and audio content nodes.
5. Finally, add your content nodes to your channel (see last \*\* statements)

Note the video example also attaches a subtitle file — see the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been reported that the tutorial videos are no longer publicly accessible. We should remove any references to them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — the archive.org item backing the tutorial's example video (RiceChef) 404s. Replaced the uri in examples/tutorial/sushichef.py with a live, similarly-small archive.org video (CM_National_Rice_Cooker_1982.mp4, ~1.5MB, already referenced elsewhere in these docs in video_compression.md) and verified the chef still imports and constructs its tree cleanly. tutorial.rst's prose itself didn't reference the URL directly, so no further text change was needed there.

Comment thread docs/files.md
)
```

You can also get subtitles using `YouTubeSubtitleFile` which takes a `youtube_id`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth mentioning how to automatically get subtitles from a youtube URL using the ContentNode approach?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an example right after the YouTubeSubtitleFile paragraph pairing it with the ContentNode(uri=...) approach — passing a YouTube URL as uri downloads the video, and YouTubeSubtitleFile(youtube_id=..., language=...) fetches the caption track directly from YouTube (confirmed via the pipeline code that this is not automatic from uri alone — it requires the explicit add_file call, same as the local-file SubtitleFile case, just without needing a local .vtt).

Comment thread docs/files.md

Thumbnail files
---------------
### Thumbnail files

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add that it's generally recommended to add thumbnails using the "thumbnail" attribute on ContentNode - adding as a separate file is mostly unnecessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note recommending ContentNode(..., thumbnail="path/or/url") (also works on ChannelNode/TopicNode) over constructing a standalone ThumbnailFile — confirmed thumbnail accepts the same local path or URL and wraps it in a ThumbnailFile internally via set_thumbnail().

- Fix dead le-utils master-branch links across docs (repo's default
  branch is main, not master)
- Replace the dead archive.org RiceChef tutorial video URL with a live
  public-domain video
- Document getting YouTube subtitles automatically via YouTubeSubtitleFile
  alongside ContentNode(uri=)
- Recommend ContentNode's thumbnail attribute over a standalone
  ThumbnailFile
- Land the previously-missing Phase 4 doc fixes: installation.md
  uv/pyproject rewrite, chefops.md CLI-flag drift, video_compression.md
  ffmpeg_settings cleanup, downloader.md import-path fix
- Verify the full doc set builds cleanly under Sphinx (no new warnings
  vs. baseline)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Addressed all review feedback in 2de59ad — replied inline on each of the four threads, and landed the previously-missing Phase 4 work (installation.md, chefops.md, video_compression.md, downloader.md) plus a clean Sphinx build.

Simplify wording across chefops, files, installation, nodes, and
tutorial docs: separate staging/deploy into its own subsection, drop
redundant explanations of uri inference, and point to AGENTS.md instead
of duplicating the contributor quick start.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more things need cleaning up.

ricecooker behaviour is to cache downloaded files forever.
Use the `--update` argument to bypass the cached and re-download all files.
The `--update` must be used whenever files are modified but the path stays the same.
Line (9) shows the `uri` attribute — the path or URL to the actual pdf file. `uri` can be

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the ContentNode URI path also automatically set the language on the file? If it isn't doing that would be good to fix that concurrently with this documentation update.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — it wasn't. Fixed in ricecooker/classes/nodes.py: ContentNode._process_uri now does metadata_dict.setdefault("language", self.language) before constructing the File object, so a file built from uri inherits the node's language unless the pipeline itself set one explicitly (e.g. a YouTube subtitle track keeps its own per-language code from YoutubeDownloadHandler). Added test_automatic_resource_node_document_inherits_node_language in tests/test_tree.py covering the inheritance case, and a sentence in docs/tutorial/explanations.md noting it. Verified with uv run --group test pytest.

Comment thread docs/tutorial/gettingstarted.rst Outdated
<a href="http://35.196.115.213/en/learn/#/topics/c/3bd5eca9a81557efbab488849058c8c7" target='_blank'>
<iframe width="560" height="315" src="https://www.youtube.com/embed/tmCllZOzY0Q" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen></iframe>
</a>
<iframe width="560" height="315" src="https://www.youtube.com/embed/tmCllZOzY0Q" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen></iframe>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This video isn't available.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — removed the whole "Video overview" section (prose link + iframe) from docs/tutorial/gettingstarted.rst rather than swapping in another dead/unverifiable link, since there's no known-good replacement for this one. The rest of the getting-started walkthrough doesn't depend on it.

Comment thread docs/files.md Outdated
ones that are not currently supported. Please let the LE content team know when
you run into language codes that are not supported so we can add them.

`YouTubeSubtitleFile` pairs with the `ContentNode(uri=...)` approach for YouTube

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't what I meant. You can pass "subtitle_languages" to the pipeline context to automatically trigger subtitle download from YouTube: https://github.com/learningequality/ricecooker/blob/main/ricecooker/utils/pipeline/transfer.py#L192

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it — replaced the YouTubeSubtitleFile.add_file(...) example with ContentNode(uri=..., context={"subtitle_languages": ["en", "es"]}), which is what actually triggers automatic subtitle download from YouTube on the uri path (confirmed against YoutubeDownloadHandler.get_file_kwargs in ricecooker/utils/pipeline/transfer.py:180-205, which loops over context.subtitle_languages to fetch a caption track per language). Linked directly to that handler in the doc so the mechanism is traceable.

rtibblesbot and others added 2 commits July 5, 2026 14:36
- ContentNode now passes a `context` dict through to the file pipeline
  and inherits the node's `language` onto files it builds from `uri`,
  fixing an inconsistency the reviewer flagged where a file built from
  `uri` had no language unless the pipeline inferred one itself.
- docs/files.md: point YouTube-subtitle docs at `context={"subtitle_languages": [...]}`
  instead of a manual `YouTubeSubtitleFile.add_file` call, since that's
  the mechanism that actually triggers automatic subtitle download from
  the `ContentNode(uri=)` path.
- docs/tutorial/gettingstarted.rst: drop the "Video overview" section
  and its dead video embed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Remove duplicated/redundant explanations of staging-vs-deploy and
uri-based file inference now that the canonical explanation lives
in one place, and point readers there instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

Re-architect docs around the pipeline-first ContentNode(uri=) API

2 participants