[tamohannes]

Summary

• Reopened from an internal NVIDIA-NeMo/Skills branch so CI can access repository secrets.
• Replaces Add radioactive decay MCP tool #1416.
• Add a built-in radioactivedecay MCP tool for nuclide lookup and decay-chain calculations.
• Document the new built-in tool and add focused MCP client tests.

Test plan

• python -m py_compile nemo_skills/mcp/servers/radioactivedecay_tool.py
• python -m pytest tests/test_mcp_clients.py::TestRadioactivedecayTool -q --tb=short
• python -m pytest tests/test_mcp_clients.py -q --tb=short

Summary by CodeRabbit

• New Features

  • Added a radioactive-decay tool for nuclide lookup and decay-chain activity calculations with a configurable default time unit.

• Bug Fixes / Validation

  • Enforced supported time units, non-negative/finite time values, and graceful handling of unknown nuclides with clear error responses.

• Documentation

  • Reference guide updated to list the new radioactive-decay tool.

• Tests

  • Added tests for configuration, tool listing, and input validation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4be73c09-368f-4d4c-803f-1fcdf0e31757

📥 Commits

Reviewing files that changed from the base of the PR and between ff1bbd1 and 7f83933.

📒 Files selected for processing (2)

• docs/agentic_inference/tool_calling.md
• tests/test_mcp_clients.py

✅ Files skipped from review due to trivial changes (2)

• docs/agentic_inference/tool_calling.md
• tests/test_mcp_clients.py

---

📝 Walkthrough

Walkthrough

Adds a new MCP server tool RadioactivedecayTool exposing two tools, nuclide-info and decay-chain, backed by the radioactivedecay library. Implements input validation, server-side time_unit config, dynamic tool registration, tests, and a docs reference.

Changes

RadioactivedecayTool MCP Server Implementation

Layer / File(s)	Summary
Core Helpers nemo_skills/mcp/servers/radioactivedecay_tool.py (lines 40–114)	Adds nuclide_info(nuclide: str) -> str (formats nuclide metadata) and decay_chain(nuclide: str, time: float, time_unit: str = "s") -> str (validates time_unit, enforces finite/non-negative time, computes activities via radioactivedecay.Inventory).
Tool Server & Routing nemo_skills/mcp/servers/radioactivedecay_tool.py (lines 116–171)	Adds RadioactivedecayTool(Tool) with _config defaulting time_unit: "s", default_config(), configure() (validates time_unit), list_tools() (registers nuclide-info and decay-chain with input schemas), and execute() (routes by tool_name, injects configured time_unit when omitted).
Tests tests/test_mcp_clients.py (lines 1005–1037)	Adds TestRadioactivedecayTool tests: default config (time_unit == "s"), list_tools() contains both tools and does not expose time_unit in decay-chain schema, and execute("decay-chain", ...) rejects non-finite time with exact message "Time must be a finite number.".
Documentation docs/agentic_inference/tool_calling.md (line 377)	Adds Built-in Tools reference entry for nemo_skills.mcp.servers.radioactivedecay_tool.RadioactivedecayTool, noting the radioactivedecay dependency.

sequenceDiagram
    rect rgba(10, 90, 180, 0.5)
    participant Client
    end
    rect rgba(20, 160, 80, 0.5)
    participant MCP_Server as RadioactivedecayTool
    end
    rect rgba(200, 80, 40, 0.5)
    participant Lib as radioactivedecay
    end

    Client->>MCP_Server: call "nuclide-info" or "decay-chain" with arguments
    MCP_Server->>MCP_Server: validate inputs, inject configured time_unit if missing
    MCP_Server->>Lib: construct Nuclide/Inventory and compute info or decay
    Lib-->>MCP_Server: return nuclide metadata or activity results
    MCP_Server-->>Client: return formatted string or error message

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add radioactive decay MCP tool' directly and clearly summarizes the main change: adding a new radioactive decay tool to the MCP server framework.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/radioactivedecay-mcp-tool

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

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

[coderabbitai]

Actionable comments posted: 2

🤖 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 built-in tool entry for
RadioactivedecayTool: keep a single list item for
`nemo_skills.mcp.servers.radioactivedecay_tool.RadioactivedecayTool` (the entry
linking to the radioactivedecay_tool.py and noting the `radioactivedecay`
requirement) and delete the repeated line so the tool appears only once in the
list; ensure the remaining item preserves the correct link and description.

In `@nemo_skills/mcp/servers/radioactivedecay_tool.py`:
- Around line 123-126: The configure method currently blindly updates
self._config with any overrides, which can mask typos and lets missing required
keys like time_unit be silently ignored (since other code uses
self._config.get(...)); change configure(self, overrides, ...) to validate
overrides against the set of allowed keys (the keys already present in
self._config or an explicit ALLOWED_CONFIG_KEYS), raise a ValueError for any
unknown key, and require/validate presence of required keys such as "time_unit"
(do not rely on _config.get for required fields) so callers fail fast; update
references in code that read time_unit (the usage around line 159) to access the
validated key directly (e.g., self._config["time_unit"]) after configure ensures
it exists.

🪄 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: ae8e4146-8dfb-4ef6-97b4-301fd405606f

📥 Commits

Reviewing files that changed from the base of the PR and between 6877a00 and 584b7ae.

📒 Files selected for processing (3)

• docs/agentic_inference/tool_calling.md
• nemo_skills/mcp/servers/radioactivedecay_tool.py
• tests/test_mcp_clients.py

[coderabbitai]

♻️ Duplicate comments (1)

nemo_skills/mcp/servers/radioactivedecay_tool.py (1)

132-132: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use direct key access after override-key validation

Line 132 uses overrides.get(...), but after Lines 124–130, a non-empty valid overrides can only contain "time_unit". Direct access is clearer and fail-fast.

Suggested change

-        time_unit = overrides.get("time_unit", self._config["time_unit"])
+        time_unit = overrides["time_unit"]

As per coding guidelines, "Don't use .get() for accessing dictionary keys if the code expects them to be present; use direct access data[key_name] to fail with a clear error instead of silently corrupting data".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemo_skills/mcp/servers/radioactivedecay_tool.py` at line 132, Replace the
permissive dictionary .get() access with direct indexing: where you currently
have time_unit = overrides.get("time_unit", self._config["time_unit"]), change
to use the validated override when overrides is non-empty (e.g. time_unit =
overrides["time_unit"]) and fall back to self._config["time_unit"] only when
overrides is empty; this relies on the existing validation logic around
overrides (Lines ~124–130) so that missing/invalid keys raise immediately
instead of silently masking errors.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@nemo_skills/mcp/servers/radioactivedecay_tool.py`:
- Line 132: Replace the permissive dictionary .get() access with direct
indexing: where you currently have time_unit = overrides.get("time_unit",
self._config["time_unit"]), change to use the validated override when overrides
is non-empty (e.g. time_unit = overrides["time_unit"]) and fall back to
self._config["time_unit"] only when overrides is empty; this relies on the
existing validation logic around overrides (Lines ~124–130) so that
missing/invalid keys raise immediately instead of silently masking errors.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2e5a3f15-3044-47f6-ac2e-ef4369e0bc9f

📥 Commits

Reviewing files that changed from the base of the PR and between 584b7ae and ff1bbd1.

📒 Files selected for processing (2)

• docs/agentic_inference/tool_calling.md
• nemo_skills/mcp/servers/radioactivedecay_tool.py

✅ Files skipped from review due to trivial changes (1)

• docs/agentic_inference/tool_calling.md