Skip to content

test: Break parsers -> llm dependency#3607

Merged
saturley-hall merged 1 commit into
mainfrom
gk-parsers-no-llm
Oct 14, 2025
Merged

test: Break parsers -> llm dependency#3607
saturley-hall merged 1 commit into
mainfrom
gk-parsers-no-llm

Conversation

@grahamking
Copy link
Copy Markdown
Contributor

@grahamking grahamking commented Oct 14, 2025

The parsers crate is intended to be standalone, it cannot depend on llm. There was an integration test in there that used llm. This PR moves that test to the llm crate.

The `parsers` crate is intended to be standalone, it cannot depend on
`llm`. There was an integration test in there that used llm. This PR
moves that test to the llm crate.

Signed-off-by: Graham King <grahamk@nvidia.com>
@grahamking grahamking requested a review from a team as a code owner October 14, 2025 14:20
@github-actions github-actions Bot added the test label Oct 14, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 14, 2025

Walkthrough

Removed a dev-only dependency from lib/parsers/Cargo.toml and deleted the test module content in lib/parsers/tests/mod.rs. No runtime dependencies or public APIs were altered.

Changes

Cohort / File(s) Summary
Dependency cleanup
lib/parsers/Cargo.toml
Deleted the [dev-dependencies] section, removing dynamo-llm = { workspace = true }.
Test removal
lib/parsers/tests/mod.rs
Cleared file contents, removing SPDX header, docs, cfg(test), and the parallel_tool_call_integration test module declaration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I trimmed the vines in nightly light,
A test hop gone, dev twigs pulled tight.
Thump-thump—clean paths for future runs,
Fewer crates, more rising suns.
Ears up, I commit with glee—
Less is more for this bunny spree. 🐇✨

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the required template as it lacks the mandatory section headings such as Overview, Details, Where should the reviewer start, and Related Issues, and it does not provide specific guidance on where reviewers should focus. The content is useful but incomplete against the repository’s structured template. Please update the description to match the repository template by adding an Overview summarizing the change, a Details section outlining specific code modifications, a Where should the reviewer start section pointing to key files, and a Related Issues section linking any relevant issue numbers.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes breaking the dependency from the parsers crate to the llm crate, which matches the main change of moving the integration test out of parsers. It is specific and clearly conveys the key purpose of the pull request without unnecessary information.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 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 9bb2bc9 and 67ec066.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • lib/parsers/Cargo.toml (0 hunks)
  • lib/parsers/tests/mod.rs (0 hunks)
💤 Files with no reviewable changes (2)
  • lib/parsers/tests/mod.rs
  • lib/parsers/Cargo.toml
⏰ 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). (10)
  • GitHub Check: trtllm (amd64)
  • GitHub Check: vllm (amd64)
  • GitHub Check: Build and Test - dynamo
  • GitHub Check: tests (launch/dynamo-run)
  • GitHub Check: tests (lib/bindings/python)
  • GitHub Check: clippy (.)
  • GitHub Check: tests (.)
  • GitHub Check: tests (lib/runtime/examples)
  • GitHub Check: clippy (launch/dynamo-run)
  • GitHub Check: clippy (lib/bindings/python)

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

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

Copy link
Copy Markdown
Contributor

@elyasmnvidian elyasmnvidian left a comment

Choose a reason for hiding this comment

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

LGTM

@saturley-hall saturley-hall merged commit 972adc3 into main Oct 14, 2025
29 checks passed
@saturley-hall saturley-hall deleted the gk-parsers-no-llm branch October 14, 2025 16:19
saturley-hall pushed a commit that referenced this pull request Oct 14, 2025
Signed-off-by: Graham King <grahamk@nvidia.com>
Signed-off-by: Harrison Saturley-Hall <hsaturleyhal@nvidia.com>
saturley-hall added a commit that referenced this pull request Oct 14, 2025
Signed-off-by: Graham King <grahamk@nvidia.com>
Signed-off-by: Harrison Saturley-Hall <hsaturleyhal@nvidia.com>
Co-authored-by: Graham King <grahamk@nvidia.com>
shpgy-shpgy pushed a commit to shpgy-shpgy/dynamo that referenced this pull request Oct 15, 2025
Signed-off-by: Graham King <grahamk@nvidia.com>
ziqifan617 pushed a commit that referenced this pull request Oct 20, 2025
Signed-off-by: Graham King <grahamk@nvidia.com>
nv-tusharma pushed a commit that referenced this pull request Oct 20, 2025
Signed-off-by: Graham King <grahamk@nvidia.com>
yao531441 pushed a commit to yao531441/dynamo that referenced this pull request May 13, 2026
Signed-off-by: Graham King <grahamk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants