refactor(planner): Clean up planner core, abstract out peripheral logic

[sachalmalick]

Overview:

Hello Hongkuan...

So this PR is the first step in my effort to clean up Planner Core and the logic around how we initiate and validate deployments.

The heart of this new design pattern is that we abstract all of what I call the "peripherals" outside of Planner Core. This is the trajectory that Planner was already headed with the connector interface, this takes it a step further by

a) Also abstracting out observation (FPM and Traffic metrics) behind a generic interface for a TrafficMetricsProvider and FpmProvider
b) Letting PlannerCore be a consumer of a new interface called PlannerEnvironment which lets Planner not have to own any specific connector logic at all!

PlannerCore, just needs to know a PlannerEnvironment will be provided to me which I can use to

1. Making scaling decisions
2. Know the current deployment state (number of workers and their configurations)
3. Ask for metrics it needs, Fpm or Traffic

So in addition to this being cleaner, this also sets up Planner long term for a wider "total addressable market" / wide adoption.

Planner is a brilliant state of the art prediction and decision engine, but it need not be environment specific at all.

Someone can go ahead and write your own FpmProvider that will just read events from the cloud if you want! Or a PlannerConnector that makes scaling actions through a generic API call rather than to the Kubernetes management API.

Related Issues

⚠️ This section is required. Choose one path below and delete the other.

🔗 This PR is linked to an issue:

• Closes #

🚫 This PR is NOT linked to an issue:

• Confirmed — no related issue

---

Summary by CodeRabbit

• Refactor

  • Restructured planner architecture to use protocol-based interfaces instead of abstract base classes for connectors.
  • Separated planner initialization concerns: deployment state management, metrics collection, and scaling operations now handled via a dedicated environment abstraction layer.
  • Refactored native planner core to delegate lifecycle and diagnostics to the planner environment.

• New Features

  • Added factory functions for constructing connectors, environments, and planners from configuration.
  • Introduced runtime namespace binding for dynamic worker endpoint resolution.

[copy-pr-bot]

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

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[devin-ai-integration]

Devin Review found 4 potential issues.

[coderabbitai]

Walkthrough

The PR introduces a PlannerEnvironment abstraction layer in the Dynamo planner. PlannerConnector and WorkerInfoProvider are converted from ABCs to Protocols with expanded method contracts. New DeploymentState dataclasses, PlannerEnvironment/RuntimeNamespaceSource protocols, PlannerEnvironmentImpl, PrometheusTrafficProvider, RuntimeFpmProvider, RuntimeNamespaceBinding, and a planner_factory module are added. NativePlannerBase is refactored to delegate all I/O to the environment, and adapter subclasses, connectors, and tests are updated accordingly.

Changes

Planner Environment Abstraction

Layer / File(s)	Summary
Connector protocol contracts and conforming implementations components/src/dynamo/planner/connectors/base.py, connectors/kubernetes.py, connectors/global_planner.py, connectors/virtual.py	PlannerConnector and WorkerInfoProvider are redefined as Protocols; KubernetesConnector and GlobalPlannerConnector add async_init and get_gpu_counts; VirtualConnector drops MDC parsing, gains injected WorkerInfoProvider, and adds get_actual_worker_counts/get_worker_runtime_namespace.
Environment state dataclasses, interface protocols, and change-detection utilities environment/state.py, environment/interface.py, environment/metrics_provider/interface.py, core/util.py	Defines ReplicaState, ComponentState, DeploymentState dataclasses; PlannerEnvironment, DeploymentStateSource, RuntimeNamespaceSource, TrafficMetricsProvider, FpmMetricsProvider protocols; and worker_info_changed/deployment_state_changed helpers.
Runtime namespace resolver and binding environment/runtime.py, connectors/runtime.py	Adds RuntimeNamespaceResolver protocol and RuntimeNamespaceBinding class that re-resolves the worker namespace with exception handling; adds a compatibility re-export shim in the connectors package.
Prometheus traffic provider and runtime FPM provider environment/metrics_provider/prometheus_traffic_provider.py, environment/metrics_provider/runtime_provider.py	Implements PrometheusTrafficProvider (queries Prometheus for TTFT, ITL, KV hit rate, accept length) and RuntimeFpmProvider (subscribes to FPM event streams, parses MDC model cards, implements get_worker_info and collect_fpm).
PlannerEnvironmentImpl: deployment state management and lifecycle environment/base.py	Implements PlannerEnvironmentImpl as an async facade over PlannerConnector: initializes/validates/waits-for-readiness, refreshes worker info, GPU counts with config fallbacks, replica stability flags, and model name; delegates traffic/FPM collection and scaling to providers.
Planner factory: connector, environment, and planner construction core/planner_factory.py	New construct_connector, construct_environment, and construct_planner factory functions wire the connector, PlannerEnvironmentImpl, PrometheusTrafficProvider, RuntimeNamespaceBinding, and RuntimeFpmProvider from PlannerConfig.
NativePlannerBase and adapter subclasses refactored to environment-driven core/base.py, core/adapters.py	NativePlannerBase now accepts PlannerEnvironment, delegates tick I/O (traffic, FPM, worker counts, GPU hours, scaling) to it, and uses deployment state change detection to refresh engine capabilities; adapter subclasses derive worker info via _required_worker_info() instead of cached attributes.
Tests updated for environment-backed planner and renamed public APIs tests/integration/test_virtual_connector.py, tests/unit/test_kube.py, tests/unit/test_load_based_scaling.py, tests/unit/test_metric_publication.py, tests/unit/test_remote_planner.py	Integration and unit tests updated to use PlannerEnvironmentImpl, the renamed async_init, environment-sourced deployment state/GPU counts, and AsyncMock for environment observation calls.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.88% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description provides a clear overview of the changes, explains the new design pattern, lists the three key abstractions introduced, and includes a fully completed 'Related Issues' section. All required template sections are present and complete.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title matches the main refactor: planner core cleanup and abstraction of peripheral logic.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 9

Caution

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

⚠️ Outside diff range comments (1)

components/src/dynamo/planner/connectors/global_planner.py (1)

129-131: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the stale initialization hint in the RuntimeError message.

Line 130 still instructs callers to invoke _async_init(), but the method is now async_init(). This makes failure guidance misleading.

Proposed fix

-                "GlobalPlannerConnector not initialized. Call _async_init() first."
+                "GlobalPlannerConnector not initialized. Call async_init() first."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/planner/connectors/global_planner.py` around lines 129
- 131, The RuntimeError message in the GlobalPlannerConnector class is providing
stale guidance by instructing callers to invoke _async_init(), but this method
has been renamed to async_init(). Update the error message string in the
RuntimeError to reference the correct method name async_init() instead of
_async_init() so that users receive accurate initialization instructions when
this error is raised.

🧹 Nitpick comments (3)

components/src/dynamo/planner/core/base.py (1)

152-154: ⚡ Quick win

Keep engine adapter import at module top.

Line 152 introduces a local import inside _ensure_engine, which conflicts with the repository Python rule for import placement.

As per coding guidelines, "keep all imports at module top (flag imports inside functions/classes)."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/planner/core/base.py` around lines 152 - 154, The
import of OrchestratorEngineAdapter inside the _ensure_engine function violates
the module-level import rule. Move the import statement from inside the
_ensure_engine function to the top of the file where other imports are declared,
placing it with the other dynamo.planner imports to maintain consistency with
the codebase conventions.

Source: Coding guidelines

components/src/dynamo/planner/core/planner_factory.py (1)

133-138: ⚡ Quick win

Move adapter imports to module scope.

Function-scoped imports in Line 133 violate the repository Python import placement rule.

As per coding guidelines, "keep all imports at module top (flag imports inside functions/classes)."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/planner/core/planner_factory.py` around lines 133 -
138, The imports of AggPlanner, DecodePlanner, DisaggPlanner, and PrefillPlanner
from dynamo.planner.core.adapters are currently located inside a function scope,
which violates the repository's import placement rules. Move these imports to
the top of the module file with the other imports at module scope to comply with
the coding guidelines that require all imports to be placed at the module level.

Source: Coding guidelines

components/src/dynamo/planner/environment/metrics_provider/runtime_provider.py (1)

132-133: ⚡ Quick win

Move function-local imports to module scope.

Please move these imports to the top-level import section to match repo Python rules.

As per coding guidelines: "keep all imports at module top (flag imports inside functions/classes)."

Proposed fix

 from typing import Optional
 
 from dynamo.common.forward_pass_metrics import ForwardPassMetrics
+from dynamo.common.forward_pass_metrics import decode as decode_fpm
+from dynamo.llm import FpmEventSubscriber
 from dynamo.planner.config.defaults import SubComponentType
@@
     async def _init_fpm_subscriber(self, component: str):
-        from dynamo.llm import FpmEventSubscriber
-
         if self.state_source is None or self.namespace_source is None:
@@
     def _decode_fpm_bytes(
         self, subscriber
     ) -> dict[tuple[str, int], ForwardPassMetrics]:
-        from dynamo.common.forward_pass_metrics import decode as decode_fpm
-
         if subscriber is None:

Also applies to: 180-181

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/src/dynamo/planner/environment/metrics_provider/runtime_provider.py`
around lines 132 - 133, Move the function-local import statement `from
dynamo.llm import FpmEventSubscriber` from inside the function (around line
132-133) to the top-level import section of the module with other imports. Apply
the same fix to the other local import statement mentioned at lines 180-181.
Remove these imports from their current function-local locations and consolidate
them with the module-level imports at the beginning of the file to comply with
the repository's Python style guidelines.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/src/dynamo/planner/connectors/virtual.py`:
- Line 43: Remove the `super().__init__(model_name)` call in the __init__ method
because PlannerConnector is now a Protocol and does not provide an __init__
implementation. Simply delete this line as Protocols are structural types that
don't require parent class initialization, and this will prevent the TypeError
that occurs at runtime when attempting to call __init__ on a Protocol.

