Repository navigation
Add CoolProp MCP tool - #1420
Conversation
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
…tool Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new MCP tool module ChangesCoolPropTool Feature
sequenceDiagram
participant Client
participant MCP_Server as "CoolPropTool (MCP)"
participant CoolProp as "CoolProp Library"
Client->>MCP_Server: execute("fluid-property", {fluid, output_property, temperature, pressure})
MCP_Server->>CoolProp: PropsSI(output_property, "T", temperature, "P", pressure, fluid)
CoolProp-->>MCP_Server: value or error
MCP_Server-->>Client: markdown-formatted result or error
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 55 seconds. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_mcp_clients.py (1)
1010-1025: ⚡ Quick winAdd a direct
execute()smoke test for the new tool.Right now the suite only checks config and tool names, so a broken dispatch path or CoolProp import would still pass. A single
fluid-listcall would cover the actual runtime entry point.♻️ Proposed test addition
tool = CoolPropTool() tool.configure() tool_names = {t["name"] for t in await tool.list_tools()} assert "fluid-property" in tool_names assert "fluid-list" in tool_names + + result = await tool.execute("fluid-list", {}) + assert "fluids available" in result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_mcp_clients.py` around lines 1010 - 1025, Add a smoke test that actually calls the tool runtime entrypoint: instantiate CoolPropTool(), call configure(), find the tool descriptor for "fluid-list" from await tool.list_tools(), then call await tool.execute(tool_descriptor, {}) (or the execute signature used by CoolPropTool) and assert the call returns a successful result (e.g., non-empty list or expected keys). Update the TestCoolPropTool class (near test_coolprop_direct_list_tools) to include a new async test method that locates the "fluid-list" tool and exercises CoolPropTool.execute to catch import/dispatch/runtime errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/agentic_inference/tool_calling.md`:
- Around line 374-375: Remove the duplicated bullet for
nemo_skills.mcp.servers.coolprop_tool.CoolPropTool in the tool list: keep a
single entry for CoolPropTool (with the preferred description—e.g.,
"Thermophysical fluid property lookup via CoolProp (requires `CoolProp`)") and
delete the other identical line so the built-in-tools list contains only one
CoolPropTool entry.
---
Nitpick comments:
In `@tests/test_mcp_clients.py`:
- Around line 1010-1025: Add a smoke test that actually calls the tool runtime
entrypoint: instantiate CoolPropTool(), call configure(), find the tool
descriptor for "fluid-list" from await tool.list_tools(), then call await
tool.execute(tool_descriptor, {}) (or the execute signature used by
CoolPropTool) and assert the call returns a successful result (e.g., non-empty
list or expected keys). Update the TestCoolPropTool class (near
test_coolprop_direct_list_tools) to include a new async test method that locates
the "fluid-list" tool and exercises CoolPropTool.execute to catch
import/dispatch/runtime errors.
🪄 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: 53a37043-5f5d-44ee-bcdd-c82d1282e5dc
📒 Files selected for processing (3)
docs/agentic_inference/tool_calling.mdnemo_skills/mcp/servers/coolprop_tool.pytests/test_mcp_clients.py
|
please fix the DCO check |
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
90ee1ea to
ae7ecd5
Compare
Signed-off-by: Hovhannes Tamoyan <hovhannes.tamoyan@gmail.com>
Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com>
## Summary - Add a NeMo-Gym `ns_tools` config that exposes `CoolPropTool` through the existing NeMo-Skills ToolManager bridge. - Add a focused config test to keep the resource-server wiring valid. - This is the Gym-side companion to NVIDIA-NeMo/Skills#1420. Replaces fork-origin PR #1218 so CI can run with full repository secrets. ## Test plan - `PYTHONPATH=. python -m pytest resources_servers/ns_tools/tests/test_coolprop_mcp_config.py -q --tb=short` --------- Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com> Co-authored-by: Christian Munley <cmunley@nvidia.com>
## Summary - Add a NeMo-Gym `ns_tools` config that exposes `CoolPropTool` through the existing NeMo-Skills ToolManager bridge. - Add a focused config test to keep the resource-server wiring valid. - This is the Gym-side companion to NVIDIA-NeMo/Skills#1420. Replaces fork-origin PR #1218 so CI can run with full repository secrets. ## Test plan - `PYTHONPATH=. python -m pytest resources_servers/ns_tools/tests/test_coolprop_mcp_config.py -q --tb=short` --------- Signed-off-by: tamohannes <hovhannes.tamoyan@gmail.com> Co-authored-by: Christian Munley <cmunley@nvidia.com> Signed-off-by: Rita Fernandes Neves <rfernandesne@nvidia.com>
Summary
NVIDIA-NeMo/Skillsbranch so CI can access repository secrets.Test plan
python -m py_compile nemo_skills/mcp/servers/coolprop_tool.pypython -m pytest tests/test_mcp_clients.py::TestCoolPropTool -q --tb=shortpython -m pytest tests/test_mcp_clients.py -q --tb=shortSummary by CodeRabbit
New Features
Documentation
Tests