test: Simplify test_indexers_sync() with the new helper send_request_via_python_kv_router()

[zhixiongli2011]

Overview:

Simplify test_indexers_sync() with the new helper send_request_via_python_kv_router()

Details:

1. Refactored and Simplified test_indexers_sync() in test_router_e2e_with_mockers.py with the new helper send_request_via_python_kv_router()
2. Successfully tested the changed test_indexers_sync(). Test log enclosed below

test_indexers_synctest.log.txt

Where should the reviewer start?

Python file: tests/router/test_router_e2e_with_mockers.py
Function: test_indexers_sync()

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: [FEATURE]: more comprehensive e2e testings in per-merge CIs with mockers #2089

Summary by CodeRabbit

• Tests
  • Consolidated router request logic into a shared helper with retries and streaming handling.
  • Standardized readiness checks across router-related tests, reducing flakiness.
  • Expanded coverage for token generation scenarios, stop conditions, and configuration overrides.
• Refactor
  • Replaced ad-hoc test flows with the shared helper to reduce duplication and simplify maintenance.
  • Minor structural and comment updates to align with the new approach.

[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.

[github-actions]

👋 Hi zhixiongli2011! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[coderabbitai]

Walkthrough

A new helper, send_request_via_python_kv_router(...), is added to centralize test routing/streaming with retries, SSE aggregation, and stop conditions. Existing end-to-end router tests are refactored to use this helper for readiness checks and generation scenarios, replacing prior inline logic while retaining test assertions and structure.

Changes

Cohort / File(s)	Summary
Router E2E tests refactor tests/router/test_router_e2e_with_mockers.py	Added helper send_request_via_python_kv_router(...) implementing exponential backoff, SSE subscription, token_id aggregation, optional stop handling (max_tokens, ignore_eos), and success boolean. Refactored tests (readiness checks and multiple scenarios) to call the helper, removing duplicated inline logic and adjusting control flow accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Test as Test Case
  participant Helper as send_request_via_python_kv_router
  participant Router as KvPushRouter
  participant SSE as SSE Stream

  Test->>Helper: invoke(..., stop_conditions, overrides)
  loop exponential backoff until ready/success
    Helper->>Router: send request (config/overrides)
    Router-->>Helper: stream handle / URL
    Helper->>SSE: subscribe
    alt stream tokens
      SSE-->>Helper: token_id chunk
      Helper->>Helper: aggregate, check stop/max_tokens/ignore_eos
    else error/end
      SSE-->>Helper: end/error
    end
    Helper-->>Test: boolean success (and aggregated token_ids)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I thump my paw: “Refactor time!”
One helper hops, consolidating rhyme.
Tokens stream like clover leaves,
Backoffs bloom beneath the eaves.
Routers ready, mockers cheer—
Fewer paths, the warren clear.
Sip of SSE, spring is here. 🐇✨

Pre-merge checks

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title accurately notes the simplification of test_indexers_sync() using the new helper, but it narrowly focuses on a single test and does not surface the core addition of the send_request_via_python_kv_router helper or the broader refactoring of multiple router e2e mocker tests.	Please revise the title to reflect both the introduction of the send_request_via_python_kv_router helper and the comprehensive refactoring of the router e2e mocker tests, for example: “test: introduce send_request_via_python_kv_router helper and refactor e2e mocker tests.”
Description Check	⚠️ Warning	The description follows the prescribed template with all required headings, but the Details section only addresses the refactor of test_indexers_sync() and omits key changes such as the implementation of the send_request_via_python_kv_router helper, updates to readiness logic in other tests, and the addition of orchestrated test invocations.	Expand the Details section to include a summary of the new helper function implementation, modifications to readiness and streaming logic across additional tests in test_router_e2e_with_mockers.py, and any other structural adjustments introduced by this PR.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

---

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.}

[coderabbitai]

Actionable comments posted: 1

📜 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 af4c0ca and 56c962e.

📒 Files selected for processing (1)

• tests/router/test_router_e2e_with_mockers.py (8 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/router/test_router_e2e_with_mockers.py (3)

lib/bindings/python/src/dynamo/_core.pyi (7)

• KvPushRouter (1126-1225)
• generate (1147-1179)
• endpoint (82-86)
• KvRouterConfig (739-741)
• block_size (497-501)
• block_size (520-524)
• block_size (1117-1124)

lib/bindings/python/rust/llm/kv.rs (4)

• generate (1077-1144)
• block_size (437-439)
• block_size (498-500)
• block_size (962-964)

lib/llm/src/kv_router.rs (3)

• generate (441-470)
• generate (531-610)
• block_size (410-412)

🪛 GitHub Actions: Pre Merge Validation of (ai-dynamo/dynamo/refs/pull/3333/merge) by zhixiongli2011.

tests/router/test_router_e2e_with_mockers.py

[error] 1-1: Black formatting failed: 1 file reformatted by this hook. Run 'pre-commit run --all-files' locally or commit again to apply changes.

🪛 Ruff (0.13.1)

tests/router/test_router_e2e_with_mockers.py

324-324: Do not catch blind exception: Exception

(BLE001)

---

330-332: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

---

330-332: Avoid specifying long messages outside the exception class

(TRY003)

---

713-713: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

714-714: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

839-839: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

857-857: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

⏰ 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). (1)

• GitHub Check: Build and Test - dynamo

[zhixiongli2011]

@PeaBrane Good evening, please help view this PR.

[zhixiongli2011]

need to rebase the code