Skip to content

Formatting changes#838

Open
RulaHallak wants to merge 9 commits intoNVIDIA:mainfrom
RulaHallak:rula-changes
Open

Formatting changes#838
RulaHallak wants to merge 9 commits intoNVIDIA:mainfrom
RulaHallak:rula-changes

Conversation

@RulaHallak
Copy link
Contributor

Summary

I did some formatting changes.

Test Plan

  • Build it locally and reviewed the changes

Additional Notes

N/A

I did some formatting changes
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR performs a broad documentation restructuring across the doc/ directory: the tutorial steps and troubleshooting content are extracted from USER_GUIDE.rst into their own dedicated files (Tutorial.rst and troubleshooting.rst), the root-level workloads_requirements_installation.rst is moved under doc/workloads/, the index.rst is overhauled to include an Update History table and a restructured toctree, and numerous headings are renamed/capitalized consistently throughout.

Key changes:

  • New files: Tutorial.rst, troubleshooting.rst, workloads/workloads_requirements_installation.rst
  • Deleted: root-level workloads_requirements_installation.rst
  • Cross-references updated: all :doc: and :ref: links in index.rst and USER_GUIDE.rst updated to point to the new paths
  • Structural improvements: USER_GUIDE.rst Field Descriptions converted from a plain bullet list to a proper list-table, step sections now use proper :ref: anchors with navigable sub-TOCs, and reporting.rst section anchors made consistent with their new heading text
  • Minor issues found: an unfilled placeholder date (April xx, 2026) in the Update History table, a capitalization regression (Forfor in a mid-sentence parenthetical in index.rst), and a spurious comma in vllm.rst

Confidence Score: 4/5

  • Safe to merge after fixing the placeholder date and minor grammar regressions; all cross-references appear consistent.
  • This is a documentation-only PR with no code changes. The restructuring is well-executed with proper RST :ref: anchors, consistent toctree updates, and resolved prior review comments. The deductions are for: (1) an unfilled placeholder date in the Update History table that would be visible to users, (2) a small capitalization regression introduced in index.rst, and (3) a spurious comma in vllm.rst.
  • doc/index.rst (placeholder date at line 10, capitalization regression at line 56) and doc/workloads/vllm.rst (spurious comma at line 95).

Important Files Changed

Filename Overview
doc/index.rst Major restructuring: adds Update History table (with unfilled placeholder date "April xx, 2026"), adds Overview section, restructures toctree to use :hidden: with explicit bullet list, renames/reorganizes headings, and updates cross-references to point to the moved workloads/workloads_requirements_installation. Also introduces a capitalization regression in a parenthetical (lowercase "for" → uppercase "For").
doc/Tutorial.rst New file: extracts the step-by-step tutorial (Steps 1–7 plus Test-in-Scenario) from USER_GUIDE.rst into its own document. Uses proper RST :ref: cross-references for the TOC, and section headings with underlines throughout. Clean and structurally sound.
doc/USER_GUIDE.rst Heavy restructuring: removes Steps 1–7 (moved to Tutorial.rst), renames/shortens several section headings (e.g. "Describing a System in the System Schema" → "System Schema"), converts the Field Descriptions bullet list to a proper RST list-table, adds :ref: anchors with sub-TOC lists for multi-step subsections, and capitalizes sub-heading words consistently.
doc/troubleshooting.rst New file: extracts the Troubleshooting section from USER_GUIDE.rst into its own document. Content is identical to the original, minor wording polish only. Structurally clean.
doc/workloads/workloads_requirements_installation.rst New file (replaces root-level workloads_requirements_installation.rst): moves the installation requirements doc under workloads/. Removes the emoji from the "Hugging Face Models" heading and reorganizes the enroot credentials step inline, resolving the previous structural inconsistency flag.
doc/workloads_requirements_installation.rst Deleted: content moved to doc/workloads/workloads_requirements_installation.rst. All cross-references in index.rst and USER_GUIDE.rst have been updated accordingly.
doc/reporting.rst Adds a TOC with proper :ref: links, renames sections (e.g. "Enable, Disable and Configure Reports" → "Enabling, Disabling and Configuring Reports"), updates internal :ref: usage consistently, and converts numbered list items to bullet points.
doc/workloads/vllm.rst Heading capitalization updates, splits a long sentence about GPU split into a bullet list, but introduces a spurious comma in "The config above, will automatically split…".
doc/DEV.rst Minor wording changes: "document" → "chapter", paragraph break inserted in the Cache section, "system install_path" → "system's install_path" (fixing the previously flagged awkward phrasing), and "So" → "Therefore".
doc/workloads/ai_dynamo.rst Minor heading capitalization updates and small wording changes (e.g., passive voice improvements).
doc/workloads/index.rst Title shortened from "Workloads Documentation" to "Workloads", and "section" → "chapter" for consistency with restructured docs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[doc/index.rst\nEntry point + toctree] --> B[Tutorial.rst\n🆕 Steps 1–7 + Test-in-Scenario]
    A --> C[USER_GUIDE.rst\nSystem/Scenario schemas\nAgents, HTTP repo, Slurm, Nsys]
    A --> D[reporting.rst\nReporting system]
    A --> E[systems.rst\nSupported systems]
    A --> F[workloads/index.rst\nWorkloads overview]
    A --> G[DEV.rst\nDeveloper guide]
    A --> H[troubleshooting.rst\n🆕 Extracted from USER_GUIDE]
    F --> I[workloads/ai_dynamo.rst]
    F --> J[workloads/vllm.rst]
    F --> K[workloads/... other workloads]
    F --> L[workloads/workloads_requirements_installation.rst\n🆕 Moved from doc/ root]
    M[doc/workloads_requirements_installation.rst\n🗑️ Deleted] -.->|replaced by| L
Loading

Last reviewed commit: "Formatting changes"

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Wide-ranging documentation updates: added Tutorial and Troubleshooting pages; reorganized and refocused USER_GUIDE and reporting docs; refined DEV and index pages; introduced workloads installation/requirements docs; and applied many heading-capitalization and wording edits across workloads and systems documentation. (All changes are documentation-only.)

Changes

