-
Notifications
You must be signed in to change notification settings - Fork 649
Conversation
Override the Read the Docs HTML command to use two Sphinx workers while preserving the existing builder, language, doctree, and output settings. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
No actionable comments were generated in the recent review. 🎉 i️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Read the Docs configuration adds an explicit Sphinx HTML build command using two workers, English language settings, ChangesRead the Docs build
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.36%. Comparing base (cc689a7) to head (c43224b).
Additional details and impacted files
@@ Coverage Diff @@ ## master #5888 +/- ## ========================================== - Coverage 79.47% 79.36% -0.11% ========================================== Files 1072 1085 +13 Lines 125041 126405 +1364 Branches 4536 4592 +56 ========================================== + Hits 99373 100319 +946 - Misses 24044 24431 +387 - Partials 1624 1655 +31
☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Run the custom Sphinx command from the documentation directory so the source checkout does not shadow the installed package. Add a 45-minute timeout with a forced-kill grace period to turn stalled parallel builds into explicit failures. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
dargs.sphinx.DargsDomain advertises parallel_read_safe but does not implement merge_domaindata, so Sphinx refuses to combine the per-worker inventories and -j aborts the build. The domain's only state is a flat targetid -> (docname, objtype) map, so merging is a dict update restricted to the documents the worker read. Verified on a minimal two-page project using dargs directives: -j 2 fails with the same NotImplementedError without the shim and succeeds with it.
njzjz
commented
Jul 27, 2026
Rebased onto master and made -j 2 actually work in eed8f9cfc.
The RTD build on this branch was failing at 655 s with:
NotImplementedError: merge_domaindata must be implemented in
<class 'dargs.sphinx.DargsDomain'> to be able to do parallel builds!
dargs.sphinx returns {"parallel_read_safe": True} from its setup, but DargsDomain inherits Domain.merge_domaindata, which raises. Sphinx then tears down the worker pool and the failure surfaces as a secondary KeyError in sphinx/util/parallel.py. doc/conf.py now installs a merge_domaindata on DargsDomain when it is still the base implementation; the domain's only state is a flat targetid -> (docname, objtype) map, so merging is a dict update restricted to the documents that worker read. Worth upstreaming to dargs, at which point the guard makes the shim a no-op.
Verified on a minimal two-page project using .. dargs:: directives: python -m sphinx -j 2 reproduces the exact NotImplementedError without the shim and reports build succeeded with it.
For context on why this matters beyond this PR: the docs/readthedocs.org:deepmd check is red on a large number of open PRs, and every one I sampled is the same 40-minute RTD wall-clock limit — builds 33625117, 33625500, 33626766 all ended at 2409-2411 s with the sphinx step still running. So this PR is the fix for that whole class of failures, not just a speed-up.
Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
njzjz
commented
Aug 1, 2026
The build process is not sped up.
@njzjz-bot
njzjz-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed draft head 347ea8d8daa927302c7bf903df4d260c0d98c380 with three independent full subagent reviews. The Sphinx parallel-domain shim and Read the Docs custom-build syntax are sound, and the preview build succeeds. One P2 configuration issue remains inline: the command timeout starts too late and exceeds the platform's observed hard build limit, so it cannot provide the graceful termination described by the comment. Some C++ checks on this draft are currently failing or cancelled, with no evidence tying them to this documentation-only diff. Because this PR was opened by the active njzjz-bot account, this is a comment-only review rather than a self-request-changes event.
The quota is about to reset, so I am concentrating the remaining token budget on these reviews.
Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Uh oh!
There was an error while loading. Please reload this page.
Summary
-j 2.Performance measurement
This PR intentionally contains only the Sphinx worker-count change so its Read the Docs preview measures parallelism independently.
Baseline Read the Docs build 33680413:
Both performance PRs use upstream commit
83e7245646185a2de8f7d6277edc4f64b4f500f0as their base.Testing
.readthedocs.ymlwith PyYAML-j 2ruff check .ruff format .Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
Summary by CodeRabbit