Skip to content

docs: add docstrings to Python components and bindings - #7056

Closed
dagil-nvidia wants to merge 4 commits into
mainfrom
dagil-nvidia/api-docstrings
Closed

docs: add docstrings to Python components and bindings#7056
dagil-nvidia wants to merge 4 commits into
mainfrom
dagil-nvidia/api-docstrings

Conversation

@dagil-nvidia

@dagil-nvidia dagil-nvidia commented Mar 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • 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

Hallucination Fixes

The initial AI-generated docstrings included hallucinated behavioral changes (field renames, new fields, function deletions) disguised as documentation. These were identified via CodeRabbit review and git diff against origin/main, then systematically reverted:

  • frontend_args.py — reverted rename of enforce_disaggdecode_fallback and removal of the added debug_perf field
  • mocker/main.py — restored deleted _build_runtime_config() function, socket import, and ModelRuntimeConfig import
  • _core.pyi — restored removed fields (data_parallel_size, bootstrap_host), methods (set_tensor_model_config), and parameters (media_decoder, enforce_disagg)

Only pure docstring additions remain in the final diff.

PR Series (split from #6989)

Merge in order:

  1. Docstringsdocs: add docstrings to Python components and bindings #7056 (this PR)
  2. API reference generators + navigationdocs: add API reference generators and navigation #7057
  3. Fernify transform scripts + CIdocs: add Fern MDX transform scripts and CI integration #7058

Test Plan

  • Pre-commit hooks pass (isort, black, flake8, ruff)
  • No behavior changes — docstrings only
  • Verify docstrings render correctly in IDE hover tooltips

Summary by CodeRabbit

Release Notes

  • New Features

    • Added KvIndexer public API with match-finding and block-size query methods
    • Added routing decision processing for ApproxKvIndexer
    • Added JSON serialization/deserialization methods for ModelDeploymentCard
    • Added new NIXL_READ embedding transfer mode option
  • Documentation

    • Enhanced documentation with comprehensive usage examples across configuration, planner, router, and storage components

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
@dagil-nvidia
dagil-nvidia requested a review from a team as a code owner March 7, 2026 05:08
@dagil-nvidia
dagil-nvidia requested a review from a team March 7, 2026 05:08
@github-actions github-actions Bot added docs planner frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` router Relates to routing, KV-aware routing, etc. labels Mar 7, 2026
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
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
@coderabbitai

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (2)
lib/bindings/python/src/dynamo/_core.pyi (1)

1230-1244: ⚠️ Potential issue | 🔴 Critical

Critical: register_model signature missing media_decoder and media_fetcher parameters.

The register_model function signature does not include media_decoder and media_fetcher parameters, but components/src/dynamo/vllm/main.py at lines 591-592 passes these as keyword arguments:

media_decoder=media_decoder,
media_fetcher=media_fetcher,

This will cause TypeError: register_model() got an unexpected keyword argument 'media_decoder' at runtime.

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

In `@lib/bindings/python/src/dynamo/_core.pyi` around lines 1230 - 1244, The
register_model stub in _core.pyi is missing the media_decoder and media_fetcher
parameters expected by callers; update the async def register_model(...)
signature to accept media_decoder and media_fetcher (e.g., media_decoder:
Optional[Callable[..., Any]] = None, media_fetcher: Optional[Callable[..., Any]]
= None) while preserving other params and return type None, and ensure the
concrete implementation of register_model (the function referenced by
components/src/dynamo/vllm/main.py) is updated to accept and forward these
kwargs as well so calls using media_decoder= and media_fetcher= no longer raise
TypeError.
lib/bindings/python/src/dynamo/runtime/__init__.py (1)

111-113: ⚠️ Potential issue | 🔴 Critical

Replace deprecated Pydantic v1 methods with v2 equivalents.

Lines 111-113 use parse_raw() and parse_obj(), which are deprecated in Pydantic v2 that the project requires (pydantic>=2.0). Replace with model_validate_json() and model_validate():

Replacement diff
- args_list[-1] = request_model.parse_raw(args[-1])
+ args_list[-1] = request_model.model_validate_json(args[-1])
- args_list[-1] = request_model.parse_obj(args[-1])
+ args_list[-1] = request_model.model_validate(args[-1])

Similar deprecated calls exist in examples/multimodal/utils/chat_processor.py and components/src/dynamo/vllm/multimodal_utils/chat_processor.py that should also be updated.

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

In `@lib/bindings/python/src/dynamo/runtime/__init__.py` around lines 111 - 113,
Replace deprecated Pydantic v1 calls by using the v2 validation APIs: in
dynamo.runtime.__init__.py update the code that currently does
request_model.parse_raw(...) and request_model.parse_obj(...) to use
request_model.model_validate_json(...) for raw JSON strings and
request_model.model_validate(...) for dicts; apply the same substitutions in the
other occurrences inside examples/multimodal/utils/chat_processor.py and
components/src/dynamo/vllm/multimodal_utils/chat_processor.py so all parse_raw
-> model_validate_json and parse_obj -> model_validate conversions are
consistent with pydantic>=2.0.
🧹 Nitpick comments (1)
lib/bindings/python/src/dynamo/runtime/__init__.py (1)

119-126: Dead code: response validation try/except block.

The try/except ValidationError block at lines 123-126 is ineffective because yield item cannot raise ValidationError. The TODO comment at line 122 acknowledges this is placeholder code. Consider removing the dead try/except or implementing actual response validation.

♻️ Suggested simplification
             # Wrap the async generator
             async for item in func(*args_list, **kwargs):
                 # Validate the response
                 # TODO: Validate the response
-                try:
-                    yield item
-                except ValidationError as e:
-                    raise ValueError(f"Invalid response: {e}")
+                yield item
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/bindings/python/src/dynamo/runtime/__init__.py` around lines 119 - 126,
The try/except around "yield item" is dead code because yield cannot raise
ValidationError; remove the pointless try/except (and the surrounding TODO) from
the async generator that iterates over func(*args_list, **kwargs) in
__init__.py, or alternatively implement real validation by calling the
appropriate validator on each item before yielding (e.g., call a
validate_response(item) that can raise ValidationError and catch/convert it to
ValueError), referencing the async wrapper that currently does "async for item
in func(*args_list, **kwargs):" and the ValidationError handling block.
🤖 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/configuration/groups/kv_router_args.py`:
- Around line 41-56: The docstring example for KvRouterConfigBase is misleading
because KvRouterConfigBase only defines annotations and MyRouterConfig() won’t
populate the 16 KV router fields, causing kv_router_kwargs() to raise
AttributeError when getattr is called; update the example to show constructing
the config via the CLI parsing path (or explicitly setting the required KV
fields on the instance) and reference KvRouterConfigBase, MyRouterConfig, and
kv_router_kwargs in the example so the created instance contains the required
fields before calling kv_router_kwargs().

In `@components/src/dynamo/common/storage.py`:
- Around line 49-58: The doctest examples in get_fs and get_media_url are not
hermetic: they reference absolute filesystem paths and optional s3 dependencies;
update the examples in storage.py (references: get_fs, get_media_url) to either
mark them with doctest skip markers (e.g. add " # doctest: +SKIP") or replace
them with side-effect-free local fixtures (temp directories or an
in-memory/local fsspec backend) similar to the existing upload_to_fs example
which already uses "# doctest: +SKIP". Ensure both the file:/// and s3://
examples are handled so CI tests won't require external paths or optional
dependencies.

In `@components/src/dynamo/frontend/frontend_args.py`:
- Around line 225-236: Update the test helper so it uses the new
--decode-fallback flag and inverted semantics: rename or replace the helper
parameter enforce_disagg to decode_fallback (or add decode_fallback alongside
it), change the command-line argument appended from "--enforce-disagg" to
"--decode-fallback", and invert the boolean mapping (previous
enforce_disagg=True should produce decode_fallback=False). Also update the test
invocation that currently passes enforce_disagg=True to pass
decode_fallback=False so tests reflect the new flag behavior.
- Line 72: main.py still passes the old parameter name enforce_disagg to
RouterConfig causing an AttributeError; update the RouterConfig instantiation in
main.py (where RouterConfig(...) is called around the previous line 200) to use
decode_fallback=config.decode_fallback instead of
enforce_disagg=config.enforce_disagg so the argument name matches the renamed
field in frontend_args.py and RouterConfig.__init__.

---

Outside diff comments:
In `@lib/bindings/python/src/dynamo/_core.pyi`:
- Around line 1230-1244: The register_model stub in _core.pyi is missing the
media_decoder and media_fetcher parameters expected by callers; update the async
def register_model(...) signature to accept media_decoder and media_fetcher
(e.g., media_decoder: Optional[Callable[..., Any]] = None, media_fetcher:
Optional[Callable[..., Any]] = None) while preserving other params and return
type None, and ensure the concrete implementation of register_model (the
function referenced by components/src/dynamo/vllm/main.py) is updated to accept
and forward these kwargs as well so calls using media_decoder= and
media_fetcher= no longer raise TypeError.

In `@lib/bindings/python/src/dynamo/runtime/__init__.py`:
- Around line 111-113: Replace deprecated Pydantic v1 calls by using the v2
validation APIs: in dynamo.runtime.__init__.py update the code that currently
does request_model.parse_raw(...) and request_model.parse_obj(...) to use
request_model.model_validate_json(...) for raw JSON strings and
request_model.model_validate(...) for dicts; apply the same substitutions in the
other occurrences inside examples/multimodal/utils/chat_processor.py and
components/src/dynamo/vllm/multimodal_utils/chat_processor.py so all parse_raw
-> model_validate_json and parse_obj -> model_validate conversions are
consistent with pydantic>=2.0.

---

Nitpick comments:
In `@lib/bindings/python/src/dynamo/runtime/__init__.py`:
- Around line 119-126: The try/except around "yield item" is dead code because
yield cannot raise ValidationError; remove the pointless try/except (and the
surrounding TODO) from the async generator that iterates over func(*args_list,
**kwargs) in __init__.py, or alternatively implement real validation by calling
the appropriate validator on each item before yielding (e.g., call a
validate_response(item) that can raise ValidationError and catch/convert it to
ValueError), referencing the async wrapper that currently does "async for item
in func(*args_list, **kwargs):" and the ValidationError handling block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fa9750a8-c0dc-4509-abbc-d2977f867d59

📥 Commits

Reviewing files that changed from the base of the PR and between f1dbea4 and dc04cba.

📒 Files selected for processing (21)
  • components/src/dynamo/common/configuration/arg_group.py
  • components/src/dynamo/common/configuration/config_base.py
  • components/src/dynamo/common/configuration/groups/kv_router_args.py
  • components/src/dynamo/common/configuration/utils.py
  • components/src/dynamo/common/constants.py
  • components/src/dynamo/common/lora/manager.py
  • components/src/dynamo/common/storage.py
  • components/src/dynamo/frontend/frontend_args.py
  • components/src/dynamo/mocker/main.py
  • components/src/dynamo/planner/defaults.py
  • components/src/dynamo/planner/global_planner_connector.py
  • components/src/dynamo/planner/kubernetes_connector.py
  • components/src/dynamo/planner/planner_connector.py
  • components/src/dynamo/planner/remote_planner_client.py
  • components/src/dynamo/planner/scale_protocol.py
  • components/src/dynamo/planner/virtual_connector.py
  • components/src/dynamo/router/args.py
  • lib/bindings/python/src/dynamo/_core.pyi
  • lib/bindings/python/src/dynamo/health_check.py
  • lib/bindings/python/src/dynamo/logits_processing/base.py
  • lib/bindings/python/src/dynamo/runtime/__init__.py

Comment thread components/src/dynamo/common/configuration/groups/kv_router_args.py
Comment thread components/src/dynamo/common/storage.py
Comment thread components/src/dynamo/frontend/frontend_args.py Outdated
Comment thread components/src/dynamo/frontend/frontend_args.py
- frontend_args.py: restore enforce_disagg field (was incorrectly
  renamed to decode_fallback), remove hallucinated debug_perf field
- mocker/main.py: restore deleted _build_runtime_config() function
  and original imports
- _core.pyi: restore removed fields (data_parallel_start_rank,
  bootstrap_host/port), methods (set/get_tensor_model_config),
  and parameters (media_decoder, media_fetcher, enforce_disagg)
- kv_router_args.py: add doctest +SKIP to example that requires
  CLI parsing
- storage.py: add doctest +SKIP to examples requiring filesystem
  access and optional S3 dependencies

Signed-off-by: Dan Gil <dagil@nvidia.com>
Made-with: Cursor
@dagil-nvidia

Copy link
Copy Markdown
Collaborator Author

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Mar 8, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7

♻️ Duplicate comments (1)
components/src/dynamo/common/storage.py (1)

133-141: ⚠️ Potential issue | 🟡 Minor

upload_to_fs() still leaves the S3 setup runnable.

Line 137 is its own doctest example, so the snippet still tries to initialize S3 before the skipped upload_to_fs() call. Mark that line with # doctest: +SKIP too, or switch the example to a local side-effect-free backend.

Suggested fix
-        >>> fs = get_fs("s3://my-media-bucket")
+        >>> fs = get_fs("s3://my-media-bucket")  # doctest: +SKIP
🤖 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 133 - 141, The doctest
is still executing S3 initialization because the line calling
get_fs("s3://my-media-bucket") is not skipped; update the example in
components/src/dynamo/common/storage.py so the S3 setup line is harmless in
doctests by either adding "# doctest: +SKIP" to the fs =
get_fs("s3://my-media-bucket") line or replace that line with a side-effect-free
backend (e.g., get_fs("memory://") or a local file backend) so only the intended
upload_to_fs call remains skipped.
🧹 Nitpick comments (2)
components/src/dynamo/mocker/main.py (1)

74-81: Keep this example purely CLI-based.

The docstring says the standard path is python -m dynamo.mocker, but the snippet below switches to uvloop.run(worker()) even though worker() still parses sys.argv. As written, it reads like a standalone Python example when it really depends on CLI state. I’d either keep just the CLI command or show the argv setup explicitly.

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

In `@components/src/dynamo/mocker/main.py` around lines 74 - 81, The Examples
section mixes CLI and programmatic invocation which is misleading because
worker() parses sys.argv; update the docstring so the example is purely
CLI-based or else show explicit argv setup: either keep only the "python -m
dynamo.mocker --model-path ..." line under Examples, or replace the
uvloop/worker() lines with an explicit statement that you must set sys.argv (or
use argparse) before calling worker(); reference the Examples block and the
worker() function when making the change.
lib/bindings/python/src/dynamo/runtime/__init__.py (1)

28-36: Example missing asyncio import.

The example uses asyncio.run(worker()) on line 36 but doesn't show the import asyncio statement. While the example is illustrative, consider adding the import for completeness.

📝 Suggested docstring fix
     Examples:
         >>> from dynamo.runtime import DistributedRuntime, dynamo_worker
+        >>> import asyncio
         >>>
         >>> `@dynamo_worker`()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/bindings/python/src/dynamo/runtime/__init__.py` around lines 28 - 36, The
example docstring is missing the asyncio import used by asyncio.run(worker());
update the example to show importing asyncio at top (so add an import asyncio
line before using asyncio.run) near the code that defines/uses
DistributedRuntime, dynamo_worker and worker to make the snippet self-contained
and clear.
🤖 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/planner/global_planner_connector.py`:
- Around line 32-43: The doctest example for GlobalPlannerConnector is invalid
because it uses top-level await and an undefined runtime; update the example in
the docstring to either (a) show an executable async usage by wrapping
initialization in asyncio.run (e.g., create an async helper that instantiates
GlobalPlannerConnector, calls await connector._async_init(), and then prints
connector.get_model_name()) and define or mock a minimal runtime object used to
construct the connector, or (b) mark the snippet as non-executable with a
doctest skip comment (# doctest: +SKIP) and keep the illustrative calls to
GlobalPlannerConnector, _async_init, and get_model_name as-is — choose one
approach and apply it consistently to the example.

In `@components/src/dynamo/planner/virtual_connector.py`:
- Around line 34-44: The doctest example for VirtualConnector uses top-level
await and an undefined runtime, making it non-runnable; update the example in
the docstring for VirtualConnector to either (a) remove the interactive doctest
prompts and present it as a plain code block with a note about async usage, or
(b) wrap async calls in a runner like asyncio.run(...) and ensure a mock or
example runtime is defined, and mark the doctest with "# doctest: +SKIP" if you
don’t want it executed; specifically update the snippet that calls
VirtualConnector(...), _async_init(), and
add_component(SubComponentType.PREFILL) to follow one of these patterns so it no
longer relies on top-level await or an undefined runtime.

In `@lib/bindings/python/src/dynamo/_core.pyi`:
- Around line 884-887: The example for HttpService references a method not
declared in the stub; update the stub so the public API matches the example or
change the example to use only declared members. Either add a declaration for
add_chat_completions_model(self, name: str, checksum: str, engine: Any) -> None
(or the correct signature) to the HttpService stub alongside __init__, run, and
shutdown, OR revise the Examples block to demonstrate usage with only __init__,
run, and shutdown; ensure the chosen fix keeps HttpService's public surface
consistent with the documentation and include the symbol names HttpService and
add_chat_completions_model in your change so reviewers can find it easily.
- Around line 795-798: The examples for KvEventPublisher (the zmq_endpoint
argument) and similar snippets embed a hard-coded port ("tcp://127.0.0.1:5557"),
which can collide in shared dev/CI; change these examples to use a dynamic or
symbolic port variable (e.g., allocate via the repo's port allocator or the
alloc_port library) and pass that into zmq_endpoint instead of a literal 5557,
updating all occurrences that construct zmq_endpoint (including the other
KvEventPublisher and related examples referenced) so examples remain portable
and collision-free.
- Around line 459-463: The example wrongly shows calling ModelDeploymentCard()
as a public no-arg constructor; remove that direct-constructor usage and update
the example to obtain a ModelDeploymentCard via the Rust-exposed factory-style
entry point instead (do not claim a no-arg ctor). Specifically, replace the line
that creates the card with a call to the appropriate factory used in the binding
(rather than ModelDeploymentCard()), and keep the existing references to
ModelDeploymentCard.to_json_str and ModelDeploymentCard.from_json_str so the
example demonstrates serialization/deserialization without implying a direct
constructor.

In `@lib/bindings/python/src/dynamo/health_check.py`:
- Around line 87-101: The doctest mutates the DYN_HEALTH_CHECK_PAYLOAD env var
without restoring it; update the example around
MyBackendHealthCheck/HealthCheckPayload to save the original value (e.g., orig =
os.environ.get("DYN_HEALTH_CHECK_PAYLOAD")), then pop the key for the test, and
finally restore the original value (set it back if orig is not None, otherwise
ensure it's removed) so the example is independent and matches the
save-and-restore pattern used by load_health_check_from_env().
- Around line 45-46: The doctest restore block places statement bodies on the
same line as the 'if'/'else', which breaks doctest syntax; change the two lines
to split the headers and bodies so each conditional ends with ':' on the '>>>'
line and the body is on the next indented '...' line—i.e., replace '>>> if _prev
is None: os.environ.pop(...)' with '>>> if _prev is None:' then '...    
os.environ.pop("DYN_HEALTH_CHECK_PAYLOAD", None)', and similarly replace the
'else' line with '>>> else:' then '...    
os.environ["DYN_HEALTH_CHECK_PAYLOAD"] = _prev' so the doctest for the
restoration block follows proper doctest indentation rules.

---

Duplicate comments:
In `@components/src/dynamo/common/storage.py`:
- Around line 133-141: The doctest is still executing S3 initialization because
the line calling get_fs("s3://my-media-bucket") is not skipped; update the
example in components/src/dynamo/common/storage.py so the S3 setup line is
harmless in doctests by either adding "# doctest: +SKIP" to the fs =
get_fs("s3://my-media-bucket") line or replace that line with a side-effect-free
backend (e.g., get_fs("memory://") or a local file backend) so only the intended
upload_to_fs call remains skipped.

---

Nitpick comments:
In `@components/src/dynamo/mocker/main.py`:
- Around line 74-81: The Examples section mixes CLI and programmatic invocation
which is misleading because worker() parses sys.argv; update the docstring so
the example is purely CLI-based or else show explicit argv setup: either keep
only the "python -m dynamo.mocker --model-path ..." line under Examples, or
replace the uvloop/worker() lines with an explicit statement that you must set
sys.argv (or use argparse) before calling worker(); reference the Examples block
and the worker() function when making the change.

In `@lib/bindings/python/src/dynamo/runtime/__init__.py`:
- Around line 28-36: The example docstring is missing the asyncio import used by
asyncio.run(worker()); update the example to show importing asyncio at top (so
add an import asyncio line before using asyncio.run) near the code that
defines/uses DistributedRuntime, dynamo_worker and worker to make the snippet
self-contained and clear.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ef16e11-81b3-44ca-8550-1c68d9b46fbf

📥 Commits

Reviewing files that changed from the base of the PR and between 6831020 and e851714.

📒 Files selected for processing (21)
  • components/src/dynamo/common/configuration/arg_group.py
  • components/src/dynamo/common/configuration/config_base.py
  • components/src/dynamo/common/configuration/groups/kv_router_args.py
  • components/src/dynamo/common/configuration/utils.py
  • components/src/dynamo/common/constants.py
  • components/src/dynamo/common/lora/manager.py
  • components/src/dynamo/common/storage.py
  • components/src/dynamo/frontend/frontend_args.py
  • components/src/dynamo/mocker/main.py
  • components/src/dynamo/planner/defaults.py
  • components/src/dynamo/planner/global_planner_connector.py
  • components/src/dynamo/planner/kubernetes_connector.py
  • components/src/dynamo/planner/planner_connector.py
  • components/src/dynamo/planner/remote_planner_client.py
  • components/src/dynamo/planner/scale_protocol.py
  • components/src/dynamo/planner/virtual_connector.py
  • components/src/dynamo/router/args.py
  • lib/bindings/python/src/dynamo/_core.pyi
  • lib/bindings/python/src/dynamo/health_check.py
  • lib/bindings/python/src/dynamo/logits_processing/base.py
  • lib/bindings/python/src/dynamo/runtime/__init__.py

Comment thread components/src/dynamo/planner/global_planner_connector.py
Comment thread components/src/dynamo/planner/virtual_connector.py Outdated
Comment thread lib/bindings/python/src/dynamo/_core.pyi Outdated
Comment thread lib/bindings/python/src/dynamo/_core.pyi Outdated
Comment thread lib/bindings/python/src/dynamo/_core.pyi Outdated
Comment thread lib/bindings/python/src/dynamo/health_check.py Outdated
Comment thread lib/bindings/python/src/dynamo/health_check.py
- kv_router_args: use CLI parsing path in example
- planner connectors: add doctest: +SKIP for top-level await
- _core.pyi: add SKIP markers for runtime-dependent examples
- health_check: fix multiline if/else doctest syntax

Signed-off-by: Dan Gil <dagil@nvidia.com>
Made-with: Cursor
... "--router-temperature", "0.5",
... ])
>>> args.overlap_score_weight
0.8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These don't need examples, they are trivial.

>>> DisaggregationMode.PREFILL.value
'prefill'
>>> DisaggregationMode("agg") == DisaggregationMode.AGGREGATED
True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, come on. It's an enum. Everyone looking at this code knows how to use an enum.

@grahamking grahamking left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like Claude added examples everywhere, even for the most trivial of types. Can we have it be smarter? Some of these are just noise and context pollution.

And some are really helpful!.

Maybe we can ask it to only add examples for types / functions that a senior engineer would find complex.

@dagil-nvidia
dagil-nvidia marked this pull request as draft March 12, 2026 21:14
@dagil-nvidia

Copy link
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` planner router Relates to routing, KV-aware routing, etc. size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants