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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
📝 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, two announcement documents are added, and technical blog review guidance is documented. ChangesAnnouncements hub
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This documentation-only change adds an announcements page while preserving the existing API documentation path; no actionable merge-blocking risk remains beyond normal checks and review. 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
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 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 67.09% 67.09%
=======================================
Files 522 522
Lines 60461 60461
=======================================
Hits 40567 40567
Misses 19894 19894
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:
|
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.
| 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?
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>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
b3adca8 to
485356b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
|
/claude review technical blog only. Read .agents/review-guidelines/technical-blog.md as the review rubric. Review the announcement content and landing-page card. |
| The figure below compares training acceptance length on Qwen3-8B across the DFlash baseline, Domino GRU, and a DSpark implementation. | ||
|
|
||
| .. image:: assets/dspark_domino_al_qwen3_8b.png | ||
| :alt: Training acceptance length on Qwen3-8B: DFlash baseline vs Domino GRU vs DSpark | ||
| :width: 100% |
There was a problem hiding this comment.
[CRITICAL Content] The caption and alt text misdescribe what this figure actually shows.
I opened assets/dspark_domino_al_qwen3_8b.png. It is a two-panel plot titled "Qwen3-8B speculative-decoding A/B — train block-acc + GSM8K AL":
- Left panel:
train block-accuracy(a training metric) - Right panel:
GSM8K acceptance length (z-lab domino_eval)— an evaluation measurement on GSM8K, not a training metric - Four series, not three:
DFlash,Domino,DFlash+DPACE,DSpark(finals 5.72 / 6.52 / 5.96 / 6.34)
Three separate problems:
- "training acceptance length" is wrong. Acceptance length here is measured by z-lab's
domino_evalon GSM8K. Calling a benchmark eval result "training acceptance length" collapses the training-vs-eval distinction the rubric asks to preserve, and hides the fact that a specific benchmark (GSM8K) drives every number a reader will take away. - A whole series is omitted.
DFlash+DPACEappears in both panels but in neither the sentence nor the alt text, so the enumeration is 3-of-4.DPACEis also never expanded anywhere in the post, even though it is public in this repo (examples/speculative_decoding/doc/dflash.md,CHANGELOG.rst) — a reader just sees an unexplained acronym in the legend. - No reproducible setup, and the runs are not length-matched. This is the post's only original measurement, so per the rubric it needs to be a "clearly identified reproducible measurement." Missing: training dataset, block size,
markov_rank, hardware, and the GSM8K eval config. More importantly, the curves terminate at different step counts —DSparkstops near 68k steps whileDomino/DFlashrun to ~92k, and DSpark's curve is still rising where it ends. Quoting "final" numbers from unequal-length runs as a head-to-head is not apples-to-apples and should be disclosed.
Suggested replacement that matches the image and names the setup gap explicitly:
| The figure below compares training acceptance length on Qwen3-8B across the DFlash baseline, Domino GRU, and a DSpark implementation. | |
| .. image:: assets/dspark_domino_al_qwen3_8b.png | |
| :alt: Training acceptance length on Qwen3-8B: DFlash baseline vs Domino GRU vs DSpark | |
| :width: 100% | |
| The figure below is an internal ModelOpt A/B run on Qwen3-8B. The left panel shows | |
| training block-accuracy; the right panel shows acceptance length measured on GSM8K | |
| with z-lab's ``domino_eval``, for the DFlash baseline, DFlash+DPACE, Domino's GRU head, | |
| and ModelOpt's DSpark Markov head. Note that the DSpark run stops at roughly 68k steps | |
| while the others continue to about 92k, so the end-of-run values are not length-matched. | |
| .. image:: assets/dspark_domino_al_qwen3_8b.png | |
| :alt: Qwen3-8B A/B run: training block-accuracy (left) and GSM8K acceptance length | |
| via z-lab domino_eval (right) for DFlash, DFlash+DPACE, Domino GRU, and DSpark | |
| :width: 100% |
Please also add the training/eval configuration (dataset, block size, hardware) either inline or as a link, so the numbers are attributable.
|
|
||
| #. DFlash draft generation is shared; the correction head is the main differentiator. | ||
| #. VanillaMarkov is cheaper per step than GRU, although both are sequential at inference. | ||
| #. GRU is more expressive, but the extra recurrence may not translate into a large acceptance-rate gain. |
There was a problem hiding this comment.
[CRITICAL Content] This takeaway is contradicted by the post's own figure.
The claim is that the GRU's "extra recurrence may not translate into a large acceptance-rate gain." But assets/dspark_domino_al_qwen3_8b.png — the only data in this post — shows the opposite:
| Variant | GSM8K acceptance length | Train block-acc |
|---|---|---|
| DFlash (baseline) | 5.72 | ~0.45 |
| DFlash+DPACE | 5.96 | ~0.44 |
| Domino (GRU) | 6.52 | ~0.68 |
| DSpark (Markov) | 6.34 | ~0.58 |
Domino's GRU posts the largest gain over baseline (+0.80 AL), beats the DSpark Markov head (6.52 vs 6.34), and leads train block-accuracy by a wide margin (~0.68 vs ~0.58). So the recurrence did translate into the biggest acceptance gain measured here.
Why this matters: this is a public NVIDIA post whose stated conclusion is refuted by the chart printed two screens above it. A reader who looks at the figure will not trust the rest of the analysis, and a reader who only reads the takeaways will come away with the wrong ranking. The asymmetry runs through the framing more broadly — the prose consistently favors DSpark ("much lighter," "DSpark's design is broader") while every number presented favors Domino.
Concrete fix — state what was measured and keep the cost/quality tradeoff as the actual point:
| #. GRU is more expressive, but the extra recurrence may not translate into a large acceptance-rate gain. | |
| #. In this run the GRU's added recurrence did deliver the largest acceptance gain | |
| (6.52 AL vs 5.72 for the DFlash baseline and 6.34 for the Markov head), so the | |
| tradeoff is quality per step against cost per step rather than a free win for either. |
If the intent was a cost-normalized claim (gain per unit of correction-head compute), say so explicitly and show the cost measurement — right now no per-step cost numbers appear anywhere in the post.
| .. image:: assets/dspark_fig1.png | ||
| :alt: DSpark overall architecture and decoding cycle | ||
| :width: 100% |
There was a problem hiding this comment.
[CRITICAL Provenance] Three of the four figures are reproduced from third-party arXiv papers with no attribution or license note. This needs resolving before this publishes to NVIDIA's public docs site.
Evidence from the images themselves:
assets/dspark_fig1.png(this directive) still contains the paper's own caption text, cropped mid-sentence at the bottom edge: "Figure 1 | The DSpark architecture and decoding cycle. Given prompt tokens [ABC], the". The filename (_fig1) confirms it is Figure 1 lifted from the DSpark paper.assets/dspark_fig7.png(line 88) is a two-panel throughput/TPS Pareto plot from the same paper, with panel titlesDeepSeek-V4-Flash/DeepSeek-V4-Proand aMTPvsDSparklegend.assets/domino_fig.png(line 53) is the Domino paper's architecture diagram ("Parallel Backbone", "Domino Head", "Causal Encoder",d_i = sample(l_i + c_i)).
Why this matters: the default arXiv submission license grants a non-exclusive license to arXiv to distribute — it does not grant third parties redistribution rights. Unless each paper is explicitly CC-BY (or you have the authors' permission), republishing their figures on NVIDIA.github.io is a rights problem, and even under CC-BY it requires a visible credit line. The PR description records approval to publish the post and to copy assets from modelopt-site, but that is an internal approval and does not speak to the upstream authors' licensing.
Concrete fix, in order of preference:
-
Verify the license for each paper (check the arXiv abstract page's license field). If CC-BY, add an explicit credit under every reproduced figure, e.g.:
.. image:: assets/dspark_fig1.png :alt: DSpark architecture and decoding cycle: parallel block draft generation, sequential correction, hardware-aware prefix scheduler, and target verification :width: 100% *Figure 1 from Cheng et al., arXiv:2607.05147, reproduced under CC-BY 4.0.*
-
If the license does not permit reuse, replace them with original diagrams drawn for this post, or drop the figures and link to the papers instead.
Also, independent of licensing: please re-crop dspark_fig1.png so the truncated caption fragment is not baked into the image — a half-sentence at the bottom of the frame reads as an accident on a published page. And the alt text here ("DSpark overall architecture and decoding cycle") should describe the content for a reader who cannot see it, rather than restating the paper's figure title.
| DSpark includes a hardware-aware prefix scheduler through a confidence head. The scheduler estimates acceptance probability and selects how many draft tokens to submit for verification. It is a serving-time throughput optimization, not a draft-quality feature. | ||
|
|
||
| For MoE models, DSpark checkpoints also include manifold-constrained Hyper-Connections. Dense models use the simpler backbone plus Markov-head path. | ||
|
|
||
| .. image:: assets/dspark_fig7.png | ||
| :alt: DSpark throughput and TPS Pareto frontier | ||
| :width: 100% |
There was a problem hiding this comment.
[IMPORTANT Content] This figure's placement implies a result it does not establish.
assets/dspark_fig7.png sits directly beneath the prefix-scheduler paragraph and the Hyper-Connections sentence, with alt text "DSpark throughput and TPS Pareto frontier." What the image actually shows is an end-to-end DSpark vs MTP comparison on DeepSeek-V4-Flash and DeepSeek-V4-Pro, annotated with +51% throughput, +60% TPS, +661% throughput, +85% TPS (Flash) and +52%, +57%, +406%, +78% (Pro).
Problems with reading this as support for the surrounding text:
- It does not isolate the scheduler or Hyper-Connections. Those headline gains are whole-system DSpark-vs-MTP deltas — backbone, Markov head, scheduler, and (on MoE) Hyper-Connections combined. Placed here, a reader will attribute
+661% throughputto the confidence-head scheduler specifically. Nothing in the figure or text supports that decomposition. - The eye-catching numbers are never mentioned in the prose. Figures up to +661% appear with no baseline definition (what MTP configuration?), no hardware, and no explanation that these are Pareto-frontier comparisons at matched TPS / matched throughput operating points. On a public post, unexplained multi-hundred-percent numbers are exactly the kind of claim the rubric asks to qualify or cite.
- The x-axis label is cropped out of the image entirely. Only the y-axis (
Throughput (token/s/gpu)) is legible; the x-axis values (roughly 20–225) have no label, so the reader cannot tell it is per-user TPS.
Suggested fix — attribute the figure to the paper, state the axes, and scope the claim:
The figure below is from the DSpark paper and compares end-to-end DSpark against MTP
on DeepSeek-V4-Flash and DeepSeek-V4-Pro: per-GPU throughput (y) against per-user
tokens/s (x). The reported gains reflect the full DSpark system, not the prefix
scheduler in isolation.
.. image:: assets/dspark_fig7.png
:alt: Throughput versus per-user tokens/s Pareto frontiers for MTP and DSpark on
DeepSeek-V4-Flash and DeepSeek-V4-Pro
:width: 100%(See my comment on line 27 regarding attribution/licensing for this image.) Also worth noting for precision: ModelOpt's DSpark confidence head is a training-time loss term that defaults to off — dflash_confidence_head_alpha has default=0.0 in modelopt/torch/speculative/config.py:222 and requires use_confidence_head=true. Saying DSpark "includes a hardware-aware prefix scheduler through a confidence head" describes the paper's serving design; it does not describe what ModelOpt ships today. Please make that boundary explicit so readers do not expect scheduler behavior from a ModelOpt DSpark checkpoint.
|
|
||
| DSpark includes a hardware-aware prefix scheduler through a confidence head. The scheduler estimates acceptance probability and selects how many draft tokens to submit for verification. It is a serving-time throughput optimization, not a draft-quality feature. | ||
|
|
||
| For MoE models, DSpark checkpoints also include manifold-constrained Hyper-Connections. Dense models use the simpler backbone plus Markov-head path. |
There was a problem hiding this comment.
[IMPORTANT Content] Unsupported architectural claim, and it conflicts with what ModelOpt actually implements.
"For MoE models, DSpark checkpoints also include manifold-constrained Hyper-Connections" is stated as fact with no citation to a specific paper section, and "manifold-constrained Hyper-Connections" is unusual enough terminology that a reader cannot map it to anything without a pointer.
More concretely, it is unverifiable against this repo: grep -ri "hyper.connection" modelopt/ returns nothing. ModelOpt's DSpark module docstring (modelopt/torch/speculative/plugins/modeling_dspark.py:50-60) enumerates exactly three head variants — vanilla, gated, rnn — and describes the module as "DFlash backbone plus a lightweight sequential (Markov) head" with no MoE-specific structure at all.
Why this matters: the sentence sits in a post published under the ModelOpt docs, immediately after a paragraph about ModelOpt's confidence_head. A reader will reasonably infer that ModelOpt's DSpark path supports MoE Hyper-Connections. It does not. The second half of the sentence ("Dense models use the simpler backbone plus Markov-head path") reinforces that misreading by implying the two paths are alternatives within this implementation.
Concrete fix — cite the source and mark the ModelOpt scope:
| For MoE models, DSpark checkpoints also include manifold-constrained Hyper-Connections. Dense models use the simpler backbone plus Markov-head path. | |
| The DSpark paper additionally describes manifold-constrained Hyper-Connections for MoE | |
| checkpoints (see :ref:`dspark-domino-references`); ModelOpt's DSpark implementation | |
| currently covers the DFlash backbone plus the Markov head only. |
Adjust the wording to match whatever the paper actually says — the point is that the claim needs a citation and an explicit "not in ModelOpt" boundary.
| <div class="announcement-tags" aria-label="Announcement tags"> | ||
| <button class="announcement-tag is-active" type="button" data-tag="all" aria-pressed="true">All</button> | ||
| <button class="announcement-tag" type="button" data-tag="speculative-decoding" aria-pressed="false">Speculative decoding</button> | ||
| <button class="announcement-tag" type="button" data-tag="docs" aria-pressed="false">Docs</button> | ||
| <button class="announcement-tag" type="button" data-tag="github-pages" aria-pressed="false">GitHub Pages</button> |
There was a problem hiding this comment.
[IMPORTANT Content] The card metadata and the tag filters disagree with each other and with the posts, so most tags are undiscoverable.
Comparing the three places tags are declared:
| Source | Tags |
|---|---|
dspark-vs-domino.rst:8 (:Tags:) |
speculative-decoding, dflash, dspark, domino, architecture |
index.rst:27 (data-tags) |
speculative-decoding, dflash, dspark, domino, architecture |
index.rst:31 (visible chips) |
speculative-decoding, dflash, architecture — dspark and domino dropped |
index.rst:13-16 (filter buttons) |
all, speculative-decoding, docs, github-pages |
Two concrete gaps:
- The visible chips on the DSpark card (line 31) omit
dsparkanddominoeven though both are in the post's:Tags:and indata-tags. The rubric asks the card metadata and the post's own tags to agree; here the reader sees a different tag set than either. - Five of the seven declared tags have no filter button:
dflash,dspark,domino,architecture, andreleaseare all filterable indata-tagsbut unreachable from the toolbar. The companion post claims announcement pages "support tags, search, filtering" (github-pages-announcements.rst:17), andreleasein particular is the tag a reader is most likely to want on a project like this.
Because the chips are rendered as plain <span>s rather than buttons, a reader who sees dflash on a card has no way to filter by it — the feature reads as broken rather than partial.
Suggested minimal fix — cover the declared tags in the toolbar:
| <div class="announcement-tags" aria-label="Announcement tags"> | |
| <button class="announcement-tag is-active" type="button" data-tag="all" aria-pressed="true">All</button> | |
| <button class="announcement-tag" type="button" data-tag="speculative-decoding" aria-pressed="false">Speculative decoding</button> | |
| <button class="announcement-tag" type="button" data-tag="docs" aria-pressed="false">Docs</button> | |
| <button class="announcement-tag" type="button" data-tag="github-pages" aria-pressed="false">GitHub Pages</button> | |
| <div class="announcement-tags"> | |
| <button class="announcement-tag is-active" type="button" aria-pressed="true">All</button> | |
| <button class="announcement-tag" type="button" aria-pressed="false">Release</button> | |
| <button class="announcement-tag" type="button" aria-pressed="false">Speculative decoding</button> | |
| <button class="announcement-tag" type="button" aria-pressed="false">Architecture</button> | |
| <button class="announcement-tag" type="button" aria-pressed="false">Docs</button> | |
| <button class="announcement-tag" type="button" aria-pressed="false">GitHub Pages</button> | |
| </div> |
and separately bring line 31's chips back in line with data-tags. Longer term, since the authoring flow in github-pages-announcements.rst requires hand-maintaining data-tags, the visible chips, and the filter list per post, these will keep drifting — consider deriving the toolbar from the union of card data-tags in announcements.js so there is one source of truth.
| #. DFlash draft generation is shared; the correction head is the main differentiator. | ||
| #. VanillaMarkov is cheaper per step than GRU, although both are sequential at inference. | ||
| #. GRU is more expressive, but the extra recurrence may not translate into a large acceptance-rate gain. | ||
| #. DSpark's design is broader: VanillaMarkov, GatedMarkov, and RNN-style heads all fit the same family. |
There was a problem hiding this comment.
[SUGGESTION] The head names in this post don't match the names a reader would need to actually use them.
"VanillaMarkov" (used 5×, including the comparison table at line 72) and "GatedMarkov" appear nowhere in ModelOpt — grep -ri "vanillamarkov\|gatedmarkov" . is empty. The real knobs, per modelopt/torch/speculative/plugins/modeling_dspark.py:50-60 and modelopt/torch/speculative/config.py, are:
projector_type="dspark"selects the DSpark pathmarkov_head_type∈{"vanilla", "gated", "rnn"}, defaulting to"vanilla"markov_rankis required (the head's low-rank dimension)
Since this post is published under the ModelOpt docs and is largely about a ModelOpt feature, a reader who wants to reproduce the comparison will search for "VanillaMarkov" and find nothing. Worth using the configuration names, e.g.:
| #. DSpark's design is broader: VanillaMarkov, GatedMarkov, and RNN-style heads all fit the same family. | |
| #. DSpark's design is broader: the same family covers the ``vanilla``, ``gated``, and | |
| ``rnn`` values of ``markov_head_type`` (``projector_type="dspark"``), where ``rnn`` is | |
| itself a recurrent head closely analogous to Domino's GRU. |
Two smaller precision notes in the same area:
- The
rnnvariant is described in the source as "a GRU-like recurrent head carrying a states_kacross positions … (closest analogue to Domino's GRU)". That is a genuinely interesting point for this post — DSpark and Domino converge when DSpark is configured withrnn— and it's currently buried as "RNN-style heads." - The "Correction Head Overhead" heading (line 63) promises overhead but the table is entirely qualitative ("Full GRU cell over a high-dimensional input", "much lighter" at line 79). Either add measured per-step cost or retitle to something like "Correction Head Comparison" so the heading doesn't promise numbers the section doesn't have.
| :Date: July 13, 2026 | ||
| :Tags: speculative-decoding, dflash, dspark, domino, architecture | ||
|
|
||
| DSpark (DeepSpec) and Domino both build on block-parallel DFlash draft generation but diverge sharply in their token-level correction heads. DSpark uses a stateless VanillaMarkov head that is fast and parallelizable during training; Domino uses a GRU that is more expressive but sequential at inference. |
There was a problem hiding this comment.
[SUGGESTION] The lede's parallel construction implies a distinction the post then retracts.
"DSpark uses a stateless VanillaMarkov head that is fast and parallelizable during training; Domino uses a GRU that is more expressive but sequential at inference." The two clauses are set up as a contrast but describe different axes — one training, one inference. The natural reading is "DSpark parallel, Domino sequential," which the third highlight (line 20) immediately contradicts: "Both correction heads are sequential at inference because x_{k-1} must be sampled before step k."
Since training-vs-serving is precisely the distinction this post is trying to teach, it's worth getting right in the first sentence:
| DSpark (DeepSpec) and Domino both build on block-parallel DFlash draft generation but diverge sharply in their token-level correction heads. DSpark uses a stateless VanillaMarkov head that is fast and parallelizable during training; Domino uses a GRU that is more expressive but sequential at inference. | |
| DSpark (DeepSpec) and Domino both build on block-parallel DFlash draft generation but diverge sharply in their token-level correction heads. DSpark's default head is a stateless first-order Markov transition; Domino's is a GRU that conditions on the whole draft prefix. Both must unroll sequentially at inference, so the tradeoff is per-step cost against how much prefix context the correction can see — DSpark's head additionally parallelizes over positions during teacher-forced training, where Domino's recurrence does not. |
Related, one level down: it would help readers to note that ModelOpt's Domino support is training-only today — PR #1710 (linked at line 120) states "the inference side (vLLM / AR evaluation) is intentionally not wired up yet — the correction head is not applied in serving." The post discusses inference cost at length without mentioning that neither correction head currently runs in ModelOpt's serving path.
| Links | ||
| ***** | ||
|
|
||
| These public references were checked during review. | ||
|
|
||
| * `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] "These public references were checked during review" is review-process metadata that shouldn't ship to readers, and I could not independently confirm the links.
Two things:
-
Drop the sentence at line 114. It addresses reviewers, not readers, and it makes an assurance on the published page that nobody can act on or re-verify. It also dates badly — links rot, and the sentence will still claim they were checked. Consider merging this section into References above (the split between "References" and "Links" is itself unclear) and dropping the line:
Suggested changeLinks ***** These public references were checked during review. * `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>`_ Resources ********* * `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>`_ -
Link verification status. Network access was unavailable in this review environment, so I could not resolve any of these URLs. What I could corroborate from inside the repo:
- ✅
github.com/deepseek-ai/DeepSpec— corroborated:modelopt/torch/speculative/plugins/modeling_dspark.py:1citeshttps://github.com/deepseek-ai/DeepSpec/blob/add63ba/deepspec/modeling/dspark/markov_head.pywith a pinned commit. - ✅
huggingface.co/Huang2020/Qwen3-8B-Domino-b16— corroborated: PR [Feat]: Domino support #1710's description names this exact drafter-format checkpoint. - ✅
ModelOpt PR #1710— verified: "[Feat]: Domino support" byh-guo18, MERGED 2026-06-13, and its content matches the surrounding claims. - ❓ Both arXiv IDs,
deepseek-ai/DeepSeek-V4-Pro-DSpark, andjianuo-huang/Domino— unverified. Please resolve each before merge.
On dates:
arXiv:2607.05147(July 2026) is now consistent with the post's July 13, 2026 date — the contradiction raised in the earlier review round is resolved by the date change. One thing to double-check isarXiv:2605.29707: a sequence number near 29.7k is high for a single month's submissions, so it's worth confirming that ID is exact rather than transposed. - ✅
There was a problem hiding this comment.
Claude review — technical blog (PR #1971)
Scope applied, per the trigger comment: technical blog only, using .agents/review-guidelines/technical-blog.md as the rubric — the two announcement posts plus their landing-page cards. I did not review announcements.js, announcements.css, or conf.py as implementation (out of the requested scope), except where the landing-page card metadata depends on them. Per the rubric's "Review scope" clause, I did read modelopt/torch/speculative/ as evidence for claims made in the post.
I also opened all four PNG assets, since figure provenance and caption accuracy (rubric checks #1 and #4) can't be assessed from the diff.
Findings
- CRITICAL: 3
- IMPORTANT: 3
- SUGGESTION: 3
Most impactful
1. The post's central takeaway is refuted by the post's own figure. (line 97) Takeaway #3 says the GRU's "extra recurrence may not translate into a large acceptance-rate gain." The chart in dspark_domino_al_qwen3_8b.png shows Domino's GRU at 6.52 GSM8K acceptance length vs 5.72 for the DFlash baseline and 6.34 for DSpark's Markov head — the largest gain of any variant, and ahead of DSpark. Train block-accuracy tells the same story (~0.68 vs ~0.58). The prose leans DSpark-favorable throughout while every number presented favors Domino.
2. That figure is mislabeled in both caption and alt text. (lines 57–61) It is described as "training acceptance length … across the DFlash baseline, Domino GRU, and a DSpark implementation." It is actually a two-panel plot — train block-accuracy on the left and GSM8K acceptance length (z-lab domino_eval) on the right, an eval metric on a specific benchmark — with four series, not three (DFlash+DPACE is omitted from the text, and DPACE is never expanded). The runs are also not length-matched: DSpark terminates near 68k steps while the others run to ~92k, with DSpark's curve still rising. No dataset, block size, markov_rank, or hardware is given for the post's only original measurement.
3. Three of four figures are reproduced from third-party arXiv papers with no attribution or license note. (line 27) dspark_fig1.png still has the paper's own caption baked in, cropped mid-sentence ("Figure 1 | The DSpark architecture and decoding cycle. Given prompt tokens [ABC], the"); dspark_fig7.png is the paper's DeepSeek-V4 throughput panels; domino_fig.png is the Domino architecture diagram. The default arXiv license does not grant third parties redistribution rights, so this needs a license check plus visible credit lines — or original diagrams — before it publishes to NVIDIA.github.io. The PR's internal publishing approval doesn't speak to the upstream authors' licensing.
Also flagged: dspark_fig7.png is placed so its +51%…+661% end-to-end DSpark-vs-MTP gains read as evidence for the prefix-scheduler and Hyper-Connections paragraph it sits under, which it does not isolate; the "manifold-constrained Hyper-Connections" claim has no citation and no counterpart anywhere in modelopt/ (a reader will assume ModelOpt's DSpark path supports it); and the landing-page card tags disagree three ways — five of seven declared tags have no filter button, and the DSpark card's visible chips drop dspark and domino.
Verified clean / prior rounds resolved
- The date-vs-citation contradiction from the earlier review round is resolved. The post is now dated July 13, 2026 and
arXiv:2607.05147is a July 2026 ID, so the citation no longer postdates the post. (Worth double-checkingarXiv:2605.29707— a ~29.7k sequence number is high for one month.) - CodeRabbit's earlier authoring-flow finding is resolved —
github-pages-announcements.rst:23now explicitly instructs authors to add the.announcement-cardmetadata toindex.rst. - Three links corroborated from inside the repo: the DeepSpec repo (cited with pinned commit
add63baatmodeling_dspark.py:1), theHuang2020/Qwen3-8B-Domino-b16checkpoint (named in PR #1710), and PR #1710 itself (verified: "[Feat]: Domino support", MERGED 2026-06-13, content matches the surrounding claims). Network access was unavailable here, so the two arXiv IDs,deepseek-ai/DeepSeek-V4-Pro-DSpark, andjianuo-huang/Dominoremain unverified — please resolve each before merge. - The DSpark Markov formula in the post (
e = W1[x],bias = W2·e,p = softmax(U + bias)) matches the implementation atmodeling_dspark.py:52-54, and "DSpark defaults to VanillaMarkov" matches the"vanilla"default inhf_spec_export.py:531. The Domino formula matchesdomino_fig.pngand PR #1710. The technical substance is sound — the problems are in the framing, the figure captions, and provenance. - No confidentiality exposure found.
DPACE,DSpark, andDominoare all already public in this repo; no internal URLs, credentials, or unreleased-product references in either post.
Risk assessment
Medium for publication, negligible for the library. No modelopt/ or examples/ code changes, so there is no runtime, mode/state, export, or backward-compatibility risk — codecov confirms zero source lines touched. The risk is entirely reputational and legal on a public NVIDIA-branded page: a stated conclusion contradicted by its own chart, an eval result labeled as a training metric, and three unattributed third-party paper figures. None is hard to fix, but I'd want the figure attribution/licensing question settled and the takeaway reconciled with the data before this goes live.
Posting as a comment review rather than an approval on the strength of findings 1–3.
There was a problem hiding this comment.
Does this file need to be linked in top-level AGENTS.md or its automatically picked up?
There was a problem hiding this comment.
You have to mention explicitly. I will add some simple instructions for folks to trigger the technical blog review.
| ######################################################### | ||
|
|
||
| :Author: Model Optimizer Team | ||
| :Date: July 13, 2026 |
There was a problem hiding this comment.
This needs to be today's date right?
There was a problem hiding this comment.
Do we want to also require a ModelOpt commit hash / version and container used in case of tutorial blogs / experimental results?
There was a problem hiding this comment.
I think that will be a next step.
kevalmorabia97
left a comment
kevalmorabia97
left a comment
There was a problem hiding this comment.
Left some non-blocking comments and approved
|
/ok to test 7713d5f |
Signed-off-by: Chenhan Yu <chenhany@nvidia.com>
|
/ok to test 238ad0c |
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