fix: suggest valid HuggingFace model IDs when the format check fails#43
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
google-oss-prow
Bot
requested review from
Electronic-Waste,
kramaranya and
szaher
|
🎉 Welcome to the Kubeflow MCP Server! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
There was a problem hiding this comment.
Pull request overview
This PR aims to improve HuggingFace model ID validation feedback by adding best-effort “did you mean” suggestions when the model ID fails the format regex check, reducing unproductive retries for malformed IDs.
Changes:
- Added
_suggest_hf_model_ids()to normalize malformed inputs and queryhuggingface_hub.list_models()for up to 3 close matches. - Updated
_get_model_info_from_hf()to attach asuggestionslist to the invalid-format error response when available. - Added unit tests covering normalization, hub error handling, and suggestion attachment/omission behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
kubeflow_mcp/trainer/api/planning.py |
Add suggestion helper and include suggestions in invalid-format HF validation errors. |
tests/unit/trainer/test_planning.py |
Add unit tests for normalization and suggestion attachment logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result: dict[str, Any] = {"error": f"Invalid HuggingFace model ID format: '{model}'"} | ||
| suggestions = _suggest_hf_model_ids(model) | ||
| if suggestions: | ||
| result["suggestions"] = suggestions | ||
| return result |
| def test_invalid_format_attaches_suggestions(): | ||
| with patch("huggingface_hub.list_models") as mock_list: | ||
| mock_list.return_value = _fake_models("Qwen/Qwen3-8B", "Qwen/Qwen2.5-7B-Instruct") | ||
| result = _get_model_info_from_hf("qwen3:8b") | ||
|
|
||
| assert result["error"] == "Invalid HuggingFace model ID format: 'qwen3:8b'" | ||
| assert result["suggestions"] == ["Qwen/Qwen3-8B", "Qwen/Qwen2.5-7B-Instruct"] | ||
|
|
||
|
|
||
| def test_invalid_format_omits_suggestions_when_none_found(): | ||
| with patch("huggingface_hub.list_models") as mock_list: | ||
| mock_list.return_value = _fake_models() | ||
| result = _get_model_info_from_hf("qwen3:8b") | ||
|
|
||
| assert result["error"] == "Invalid HuggingFace model ID format: 'qwen3:8b'" | ||
| assert "suggestions" not in result |
When pre_flight or estimate_resources received a malformed model reference (an Ollama-style tag like qwen3:8b, or an hf:// prefixed value), the response was a bare "Invalid HuggingFace model ID format" error with no guidance, so users and agents retried with similarly wrong IDs. Add a best-effort _suggest_hf_model_ids helper that normalizes the input (drops an hf:// prefix and any Ollama-style :tag suffix) and queries huggingface_hub.list_models for close matches, attaching them as a "suggestions" list. estimate_resources re-wraps the helper error into a ToolError, so propagate the suggestions into the error details there too, which also covers pre_flight (it delegates to estimate_resources). The lookup degrades to no suggestions when it errors or finds nothing, so validation never depends on network access. huggingface_hub is already a dependency. Covered by unit tests (mocking list_models) at both the helper and the estimate_resources tool boundary. Fixes kubeflow#40 Signed-off-by: Ahmed <ahmed.dlshad.m@gmail.com>
AhmedDlshad007
force-pushed
the
fix/suggest-hf-model-ids-on-invalid-format
branch
from
fe98afe to
0ff34a6
Compare
|
Thanks for the review, good catch on the boundary. Fixed: |
Description
pre_flight(andestimate_resources, which share_get_model_info_from_hf) returned a bareInvalid HuggingFace model ID formaterror for a malformed model reference, with no guidance. Users and AI agents then retry with similarly wrong IDs, wasting round-trips. This attaches best-effort suggestions of real Hub model IDs, following the approach proposed in the issue.Type of Change
Checklist
Related Issues
Fixes #40
Problem
With an invalid model id (e.g. an Ollama-style tag
qwen3:8b, or anhf://prefixed value),_get_model_info_from_hffails the_HF_MODEL_ID_REcheck and returns only:{"error": "Invalid HuggingFace model ID format: 'qwen3:8b'"}Fix
Add a small
_suggest_hf_model_ids(model)helper that:hf://prefix and any Ollama-style:tagsuffix).huggingface_hub.list_models(search=..., limit=3, full=False)for close matches.suggestionslist on the error response.It returns no suggestions (and adds no
suggestionskey) when the Hub lookup errors or finds nothing, so the validation path stays robust offline and never depends on network access.huggingface_hubis already a dependency.Result now:
{"error": "Invalid HuggingFace model ID format: 'qwen3:8b'", "suggestions": ["Qwen/Qwen3-8B", "Qwen/Qwen2.5-7B-Instruct"]}Tests Added
New
tests/unit/trainer/test_planning.py(mockslist_models, no network)::tagandhf://prefix are normalized before searching the Hub.Full suite passes locally: 151 passed (144 existing + 7 new).
Note on CI
mainis currently red independently of this change: thepre-commitjob'sruff-formathook reports drift on files this PR does not touch, andsecurity-audit(pip-audit) flags a transitive CVE. Becausetestis gated onpre-commit, the test matrix is skipped repo-wide until that is resolved. The change here is clean under both the pre-commit ruff pin (0.14.14) and themake verifyruff (0.15.12), and all unit tests pass locally. Happy to rebase once main is green./kind bug