docs: add announcements landing page - #1971
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe documentation landing page now presents an announcements hub with searchable, tag-filtered, paginated cards. Sphinx loads supporting CSS and JavaScript, navigation is adjusted for announcement pages, and two announcement documents are added. ChangesAnnouncements hub
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Visitor
participant AnnouncementPage
participant announcements.js
participant AnnouncementCards
Visitor->>AnnouncementPage: Open announcements landing page
AnnouncementPage->>announcements.js: Handle DOMContentLoaded
announcements.js->>AnnouncementCards: Sort, filter, and paginate cards
announcements.js-->>AnnouncementPage: Toggle card visibility and pager state
Visitor->>AnnouncementPage: Search or select a tag
AnnouncementPage->>announcements.js: Dispatch input or click event
announcements.js->>AnnouncementCards: Recompute matching cards
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1971 +/- ##
==========================================
- Coverage 66.83% 66.82% -0.02%
==========================================
Files 519 519
Lines 58916 58916
==========================================
- Hits 39376 39370 -6
- Misses 19540 19546 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
b26e4e9 to
278c3ca
Compare
|
/ok to test 278c3ca |
|
/claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| menu.innerHTML = ` | ||
| <p class="caption" role="heading"><span class="caption-text">Announcements</span></p> | ||
| <ul> | ||
| <li class="toctree-l1"><a class="reference internal" href="../index.html">Announcements</a></li> | ||
| <li class="toctree-l1"><a class="reference internal" href="../reference/1_modelopt_api.html">API Docs</a></li> | ||
| </ul> | ||
| `; | ||
| }; |
There was a problem hiding this comment.
[SUGGESTION] This replaces the theme-generated sidebar wholesale via innerHTML with two hardcoded relative links. It works today (both ../index.html and ../reference/1_modelopt_api.html resolve correctly from /announcements/*.html), but it's fragile in two ways worth noting:
- The relative paths assume announcement posts live exactly one directory deep. A future post under
announcements/<subdir>/would get a broken../reference/...link. - It discards the RTD theme's built-in search box and version selector on announcement pages, which are present everywhere else in the docs — a small navigation inconsistency for readers who land on a post.
Not blocking. If you want to keep the trimmed sidebar, consider deriving the API-docs href from an existing link in the menu rather than hardcoding the depth, or gating the replacement so it only fires at the expected depth.
| * Xin Cheng et al., `DSpark: Confidence-Scheduled Speculative Decoding with Semi-Autoregressive Generation <https://arxiv.org/abs/2607.05147>`_, | ||
| arXiv:2607.05147, 2026. | ||
| * Jianuo Huang et al., `Domino: Decoupling Causal Modeling from Autoregressive Drafting in Speculative Decoding <https://arxiv.org/abs/2605.29707>`_, | ||
| arXiv:2605.29707, 2026. | ||
|
|
||
| Links | ||
| ***** | ||
|
|
||
| * `DeepSpec / DSpark repo <https://github.com/deepseek-ai/DeepSpec>`_ | ||
| * `DeepSeek-V4-Pro-DSpark checkpoint <https://huggingface.co/deepseek-ai/DeepSeek-V4-Pro-DSpark>`_ | ||
| * `Domino repo <https://github.com/jianuo-huang/Domino>`_ | ||
| * `Domino checkpoint: Qwen3-8B-Domino-b16 <https://huggingface.co/Huang2020/Qwen3-8B-Domino-b16>`_ | ||
| * `ModelOpt PR #1710 <https://github.com/NVIDIA/Model-Optimizer/pull/1710>`_ |
There was a problem hiding this comment.
[SUGGESTION] Since this post publishes to the public NVIDIA.github.io/Model-Optimizer site, it's worth a final manual check that every external reference here resolves before merge: the two arXiv IDs (2607.05147, 2605.29707), the HuggingFace checkpoints (deepseek-ai/DeepSeek-V4-Pro-DSpark, Huang2020/Qwen3-8B-Domino-b16), and the GitHub repos (deepseek-ai/DeepSpec, jianuo-huang/Domino). I couldn't verify these from the review environment. A dead or wrong link on an official NVIDIA announcement is a bad look; a 30-second click-through avoids it.
There was a problem hiding this comment.
Claude review passed — no blocking issues found. LGTM
Scope applied: This is a docs-only PR (10 files, all under docs/). No modelopt/ source, mode registration, config schema, export path, or public API is touched, so the ModelOpt-specific correctness / mode-state / export categories do not apply. I reviewed all reviewable files: the two .rst posts, index.rst, conf.py, and the new announcements.js / announcements.css. The four .png assets were not opened (binary).
Findings by severity: CRITICAL: 0, IMPORTANT: 0, SUGGESTION: 2
What I verified:
conf.pycorrectly registersannouncements.css(alongside existingcustom.css) andannouncements.js; both files exist in_static/.- The hardcoded sidebar links injected by
announcements.js(../index.html,../reference/1_modelopt_api.html) resolve —reference/1_modelopt_api.rstexists. announcements.jsearly-returns on non-announcement pages and guards on!search || cards.length === 0, so it's inert on the rest of the docs.- The
.. raw:: htmlblock inindex.rstis tag-balanced (section/div/navall closed). - Both new posts carry
:orphan:and the toctrees are switched to:hidden:, consistent with the new announcements-first landing layout.
Non-blocking suggestions (posted inline):
- The
innerHTMLsidebar replacement hardcodes one-level-deep relative paths and drops the RTD search box / version selector on announcement pages — fine today, fragile for future nested posts. - Recommend a final manual click-through of the external arXiv / HuggingFace / GitHub links in
dspark-vs-domino.rstbefore merge, since this publishes to the public NVIDIA site.
Risk level: Low. Documentation/static-site change with no impact on library behavior; codecov confirms no source lines changed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/announcements/github-pages-announcements.rst`:
- Line 23: Update the announcement workflow guidance in
github-pages-announcements.rst to require adding the corresponding
.announcement-card entry in docs/source/index.rst, including its title, summary,
date, tags, and HTML link, alongside the Sphinx page and toctree link;
alternatively, document automation that generates this card.
In `@docs/source/index.rst`:
- Around line 12-16: Update the announcement filter buttons and the
tag-selection logic in announcements.js to expose selection through
aria-pressed, initializing “All” as true and the other tags as false. Whenever
the selected tag changes, keep each button’s aria-pressed value synchronized
with its is-active class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 717affd1-3f45-48b2-ad01-25447b388158
⛔ Files ignored due to path filters (4)
docs/source/announcements/assets/domino_fig.pngis excluded by!**/*.pngdocs/source/announcements/assets/dspark_domino_al_qwen3_8b.pngis excluded by!**/*.pngdocs/source/announcements/assets/dspark_fig1.pngis excluded by!**/*.pngdocs/source/announcements/assets/dspark_fig7.pngis excluded by!**/*.png
📒 Files selected for processing (6)
docs/source/_static/announcements.cssdocs/source/_static/announcements.jsdocs/source/announcements/dspark-vs-domino.rstdocs/source/announcements/github-pages-announcements.rstdocs/source/conf.pydocs/source/index.rst
| <div class="announcement-tags" aria-label="Announcement tags"> | ||
| <button class="announcement-tag is-active" type="button" data-tag="all">All</button> | ||
| <button class="announcement-tag" type="button" data-tag="speculative-decoding">Speculative decoding</button> | ||
| <button class="announcement-tag" type="button" data-tag="docs">Docs</button> | ||
| <button class="announcement-tag" type="button" data-tag="github-pages">GitHub Pages</button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose the selected tag state to assistive technology.
The active filter is represented only by the is-active class. Add aria-pressed to each button and update it when the selected tag changes; otherwise screen readers cannot determine which filter is active.
Suggested change
-<button class="announcement-tag is-active" type="button" data-tag="all">All</button>
+<button class="announcement-tag is-active" type="button" data-tag="all" aria-pressed="true">All</button>
-<button class="announcement-tag" type="button" data-tag="docs">Docs</button>
+<button class="announcement-tag" type="button" data-tag="docs" aria-pressed="false">Docs</button>Update the corresponding state in announcements.js alongside the class toggle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/index.rst` around lines 12 - 16, Update the announcement filter
buttons and the tag-selection logic in announcements.js to expose selection
through aria-pressed, initializing “All” as true and the other tags as false.
Whenever the selected tag changes, keep each button’s aria-pressed value
synchronized with its is-active class.
|
/claude review perform a technical blog review |
There was a problem hiding this comment.
Claude review — technical blog review (PR #1971)
Docs-only PR (10 files changed, +444/-4): an announcements landing page for GitHub Pages (RST + CSS + JS), plus two announcement posts — a GitHub-Pages migration note and a DSpark-vs-Domino technical comparison — and image assets. Reviewed per the requested technical blog review: content/citations of the posts, the JS/CSS wiring, and the index.rst/conf.py changes. No modelopt/ or examples/ code touched, so mode/state/export categories are N/A. Image assets and CSS were not deeply audited.
Findings
- CRITICAL: 0
- IMPORTANT: 1
- SUGGESTION: 2
Most impactful
- [IMPORTANT] Date vs. citation contradiction in
dspark-vs-domino.rst. The post is dated June 29, 2026, yet cites DSpark as arXiv:2607.05147 — arXiv'sYYMMscheme puts2607in July 2026, i.e. after the post's own date. On a public NVIDIA blog this internal contradiction is worth fixing before publish. (Domino's2605…= May 2026 is consistent.) - [SUGGESTION] External links unverified. Web access wasn't available in this review, so I could not confirm the two arXiv IDs, the DeepSpec repo, and the two HF checkpoints actually resolve. Please spot-check each before merge — a dead link on the public docs site is worse than none.
- [SUGGESTION] Brittle sidebar override in
announcements.js.trimAnnouncementPostSidebar()replaces the whole.wy-menu-verticalwith a hardcoded two-item list including a literalreference/1_modelopt_api.htmlpath (fragile against glob-renumbering) and drops all other doc-nav sections on announcement pages — confirm that trimmed nav is the intended UX.
Risk assessment
Low risk. Docs/site-only; no library code, no dependency or security-surface changes (CodeRabbit's pre-merge checks passed). The one IMPORTANT item is a content-accuracy issue on a public-facing post, not a functional defect. Once the date/citation is reconciled and the external links are confirmed to resolve, this is good to publish.
| * Xin Cheng et al., `DSpark: Confidence-Scheduled Speculative Decoding with Semi-Autoregressive Generation <https://arxiv.org/abs/2607.05147>`_, | ||
| arXiv:2607.05147, 2026. | ||
| * Jianuo Huang et al., `Domino: Decoupling Causal Modeling from Autoregressive Drafting in Speculative Decoding <https://arxiv.org/abs/2605.29707>`_, | ||
| arXiv:2605.29707, 2026. |
There was a problem hiding this comment.
[IMPORTANT Compatibility] Internal date inconsistency between the post and its citation.
What: The post is dated June 29, 2026 (line 7), but it cites DSpark as arXiv:2607.05147. Under arXiv's YYMM.NNNNN identifier scheme, 2607 decodes to July 2026 — so the post is dated before the paper it references would have been submitted. (Domino's 2605... → May 2026 is consistent with a June post.)
Why it matters: This is a public-facing NVIDIA technical blog. A post that cites a paper dated after its own publication date is an internal contradiction that undercuts the post's credibility, and it's the kind of thing readers notice immediately.
Fix: Reconcile the two — either bump the post :Date: to a date on/after the DSpark submission, or correct the arXiv ID if 2607 is a typo.
Additionally (SUGGESTION): I was unable to verify that the external references resolve (web access wasn't available in this review). Before publishing, please confirm each link actually loads — the two arXiv IDs (2607.05147, 2605.29707), the github.com/deepseek-ai/DeepSpec repo, and the deepseek-ai/DeepSeek-V4-Pro-DSpark / Huang2020/Qwen3-8B-Domino-b16 HF checkpoints. A dead or wrong link on the public docs site is worse than no link.
| menu.innerHTML = ` | ||
| <p class="caption" role="heading"><span class="caption-text">Announcements</span></p> | ||
| <ul> | ||
| <li class="toctree-l1"><a class="reference internal" href="../index.html">Announcements</a></li> | ||
| <li class="toctree-l1"><a class="reference internal" href="../reference/1_modelopt_api.html">API Docs</a></li> | ||
| </ul> | ||
| `; | ||
| }; |
There was a problem hiding this comment.
[SUGGESTION] Hardcoded sidebar replacement is brittle and silently drops the rest of the doc nav.
trimAnnouncementPostSidebar() overwrites the entire .wy-menu-vertical with a two-item list on any /announcements/ page. Two concerns:
- Hardcoded path.
../reference/1_modelopt_api.htmlis coupled to the current filename. Ifreference/1_modelopt_api.rstis ever renamed/renumbered (these files are glob-numbered, so reordering is plausible), this link 404s with no build-time signal. Consider deriving the target from the theme's generated nav instead of a literal string. - Wholesale
innerHTMLreplace discards every other section (Getting Started, Guides, Deployment, Examples, Reference sub-pages) from the sidebar when viewing an announcement post. If that's intentional (a deliberately trimmed reading view) it's fine — but it means announcement pages become a nav dead-end back into the rest of the docs except via the single "API Docs" link. Worth confirming that's the intended UX.
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
| DSpark vs Domino: Same DFlash Backbone, Different Correction Heads | ||
| ################################################################## | ||
|
|
||
| :Author: ModelOpt Team |
There was a problem hiding this comment.
we user ModelOpt Team in some places and Model Optimizer Team in other places. Can you use same everywhere?
2 participants
Summary
api/subpathJira: https://jirasw.nvidia.com/browse/OMNIML-5476
Verification
python3 docs/build_site.py --output docs/build/htmlhttp://127.0.0.1:8088/Publishing approval
User explicitly approved publishing the
dspark-vs-dominosample post and copied image assets frommodelopt-siteto public GitHub inNVIDIA/Model-Optimizer.Notes
uv run nox -s docswas attempted locally, but dependency setup/download did not complete in a reasonable time; CI should provide the authoritative full docs build and PR Pages preview.Summary by CodeRabbit
New Features
Documentation