Skip to content

Feature/merge instantiate tools#472

Open
Shashankss1205 wants to merge 1 commit into
mainfrom
feature/merge_instantiate_tools
Open

Feature/merge instantiate tools#472
Shashankss1205 wants to merge 1 commit into
mainfrom
feature/merge_instantiate_tools

Conversation

@Shashankss1205

@Shashankss1205 Shashankss1205 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Reference Issues/PRs

Independence: This PR is rebased on latest main and does not depend on #463, #466, #468, or #469. It can be merged on its own.

What does this implement/fix? Explain your changes.

Merges instantiate_estimator and instantiate_pipeline into a single unified instantiate_estimator tool.

Before: LLM clients chose between two tools:

  • instantiate_estimator(estimator="ARIMA", params={...}) — single estimator
  • instantiate_pipeline(components=["Detrender", "ARIMA"], params_list=[...]) — pipeline

After: One tool handles both:

  • instantiate_estimator(estimator="ARIMA", params={...}) — single estimator
  • instantiate_estimator(components=["Detrender", "ARIMA"], params_list=[...]) — pipeline

The tool validates that exactly one of estimator or components is provided. A single-element components list is equivalent to a single estimator. Pipeline composition validation runs automatically when components has multiple entries.

Files changed (7):

  • src/sktime_mcp/tools/instantiate.py — unified API with estimator|components dispatch; backward-compat alias instantiate_pipeline_tool kept internally
  • src/sktime_mcp/server.py — merged tool schema and dispatch
  • src/sktime_mcp/tools/__init__.py — removed instantiate_pipeline_tool from exports
  • tests/test_param_validation.py — keyword args; mutual-exclusion tests
  • tests/test_codegen.py, tests/test_core.py — updated call sites
  • docs/source/implementation.md — updated documentation

Does your contribution introduce a new dependency? If yes, which one?

No.

What should a reviewer concentrate their feedback on?

  • Unified instantiate_estimator_tool API — mutual exclusion logic for estimator vs components
  • Merged tool schema in server.py — no required fields since either estimator or components suffices
  • Backward-compat alias instantiate_pipeline_tool = instantiate_estimator_tool
  • New tests: test_requires_estimator_or_components, test_rejects_both_estimator_and_components

Any other comments

Rebased on latest main (1 commit, +224 / −151). 184 tests pass under make check. Ruff lint clean.

PR checklist

For all contributions
  • I've added unit tests and made sure they pass locally (make check).
  • I've added the tool to the online documentation in docs/source/.

@Shashankss1205 Shashankss1205 self-assigned this Jun 4, 2026
@Shashankss1205 Shashankss1205 force-pushed the feature/merge_instantiate_tools branch from c1b300e to f7766e6 Compare June 4, 2026 20:03
Rebase onto main and unify instantiation behind a single MCP tool that
accepts either estimator+params or components+params_list. Removes the
separate instantiate_pipeline tool registration while keeping a backward-
compat alias for internal callers.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Shashankss1205 Shashankss1205 force-pushed the feature/merge_instantiate_tools branch from f7766e6 to 9eeaa40 Compare June 12, 2026 12:57
Shashankss1205 added a commit that referenced this pull request Jun 12, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
Shashankss1205 added a commit that referenced this pull request Jun 12, 2026
Adds instantiate/set_params craft tools, craft_utils sandboxing, and
merges craft-backed instantiation with the unified instantiate_estimator
API from PR #472. Includes test_craft_instantiate suite (219 tests pass).

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants