[BUG] Fix #220 #221 #222 — PandasAdapter dead code, negative limit validation, async background loop#243
Open
harsh2025-sketch wants to merge 2 commits intosktime:mainfrom
Open
[BUG] Fix #220 #221 #222 — PandasAdapter dead code, negative limit validation, async background loop#243harsh2025-sketch wants to merge 2 commits intosktime:mainfrom
harsh2025-sketch wants to merge 2 commits intosktime:mainfrom
Conversation
…ation, async background loop
There was a problem hiding this comment.
Pull request overview
This PR fixes three audited bugs across data loading, estimator listing, and async background job execution in sktime_mcp, and adds regression tests to prevent recurrence.
Changes:
- Fix
PandasAdaptertime-column auto-detection so_detect_time_column()is reachable when the index is a defaultRangeIndex. - Validate
limitinlist_estimators_toolto reject negative values that would otherwise return a near-full page. - Introduce a shared background asyncio loop (
BG_LOOP) and route async tool submissions through it so background jobs actually execute.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/sktime_mcp/data/adapters/pandas_adapter.py |
Adjusts index-type guard so time-column detection runs when time_column is omitted. |
src/sktime_mcp/tools/list_estimators.py |
Adds negative limit validation before slicing. |
src/sktime_mcp/runtime/_background_loop.py |
Adds a singleton background event loop running in a daemon thread and a submit() API. |
src/sktime_mcp/tools/fit_predict.py |
Uses BG_LOOP.submit(...) for async fit/predict jobs instead of submitting to a non-running loop. |
src/sktime_mcp/tools/data_tools.py |
Uses BG_LOOP.submit(...) for async data-loading jobs instead of submitting to a non-running loop. |
tests/test_core.py |
Adds regression tests for negative/zero limit and pandas time-column auto-detection. |
tests/test_background_jobs.py |
Adds a tool-level regression test ensuring async fit/predict jobs move beyond pending. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Summary
Fixes three confirmed bugs found during ESoC 2026 codebase audit.
Fixes #220
Fixes #221
Fixes #222
Fix #220 — PandasAdapter auto-detection dead code
File:
src/sktime_mcp/data/adapters/pandas_adapter.pyCondition was always False —
pd.Indexis base class of all pandasindex types so
_detect_time_columnwas never called.Fix: Removed
pd.Indexfrom check — detection now triggers correctlyon RangeIndex inputs.
Fix #221 — Negative limit returns 400+ results
File:
src/sktime_mcp/tools/list_estimators.pylimitwas never validated.limit=-5causedall_estimators[0:-5]to return 410 results, flooding LLM agent context window.
Fix: Added guard before slice:
if limit is not None and limit < 0: return errorFix #222 — Async jobs stay pending forever
Files:
_background_loop.py(new),fit_predict.py,data_tools.pyrun_coroutine_threadsafewas submitting to a loop never startedin a background thread. All async jobs stayed pending forever.
Fix: Created
_BackgroundLoopwith daemon thread runningloop.run_forever(). All async tools now submit viaBG_LOOP.submit(coro).Tests — all passing