Add Sphinx documentation site - #311
Conversation
tomastola
left a comment
There was a problem hiding this comment.
Nice approach — single-sourcing from README.md/CONTRIBUTING.md via MyST {include} avoids the usual drift between the repo and the docs site. I built the site locally (rocm-docs-core resolved to 1.40.0) and it does build; the pages render and the section anchors land where you intended. A few things to resolve before this is mergeable.
Blocking — CI is red
1. License header check (hawkeye) — 7 new files are missing the MIT header:
docs/rocm-docs/conf.py
docs/rocm-docs/commands.md
docs/rocm-docs/demos.md
docs/rocm-docs/getting-started.md
docs/rocm-docs/about/contributing.md
docs/rocm-docs/about/license.md
docs/rocm-docs/install/installation.md
For the .md files that's the HTML-comment block already used across docs/*.md:
<!--
Copyright © Advanced Micro Devices, Inc., or its affiliates.
SPDX-License-Identifier: MIT
-->
and the #-comment equivalent for conf.py. index.rst, _toc.yml.in, and requirements.txt aren't in licenserc.toml's includes, so they're fine as-is. hawkeye check reproduces this locally.
2. Commit signatures + sign-off — both commits are unsigned and carry no Signed-off-by trailer:
065f231 N | signoff:
1f5d607 N | signoff:
Needs a re-sign plus git rebase --signoff (or recommit with -s -S) and a force-push.
Real defect
3. The platform-support table has a dead link on the install page. docs/wsl.md is the one relative link in an included region that wasn't converted, and it's the only one that actually breaks:
README.md:1056: WARNING: 'myst' cross-reference target not found: 'docs/wsl.md' [myst.xref_missing]
It renders as <a href="#docs/wsl.md">docs/wsl.md</a> — an in-page anchor to nothing, so clicking it does nothing on the install page. (docs/vllm.md at README:41 and LICENSE.TXT at README:10 have the same shape but fall outside every include range, so they're currently harmless — worth deciding whether to normalize them too or leave them.)
Gaps worth settling before merge
4. Nothing builds or publishes this site. There's no .readthedocs.yaml at the repo root, so as it stands this adds docs sources that never become a site. Is the RTD project config landing in a follow-up, or should it be part of this PR?
5. No CI job builds the docs, and this design specifically needs one. The includes are anchored on prose strings — :end-before: "See [CONTRIBUTING.md]", :end-before: "<!--", and heading text. Rewording that one sentence in README.md, or adding an HTML comment anywhere inside the Demos section, silently truncates or empties a page with no signal anywhere. That's the cost of single-sourcing, and a sphinx-build -W job is what makes it safe. Related: the build currently emits 16 Non-consecutive header level increase; H1 to H3 warnings (the included chunks start at ### because start-after drops the ##). I checked the rendered output — docutils normalizes them, so pages get correct <h1>/<h2> and the in-page TOCs are right; they're cosmetic. But they'd block -W, so they need suppressing or restructuring as part of adding the job.
Minor
6. rocm-docs-core>=1.7.1 is unpinned — it resolved to 1.40.0 for me. ROCm docs repos generally pin == so a docs build can't break from an upstream release landing.
7. Two different strategies for the same link problem. README.md anchors were rewritten to absolute https://github.com/ROCm/rocm-cli#... URLs, while installation.md solves the same problem the opposite way — cutting at :end-before: "See [CONTRIBUTING.md]" and re-authoring a docs-relative link. The absolute-URL route works, but it has costs on the primary surface: on GitHub, #model-serving and #interactive-interfaces used to be in-page scrolls and are now full page loads that always resolve against main, and they're wrong on a fork or a tag. On the docs site they send the reader off-site entirely. Since installation.md shows the cut-and-re-author pattern works, is it worth using that consistently and leaving the README anchors relative?
Adds a Sphinx documentation site under docs/rocm-docs/, single-sourced
from README.md and CONTRIBUTING.md via MyST {include} directives so
the docs and repo content stay in sync automatically. Covers
installation, getting started, command reference, demos, and
contributing/license, and is set up to publish with the rocm-docs-core
theme. Updates some links in README.md and CONTRIBUTING.md to be
absolute so they render correctly when reused via {include}.
Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>
Anchor the table in README.md and pull it into
docs/rocm-docs/install/installation.md via {include} instead of
duplicating it by hand.
Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>
065f231 to
911da53
Compare
|
@tomastola Thanks for the thorough review — believe I've addressed all 7 items:
One heads-up: there's a small follow-on PR (#312) with an editorial pass from Anisha still open against this branch — not touching anything above, but once it merges in, branch protection may dismiss your approval on the new commits. I'll ping you again then if that happens, but it should be a quick recheck. |
|
Added editorial updates from @anisha-amd. |
- Add the MIT license header to the 7 new files hawkeye flagged as missing it. - Fix the WSL2 platform-support-table link to docs/wsl.md: it's now an absolute GitHub URL so it resolves when the table is pulled into install/installation.md via MyST include, matching the pattern already used for the other README anchors. - Pin rocm-docs-core to the exact version that resolved locally (1.40.0) instead of an open->= range. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>
- Add .readthedocs.yaml so RTD builds against docs/rocm-docs/conf.py with the same pinned rocm-docs-core toolchain CI uses. - Add a docs-build CI job that runs sphinx-build -W (warnings as errors) against the same source RTD points at, gated on the new `docs` path-filter category so it only runs when doc sources, README.md/CONTRIBUTING.md (single-sourced via MyST includes), or the build config change. - Suppress the myst.header warning from :start-after:/:end-before: README includes stripping the anchor heading, which is cosmetic: docutils re-normalizes the resulting heading depth in the rendered output. - Scope external_projects to [] instead of the implicit "all" default, which was fetching intersphinx inventories for every project in rocm_docs' bundled catalog (~90 projects) - several of which 404 or have moved upstream. No page here uses a cross-project intersphinx role, so this is lossless and makes -W builds deterministic instead of flaky against infrastructure this repo doesn't control. - Filter the "current project not found in projects" warning: rocm-cli isn't registered in rocm-docs-core's shared projects catalog yet, so external_projects_current_project can never resolve until that's added upstream. rocm_docs.projects handles the unresolved project as None everywhere it's used, so this is informational, not a defect. Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>
README.md's own #model-serving and #interactive-interfaces anchors were
rewritten to absolute GitHub URLs so they'd still work once pulled into
the docs site, but that breaks in-page anchor scrolling when the README
is viewed on GitHub. Revert both to relative anchors and instead apply
the cut-and-reauthor pattern already used for the CONTRIBUTING.md link
in installation.md: narrow the surrounding {include} in getting-started.md
and commands.md around each sentence and hand-author a docs-relative
replacement (commands.md#model-serving, getting-started.md#interactive-interfaces).
Signed-off-by: pmoutsia_amdeng <peter.moutsias@amd.com>
Avoids the sidebar rendering as duplicated Demos > Demos for the single-entry section by overriding the nav link text to match the homepage tile's wording. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
Joins the "single prebuilt binary" fragments into one complete sentence in both README.md and the standalone index.rst homepage copy, and adds a lead-in sentence before the bare `rocm` code fence in the Getting Started "First run" section. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
e844c87 to
42abffa
Compare
Sentence-case headings, expand DCO acronym, replace banned/informal wording (may, e.g.), normalize slash usage, lowercase version placeholder. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
42abffa to
77ea773
Compare
michaelroy-amd
left a comment
There was a problem hiding this comment.
Reviewed current head 77ea773c240263b1a1fa609ae9b70959c3fb5644 after the follow-up commits. The prior blockers are addressed: required MIT headers are present, commits/sign-offs validate, .readthedocs.yaml and a pinned rocm-docs-core==1.40.0 requirement exist, Sphinx runs with -W, the included docs/wsl.md reference no longer produces a dead anchor, and the include-boundary/link strategy is internally consistent. All 17 required checks pass, including the Sphinx docs build, DCO, licensing, and link-sensitive build. One non-linked docs/vllm.md code reference is a minor usability follow-up, not a broken link. No blocking finding. Approving.
|
Pushed 3 README updates on top of the approved changes:
Flagging since this PR was already approved — the push didn't reset that, so no re-review required, just visibility on the new content. |
* fix(lemonade): retry interrupted backend setup Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * ci(e2e): build prebuilt lanes with the test-hooks feature The suite's deterministic failure seams are compiled out without `rocm/e2e-test-hooks`. `cargo xtask e2e` passes the feature only when it builds the binaries itself, so every lane that pre-builds and exports ROCM_CLI_BINARY silently tested a binary without those seams. The scripted-Lemonade-failure scenario then never reached its premise and failed as a regression on the one lane that selects it. Pass the feature on all prebuilt e2e lanes, and add a workflow-contract test so a lane cannot drift back. Release and packaging builds keep the feature off — a shipped binary must not carry a failure-injection seam. Also name the cause in the retry assertion, which otherwise reports a baffling missing announcement rather than the real misconfiguration. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> --------- Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
The hook pins in .pre-commit-config.yaml were bumped by hand and drifted between releases. Dependabot's pre-commit ecosystem resolves each rev against the hook repository's tags, and skips the builtin and local blocks, which have no upstream release to track. Weekly and grouped, with the same 7-day cooldown as the other two ecosystems: a hook is executable code that runs on every contributor's machine at commit time and in the prek CI job, so a compromised release would run before anyone reads the bump. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
* fix: raise the supported Linux minimum to Ubuntu 24.04 Every published Lemonade embeddable, v10.2.0 through v11.5.2, is linked against GLIBC_2.38 and GLIBCXX_3.4.32. Ubuntu 22.04 ships glibc 2.35, so the Lemonade engine cannot start there at all, yet README.md advertised Linux x86_64 as "full support ... both inference engines" and docs/wsl.md named 22.04 as a supported WSL base. Document a single minimum of Ubuntu 24.04 for Linux and WSL2, keeping the glibc number as the reason so the requirement stays meaningful on other distributions, and drop 22.04 from the WSL preflight's supported set so the automated check matches the prerequisite it enforces. Fixes #258 Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com> * fix(wsl): accept Ubuntu releases after 24.04 Signed-off-by: Michael Roy <michael.roy@amd.com> --------- Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com> Signed-off-by: Michael Roy <michael.roy@amd.com> Co-authored-by: Michael Roy <michael.roy@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
A scenario reaches the shared pre-warm tree through a symlink at its own `data/runtimes`. An `install sdk` run that way writes the link's path — a per-scenario temp dir — into the manifest that lands in the SHARED registry, and into the venv's console-script shebangs. The temp dir goes away with the scenario and the shared runtime is left naming a folder that no longer exists, so later, unrelated runs fail. Drop such a runtime before deciding whether the tree is fresh, so the pre-warm reinstalls instead of serving a dead one. `rocm update` cannot see this: it compares versions against the index, not the runtime against the disk. Removing the folder is the repair, not a precaution. A poisoned venv keeps a working `bin/python` — a symlink to the base interpreter, still present — so the install reuses it and audits already-satisfied packages rather than reinstalling them, leaving every shebang pointing at the folder that went away. Measured on uv 0.9.30: reinstalling over a poisoned venv reports success and repairs nothing. The signal is an install root outside the tree, deliberately not `status=unusable`. Unusable has many causes — a missing rocm_sdk probe block alone reports it — and this deletes what it selects, so a healthy multi-GiB runtime must not hang on a validation detail. Read-only runtimes are exempt: `runtimes adopt` records an external folder on purpose. Refs: #315 Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
* fix(install): record the folder the files land in, not the link `install sdk` built the install root by joining onto the data dir and recorded whatever that produced. Reaching `data/runtimes` through a symlink is enough to make that name a route rather than a place: correct while the link exists, and wrong the moment it goes. The path outlives the command. It goes into the registry manifest, the sidecar beside the runtime, and — through uv — the `#!` line of every console script in the environment. So a runtime installed that way keeps reporting itself installed at a folder that is not there, while the files sit untouched next door and every entry point fails to start. Resolve the root before anything is written to it. Only the root: `python_executable` is derived from it and must keep the venv's own `bin/python`, which is itself a symlink to the base interpreter, so resolving that would record the system Python instead. The adopt path already draws the line in the same place, canonicalizing the install root next to `absolute_existing_file_path_preserving_symlink` for the interpreter; this gives the install path the same treatment, `--prefix` included. The E2E harness creates exactly this shape when a scenario opts into the shared pre-warmed runtime, so the shared tree on a runner was what got poisoned — but nothing about the fault is test-only. Any data dir reached through a link has it. Refs: #315 Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(vllm): name the missing interpreter behind a failed launch Two messages described a stale runtime as an absence. A spawn that fails on a console script whose `#!` interpreter is gone reports ENOENT against the script, so the error named a file that is plainly there. Say which interpreter is missing when that is what happened, and stay quiet when the ordinary reading is right — a genuinely absent file, a binary, or a live interpreter. Resolution drops a registered runtime whose recorded interpreter is not there. That is correct — a runtime that cannot run is not a candidate — but it left the failure describing an empty registry while `rocm runtimes list` prints the entry. Name the manifests that were passed over and the interpreters they record. Both are additive: the note only ever decorates an error already being returned, and a registry it cannot read yields no note rather than replacing the original failure. First tests for this area, which had none. Refs: #315 Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * test(install): compare resolved paths, not verbatim ones, on Windows The Windows lane failed on the prefix rather than the behaviour. `canonicalize` hands back `\\?\C:\…` there, so a test that built its expectation that way compared a verbatim path against the plain one the resolver deliberately returns, and three assertions turned on the difference. Build the expectations with the resolver instead, which strips the prefix for the same reason the product does: a stored path is later compared against ordinary ones, and `\\?\C:\…` never `starts_with`-matches `C:\…`. That the prefix really is stripped stays asserted on its own, so this does not become the function agreeing with itself. This also removes a second Windows-only trap: `temp_dir()` there can return an 8.3 short path, which `canonicalize` expands, so the two sides disagreed on the folder name as well as the prefix. Same fault in the scenario's step definitions, where the comparison would have failed only on the Strix Windows lane. That crate cannot reach `rocm-core`, and a dependency for six lines of string handling is the worse trade, so it strips the prefix locally with the reason recorded. Refs: #315 Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> * fix(install): give up on a parent component consistently across hosts The guard against resolving a `..` past a missing directory never fired on Windows, because the two platforms disagree about what such a path means. Windows collapses `..` lexically, so `<root>/missing/..` "exists" and canonicalizes to `<root>`; Unix walks the path through the filesystem and finds nothing. The walk therefore stopped a level early on Windows, resolved to the wrong folder, and re-attached the tail — recording a different folder depending on the host, which is precisely what this function exists to prevent. Check for the parent component before asking whether the candidate exists, so both hosts give up in the same place. The give-up stays narrow: a `..` whose parent is really there is unambiguous everywhere and still resolves, which is now pinned by its own test so a future tightening cannot quietly leave ordinary paths unresolved. Found by the Windows lane; it does not reproduce on Linux. Refs: #315 Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> --------- Signed-off-by: Eugene Volen <Eugene.Volen@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
#307) * fix(install): resolve driver repo version and package release in plans The driver install plan stored the repo version and package release as raw shell parameter-expansion templates (${ROCM_CLI_AMDGPU_VERSION:-7.2.4} and ${ROCM_CLI_AMDGPU_PACKAGE_RELEASE:-70204}). Two problems followed. The human-readable plan summary printed the version template verbatim on its "repo_version:" line, leaking an unexpanded shell placeholder into user-facing dry-run output. And the Debian/Ubuntu apt source line wrapped the template in single quotes, so the shell never expanded it and the literal placeholder would have been written into /etc/apt/sources.list.d/amdgpu.list. Resolve both templates once, at plan-build time, to their effective values (the env var when set and non-empty, matching shell :- default semantics, otherwise the documented default) and bake the concrete values into every rendered URL, repo path, and command. The resolver is generalized to resolve_shell_default_template and hardened to pass bare ${VAR} and nested-default shapes through unchanged rather than emitting a partially rewritten value. Add unit tests for the resolver (default, env override, empty-as-unset, non-template pass-through, bare-var and nested-default pass-through) and update every distro plan test to assert the concrete values, plus a render-level regression test and an end-to-end scenario asserting the dry-run summary shows the resolved version rather than the raw placeholder. Tests that read these process env vars serialize on a shared lock and save/restore prior values so they stay deterministic under edition-2024 env semantics. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(install): guard the two remaining env-reading driver-plan tests build_driver_install_plan now resolves the repo-version template via std::env::var at the top of the function, so every test calling it reads process env. The sweep that added ScopedTestEnv::with_amd_overrides_cleared() to the other call sites missed windows_install_driver_is_validate_only and wsl_install_driver_uses_rocdxg_guidance_without_dkms, leaving two unguarded readers racing the guarded mutators in the same test binary. Add the guard to both. No production change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
…306) * fix(cli): accept negative float flag values in space form The space form of negative-number float flags (e.g. `serve --temperature -1`) was rejected by clap with a confusing "unexpected argument '-1'" error, because clap parsed the leading-dash token as a flag rather than a value. Only the equals form (`--temperature=-1`) reached the range validator that reports the valid range. Enable `allow_negative_numbers` on the affected float flags (`--temperature` and `--top-p` on both `chat` and `serve`) so the space form reaches the value parser and both forms validate identically, surfacing a clear range-validation message instead of an unexpected argument error. Add regression tests covering both the space and equals forms for `--temperature` and `--top-p` on `chat` and `serve`. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(cli): extend negative-value fix to --max-tokens and add serve scenario The negative-number-as-flag gotcha is a tokenizer-level ambiguity that fires before any value parser runs, so it is not float-specific: `--max-tokens -1` (space form) on both chat and serve was still rejected as an unexpected argument while `--max-tokens=-1` reached parse_positive_u32. Add allow_negative_numbers to --max-tokens on both commands so the space form reaches the value parser and reports a clear error, matching the --temperature/--top-p behavior fixed here. Per AGENTS.md §3, the user-observable stderr change is now covered by an ungated Gherkin scenario, @id:serve-negative-temperature-rejected, in model_serving.feature. The value parser runs inside argument parsing before engine selection or GPU pre-flight, so the scenario needs no GPU and no engine and gates every PR (mirrors the ungated @id:fix-position-argument-rejected precedent). Also: reword the shared rationale comment to cover all three numeric value flags (and note clap only treats a token as negative when the whole remainder parses as one, so missing/malformed values still error at parse time), give the chat regression test a symmetric comment, and extend both space-form regression tests with the --max-tokens cases. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(e2e): number the negative-temperature serve scenario The new scenario was the only unnumbered title in model_serving.feature; every other scenario uses 'Scenario: <N> - <Title>'. Assign the next free number (15) to keep the file consistent. No behavior change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
* fix(serve): validate --device via clap possible values (exit 2) Model `rocm serve --device` as a clap `ValueEnum` (`DevicePolicyArg`) so an invalid value is rejected by clap's usage validation (exit code 2) with the valid choices listed, instead of parsing as a free-form string and failing later in application logic with a generic "unsupported device policy" error (exit code 1). This makes `--device` consistent with every other enum-style argument in the CLI (`--engine`, config set-*, engines install, etc.), all of which use clap possible values. The historical aliases (`auto`/`gpu` for gpu_required, `cpu` for cpu_only) stay accepted for backward compatibility but are hidden from the advertised list, and the intentional cpu_only app-level rejection (exit 1 with a GPU-required message) is preserved. Adds regression tests asserting invalid `--device` is a clap InvalidValue usage error and that every value and alias still parses, and reworks the device possible-values sync test to read clap's structural possible values rather than a hand-written doc string. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(serve): correct --device help text and hide cpu_only choice The per-variant ValueEnum doc comments render in `serve --help`, shell completions, and the invalid-value suggestion list. `gpu_preferred` previously read "Prefer a ROCm GPU when one is available", which is misleading: parse_device_policy maps gpu_preferred to GpuRequired and rocm serve has no CPU fallback path. Reword it to state it behaves identically to gpu_required. cpu_only is always rejected (rocm serve requires GPU execution), so hide it from the advertised choices with #[value(hide = true)] while keeping it parseable so the deliberate rejection message is preserved. Add device_policy_arg_maps_through_parse_device_policy to guard the DevicePolicyArg::as_policy_str -> parse_device_policy mapping (gpu_required and gpu_preferred resolve to GpuRequired; cpu_only is rejected), closing the previously untested conversion path. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * docs(serve): stop advertising hidden cpu_only device value Hiding `cpu_only` from `--device` help/completion left three surfaces still presenting it as a real choice. Align them with what the CLI now advertises: - README serve synopsis dropped `cpu_only` from the --device value list. - Removed the '--gpu is ignored with --device cpu_only (the model runs on CPU)' sentence: cpu_only is rejected (exit 1), never runs on CPU, so the line advertised a fallback path that does not exist (AGENTS.md §6). - Corrected the drifted test comment to note cpu_only is #[value(hide = true)] and that serve_device_help_lists_match_device_policy_names compares against the full DevicePolicy set (hidden entries included). Doc/comment only; no behavior change. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
…ff critical path (eai-8190) (#295) * fix(dash): seed launcher front door with live serving instances The launcher front door built an empty AppState, so it always rendered the idle variant even when a model was actively serving. Read the managed-service registry (the same authority `rocm services` reads) once per hub-loop pass and seed the AppState's instances from it. Also treat `Ready` as serving everywhere the dashboard counts running instances (`is_serving()`), matching the `Running`+`Ready` treatment already used elsewhere (e.g. home.rs) -- a served model reports `Ready`, not `Running`, so the count previously undercounted actual serving models. Adds apps/rocm's direct dependency on rocm-dash-core (previously only a transitive dep) so the launcher can build `Instance`s from the registry's `DiscoveredService` records. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * fix(dash-daemon): run amd-smi detection off the run loop's critical path Detecting amd-smi (`amd-smi version` plus the first `system_info()`) can take up to ~15s on real hardware. Running it inline before the run loop's first tick blocked managed-service discovery and the first snapshot broadcast behind it, so an already-running model did not surface in the dashboard until GPU detection finished -- a visible ~15-20s "0 models running" lag while `rocm services` already reported it live. Spawn detection in the background and adopt the result via a oneshot channel the moment it lands, without ever blocking the loop while it is in flight. The loop now starts ticking immediately, so serving instances appear within one discovery tick; GPU metrics fill in once detection completes. Adds a regression test asserting on ordering (the instance must surface in a snapshot whose gpu_system_info is still None) rather than wall-clock timing, since a pure "arrived within Ns" check would be flaky under subscriber starvation. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> * test(dash): exercise off-critical-path detection and launcher front door Address review feedback on the amd-smi-off-critical-path change. - Gate the "amd-smi unavailable" warning behind gpu_init_done so a healthy host never flashes it during the detection window; settle as unavailable (and say so once) if the detection task ever ends without a result. - Add an amd_smi_skip_kfd_preflight test seam so the daemon regression test drives a fake amd-smi through the real detection path instead of short-circuiting on a GPU-less CI host (the /dev/kfd guard stays mandatory in production). The test now genuinely fails if detection moves back onto the critical path, and asserts the surfaced snapshot carries no premature "amd-smi unavailable" warning. - Add direct unit tests for launcher_serving_instances (ready record maps to a live Instance; unbound :0 record is dropped) and a behavioural launcher scenario driving bare rocm through a PTY to prove the front door shows "Serving <model>" rather than "Idle" for a live registry service. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> --------- Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
check_4_render_group built the render-group remediation command from the /dev/kfd owner group obtained via `stat -c %G`. When the device GID has no matching group name, stat prints the literal "UNKNOWN", and the existing guard only rejected empty strings. That let UNKNOWN leak into the suggested fix, producing `sudo usermod -a -G UNKNOWN,video "$USER"` in the plan, summary, and --json output -- a command that fails because no such group exists. Reject "UNKNOWN" (case-insensitive) alongside empty values so the group falls back to "render". This makes diagnose's fallback coincide with the "render,video" that fix.rs hardcodes in the UNKNOWN/empty case; it does not unify the two group sources, which are still computed independently and can diverge when /dev/kfd has a real group name that is not "render". Add a regression test covering both the "UNKNOWN" and "unknown" sentinel casings falling back to the render group. Signed-off-by: Roman Sirokov <roman.sirokov@amd.com> Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
…entation-site Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
…ment rocm examine, describe Configuration section Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
Not one of the commands being hardened for this release; team decision was to drop it from the reference for now rather than document it. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
35d4580 to
b7300c3
Compare
Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
…entation-site Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
use_repository_button and use_download_button rely on rocm-docs-core resolving a repository_url from the current git branch. GitHub Actions' pull_request checkout leaves a detached HEAD on the synthetic merge ref, so get_branch() returns an empty URL, and sphinx_book_theme crashes trying to unpack it, failing the -W build. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
- Broaden the known-warning suppression filter to also cover the "rocm-ai" flavor, which isn't in the published rocm-docs-core release yet and would otherwise fail the -W build even though rocm_docs.theme already falls back to "rocm" gracefully. - Re-enable use_repository_button/use_download_button, pinning repository_url/repository_branch explicitly instead of letting rocm-docs-core infer them from the local git branch. CI's pull_request checkout leaves a detached HEAD on the synthetic merge ref, which resolves to an empty repository_url and crashes sphinx_book_theme's repository button; use_download_button was never actually affected by this, it only reads local source files. Verified by building against the pinned rocm-docs-core==1.40.0 in a clean venv (matching CI) -- build succeeds with 0 warnings. Signed-off-by: pmoutsias-amd <peter.moutsias@amd.com>
Summary
docs/rocm-docs/, single-sourced fromREADME.mdandCONTRIBUTING.mdvia MyST{include}directives so the docs and repo content stay in sync automaticallyrocm-docs-corethemeREADME.mdandCONTRIBUTING.mdto be absolute so they render correctly when reused via{include}Test plan
pip install -r docs/rocm-docs/sphinx/requirements.txt && sphinx-build docs/rocm-docs docs/rocm-docs/_build) and spot-check each page renders correctly