Skip to content

Conversation

@kmkelle-nv
Copy link
Contributor

@kmkelle-nv kmkelle-nv commented Sep 19, 2025

Overview:

Fixing Sphinx build issues in these categories:

  • index.rst and hidden_toc.rst -- Topics that were not in any table of contents
  • hidden_toc.rst: Topics listed in the hidden_toctree.rst that were renamed or deleted from the project
  • health-check.md: Non-sequential section headings in the health check topic

Details:

Where should the reviewer start?

  • index.rst and hidden_toc.rst. Topics that were not in a table of contents: these are moved to hidden_toctree.rst (if they're already well-integrated with other topics) or index.rst (if they are high-importance topics lacking proper discoverability).
  • hidden_toc.rst. Topics listed in the hidden_toctree.rst that were renamed or deleted from the project: these references were corrected or deleted
  • health-check.md: Non-sequential section headings in the health check topic: Headings now follow their logical levels, that is to say, H2 is followed by H3 instead of H4.

Summary by CodeRabbit

  • Documentation
    • Improved Health Check guide formatting by standardizing example section headers.
    • Expanded navigation with Detailed Installation Guide, Benchmarking Guide, Planner Benchmark Example, Logging, and Health Checks.
    • Added new deployment/runtime references, including Dynamo deployment guides and request cancellation overview.
    • Removed an outdated Kubernetes metrics document and reordered the SLA planner deployment guide for clarity.
    • Updated hidden documentation structure to surface new guides and improve discoverability.
    • No functional changes to the application or APIs.

@kmkelle-nv kmkelle-nv requested a review from rmccorm4 September 19, 2025 22:51
@kmkelle-nv kmkelle-nv self-assigned this Sep 19, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 19, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

Walkthrough

Documentation-only updates: header levels adjusted in health_check.md; hidden_toctree.rst updated to add/remove/reorder guide entries; index.rst updated to include new navigation links for Kubernetes deployment and developer guides. No code or behavior changes.

Changes

Cohort / File(s) Summary
Health check doc formatting
docs/guides/health_check.md
Changed “Example Request” and “Example Response” headers from H4 to H3 in Frontend Liveness and Frontend Health sections. Content unchanged.
Docs navigation and toctree updates
docs/hidden_toctree.rst, docs/index.rst
Updated hidden toctree: added several Dynamo deployment and runtime references, removed guides/dynamo_deploy/k8s_metrics.md, reordered guides/dynamo_deploy/sla_planner_deployment.md. Updated index toctrees: added Detailed Installation Guide, Benchmarking Guide, Planner Benchmark Example, Logging, and Health Checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

I thump my paw at docs aligned,
New trails in toctrees now defined.
Headers hop a level high,
Guides conga in a tidy line.
With whiskered pride I sign—
“All carrots point to ‘Install’—divine!” 🥕✨

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive The PR description provides a useful Overview and a "Where should the reviewer start" section, but the "Details" section is empty and the repository's "Related Issues" template field is missing, so the template is not fully followed. Because required sections are incomplete, the description does not give reviewers the concrete change list or issue linkage needed to validate the fix. The current content is informative but insufficiently specific for a full review. Populate the "Details" section with a concise list of changed files and the exact edits (e.g., headings adjusted in docs/guides/health_check.md; entries added/removed in docs/hidden_toctree.rst and docs/index.rst), add a "Related Issues" line or "None" per the template, and optionally note verification steps (for example, confirming a successful local Sphinx build). After those fields are completed the description will meet the repository template and reviewers will have sufficient context to proceed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly communicates the primary intent—fixing Sphinx errors caused by structural documentation problems—and directly maps to the changes described (index/hidden_toctree updates and health-check heading fixes). It is focused and relevant to the changeset. However it contains a typographical error ("casued" instead of "caused") and could be tightened for clarity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/guides/health_check.md (1)

23-25: Consider adding language specifiers to code blocks.

The curl command blocks should specify 'bash' or 'shell' as the language for better syntax highlighting.

-```
+```bash
 curl -s localhost:8080/live -q | jq

```diff
-```
+```bash
 curl -v localhost:8080/health -q | jq


Also applies to: 46-48

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 406c4d4e874ad1002b3fad9d06caf8ada5706b0e and d05f128638a7bd34f6926d81a60a0393999f038d.

</details>

<details>
<summary>📒 Files selected for processing (3)</summary>

* `docs/guides/health_check.md` (2 hunks)
* `docs/hidden_toctree.rst` (1 hunks)
* `docs/index.rst` (2 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>

<details>
<summary>docs/guides/health_check.md</summary>

23-23: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

46-46: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary>

* GitHub Check: Build and Test - dynamo

</details>

<details>
<summary>🔇 Additional comments (9)</summary><blockquote>

<details>
<summary>docs/guides/health_check.md (4)</summary><blockquote>

`21-21`: **LGTM! Header level fix improves document structure.**

The change from H4 (####) to H3 (###) creates a logical heading hierarchy under the H2 "Frontend Liveness Check" section.

---

`27-27`: **LGTM! Consistent header level with Example Request.**

The change maintains consistency by matching the H3 level used for "Example Request" above.

---

`44-44`: **LGTM! Header level fix maintains consistency.**

The change from H4 to H3 matches the pattern established in the Frontend Liveness section.

---

`50-50`: **LGTM! Completes the consistent header structure.**

All "Example Request/Response" pairs now use H3 consistently throughout the document.

</blockquote></details>
<details>
<summary>docs/index.rst (3)</summary><blockquote>

`54-54`: **LGTM! Adds valuable navigation for detailed installation.**

The "Detailed Installation Guide" entry provides users with a logical progression from the K8s quickstart to more comprehensive installation instructions.

---

`74-77`: **LGTM! Well-organized developer documentation structure.**

The additions create a logical flow in the Developer Guide:
- Benchmarking Guide and Planner Benchmark Example are related and placed together
- Logging and Health Checks provide operational guidance
- All entries are appropriately positioned before the performance tuning section

---

`54-54`: **All referenced docs present — no action required.**
All documentation files referenced in index.rst exist at their specified paths.

</blockquote></details>
<details>
<summary>docs/hidden_toctree.rst (2)</summary><blockquote>

`27-27`: **LGTM! Proper hidden toctree organization.**

The additions and reorganization properly include documentation files that need to be part of the Sphinx project structure without appearing in main navigation:
- API reference and deployment guides (FluxCD, model caching)
- Runtime guides (KVBM with TensorRT-LLM)
- Architecture documentation (request cancellation)
- SLA planner deployment moved to appropriate position




Also applies to: 30-30, 33-33, 36-36, 39-39, 44-44

---

`27-27`: **All newly referenced docs exist; removal check failed**

Script confirmed existence of docs/guides/dynamo_deploy/api_reference.md, docs/guides/dynamo_deploy/fluxcd.md, docs/guides/dynamo_deploy/model_caching_with_fluid.md, docs/guides/dynamo_deploy/sla_planner_deployment.md, docs/guides/run_kvbm_in_trtllm.md, and docs/architecture/request_cancellation.md. The removed-file check for docs/guides/dynamo_deploy/k8s_metrics.md failed with "/bin/bash: -c: line 26: conditional binary operator expected" — confirm the file was intentionally removed or fix the verification script.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@kmkelle-nv kmkelle-nv enabled auto-merge (squash) September 22, 2025 16:15
@rmccorm4
Copy link
Contributor

/ok to test ecd75ab

@rmccorm4
Copy link
Contributor

/ok to test 1868bbc

@kmkelle-nv kmkelle-nv merged commit 5fc0bf9 into main Sep 22, 2025
18 of 19 checks passed
@kmkelle-nv kmkelle-nv deleted the docs-kkelle-fix-sphinx-errors branch September 22, 2025 22:10
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
Signed-off-by: Kristen Kelleher <[email protected]>
Co-authored-by: nv-nmailhot <[email protected]>
Signed-off-by: Jason Zhou <[email protected]>
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
Signed-off-by: Kristen Kelleher <[email protected]>
Co-authored-by: nv-nmailhot <[email protected]>
Signed-off-by: Jason Zhou <[email protected]>
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
Signed-off-by: Kristen Kelleher <[email protected]>
Co-authored-by: nv-nmailhot <[email protected]>
Signed-off-by: Jason Zhou <[email protected]>
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
Signed-off-by: Kristen Kelleher <[email protected]>
Co-authored-by: nv-nmailhot <[email protected]>
Signed-off-by: Jason Zhou <[email protected]>
jasonqinzhou pushed a commit that referenced this pull request Sep 24, 2025
kylehh pushed a commit that referenced this pull request Sep 25, 2025
Signed-off-by: Kristen Kelleher <[email protected]>
Co-authored-by: nv-nmailhot <[email protected]>
Signed-off-by: Kyle H <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants