fix(clp-mcp-server): Add missing clp-py-utils dependency needed for running unit tests.#1531
Merged
Merged
Conversation
Contributor
WalkthroughReworked Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-10-27T04:05:01.560ZApplied to files:
📚 Learning: 2025-10-22T21:02:31.113ZApplied to files:
📚 Learning: 2025-08-29T07:31:24.032ZApplied to files:
📚 Learning: 2025-08-16T10:24:29.316ZApplied to files:
📚 Learning: 2025-08-29T07:26:53.532ZApplied to files:
📚 Learning: 2025-08-17T16:10:38.722ZApplied to files:
📚 Learning: 2025-07-28T08:33:57.487ZApplied to files:
📚 Learning: 2025-08-20T05:38:30.720ZApplied to files:
📚 Learning: 2025-10-13T03:24:35.074ZApplied to files:
📚 Learning: 2025-08-08T21:15:10.905ZApplied to files:
📚 Learning: 2025-08-24T17:10:44.565ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (2)
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. Comment |
20001020ycx
changed the title
20001020ycx
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
components/clp-mcp-server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
components/clp-mcp-server/pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#1405
File: components/clp-package-utils/pyproject.toml:5-15
Timestamp: 2025-10-13T03:24:35.074Z
Learning: In the y-scope/clp repository, the Python 3.9 to 3.10 version requirement change was intentionally deferred to a separate PR (after PR #1405) to reduce review effort, as decided in an offline discussion between junhaoliao and kirkrodrigues.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1100
File: integration-tests/tests/fixtures/integration_test_logs.py:54-56
Timestamp: 2025-08-17T16:10:38.722Z
Learning: For PR #1100 (feat(integration-tests): Add CLP package integration tests boilerplate), do not raise cache weakness problems related to the pytest cache implementation in the integration test logs fixtures.
Learnt from: junhaoliao
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Learnt from: junhaoliao
PR: y-scope/clp#1507
File: taskfiles/deps/lock.yaml:66-76
Timestamp: 2025-10-27T04:05:01.560Z
Learning: In the CLP project, the `uv` CLI tool is documented as a prerequisite in docs/src/dev-docs/building-package.md and is not installed via a task dependency like the rust toolchain. UV is expected to be available in the environment rather than being provisioned by the taskfile system.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1271
File: components/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.sh:41-43
Timestamp: 2025-08-29T07:31:24.032Z
Learning: In CLP's build tool installation scripts, uv version constraints should use lower bound constraints (uv>=0.8) rather than exact version pinning, following the same philosophy used for other build tools to accommodate platform differences while ensuring minimum required functionality.
📚 Learning: 2025-10-27T04:05:01.560Z
Learnt from: junhaoliao
PR: y-scope/clp#1507
File: taskfiles/deps/lock.yaml:66-76
Timestamp: 2025-10-27T04:05:01.560Z
Learning: In the CLP project, the `uv` CLI tool is documented as a prerequisite in docs/src/dev-docs/building-package.md and is not installed via a task dependency like the rust toolchain. UV is expected to be available in the environment rather than being provisioned by the taskfile system.
Applied to files:
components/clp-mcp-server/pyproject.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). (1)
- GitHub Check: package-image
🔇 Additional comments (2)
components/clp-mcp-server/pyproject.toml (2)
18-19: Verify the local path reference resolves correctly.The [tool.uv.sources] section correctly configures clp-py-utils as a local editable dependency. However, the relative path ../clp-py-utils assumes a specific directory structure. Ensure this path exists relative to the pyproject.toml location (components/clp-mcp-server/pyproject.toml).
The verification script above will confirm the path resolves correctly. If the path is incorrect, the build or dependency resolution will fail during uv operations.
18-19: LGTM: Local editable mode is appropriate for development.The use of [tool.uv.sources] with editable = true aligns with the PR objective of enabling unit tests without requiring the full clp-package. This configuration supports the validation workflow mentioned in the PR (uv run --group dev pytest).
| "msgpack>=1.1.2", | ||
| "paginate>=0.5.7", | ||
| "pymongo>=4.15.3", | ||
| "clp-py-utils", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Add version constraint for clp-py-utils dependency.
The clp-py-utils dependency lacks a version constraint, inconsistent with all other dependencies in this file (e.g., aiomysql>=0.2.0, click>=8.3.0). Even for local editable dependencies, specifying a version constraint improves reproducibility and clarity.
Based on learnings from PR #1271, the repository prefers lower bound version constraints (e.g., >=X.Y.Z).
Please verify the appropriate version constraint for clp-py-utils and update line 15 to include it:
- "clp-py-utils",
+ "clp-py-utils>=VERSION",Additionally, run the following script to verify the repository structure and the clp-py-utils directory location:
🏁 Script executed:
#!/bin/bash
# Description: Verify clp-py-utils directory exists and contains a valid pyproject.toml
# Check if clp-py-utils directory exists at the expected path
if [ -d "components/clp-py-utils" ]; then
echo "✓ clp-py-utils directory found"
# Verify it contains a pyproject.toml
if [ -f "components/clp-py-utils/pyproject.toml" ]; then
echo "✓ pyproject.toml exists in clp-py-utils"
# Extract version from clp-py-utils pyproject.toml
grep "^version" components/clp-py-utils/pyproject.toml
else
echo "✗ pyproject.toml not found in clp-py-utils"
fi
else
echo "✗ clp-py-utils directory not found at components/clp-py-utils"
fiLength of output: 92
🏁 Script executed:
#!/bin/bash
# View the full pyproject.toml for clp-mcp-server to verify dependency constraints
echo "=== clp-mcp-server/pyproject.toml ==="
cat components/clp-mcp-server/pyproject.toml
echo -e "\n=== Check for similar patterns in other pyproject.toml files ==="
# Find all pyproject.toml files with [tool.uv.sources]
fd -e pyproject.toml -x grep -l "tool.uv.sources" {}Length of output: 2661
Add version constraint for clp-py-utils dependency to maintain consistency.
The clp-py-utils dependency (line 15) lacks a version constraint, while all other dependencies specify lower bound constraints (e.g., aiomysql>=0.2.0, click>=8.3.0). Even when using editable local sources via [tool.uv.sources], including a version constraint improves reproducibility and aligns with the repository's established pattern (as documented in PR #1271).
Update line 15:
- "clp-py-utils",
+ "clp-py-utils>=0.5.2",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "clp-py-utils", | |
| "clp-py-utils>=0.5.2", |
🤖 Prompt for AI Agents
In components/clp-mcp-server/pyproject.toml around line 15, the clp-py-utils
dependency has no version constraint; update that entry to include a lower-bound
version constraint consistent with other deps (e.g., use the repository's
current package version or the minimum supported version from PR #1271 such as
>=0.1.0) so the dependency line matches the project's pattern and improves
reproducibility.
junhaoliao
requested changes
Nov 12, 2025
junhaoliao
left a comment
junhaoliao
left a comment
Member
There was a problem hiding this comment.
for the title, how about:
fix(clp-mcp-server): Add missing `clp-py-utils` dependency needed for running unit tests.
| "msgpack>=1.1.2", | ||
| "paginate>=0.5.7", | ||
| "pymongo>=4.15.3", | ||
| "clp-py-utils", |
Member
There was a problem hiding this comment.
let's refer to the integration test to set up a clp group:
clp/integration-tests/pyproject.toml
Lines 19 to 24 in e276511
i.e.,
clp = [
"clp-py-utils",
]
Member
There was a problem hiding this comment.
then below we can add
[tool.uv]
default-groups = ["clp", "dev"]
junhaoliao
requested changes
Nov 12, 2025
junhaoliao
left a comment
junhaoliao
left a comment
Member
There was a problem hiding this comment.
for the title, how about:
fix(clp-mcp-server): Add missing `clp-py-utils` dependency needed for running unit tests.
20001020ycx
changed the title
clp-py-utils dependency needed for running unit tests.
added 2 commits
November 12, 2025 18:44
junhaoliao
previously approved these changes
Nov 12, 2025
junhaoliao
previously approved these changes
Nov 13, 2025
junhaoliao
approved these changes
Nov 13, 2025
junhaoliao
left a comment
junhaoliao
left a comment
Member
There was a problem hiding this comment.
we discussed offline about the differences between [dependencies] and [dependency-groups]. i can confirm the changes to [dependencies] have been reverted
junhaoliao
added a commit
to junhaoliao/clp
that referenced
this pull request
May 17, 2026
… running unit tests. (y-scope#1531) Co-authored-by: Junhao Liao <junhao.liao@yscope.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
clp-mcp-serverrelies on many constants, data structures, and util functions from clp_py_utils, e.g.clp_py_utils.clp_config import CLPConfig. However, this dependency is only resolved at run time after the entire clp package is built. This led to the problem when locally runninguv run --group dev pytest tests/* -vv, the pytest fails because it cannot find the dependencies for imports fromclp_py_utils. This PR adds support for this.Checklist
breaking change.
Validation performed
uv run --group dev pytest tests/* -vvlocally without clp-package.Summary by CodeRabbit