In `@components/src/dynamo/planner/core/base.py`:
- Around line 100-102: Replace the broad Exception handler at lines 100-102 that
catches Prometheus server startup failures with a specific exception type and
add a defensive check before calling prometheus_metrics.sla_target_ttft_ms.set()
to verify the metrics server is running, or restructure to fail fast if
initialization fails. Similarly, narrow the broad exception handlers at lines
494-495 (diagnostics handler) and lines 129-130 (live dashboard) to catch
specific exceptions and either re-raise after logging or validate state before
continuing execution. Additionally, move the function-scoped import at lines
150-155 to module level to follow standard import conventions.

In `@components/src/dynamo/planner/core/planner_factory.py`:
- Line 33: Replace the assert statement that checks
config.global_planner_namespace is not None with an explicit validation that
raises an appropriate exception (such as ValueError or ConfigError). The assert
statement will be stripped out when Python runs with the -O optimization flag,
allowing None values to flow into the GlobalPlannerConnector initialization.
Instead, use an if statement that explicitly raises an exception with a clear
error message when config.global_planner_namespace is None.

In `@components/src/dynamo/planner/core/util.py`:
- Line 1: The file util.py is missing the required SPDX license header at the
top, which is causing the CI copyright check to fail. Add the appropriate SPDX
header comment before the `from __future__ import annotations` line at the
beginning of the file. The header should be placed as the first line(s) of the
file to comply with the repository's copyright requirements.

In `@components/src/dynamo/planner/environment/base.py`:
- Around line 177-183: The try-except block around the get_worker_info call is
catching a broad Exception type and silently returning, which masks failures.
Replace the generic Exception catch with specific exception types that
get_worker_info can raise, log the error details, and re-raise the exception to
fail fast rather than continuing with stale state. This same pattern needs to be
fixed in all locations mentioned, including the handler at line 241-243.
- Around line 12-16: The import statement for PlannerConnector from
dynamo.planner.environment.interface is incorrect because PlannerConnector is
not exported from that module. Find the correct module where PlannerConnector is
actually defined and exported, then update the import statement in the file to
import PlannerConnector from the correct location instead of from
dynamo.planner.environment.interface.

In
`@components/src/dynamo/planner/environment/metrics_provider/prometheus_traffic_provider.py`:
- Around line 83-95: The logger.info call with %.2f format specifiers for
m.num_req, m.isl, and m.osl is executed before the m.is_valid() validation
check. If Prometheus returns None for these fields, the float formatting will
fail. Move the m.is_valid() check and early return to occur before the
logger.info call so that metrics are validated before attempting to format them
with float specifiers, preventing formatting errors on None values.

In `@components/src/dynamo/planner/environment/state.py`:
- Line 1: The file state.py is missing the required SPDX license header at the
very beginning, which is causing the copyright check CI job to fail. Add the
appropriate SPDX copyright and license header comment block at the top of the
file, before the existing from __future__ import annotations line. The header
should match the format and licensing used by other files in the same project
directory to ensure consistency and pass the copyright check validation.

In `@components/src/dynamo/planner/tests/unit/test_load_based_scaling.py`:
- Around line 38-39: The import of PlannerEnvironmentImpl from
dynamo.planner.environment.base at lines 38-39 is triggering a circular
dependency chain where base.py attempts to import a non-existent
PlannerConnector from dynamo.planner.environment.interface, causing test
collection to fail with ImportError. Either remove the PlannerEnvironmentImpl
import if it is not used in the test file, or defer the import by wrapping it in
a TYPE_CHECKING block from the typing module and using string annotations in
type hints where PlannerEnvironmentImpl is referenced, allowing the test file to
be imported without executing the problematic dependency chain at collection
time.