Cohort / File(s) Summary
New tutorial & troubleshooting
doc/Tutorial.rst, doc/troubleshooting.rst
Added a Tutorial with end-to-end usage steps and a Troubleshooting workflow page with debugging and repro guidance.
User guide & index rework
doc/USER_GUIDE.rst, doc/index.rst
Reorganized USER_GUIDE into schema-focused chapters, added anchors and list-table formatting; index updated with Overview, Update History, toctree and CLI/usage reorganizations.
Reporting & developer docs
doc/reporting.rst, doc/DEV.rst
Reporting restructured with new section anchors and renamed refs; DEV wording and small phrasing edits.
Workloads docs (many files)
doc/workloads/..., doc/workloads/index.rst, doc/workloads/*.{rst}
Numerous editorial edits: replaced generic "This workload" with " workload", adjusted heading capitalization, clarified sentences—no config or behavior changes.
Workloads installation / requirements
doc/workloads_requirements_installation.rst, doc/workloads/workloads_requirements_installation.rst
Added installation requirements doc: clarified installables (images, git repos, HF models), NGC credential steps, HF auth example (uv run hf auth login), HF cache/mount behavior.
Systems & small edits
doc/systems.rst, doc/workloads/ai_dynamo.rst, doc/workloads/vllm.rst, other doc/...
Minor edits: "section"→"chapter", capitalization and phrasing tweaks across multiple docs; no code changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through docs, nibbling headers bright,

I straightened anchors, made the index light;
Paved a tutorial path and left a debugging clue,
Tidied workloads, one sentence or two;
A rabbit's small hop — docs polished, snug, and new.

🚥 Pre-merge checks | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Formatting changes' is too vague and generic; it does not convey meaningful information about the actual changeset, which involves substantial restructuring of multiple documentation files across the project. Provide a more specific title that captures the primary documentation updates, such as 'Restructure documentation with new tutorial, troubleshooting guide, and reorganized user guide' or similar.
Description check ❓ Inconclusive The description mentions 'formatting changes' but the changeset involves significant documentation restructuring, new pages, section reorganization, and content updates—far beyond simple formatting. Expand the description to accurately reflect the scope of changes, including new tutorial/troubleshooting pages, user guide reorganization, and documentation structure updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
doc/workloads/ai_dynamo.rst (1)

7-87: ⚠️ Potential issue | 🟡 Minor

Consider reverting awkward grammar change.

While most heading capitalization improvements are good, Line 54 introduces awkward phrasing: "The total number of the required nodes" should remain "The total number of required nodes" (without the extra "the").

Additionally, Line 75 changes to passive voice ("The job progress monitoring can be done") which is less direct than the original active voice ("One can monitor job progress"). Consider keeping active voice for clarity.

📝 Suggested improvements
-The total number of the required nodes must be:
+The total number of required nodes must be:
-The job progress monitoring can be done using either of the following options:
+Job progress can be monitored using either of the following options:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/workloads/ai_dynamo.rst` around lines 7 - 87, Update the two awkward
phrasing changes in the ai_dynamo.rst content: replace "The total number of the
required nodes" with "The total number of required nodes" (remove the extra
"the") and change the passive sentence "The job progress monitoring can be done
using either of the following options:" back to an active form such as "One can
monitor job progress using either of the following options:" so the headings and
instructions read naturally; locate and edit the exact phrases quoted above in
the doc to apply these replacements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/DEV.rst`:
- Around line 42-46: The fragment around "For example:" in doc/DEV.rst is
incomplete; merge/rewrite the sentences so the example completes the
thought—e.g., change the three lines into a single clear sentence like "Some
prerequisites can be installed; for example, Docker images and git repos with
executable scripts." and then keep the follow-up sentence "All such
'installables' are kept under ``install_path`` system." Locate the text
containing the quoted phrases "For example:" and "All such \"installables\" are
kept under ``install_path`` system." and update them accordingly.

In `@doc/USER_GUIDE.rst`:
- Line 215: Replace the passive sentence "It is possible to override some args
or even fully define a workload inside a scenario file:" with an active-voice
variant that directly addresses the reader or the agent performing the action,
for example starting with "You can override..." or "Override some args or even
fully define a workload inside a scenario file:"; locate the exact sentence (the
string "It is possible to override some args or even fully define a workload
inside a scenario file:") in USER_GUIDE.rst and update it to one of those active
phrasings to improve clarity and readability.
- Line 256: Replace the passive sentence "Once the test scenario is completed,
it is possible to generate reports using the following command:" with an
active-voice construction; e.g., rephrase to "Once you complete the test
scenario, generate reports using the following command:" so the instruction
reads more direct and clear in USER_GUIDE.rst.

---

Outside diff comments:
In `@doc/workloads/ai_dynamo.rst`:
- Around line 7-87: Update the two awkward phrasing changes in the ai_dynamo.rst
content: replace "The total number of the required nodes" with "The total number
of required nodes" (remove the extra "the") and change the passive sentence "The
job progress monitoring can be done using either of the following options:" back
to an active form such as "One can monitor job progress using either of the
following options:" so the headings and instructions read naturally; locate and
edit the exact phrases quoted above in the doc to apply these replacements.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 133e7610-b2c0-407f-98bf-81b3a3b7c919

📥 Commits

Reviewing files that changed from the base of the PR and between 61e16c7 and aba86ae.

📒 Files selected for processing (5)
  • doc/DEV.rst
  • doc/USER_GUIDE.rst
  • doc/reporting.rst
  • doc/workloads/ai_dynamo.rst
  • doc/workloads_requirements_installation.rst

@RulaHallak RulaHallak marked this pull request as draft March 17, 2026 10:21
@RulaHallak RulaHallak marked this pull request as ready for review March 19, 2026 13:42
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: 13

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/index.rst`:
- Line 10: Replace the placeholder date "April xx, 2026" in the release history
row for "v1.5.0" with the actual release date; locate the table entry containing
the string "v1.5.0" and update the middle column from "April xx, 2026" to the
correct date (e.g., "April 12, 2026") so the update history shows a concrete
release date.

In `@doc/troubleshooting.rst`:
- Around line 29-31: Update the troubleshooting instructions that currently
instruct users to rerun tests via an "sbatch" command so they are scoped by
scheduler/backend: replace the three bullets referencing "sbatch" with
conditional guidance such as "For Slurm runs: inspect debug.log for the sbatch
command and rerun the generated sbatch script; For Kubernetes runs: inspect the
submitted Job/Pod spec and controller logs or use kubectl to view and re-run the
job; For standalone/local runs: inspect the command executed in debug.log and
re-run it locally." Specifically edit the bullets that mention "sbatch" (the
phrases "sbatch" and "sbatch script") in doc/troubleshooting.rst to include
separate, brief steps per backend (Slurm, Kubernetes, standalone).

In `@doc/Tutorial.rst`:
- Line 146: Replace the circular sentence "Test configuration describes a
particular test configuration to be run" with a clearer definition: say that a
Test configuration specifies the settings used to run a test (based on a test
definition) and will be used in a test scenario — e.g., "A Test configuration
specifies the settings used to run a test; it is based on a test definition and
is used within a test scenario." Update the sentence in the Tutorial.rst section
that introduces CONFIGS_DIR/tests/nccl_test.toml and the built-in NcclTest so it
uses this non-circular phrasing.
- Line 170: The phrase "test description" in the sentence "Test Scenario uses
test description from Step 5. Below is the ``CONFIGS_DIR/scenario.toml`` file:"
should be replaced with the project's standard term "test configuration" to keep
documentation consistent; update that sentence in Tutorial.rst (the line
referencing CONFIGS_DIR/scenario.toml / "Test Scenario uses ... from Step 5") so
it reads "Test Scenario uses test configuration from Step 5." (or equivalent
phrasing using "test configuration") throughout the document.
- Line 120: Replace the circular sentence "System configuration describes how
the system configuration works" with a concise explanatory sentence such as
"This section explains how to configure the system and provides an example
configuration file" and ensure the paragraph then points readers to the example
location under conf/common/system/ and the sample CONFIGS_DIR/system.toml file;
update the sentence in the Tutorial.rst content so it clearly states purpose and
references the example paths.
- Line 4: The opening sentence uses the misleading phrase "create CloudAI
framework"; update it to clearly state the tutorial objective by replacing that
phrase with something like "build and run applications using the CloudAI
framework" or "get started with the CloudAI framework" so readers understand
they will learn to use the framework rather than create it—locate the exact text
"create CloudAI framework" and revise it to a clearer description of the
tutorial goal.
- Line 20: The sentence "To create a Docker image, follow the following steps:"
uses redundant wording; update it (in Tutorial.rst) to a concise form such as
"To create a Docker image, follow these steps:" or "To create a Docker image,
follow the steps:" by replacing "follow the following steps" with one of the
suggested alternatives to remove redundancy.
- Line 163: Replace the inconsistent phrase "test schema file" with the
project's standard term "test configuration" in the sentence that starts "You
can find more examples under ``conf/common/test``" (the phrase "test schema
file" is the unique text to change); keep the rest of the sentence intact so
"cmd_args", "extra_cmd_args" and "extra_env_vars" remain unchanged, ensuring the
doc uses "test configuration" consistently with the rest of the document.

In `@doc/USER_GUIDE.rst`:
- Line 448: Replace the sentence-fragment lead-ins "Will result in sbatch file
content like this:" (and the similar instance at the other location) with a
complete-sentence form such as "This will result in the sbatch file containing
the following:" (or "The command will result in the sbatch file containing the
following:") so the subject is explicit and the prose is grammatically complete.
- Line 353: Replace the awkward phrase "sub-items of Slurm-related topics" in
the USER_GUIDE section intro with clearer wording; locate the exact sentence
containing "sub-items of Slurm-related topics" and change it to "subtopics
related to Slurm." Ensure surrounding punctuation and capitalization remain
consistent with the document style.

In `@doc/workloads/ai_dynamo.rst`:
- Line 54: Reword two awkward user-facing sentences to improve grammar and
readability: replace "The total number of the required nodes must be:" with "The
total number of required nodes is:" and similarly simplify the other awkward
sentence (the similar user-facing sentence later in the doc) to "Ensure the
total number of nodes meets the requirement:" so both read clearly and
concisely.

In `@doc/workloads/workloads_requirements_installation.rst`:
- Line 4: The sentence "CloudAI workloads can define multiple installables as
prerequisites. It can be a container image, git repository, HF model, etc." has
a singular/plural mismatch; change the second sentence to use plural agreement
(e.g., "They can be a container image, git repository, HF model, etc." or
rephrase to "These can be...") so "installables" and the following clause agree;
update the sentence in the same paragraph where that text appears.
- Line 10: The phrase "the Docker repository" is ambiguous; update the sentence
"First, make sure you have access to the Docker repository. Proceed as follows:"
to explicitly name the target registry (e.g., "NVIDIA NGC Docker registry" or
"NGC container registry") and, if relevant, add a short parenthetical like
"(NGC)" so subsequent instructions clearly refer to the NGC registry; ensure the
new wording appears exactly where that sentence occurs in
doc/workloads/workloads_requirements_installation.rst.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 88da6b0a-4f00-4a7b-b03e-779f1b12c8b3

📥 Commits

Reviewing files that changed from the base of the PR and between aba86ae and 2d479dd.

📒 Files selected for processing (17)
  • doc/DEV.rst
  • doc/Tutorial.rst
  • doc/USER_GUIDE.rst
  • doc/index.rst
  • doc/reporting.rst
  • doc/systems.rst
  • doc/troubleshooting.rst
  • doc/workloads/ai_dynamo.rst
  • doc/workloads/aiconfigurator.rst
  • doc/workloads/bash_cmd.rst
  • doc/workloads/chakra_replay.rst
  • doc/workloads/ddlb.rst
  • doc/workloads/deepep.rst
  • doc/workloads/index.rst
  • doc/workloads/osu.rst
  • doc/workloads/vllm.rst
  • doc/workloads/workloads_requirements_installation.rst

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: 5

♻️ Duplicate comments (3)
doc/workloads/ai_dynamo.rst (1)

75-75: ⚠️ Potential issue | 🟡 Minor

Simplify awkward monitoring sentence on Line 75.

Current wording is hard to read. Prefer: “You can monitor job progress using either of the following options:”.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/workloads/ai_dynamo.rst` at line 75, Replace the awkward sentence that
currently begins "The job progress monitoring can be done using either of the
following options:" with the clearer wording: "You can monitor job progress
using either of the following options:". Locate the sentence in the
documentation paragraph describing job progress monitoring (the sentence
currently starting "The job progress monitoring...") and update it verbatim to
the suggested phrasing, preserving surrounding punctuation and formatting.
doc/workloads/workloads_requirements_installation.rst (2)

4-4: ⚠️ Potential issue | 🟡 Minor

Fix plural agreement in installables description.

Line 4 still uses a plural subject with a singular follow-up (“multiple installables” vs “The installable”). Please keep number agreement consistent.

Proposed fix
-CloudAI workloads can define multiple installables as prerequisites. The installable can be a container image, git repository, HF model, etc.
+CloudAI workloads can define multiple installables as prerequisites. Installables can be container images, git repositories, Hugging Face models, etc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/workloads/workloads_requirements_installation.rst` at line 4, The
sentence with number disagreement uses "multiple installables" followed by "The
installable can be..."; fix by making the noun and verb agree—replace "The
installable can be a container image, git repository, HF model, etc." with
either "Installables can be a container image, git repository, HF model, etc."
or "An installable can be a container image, git repository, HF model, etc." so
the plurality matches the earlier phrase; update the phrase in the same
paragraph where "multiple installables" appears.

10-10: ⚠️ Potential issue | 🟡 Minor

Clarify which Docker registry is required.

Line 10 says “the Docker repository,” which is ambiguous in this context. Naming NGC explicitly will make the setup steps clearer.

Proposed fix
-First, make sure you have access to the Docker repository. Proceed as follows:
+First, make sure you have access to the NVIDIA NGC container registry. Proceed as follows:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/workloads/workloads_requirements_installation.rst` at line 10, Replace
the ambiguous phrase "the Docker repository" with an explicit reference to
NVIDIA's registry by renaming it to "the NVIDIA NGC (NVIDIA GPU Cloud) Docker
registry" (or similar wording) where it appears in this document—specifically
the sentence containing "the Docker repository"—and optionally add a brief
parenthetical or link to NGC for clarity; ensure the phrasing is consistent
throughout the file doc/workloads/workloads_requirements_installation.rst.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/Tutorial.rst`:
- Line 194: Update the sentence containing ``test_name`` so it reads "The
``test_name`` specifies the test definition from one of the Test TOML files.
Node lists and time limits are optional." — insert the missing article "the"
before "test definition" in the existing line referencing ``test_name``.
- Line 79: Edit the sentence that currently begins "Test definition is a
Pydantic model..." to include the article "A" so it reads "A test definition is
a Pydantic model..." and keep the rest of the sentence unchanged; refer to the
documentation sentence describing the TestDefinition class to locate and update
this line.
- Line 92: Update the sentence referencing the parameter "an_arg" so it reads
"has a mixed type of" instead of "has mixed type of" — find the line containing
"an_arg" and replace "has mixed type of ``str | list[str]``" with "has a mixed
type of ``str | list[str]``" to correct the grammar.
- Around line 196-202: Clarify the dependency semantics: state that each
dependency entry is a pair of the depending test id (use the test scenario
name/id) and an optional delay in seconds, and update wording for the three
dependency types (`start_post_comp`, `start_post_init`, `end_post_comp`) to say
when the dependent test starts or must complete and whether the optional `delay`
is applied (e.g., `start_post_comp` — start after the other test completes plus
optional delay; `start_post_init` — start after the other test starts plus
optional delay; `end_post_comp` — must complete after the other test completes
plus optional delay), specify the default when `delay` is omitted (default 0
seconds), and update the example to include explicit `delay` fields and fix the
awkward phrasing ("after a specific delay following the completion" or similar).

In `@doc/USER_GUIDE.rst`:
- Line 494: Update the sentence to include the missing article before "file
system" — change "When compute nodes do not share file system with head node" to
either "When compute nodes do not share a file system with the head node" or
"When compute nodes do not share the file system with the head node"; apply this
edit in the USER_GUIDE.rst paragraph that includes the description of
"--enable-cache-without-check" for "run" and "dry-run".

---

Duplicate comments:
In `@doc/workloads/ai_dynamo.rst`:
- Line 75: Replace the awkward sentence that currently begins "The job progress
monitoring can be done using either of the following options:" with the clearer
wording: "You can monitor job progress using either of the following options:".
Locate the sentence in the documentation paragraph describing job progress
monitoring (the sentence currently starting "The job progress monitoring...")
and update it verbatim to the suggested phrasing, preserving surrounding
punctuation and formatting.

In `@doc/workloads/workloads_requirements_installation.rst`:
- Line 4: The sentence with number disagreement uses "multiple installables"
followed by "The installable can be..."; fix by making the noun and verb
agree—replace "The installable can be a container image, git repository, HF
model, etc." with either "Installables can be a container image, git repository,
HF model, etc." or "An installable can be a container image, git repository, HF
model, etc." so the plurality matches the earlier phrase; update the phrase in
the same paragraph where "multiple installables" appears.
- Line 10: Replace the ambiguous phrase "the Docker repository" with an explicit
reference to NVIDIA's registry by renaming it to "the NVIDIA NGC (NVIDIA GPU
Cloud) Docker registry" (or similar wording) where it appears in this
document—specifically the sentence containing "the Docker repository"—and
optionally add a brief parenthetical or link to NGC for clarity; ensure the
phrasing is consistent throughout the file
doc/workloads/workloads_requirements_installation.rst.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5032018d-8276-4bb9-bdcb-2041fd717f1a

📥 Commits

Reviewing files that changed from the base of the PR and between 2d479dd and be51026.

📒 Files selected for processing (4)
  • doc/Tutorial.rst
  • doc/USER_GUIDE.rst
  • doc/workloads/ai_dynamo.rst
  • doc/workloads/workloads_requirements_installation.rst

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: 2

♻️ Duplicate comments (4)
doc/USER_GUIDE.rst (2)

494-494: ⚠️ Potential issue | 🟡 Minor

Add missing article before “head node.”

Line 494 should read “with the head node” for grammatical correctness.

Suggested edit
-When compute nodes do not share the file system with head node, ``--enable-cache-without-check`` for ``run`` and ``dry-run`` skips the real check for cache existence, but still builds all paths correctly. The flow is like this:
+When compute nodes do not share the file system with the head node, ``--enable-cache-without-check`` for ``run`` and ``dry-run`` skips the real check for cache existence, but still builds all paths correctly. The flow is like this:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/USER_GUIDE.rst` at line 494, The sentence describing the cache behavior
for compute nodes is missing the article "the" before "head node"; update the
text around the "--enable-cache-without-check" mention (the paragraph that also
references "run" and "dry-run") so it reads "with the head node" instead of
"with head node" to correct the grammar while keeping the existing wording and
flags intact.

463-463: ⚠️ Potential issue | 🟡 Minor

Use a complete-sentence lead-in for the srun example.

Line 463 is still a sentence fragment (“Will result in ...”). This was already flagged earlier for the same section style.

Suggested edit
-Will result in srun command inside sbatch script like this:
+This will result in an srun command inside the sbatch script like this:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/USER_GUIDE.rst` at line 463, The lead-in for the srun example is a
sentence fragment ("Will result in srun command inside sbatch script like
this:"); change it to a complete sentence such as "This will result in the
following srun command inside the sbatch script:" so the example is introduced
with proper grammar and clarity (update the text that currently reads "Will
result in srun command inside sbatch script like this:" to the full-sentence
form).
doc/Tutorial.rst (2)

170-170: ⚠️ Potential issue | 🟡 Minor

Use consistent term: “test configuration” instead of “test description.”

Line 170 uses “test description,” which is inconsistent with the surrounding sections and prior terminology.

Suggested edit
-Test Scenario uses test description from Step 5. Below is the ``CONFIGS_DIR/scenario.toml`` file:
+Test Scenario uses the test configuration from Step 5. Below is the ``CONFIGS_DIR/scenario.toml`` file:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/Tutorial.rst` at line 170, Replace the phrase "test description" with the
consistent term "test configuration" in the sentence that begins "Test Scenario
uses test description from Step 5." (the line referencing
``CONFIGS_DIR/scenario.toml``) so the terminology matches surrounding sections
and prior usage.

198-202: ⚠️ Potential issue | 🟠 Major

Dependency semantics are internally inconsistent (delay/name vs id).

Lines 198–202 describe dependencies using “delay” and “depending test name”, but the example structure uses type + id. This is likely to cause misconfigured scenarios. Please document dependencies by referenced test id and remove outdated delay wording if no longer supported.

Suggested edit
-   - ``start_post_comp`` means that the current test should be started after a specific delay of the completion of the depending test.
-   - ``start_post_init`` means that the current test should start after the start of the depending test.
-   - ``end_post_comp`` means that the current test should be completed after the completion of the depending test.
+   - ``start_post_comp`` means that the current test starts after the depended-on test completes.
+   - ``start_post_init`` means that the current test starts after the depended-on test starts.
+   - ``end_post_comp`` means that the current test must complete after the depended-on test completes.

-All dependencies are described as a depending test name. The name should be taken from the test name as set in the test scenario.
+Each dependency is defined by the dependency ``type`` and the depended-on test ``id`` from the same scenario.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/Tutorial.rst` around lines 198 - 202, The paragraph describing dependency
semantics is inconsistent: replace references to "delay" and "depending test
name" with the canonical dependency fields used in the scenario schema (use the
dependency object's "type" and "id" fields), update the examples to show
dependencies referencing the target test by its test id (not the name), and
remove or reword mention of "delay" unless the schema explicitly supports a
delay field; specifically edit the descriptions for start_post_comp,
start_post_init, and end_post_comp to state they reference another test by id
and to align wording with the `type` + `id` structure shown in examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@doc/Tutorial.rst`:
- Around line 187-190: The dependency entry uses an invalid id "Tests.1"; update
the Tests.dependencies block (the entry with type="start_post_comp" and
id="Tests.1") to reference an existing scenario ID (e.g., replace id="Tests.1"
with id="allreduce.1" or id="allreduce.2" to match the defined IDs allreduce.1
and allreduce.2) so the dependency points to a valid target.

In `@doc/USER_GUIDE.rst`:
- Line 63: Update the documentation for the scheduler field to include runai as
a supported value: change the sentence that currently lists only "slurm" or
"standalone" to explicitly mention "runai" alongside those values and, if
helpful, add a short parenthetical clarifier linking to the RunAI section;
ensure the reference to the configuration key (scheduler = "runai") and the
terms "slurm" and "standalone" are present so readers see all supported options
consistently.

---

Duplicate comments:
In `@doc/Tutorial.rst`:
- Line 170: Replace the phrase "test description" with the consistent term "test
configuration" in the sentence that begins "Test Scenario uses test description
from Step 5." (the line referencing ``CONFIGS_DIR/scenario.toml``) so the
terminology matches surrounding sections and prior usage.
- Around line 198-202: The paragraph describing dependency semantics is
inconsistent: replace references to "delay" and "depending test name" with the
canonical dependency fields used in the scenario schema (use the dependency
object's "type" and "id" fields), update the examples to show dependencies
referencing the target test by its test id (not the name), and remove or reword
mention of "delay" unless the schema explicitly supports a delay field;
specifically edit the descriptions for start_post_comp, start_post_init, and
end_post_comp to state they reference another test by id and to align wording
with the `type` + `id` structure shown in examples.

In `@doc/USER_GUIDE.rst`:
- Line 494: The sentence describing the cache behavior for compute nodes is
missing the article "the" before "head node"; update the text around the
"--enable-cache-without-check" mention (the paragraph that also references "run"
and "dry-run") so it reads "with the head node" instead of "with head node" to
correct the grammar while keeping the existing wording and flags intact.
- Line 463: The lead-in for the srun example is a sentence fragment ("Will
result in srun command inside sbatch script like this:"); change it to a
complete sentence such as "This will result in the following srun command inside
the sbatch script:" so the example is introduced with proper grammar and clarity
(update the text that currently reads "Will result in srun command inside sbatch
script like this:" to the full-sentence form).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a1d09f5e-7c2f-4ff4-9817-89958c9153d2

📥 Commits

Reviewing files that changed from the base of the PR and between be51026 and 36a2e31.

📒 Files selected for processing (2)
  • doc/Tutorial.rst
  • doc/USER_GUIDE.rst

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.

2 participants