docs: add auto-generated API reference for Python, Rust, and K8s - #6989
Closed
dagil-nvidia wants to merge 6 commits into
Closed
docs: add auto-generated API reference for Python, Rust, and K8s#6989dagil-nvidia wants to merge 6 commits into
dagil-nvidia wants to merge 6 commits into
Conversation
Add a two-stage API documentation pipeline: 1. Generators produce GitHub-friendly Markdown with YAML frontmatter 2. Thin fernify scripts transform to Fern MDX in CI Python API (generate_python_api.py): - Uses griffe to parse docstrings from _core.pyi, runtime, frontend, planner, router, mocker, common, and nixl_connect modules - Renders classes, functions, enums with parameters, returns, examples - Deduplicates re-exported symbols across modules Rust API (generate_rust_api.py): - Discovers published crates from Cargo.toml workspace - Links to docs.rs for each crate Fernify scripts (CI-only transforms): - fernify_python_api.py: tables->cards, details->accordions, admonitions - fernify_rust_api.py: tables->cards, admonitions - fernify_k8s_api.py: resource cards, headings->accordions, tabs Shared helpers (_fern_helpers.py): - fernify_details_to_accordion, fernify_table_to_cards, fernify_headings_to_accordion Also adds docstring examples to planner, router, mocker, frontend, and configuration modules for richer API documentation. Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
github-actions
Bot
added
docs
documentation
Contributor
Contributor
WalkthroughThis PR introduces API documentation generation infrastructure and comprehensive documentation examples. It adds Python scripts to generate consolidated API references for Python, Rust, and Kubernetes APIs, integrates Fern MDX transformation helpers for doc formatting, updates CI/CD workflows to automate docs generation and publishing, and extends docstrings across components with usage examples. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (5)
lib/bindings/python/src/dynamo/health_check.py (1)
34-43: Clear and useful example.The doctest syntax is correct and demonstrates the JSON parsing behavior well.
Minor note: if these examples are run as doctests, the environment variable set here persists in the process. Consider cleaning it up at the end to avoid affecting other tests:
>>> del os.environ["DYN_HEALTH_CHECK_PAYLOAD"] # cleanup🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/bindings/python/src/dynamo/health_check.py` around lines 34 - 43, The doctest example for load_health_check_from_env leaves the DYN_HEALTH_CHECK_PAYLOAD environment variable set which can leak state into other tests; update the example to delete the environment variable at the end (use del os.environ["DYN_HEALTH_CHECK_PAYLOAD"]) so the environment is cleaned up after the example runs and does not affect subsequent doctests or code.docs/scripts/fernify_k8s_api.py (1)
141-149: Either use the injectedmetamap or drop it from the API.
build_resource_cards()acceptsmeta, but_resolve_meta()always reads globalRESOURCE_META, so callers can’t actually override metadata. Thread the argument through to keep the helper honest.Suggested fix
-def _resolve_meta(name: str, full_text: str) -> dict[str, str]: +def _resolve_meta( + name: str, + full_text: str, + meta: dict[str, dict[str, str]], +) -> dict[str, str]: """Resolve icon, description, and source path for a resource.""" - if name in RESOURCE_META: - return RESOURCE_META[name] + if name in meta: + return meta[name] desc = ( _extract_description(full_text, name) or "Custom resource for the Dynamo operator." ) return {"icon": "regular cube", "desc": desc, "src": _discover_go_source(name)} @@ cards: list[dict[str, str]] = [] for name in resources: - info = _resolve_meta(name, text) + info = _resolve_meta(name, text, meta) src = info["src"] href = f"{OPERATOR_SRC}/{src}" if src else "" cards.append( {Also applies to: 162-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/scripts/fernify_k8s_api.py` around lines 141 - 149, The helper _resolve_meta currently ignores the passed-in meta map and always reads the global RESOURCE_META, preventing callers of build_resource_cards(meta) from overriding metadata; update _resolve_meta to accept a meta: dict[str, dict[str,str]] (or Optional) parameter and use that map first (falling back to RESOURCE_META and then to _extract_description/_discover_go_source), and update all callers (notably build_resource_cards) to pass the meta through; ensure the function signature and usages (e.g., _resolve_meta(...)) are updated consistently so injected metadata is honored.docs/scripts/generate_python_api.py (1)
772-779: Deduplicate on the resolved target, not the short name.
seen_namesis global and keyed only byname, so two unrelated public symbols that share an identifier across modules/submodules collapse to one entry. That’s broader than re-export dedupe and can hide valid API items.Also applies to: 783-791
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/scripts/generate_python_api.py` around lines 772 - 779, The code currently deduplicates by short symbol name via ctx.seen_names and `name`, which collapses distinct symbols from different modules; change the dedupe key to the resolved/qualified target instead. Replace checks against ctx.seen_names/name with a dedupe set (e.g., ctx.seen_targets) and add a fully-qualified identifier such as full_path or a resolved identity like f"{member.__module__}.{getattr(member,'__qualname__', name)}" (or id(member) if necessary); then use that key for the membership test and insertion before appending to groups so _classify_member, groups, SUB_GROUP_ORDER and full_path stay the same but collisions only occur for true re-exports.docs/scripts/generate_rust_api.py (1)
41-51: Avoid silently dropping newly added workspace crates.The hard-coded allowlist means any new published crate is omitted until
DISPLAYED_CRATESis updated by hand. That makes the Rust reference easy to drift out of sync with the workspace. Consider deriving inclusion from Cargo metadata, or at least failing fast when a published crate is not represented.Also applies to: 147-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/scripts/generate_rust_api.py` around lines 41 - 51, The hard-coded DISPLAYED_CRATES set (and the similar list at the later occurrence) silently omits any newly published workspace crate; update generate_rust_api.py to derive the displayed crate list from Cargo metadata (e.g., run `cargo metadata` and collect workspace packages and their publish status) or, if you must keep a manual allowlist (DISPLAYED_CRATES), add an explicit validation step that compares the allowlist against the set of published workspace crates and raises an error (or exits non‑zero) when any published crate is missing; reference and update the DISPLAYED_CRATES symbol and the corresponding check around the later occurrence so the script either auto-populates the list from cargo metadata or fails fast when it detects a missing published crate..github/workflows/fern-docs.yml (1)
168-173: Add a generator or drift-check step before fernifying.This step only transforms the committed API READMEs. If docstrings or Cargo metadata change without re-running
generate_*_api.py, the Fern output will quietly stay stale. Running the generators here, or failing on a diff, would keep the GitHub README and Fern docs aligned.One straightforward option
+ - name: Generate API reference docs + working-directory: source-checkout + run: | + python3 docs/scripts/generate_python_api.py + python3 docs/scripts/generate_rust_api.py + - name: Fernify API reference docs working-directory: source-checkout run: | python3 docs/scripts/fernify_python_api.py python3 docs/scripts/fernify_rust_api.py python3 docs/scripts/fernify_k8s_api.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/fern-docs.yml around lines 168 - 173, The current "Fernify API reference docs" step runs fernify scripts but doesn't regenerate the source READMEs or detect drift; update this job to first run the language generator scripts (the project's generator commands that produce API READMEs) before invoking python3 docs/scripts/fernify_*.py, then perform a git diff check (e.g., run the generators and if git shows any unstaged/changed files, fail the step) so that any regenerated README/Cargo metadata changes are surfaced; modify the job named "Fernify API reference docs" to run the generators prior to python3 docs/scripts/fernify_python_api.py, python3 docs/scripts/fernify_rust_api.py, python3 docs/scripts/fernify_k8s_api.py and add a post-generation git diff check (exit non-zero on diffs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/fern-docs.yml:
- Around line 455-463: The release step only rewrites .md files; update the find
invocation under the block using TAG and VERSION that targets
"fern/pages-$TAG/api/rust" so it matches both .md and .mdx files (e.g., change
-name "*.md" to -name "*.md" -o -name "*.mdx" or an equivalent pattern) and keep
the same sed replacement that swaps docs.rs/.../latest to docs.rs/.../$VERSION
so MDX pages are updated too.
In `@components/src/dynamo/common/configuration/groups/kv_router_args.py`:
- Around line 43-55: The doctest example in the
KvRouterArgGroup/KvRouterConfigBase docstring references an undefined my_config
and imports unused names (argparse, KvRouterArgGroup); fix by either
instantiating the example config (e.g., create my_config = MyRouterConfig(...)
before calling my_config.kv_router_kwargs()) or simplify the example to show
usage without execution, and remove the unused imports; update the example lines
that mention my_config and the import list so they reference KvRouterConfigBase
and MyRouterConfig only and are self-contained.
In `@components/src/dynamo/common/lora/manager.py`:
- Around line 37-45: The example in the LoRAManager docstring is inconsistent
and non-runnable: it registers a custom source via
LoRAManager.register_custom_source("hf", ...) but calls download_lora with an
s3:// URI and uses top-level await; update the example so the registered source
is actually exercised (either register an "s3" source to match "s3://..." or
change the download URI to an hf://... identifier) and make the call runnable by
wrapping async calls with asyncio.run (e.g., result =
asyncio.run(manager.download_lora(...))) or by showing an async def main() +
asyncio.run(main()) pattern; ensure references to
LoRAManager.register_custom_source, LoRAManager.download_lora, and
LoRAManager.is_cached remain in the example.
In `@components/src/dynamo/common/storage.py`:
- Around line 132-141: The doc example shows top-level await for upload_to_fs
which won't run in a normal Python REPL; update the example around get_fs and
upload_to_fs to run the coroutine via asyncio.run(...) or by defining and
calling an async def wrapper so the snippet is copy/pasteable. Specifically
modify the example that calls upload_to_fs (referencing get_fs and upload_to_fs)
to wrap the await in asyncio.run(...) or show an async def main(): ... await
upload_to_fs(...) followed by asyncio.run(main()) so it executes outside an
async REPL.
In `@components/src/dynamo/planner/kubernetes_connector.py`:
- Around line 55-66: The example for KubernetesConnector is missing the required
parent DGD input used by KubernetesConnector.__init__ (it raises if neither
parent_dgd_name nor the DYN_PARENT_DGD_K8S_NAME env var is provided); update the
example to include either a parent_dgd_name argument when instantiating
KubernetesConnector or show setting the DYN_PARENT_DGD_K8S_NAME environment
variable before creating the connector so the snippet is self-contained and will
not raise on initialization.
In `@components/src/dynamo/planner/remote_planner_client.py`:
- Around line 20-30: The doctest/example uses a top-level await causing
SyntaxError; modify the example for RemotePlannerClient so the async call to
send_scale_request is wrapped in an async function (e.g., async def run(): ...)
and then invoked via asyncio.run(run()) so the example is runnable in standard
Python/doctest; keep RemotePlannerClient instantiation and call to
send_scale_request inside that async function and show using asyncio.run to
obtain and inspect the response.status.
In `@docs/scripts/_fern_helpers.py`:
- Around line 58-60: The slugify(text: str) function incorrectly preserves
punctuation like '.' so inputs like "dynamo.runtime" don't match generated
heading IDs; update slugify to normalize non-word characters by replacing all
sequences of non-alphanumeric/underscore characters (e.g., using a regex
targeting \W+) with a single hyphen, then collapse duplicate hyphens, strip
leading/trailing hyphens, and lowercase the result so headings like
"dynamo.runtime" become "dynamo-runtime"; modify the slugify function
accordingly.
In `@docs/scripts/fernify_k8s_api.py`:
- Around line 88-96: _get_go_source_index currently keys files by f.name and
only globs one level, which causes collisions for versioned names and misses
nested dirs; update the function (referencing _get_go_source_index,
OPERATOR_DIR, and _GO_SOURCE_INDEX) to recursively discover all Go files under
the api tree (use rglob or a recursive glob for "api/**/*.go") and key the index
by the file's relative path (e.g., f.relative_to(OPERATOR_DIR).as_posix() or
similar) instead of f.name so versioned and nested paths are preserved.
In `@docs/scripts/generate_python_api.py`:
- Around line 497-528: The generated signatures and parameter tables currently
drop explicit None defaults because both _build_signature and
_render_parameters_table check "is not None" before treating a default; change
both functions to only treat the empty-parameter sentinel
(_PARAM_EMPTY_SENTINEL) as absent so that explicit None values are preserved: in
_build_signature remove the p.default is not None guard and only skip adding a
default when str(p.default) == _PARAM_EMPTY_SENTINEL, and in
_render_parameters_table set default to empty only when str(param.default) ==
_PARAM_EMPTY_SENTINEL so explicit None appears in the rendered table; keep using
_format_annotation/_safe_parameters unchanged.
- Around line 224-242: The _source_link function currently builds the GitHub URL
using a path shortened by SEARCH_PATHS which yields a non-repo-root-relative
path and causes 404s; change it to build the href from REPO_ROOT while
optionally keeping the shortened rel for display: compute repo_rel =
str(filepath.relative_to(REPO_ROOT)) (fallback to str(filepath) if relative_to
fails) and use repo_rel in the URL portion
(https://github.com/.../blob/main/{repo_rel}{suffix}) but keep the existing rel
variable for the visible link text; update references inside _source_link to use
repo_rel for the href and rel for display.
In `@docs/scripts/requirements-apidocs.txt`:
- Around line 1-2: Add the project's standard SPDX header to the top of this new
requirements-apidocs.txt file: insert the same copyright/owner line and
SPDX-License-Identifier used across the repo as the first lines (followed by a
blank line), matching formatting and encoding conventions so the repo-wide
copyright check passes.
In `@lib/bindings/python/src/dynamo/_core.pyi`:
- Around line 873-877: The example for HttpService incorrectly calls
add_chat_completions_model which is not present on the HttpService stub; update
the example to only use the actual exposed methods (__init__, run, shutdown) or
add a proper stubbed method if that API is intended. Locate the HttpService
example in the docstring and either remove the add_chat_completions_model line
or replace it with a valid call sequence that uses HttpService.__init__,
HttpService.run(runtime), and HttpService.shutdown() so the generated docs do
not reference a nonexistent method.
- Around line 1090-1097: Update the ModelType docstring summary so it exactly
mirrors the exported enum member names: replace the lead sentence that mentions
"Tensor" and omits "Audios" with one that lists the actual members (Chat,
Completions, Embedding, TensorBased, Images, Videos, Audios, Prefill) and ensure
the examples and explanatory text reference ModelType and the members (e.g.,
ModelType.TensorBased and ModelType.Audios) consistently to match the enum
surface.
- Around line 937-945: The example shows service.run(...) returning a shutdown
awaitable but run() is declared async def -> None; fix the example to call and
await the coroutine instead (or create a background task if intended).
Specifically update the lines using KserveGrpcService.run (in the example block
with KserveGrpcService, PythonAsyncEngine) to either: await service.run(runtime)
for a foreground async call, or task = asyncio.create_task(service.run(runtime))
and then await task for background semantics; ensure you remove assignment to
shutdown_signal since run() returns None and adjust subsequent usage (e.g.,
await shutdown_signal -> await service.run(runtime) or await task).
- Around line 138-143: The example under
runtime.endpoint("dynamo.backend.generate") uses RequestHandler().generate but
RequestHandler is a type alias, so replace that with an actual handler callable:
define or reference an async function (e.g., async def handle(request): ...) or
an instance of a concrete class that implements the handler protocol and pass
that to serve_endpoint; update the example to call await
endpoint.serve_endpoint(handle) (or serve_endpoint(MyHandler().generate)) so the
sample compiles and matches the RequestHandler type alias usage.
In `@lib/bindings/python/src/dynamo/health_check.py`:
- Around line 82-96: The doctest for the HealthCheckPayload subclass can be
flaky because HealthCheckPayload.to_dict consults the DYN_HEALTH_CHECK_PAYLOAD
environment variable via load_health_check_from_env; update the example to
ensure the env var is cleared before instantiating (e.g., remove or unset
DYN_HEALTH_CHECK_PAYLOAD from os.environ) so the subclass default_payload is
returned, or alternatively call load_health_check_from_env to show precedence
explicitly; locate the example around HealthCheckPayload and its
to_dict/load_health_check_from_env usage and add a short setup step that unsets
os.environ["DYN_HEALTH_CHECK_PAYLOAD"] (or uses os.environ.pop) before creating
MyBackendHealthCheck.
In `@lib/bindings/python/src/dynamo/runtime/__init__.py`:
- Around line 75-95: The docstring for the dynamo_endpoint decorator advertises
response_model validation but the decorator never uses response_model or
validates streamed items; update either the docs or the implementation. Fix
option A: remove or reword response_model from the docstring/example to only
mention request parsing and show the generator yielding raw items; OR Fix option
B: implement response validation inside the dynamo_endpoint wrapper by checking
for a provided response_model, validating each yielded item via
response_model.parse_obj (or response_model(**item)) before yielding, and adjust
the example to yield Response instances; look for the dynamo_endpoint decorator
function and the request_model/response_model parameters to add the per-item
validation logic or to shorten the docstring accordingly.
---
Nitpick comments:
In @.github/workflows/fern-docs.yml:
- Around line 168-173: The current "Fernify API reference docs" step runs
fernify scripts but doesn't regenerate the source READMEs or detect drift;
update this job to first run the language generator scripts (the project's
generator commands that produce API READMEs) before invoking python3
docs/scripts/fernify_*.py, then perform a git diff check (e.g., run the
generators and if git shows any unstaged/changed files, fail the step) so that
any regenerated README/Cargo metadata changes are surfaced; modify the job named
"Fernify API reference docs" to run the generators prior to python3
docs/scripts/fernify_python_api.py, python3 docs/scripts/fernify_rust_api.py,
python3 docs/scripts/fernify_k8s_api.py and add a post-generation git diff check
(exit non-zero on diffs).
In `@docs/scripts/fernify_k8s_api.py`:
- Around line 141-149: The helper _resolve_meta currently ignores the passed-in
meta map and always reads the global RESOURCE_META, preventing callers of
build_resource_cards(meta) from overriding metadata; update _resolve_meta to
accept a meta: dict[str, dict[str,str]] (or Optional) parameter and use that map
first (falling back to RESOURCE_META and then to
_extract_description/_discover_go_source), and update all callers (notably
build_resource_cards) to pass the meta through; ensure the function signature
and usages (e.g., _resolve_meta(...)) are updated consistently so injected
metadata is honored.
In `@docs/scripts/generate_python_api.py`:
- Around line 772-779: The code currently deduplicates by short symbol name via
ctx.seen_names and `name`, which collapses distinct symbols from different
modules; change the dedupe key to the resolved/qualified target instead. Replace
checks against ctx.seen_names/name with a dedupe set (e.g., ctx.seen_targets)
and add a fully-qualified identifier such as full_path or a resolved identity
like f"{member.__module__}.{getattr(member,'__qualname__', name)}" (or
id(member) if necessary); then use that key for the membership test and
insertion before appending to groups so _classify_member, groups,
SUB_GROUP_ORDER and full_path stay the same but collisions only occur for true
re-exports.
In `@docs/scripts/generate_rust_api.py`:
- Around line 41-51: The hard-coded DISPLAYED_CRATES set (and the similar list
at the later occurrence) silently omits any newly published workspace crate;
update generate_rust_api.py to derive the displayed crate list from Cargo
metadata (e.g., run `cargo metadata` and collect workspace packages and their
publish status) or, if you must keep a manual allowlist (DISPLAYED_CRATES), add
an explicit validation step that compares the allowlist against the set of
published workspace crates and raises an error (or exits non‑zero) when any
published crate is missing; reference and update the DISPLAYED_CRATES symbol and
the corresponding check around the later occurrence so the script either
auto-populates the list from cargo metadata or fails fast when it detects a
missing published crate.
In `@lib/bindings/python/src/dynamo/health_check.py`:
- Around line 34-43: The doctest example for load_health_check_from_env leaves
the DYN_HEALTH_CHECK_PAYLOAD environment variable set which can leak state into
other tests; update the example to delete the environment variable at the end
(use del os.environ["DYN_HEALTH_CHECK_PAYLOAD"]) so the environment is cleaned
up after the example runs and does not affect subsequent doctests or code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2563444-f219-4f88-9ddf-8f7111b65dc8
📒 Files selected for processing (32)
.github/workflows/fern-docs.ymlcomponents/src/dynamo/common/configuration/arg_group.pycomponents/src/dynamo/common/configuration/config_base.pycomponents/src/dynamo/common/configuration/groups/kv_router_args.pycomponents/src/dynamo/common/configuration/utils.pycomponents/src/dynamo/common/constants.pycomponents/src/dynamo/common/lora/manager.pycomponents/src/dynamo/common/storage.pycomponents/src/dynamo/frontend/frontend_args.pycomponents/src/dynamo/mocker/main.pycomponents/src/dynamo/planner/defaults.pycomponents/src/dynamo/planner/global_planner_connector.pycomponents/src/dynamo/planner/kubernetes_connector.pycomponents/src/dynamo/planner/planner_connector.pycomponents/src/dynamo/planner/remote_planner_client.pycomponents/src/dynamo/planner/scale_protocol.pycomponents/src/dynamo/planner/virtual_connector.pycomponents/src/dynamo/router/args.pydeploy/operator/docs/header.mddocs/api/python/README.mddocs/api/rust/README.mddocs/scripts/_fern_helpers.pydocs/scripts/fernify_k8s_api.pydocs/scripts/fernify_python_api.pydocs/scripts/fernify_rust_api.pydocs/scripts/generate_python_api.pydocs/scripts/generate_rust_api.pydocs/scripts/requirements-apidocs.txtlib/bindings/python/src/dynamo/_core.pyilib/bindings/python/src/dynamo/health_check.pylib/bindings/python/src/dynamo/logits_processing/base.pylib/bindings/python/src/dynamo/runtime/__init__.py
- Add SPDX header to requirements-apidocs.txt (copyright-checks CI fix) - Revert header.md to original; inject Fern frontmatter via fernify_k8s_api.py instead (Operator CI fix: eliminates api-reference.md drift) - Fix slugify() to strip punctuation for correct heading anchors - Fix _source_link() to use repo-root path for valid GitHub URLs - Preserve explicit None defaults in generated signatures and tables - Include *.mdx in Rust API link rewrite during Fern release build - Add backtick_api_versions() and _inject_frontmatter() to fernify pipeline - Fix _RESOURCE_SECTION_RE regex to capture all resource card links - Add API Reference section (Python, Rust, K8s CRD) to Fern sidebar - Fix 10 docstring accuracy issues: undefined vars, top-level await, response_model claim, ModelType enum names, env var leaks Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
dagil-nvidia
force-pushed
the
dagil-nvidia/api-reference-docs
branch
from
March 6, 2026 17:03
2eab055 to
96a050c
Compare
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/fern-docs.yml (1)
165-173: Consider generating the Python/Rust README inputs before fernifying.Both fernify scripts read
docs/api/*/README.md; this job currently only transforms whatever snapshot was committed. Running the generators here would make preview/publish resilient to missed README refreshes when generator logic or docstrings change.💡 Suggested workflow change
- name: Install API doc dependencies run: pip install -r source-checkout/docs/scripts/requirements-apidocs.txt + - name: Generate API reference docs + working-directory: source-checkout + run: | + python3 docs/scripts/generate_python_api.py + python3 docs/scripts/generate_rust_api.py + - name: Fernify API reference docs working-directory: source-checkout run: | python3 docs/scripts/fernify_python_api.py python3 docs/scripts/fernify_rust_api.py python3 docs/scripts/fernify_k8s_api.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/fern-docs.yml around lines 165 - 173, The workflow runs fernify scripts that consume docs/api/*/README.md but doesn't regenerate those READMEs; add steps before the fernify_* calls to run the README generator scripts so the fernify_python_api.py, fernify_rust_api.py and fernify_k8s_api.py always operate on fresh inputs. Specifically, after installing dependencies and before invoking fernify_python_api.py/fernify_rust_api.py/fernify_k8s_api.py, invoke the README generator commands that produce docs/api/*/README.md (the project’s Python/Rust/K8s README generator scripts under docs/scripts) so the fernify scripts use up-to-date README outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/src/dynamo/common/storage.py`:
- Around line 49-57: Update the doc examples using get_fs to avoid hard-coded
/tmp paths: replace the platform-specific "file:///tmp/media" example with
either a platform-neutral placeholder (e.g., "file:///path/to/media") or show
how to construct a temp directory using Python's tempfile (referencing get_fs in
the examples), and similarly update the later example range (lines 94-99) so all
sample file paths are portable across OSes.
- Around line 52-54: Update the docstring example for get_fs so it handles
fs.fs.protocol being either a string or a tuple (as LocalFileSystem.protocol may
return ('file','local')), matching the normalization used in the implementation;
change the example to normalize the protocol (e.g., coerce to a string by taking
the first element if it's a tuple) before asserting or displaying it so the docs
are accurate and consistent with the behavior in get_fs.
In `@docs/scripts/generate_python_api.py`:
- Around line 156-157: ctx.seen_names currently dedupes re-exports by the last
path segment only, causing unrelated symbols with the same short name to be
dropped; change the dedupe key to the symbol's full identity (e.g., module +
name or fully-qualified path) wherever ctx.seen_names is created/checked/updated
(references: the variable ctx.seen_names and the places that add/check it around
the current snippet and the later block ~775-780). Specifically, replace uses
that derive a key as the short name with a deterministic full identifier (such
as f"{module}.{name}" or a (module, name) tuple) and update the membership
checks and inserts to use that full identifier so only true duplicates are
suppressed.
- Around line 838-846: The current loop over modules swallows load/parsing
errors (in the for module_name in modules loop calling loader.load(module_name))
by printing and continuing, allowing generation to succeed with missing modules;
instead, collect failed module names/errors in a list (e.g., failed_modules or
load_errors) when the except Exception as exc block runs, continue iterating to
gather all failures, and after the loop but before calling
loader.resolve_aliases() (and any rendering/export steps) raise a single
exception or sys.exit(1) that includes the aggregated failure details so the
process fails fast and CI cannot publish an incomplete API page.
---
Nitpick comments:
In @.github/workflows/fern-docs.yml:
- Around line 165-173: The workflow runs fernify scripts that consume
docs/api/*/README.md but doesn't regenerate those READMEs; add steps before the
fernify_* calls to run the README generator scripts so the
fernify_python_api.py, fernify_rust_api.py and fernify_k8s_api.py always operate
on fresh inputs. Specifically, after installing dependencies and before invoking
fernify_python_api.py/fernify_rust_api.py/fernify_k8s_api.py, invoke the README
generator commands that produce docs/api/*/README.md (the project’s
Python/Rust/K8s README generator scripts under docs/scripts) so the fernify
scripts use up-to-date README outputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a35aab2d-5445-4ffb-be64-ea914dd06f9c
📒 Files selected for processing (15)
.github/workflows/fern-docs.ymlcomponents/src/dynamo/common/configuration/groups/kv_router_args.pycomponents/src/dynamo/common/lora/manager.pycomponents/src/dynamo/common/storage.pycomponents/src/dynamo/planner/kubernetes_connector.pycomponents/src/dynamo/planner/remote_planner_client.pydocs/api/python/README.mddocs/index.ymldocs/scripts/_fern_helpers.pydocs/scripts/fernify_k8s_api.pydocs/scripts/generate_python_api.pydocs/scripts/requirements-apidocs.txtlib/bindings/python/src/dynamo/_core.pyilib/bindings/python/src/dynamo/health_check.pylib/bindings/python/src/dynamo/runtime/__init__.py
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/bindings/python/src/dynamo/health_check.py
- components/src/dynamo/planner/kubernetes_connector.py
- components/src/dynamo/common/configuration/groups/kv_router_args.py
- docs/scripts/_fern_helpers.py
- components/src/dynamo/common/lora/manager.py
- lib/bindings/python/src/dynamo/_core.pyi
- docs/scripts/requirements-apidocs.txt
- lib/bindings/python/src/dynamo/runtime/init.py
- components/src/dynamo/planner/remote_planner_client.py
Comment on lines
+49
to
+57
| Examples: | ||
| >>> from dynamo.common.storage import get_fs | ||
| >>> | ||
| >>> fs = get_fs("file:///tmp/media") | ||
| >>> fs.fs.protocol | ||
| 'file' | ||
| >>> fs = get_fs("s3://my-bucket") | ||
| >>> fs.path | ||
| 'my-bucket' |
Contributor
There was a problem hiding this comment.
Avoid /tmp in the published examples.
file:///tmp/media is Unix-specific, so these generated docs become misleading on Windows and other non-/tmp environments. If the snippet is meant to be runnable, use tempfile; if it is illustrative only, switch to a platform-neutral placeholder path.
♻️ Proposed doc tweak
- >>> fs = get_fs("file:///tmp/media")
+ >>> fs = get_fs("file:///path/to/media")
@@
- >>> fs = get_fs("file:///tmp/media")
+ >>> fs = get_fs("file:///path/to/media")
@@
- 'file:///tmp/media/videos/req-123.mp4'
+ 'file:///path/to/media/videos/req-123.mp4'Based on learnings: hard-coded constants that reduce portability, including temporary paths, should be replaced with portable alternatives such as Python's tempfile module.
Also applies to: 94-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/src/dynamo/common/storage.py` around lines 49 - 57, Update the doc
examples using get_fs to avoid hard-coded /tmp paths: replace the
platform-specific "file:///tmp/media" example with either a platform-neutral
placeholder (e.g., "file:///path/to/media") or show how to construct a temp
directory using Python's tempfile (referencing get_fs in the examples), and
similarly update the later example range (lines 94-99) so all sample file paths
are portable across OSes.
Comment on lines
+52
to
+54
| >>> fs = get_fs("file:///tmp/media") | ||
| >>> fs.fs.protocol | ||
| 'file' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and inspect the storage.py file
fd storage.py --type fRepository: ai-dynamo/dynamo
Length of output: 150
🏁 Script executed:
# Check the structure to understand the file path
find . -name "storage.py" -path "*/dynamo/common/*"Repository: ai-dynamo/dynamo
Length of output: 101
🏁 Script executed:
# Look for requirements/dependencies files
fd -e txt -e toml -e cfg -e lock | grep -E '(requirements|pyproject|setup|poetry)' | head -20Repository: ai-dynamo/dynamo
Length of output: 384
🏁 Script executed:
# Read the storage.py file to see the relevant lines
cat -n components/src/dynamo/common/storage.py | head -120Repository: ai-dynamo/dynamo
Length of output: 4804
🏁 Script executed:
# Check main pyproject.toml for fsspec dependency
cat pyproject.toml | grep -A 10 -B 5 fsspecRepository: ai-dynamo/dynamo
Length of output: 42
🏁 Script executed:
# Check requirements files for fsspec
grep fsspec container/deps/requirements.txt container/deps/requirements.standard.txt pyproject.toml 2>/dev/null || echo "Not found in checked files"Repository: ai-dynamo/dynamo
Length of output: 86
🏁 Script executed:
# Search all files for fsspec version constraint
rg "fsspec" --type toml --type txt | grep -i versionRepository: ai-dynamo/dynamo
Length of output: 42
🏁 Script executed:
# Check if there's a lockfile or version specification
cat pyproject.toml | head -100Repository: ai-dynamo/dynamo
Length of output: 2896
🏁 Script executed:
# Check for any lock files
fd -e lock | head -20Repository: ai-dynamo/dynamo
Length of output: 162
🏁 Script executed:
# Search for fsspec imports and usage in the test file
cat -n components/src/dynamo/common/tests/test_storage.py | head -80Repository: ai-dynamo/dynamo
Length of output: 3553
🌐 Web query:
fsspec LocalFileSystem protocol attribute return type - does it return string or tuple/list
💡 Result:
In fsspec, AbstractFileSystem.protocol is defined as:
ClassVar[str | tuple[str, ...]]— i.e., it may be a string or a tuple of strings. [2]
For LocalFileSystem specifically, it sets:
protocol = "file", "local"which is a tuple of two strings (("file", "local")). [1]
Fix docstring example to handle fs.fs.protocol as either string or tuple.
The docstring example at lines 52-54 assumes fs.fs.protocol returns the string 'file', but LocalFileSystem.protocol actually returns a tuple ('file', 'local') in current fsspec versions. Lines 109-111 already normalize this in the actual implementation. The docstring example should do the same to avoid confusion and ensure generated documentation is accurate.
Proposed fix
- >>> fs.fs.protocol
- 'file'
+ >>> protocol = fs.fs.protocol[0] if isinstance(fs.fs.protocol, (list, tuple)) else fs.fs.protocol
+ >>> protocol
+ 'file'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| >>> fs = get_fs("file:///tmp/media") | |
| >>> fs.fs.protocol | |
| 'file' | |
| >>> fs = get_fs("file:///tmp/media") | |
| >>> protocol = fs.fs.protocol[0] if isinstance(fs.fs.protocol, (list, tuple)) else fs.fs.protocol | |
| >>> protocol | |
| 'file' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/src/dynamo/common/storage.py` around lines 52 - 54, Update the
docstring example for get_fs so it handles fs.fs.protocol being either a string
or a tuple (as LocalFileSystem.protocol may return ('file','local')), matching
the normalization used in the implementation; change the example to normalize
the protocol (e.g., coerce to a string by taking the first element if it's a
tuple) before asserting or displaying it so the docs are accurate and consistent
with the behavior in get_fs.
Comment on lines
+156
to
+157
| seen_names: set[str] = field(default_factory=set) | ||
|
|
Contributor
There was a problem hiding this comment.
Deduplicate re-exports by symbol identity, not by short name.
ctx.seen_names is keyed on the last path segment only, so a later public Foo from another module disappears even when it is unrelated to the earlier Foo. That is broader than “dedupe re-exports” and can silently drop legitimate API entries.
Also applies to: 775-780
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/scripts/generate_python_api.py` around lines 156 - 157, ctx.seen_names
currently dedupes re-exports by the last path segment only, causing unrelated
symbols with the same short name to be dropped; change the dedupe key to the
symbol's full identity (e.g., module + name or fully-qualified path) wherever
ctx.seen_names is created/checked/updated (references: the variable
ctx.seen_names and the places that add/check it around the current snippet and
the later block ~775-780). Specifically, replace uses that derive a key as the
short name with a deterministic full identifier (such as f"{module}.{name}" or a
(module, name) tuple) and update the membership checks and inserts to use that
full identifier so only true duplicates are suppressed.
Comment on lines
+838
to
+846
| for module_name in modules: | ||
| try: | ||
| loader.load(module_name) | ||
| except Exception as exc: | ||
| print(f" SKIP {module_name}: {exc}", file=sys.stderr) | ||
| continue | ||
| print(f" Loaded: {module_name}") | ||
|
|
||
| loader.resolve_aliases() |
Contributor
There was a problem hiding this comment.
Don't succeed after skipping a whitelisted module.
A load/parsing failure currently becomes a warning and the generator keeps going, which means CI can publish an incomplete API page while still exiting 0. Please accumulate failures and raise before alias resolution/rendering instead.
🛠️ Suggested fail-fast pattern
def render_consolidated_page() -> str:
"""Render the single consolidated Python API reference page."""
modules = _discover_modules()
print(f"Loading modules ({len(modules)} discovered)...")
loader = _create_loader()
+ failures: list[str] = []
for module_name in modules:
try:
loader.load(module_name)
except Exception as exc:
- print(f" SKIP {module_name}: {exc}", file=sys.stderr)
+ failures.append(f"{module_name}: {exc}")
continue
print(f" Loaded: {module_name}")
+ if failures:
+ raise RuntimeError("Failed to load API modules:\n" + "\n".join(failures))
+
loader.resolve_aliases()🧰 Tools
🪛 Ruff (0.15.4)
[warning] 841-841: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/scripts/generate_python_api.py` around lines 838 - 846, The current loop
over modules swallows load/parsing errors (in the for module_name in modules
loop calling loader.load(module_name)) by printing and continuing, allowing
generation to succeed with missing modules; instead, collect failed module
names/errors in a list (e.g., failed_modules or load_errors) when the except
Exception as exc block runs, continue iterating to gather all failures, and
after the loop but before calling loader.resolve_aliases() (and any
rendering/export steps) raise a single exception or sys.exit(1) that includes
the aggregated failure details so the process fails fast and CI cannot publish
an incomplete API page.
Move dynamo.llm before dynamo._core in MODULE_ORDER so LLM-related classes are documented under their public import path. Users import from dynamo.llm, not dynamo._core, so the docs should reflect that. Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
Move frontmatter (sidebar-title, max-toc-depth) into the operator header template so it survives regeneration. Simplify fernify script by removing _inject_frontmatter in favor of a heading rename. Move Blog section into hidden pages and reorder API Reference above Documentation. Bump Fern SDK to 4.15.0. Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
- Replace /tmp with /data in storage.py docstring examples for portability - Handle fs.fs.protocol as string or tuple in get_fs example - Deduplicate re-exports by fully-qualified path instead of short name - Fail fast when whitelisted modules fail to load in API generator Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
dagil-nvidia
added a commit
that referenced
this pull request
Mar 7, 2026
Add Google-style docstrings to 21 Python source files across components/ and lib/bindings/. Docstrings were generated using Claude Opus 4.6 by comparing Python binding stubs (.pyi) against Rust source implementations for accuracy. Covers: configuration, constants, storage, frontend args, planner connectors, router args, runtime bindings, health checks, and logits processing. Part 1 of 3 (split from #6989): 1. Docstrings (this PR) 2. API reference generators + navigation 3. Fernify transform scripts + CI Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
3 tasks
dagil-nvidia
added a commit
that referenced
this pull request
Mar 7, 2026
Add static-analysis-based API reference generation for Python and Rust: - generate_python_api.py: uses griffe to parse Python source and produce a consolidated Markdown reference with module/class/function docs - generate_rust_api.py: generates a crate overview linking to docs.rs - requirements-apidocs.txt: pip dependencies for the generators Generated output: - docs/api/python/README.md (5940 lines, auto-generated) - docs/api/rust/README.md (crate table with docs.rs links) Navigation and config: - docs/index.yml: reorder API Reference above Documentation, hide Blog - fern/fern.config.json: bump SDK version 3.73.0 -> 4.15.0 - deploy/operator/docs/header.md: add sidebar-title and max-toc-depth - docs/kubernetes/api-reference.md: add frontmatter from header Part 2 of 3 (split from #6989): 1. Docstrings (#7056) 2. API reference generators + navigation (this PR) 3. Fernify transform scripts + CI Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
5 tasks
dagil-nvidia
added a commit
that referenced
this pull request
Mar 7, 2026
Add fernify transform scripts that convert generated API reference Markdown into Fern MDX components for the documentation site: - _fern_helpers.py: shared transforms (details->Accordion, tables->Cards, slugify, frontmatter injection) - fernify_python_api.py: transforms Python API reference - fernify_rust_api.py: transforms Rust API reference - fernify_k8s_api.py: transforms K8s CRD reference (wraps API groups in Tabs, resource lists in CardGroups, type defs in Accordions) CI integration (.github/workflows/fern-docs.yml): - Add Python setup + pip install for fernify dependencies - Run all 3 fernify scripts before syncing to docs-website - Pin Rust docs.rs links to release version on tag pushes Part 3 of 3 (split from #6989): 1. Docstrings (#7056) 2. API reference generators + navigation (#7057) 3. Fernify transform scripts + CI (this PR) Signed-off-by: Dan Gil <dagil@nvidia.com> Made-with: Cursor
7 tasks
dagil-nvidia
marked this pull request as draft
March 11, 2026 15:21
Collaborator
Author
|
Closing as stale. This work has been untouched for ~2 months and is superseded or no longer prioritized. Reopen if we pick it back up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a two-stage API documentation pipeline with auto-generated docstrings:
Docstring Generation (Claude Opus 4.6)
.pyifiles inlib/bindings/python/src/dynamo/) against the corresponding Rust source implementationdynamo.llm,dynamo.runtime,dynamo._core,dynamo.planner,dynamo.router,dynamo.mocker,dynamo.frontend, and configuration modulesPython API Generator (griffe)
generate_python_api.pyuses griffe to statically parse the Python source tree -- no import or build requiredGriffeLoaderwalkscomponents/src/andlib/bindings/python/src/with configurable search pathsdocstring_parser="auto"to auto-detect docstring style (Google, NumPy, Sphinx) per docstring, with a workaround to strip raw NumPy-style parameter blocks that the auto parser doesn't fully handleDocstringSectionKind.parameters,.returns,.examples,.raises,.attributes)<details>/<summary>blocks, parameter tables, and fenced code examplesseen_namesacross modules so items likeLlmEngineappear only at their canonical import path (dynamo.llm), not in internal submodulesdocs/api/python/README.md(5934 lines, 278 items across 11 modules) that renders correctly on GitHub without any Fern dependencyRust API Generator
generate_rust_api.pydiscovers published crates fromCargo.tomlworkspace members and links to docs.rsFernify Transform Layer
fernify_*.pyscripts convert the committed GitHub-friendly Markdown into Fern MDX components (CardGroup, Accordion, Tabs) during CI -- the committed docs remain plain Markdown_fern_helpers.pyprovide composable text-to-text transformsv1alpha1,v1beta1,OperatorConfiguration) with accordion sectionsfern/convert_callouts.pyhandles admonition syntax conversionNavigation and Layout
sidebar-title,max-toc-depth) to K8s CRD header templateFiles Changed (32)
generate_python_api.py,generate_rust_api.py,_fern_helpers.py,fernify_python_api.py,fernify_rust_api.py,fernify_k8s_api.py,requirements-apidocs.txtdocs/api/python/README.md(5934 lines),docs/api/rust/README.md.github/workflows/fern-docs.ymldeploy/operator/docs/header.mdTest Plan
python3 docs/scripts/generate_python_api.pyproducesdocs/api/python/README.mdwith 63+ code fences, 0 confidence comments, 0 duplicate entriespython3 docs/scripts/generate_rust_api.pyproducesdocs/api/rust/README.mdpython3 docs/scripts/fernify_python_api.pytransforms to Fern MDX with CardGroup, Accordion, Warning componentspython3 docs/scripts/fernify_rust_api.pytransforms to Fern MDXpython3 docs/scripts/fernify_k8s_api.pytransforms K8s docs with tabs and accordionsdocs/api/python/README.mdanddocs/api/rust/README.mdcorrectly (tables, details/summary, code fences)