---

Outside diff comments:
In `@components/src/dynamo/planner/connectors/global_planner.py`:
- Around line 129-131: The RuntimeError message in the GlobalPlannerConnector
class is providing stale guidance by instructing callers to invoke
_async_init(), but this method has been renamed to async_init(). Update the
error message string in the RuntimeError to reference the correct method name
async_init() instead of _async_init() so that users receive accurate
initialization instructions when this error is raised.

---

Nitpick comments:
In `@components/src/dynamo/planner/core/base.py`:
- Around line 152-154: The import of OrchestratorEngineAdapter inside the
_ensure_engine function violates the module-level import rule. Move the import
statement from inside the _ensure_engine function to the top of the file where
other imports are declared, placing it with the other dynamo.planner imports to
maintain consistency with the codebase conventions.

In `@components/src/dynamo/planner/core/planner_factory.py`:
- Around line 133-138: The imports of AggPlanner, DecodePlanner, DisaggPlanner,
and PrefillPlanner from dynamo.planner.core.adapters are currently located
inside a function scope, which violates the repository's import placement rules.
Move these imports to the top of the module file with the other imports at
module scope to comply with the coding guidelines that require all imports to be
placed at the module level.

In
`@components/src/dynamo/planner/environment/metrics_provider/runtime_provider.py`:
- Around line 132-133: Move the function-local import statement `from dynamo.llm
import FpmEventSubscriber` from inside the function (around line 132-133) to the
top-level import section of the module with other imports. Apply the same fix to
the other local import statement mentioned at lines 180-181. Remove these
imports from their current function-local locations and consolidate them with
the module-level imports at the beginning of the file to comply with the
repository's Python style guidelines.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5a740f70-227b-4dbb-b481-b5dabbc9f0d6

📥 Commits

Reviewing files that changed from the base of the PR and between a919289 and e41f191.

📒 Files selected for processing (23)

• components/src/dynamo/planner/connectors/base.py
• components/src/dynamo/planner/connectors/clients/kubernetes_api.py
• components/src/dynamo/planner/connectors/clients/remote_client.py
• components/src/dynamo/planner/connectors/global_planner.py
• components/src/dynamo/planner/connectors/kubernetes.py
• components/src/dynamo/planner/connectors/runtime.py
• components/src/dynamo/planner/connectors/virtual.py
• components/src/dynamo/planner/core/adapters.py
• components/src/dynamo/planner/core/base.py
• components/src/dynamo/planner/core/planner_factory.py
• components/src/dynamo/planner/core/util.py
• components/src/dynamo/planner/environment/base.py
• components/src/dynamo/planner/environment/interface.py
• components/src/dynamo/planner/environment/metrics_provider/interface.py
• components/src/dynamo/planner/environment/metrics_provider/prometheus_traffic_provider.py
• components/src/dynamo/planner/environment/metrics_provider/runtime_provider.py
• components/src/dynamo/planner/environment/runtime.py
• components/src/dynamo/planner/environment/state.py
• components/src/dynamo/planner/tests/integration/test_virtual_connector.py
• components/src/dynamo/planner/tests/unit/test_kube.py
• components/src/dynamo/planner/tests/unit/test_load_based_scaling.py
• components/src/dynamo/planner/tests/unit/test_metric_publication.py
• components/src/dynamo/planner/tests/unit/test_remote_planner.py

[dynamo-review-agent]

components/src/dynamo/planner/main.py — start_planner still constructs planner classes with (runtime, config), but NativePlannerBase.__init__ now requires an environment, so the planner worker fails on startup. Fix: construct planners through the new planner factory.

🤖 AI Fix

In components/src/dynamo/planner/__main__.py, import construct_planner from dynamo.planner.core.planner_factory and replace the mode switch in start_planner with planner = construct_planner(runtime=runtime, config=config).

components/src/dynamo/global_planner/scale_handler.py — This file still imports dynamo.planner.connectors.kubernetes_api after the module was moved, so importing the global-planner scale handler fails. Fix: update the import to the new clients package path.

🤖 AI Fix

In components/src/dynamo/global_planner/scale_handler.py, change from dynamo.planner.connectors.kubernetes_api import KubernetesAPI to from dynamo.planner.connectors.clients.kubernetes_api import KubernetesAPI.

[tedzhouhk]

Great work! Overall LGTM, a few focused questions on the new Environment/observer boundary and its contracts.

[sachalmalick]

/ok to test 53fb741

[sachalmalick]

/ok to test ca00b82