fix: Make planner VirtualConnectorClient also use v1/ prefix.

[grahamking]

Codex caught the bug! Thanks @paulhendricks for doing a review with it.

Follow-up to #3458 .

Summary by CodeRabbit

• New Features
  • Standardized key/value storage namespace to use a v1/ prefix for planner data.
• Refactor
  • Simplified configuration by consolidating versioning into the provided prefix (no separate version parameter).
• Notes
  • Existing setups referencing previous prefixes may need to update configurations or reindex keys to align with the new v1/ path.

[coderabbitai]

Walkthrough

The KvCache::new API removed its version parameter. Callers now supply a fully formed prefix. Planner root keys are updated to include a leading "v1/" segment. Planner async initialization now passes the computed prefix directly to KvCache. Etcd transport uses the provided prefix as-is for key listing and storage.

Changes

Cohort / File(s)	Summary
KvCache API update lib/runtime/src/transports/etcd.rs	Removed version parameter from KvCache::new; stopped composing version+prefix internally; now uses provided prefix directly for listing and constructing keys. Public signature changed accordingly.
Planner initialization and key path lib/bindings/python/rust/planner.rs	Updated async_init to call KvCache::new(client, prefix, HashMap::new()). root_key(namespace) now prepends v1/, changing planner key path to v1/{namespace}/planner/.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller as Python Binding
  participant Planner as Planner (Rust)
  participant Kv as KvCache
  participant Etcd as Etcd Client

  Caller->>Planner: async_init(namespace)
  Note over Planner: Build prefix with v1/namespace/planner/
  Planner->>Kv: new(client, prefix, initial_values)
  Note right of Kv: Uses prefix as-is (no version composition)
  Kv->>Etcd: list(prefix)
  Etcd-->>Kv: existing keys
  Kv-->>Planner: cache ready
  Planner-->>Caller: init complete

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A whisk of keys, a hop through time,

We dropped a version’s extra rhyme.

Now prefixes lead straight and true—

v1 in tow, the paths renew.

I twitch my ears, cache warms the burrow—

Etcd hums, no needless furrow. 🐇✨

Pre-merge checks

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description does not follow the repository’s required template and is missing key sections such as Overview, Details, Where should the reviewer start, and Related Issues, providing only a brief note and link to a prior PR.	Please update the description to use the repository’s template by adding an Overview, a detailed description of changes, guidance on where to start reviewing, and a Related Issues section referencing the relevant issue or PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly summarizes the main change by indicating that the planner’s VirtualConnectorClient now uses the v1/ prefix, and it is clear and focused on the primary fix without extraneous details.

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e1f507 and 4d60073.

📒 Files selected for processing (2)

• lib/bindings/python/rust/planner.rs (2 hunks)
• lib/runtime/src/transports/etcd.rs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

lib/bindings/python/rust/planner.rs (1)

lib/runtime/src/transports/etcd.rs (2)

• new (93-126)
• new (516-559)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: trtllm (amd64)
• GitHub Check: trtllm (arm64)
• GitHub Check: vllm (arm64)
• GitHub Check: sglang
• GitHub Check: vllm (amd64)
• GitHub Check: clippy (lib/bindings/python)
• GitHub Check: clippy (launch/dynamo-run)
• GitHub Check: clippy (.)
• GitHub Check: tests (lib/runtime/examples)
• GitHub Check: tests (launch/dynamo-run)
• GitHub Check: tests (lib/bindings/python)
• GitHub Check: tests (.)
• GitHub Check: Build and Test - dynamo

🔇 Additional comments (4)

lib/bindings/python/rust/planner.rs (2)

105-105: LGTM! Correct removal of version parameter.

The version parameter removal aligns with the updated KvCache::new API signature. The prefix variable already includes the "v1/" prefix from the root_key function (line 102), so this change is correct.

---

500-500: All KvCache::new usages updated to new signature
Verification confirmed that every KvCache::new call uses three arguments (client, prefix, initial_values); no legacy 4-argument invocations remain.

lib/runtime/src/transports/etcd.rs (2)

515-559: LGTM! Clean API simplification.

The removal of the version parameter simplifies the KvCache::new API by pushing version management to the caller. The implementation correctly uses the provided prefix as-is throughout:

• Line 524: Fetches existing keys with the prefix directly
• Line 535: Constructs full keys by concatenating prefix with key names
• Line 545: Watches the prefix directly

This is a cleaner separation of concerns—the caller owns the complete prefix including any version information.

---

712-720: LGTM! Test correctly updated for new API.

The test changes properly reflect the new KvCache::new signature:

• Line 712: The prefix now includes "v1/" explicitly
• Line 720: The KvCache::new call passes only three arguments (client, prefix, initial_values)

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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