[Feature] Model validation and graceful failure for unknown model IDs

[mimran-khan]

Summary

When a user sets SKILLSPECTOR_MODEL to an unknown model ID that isn't in any provider's model_registry.yaml, SkillSpector silently falls back to a 128k context length default (_FALLBACK_CONTEXT_LENGTH). If the model's actual context window is smaller, this causes oversized batches that get truncated by the API — or the API call fails entirely at runtime with an opaque error.

There is no upfront validation that the configured model is recognized, and no warning that token budgeting is using fallback values. Users discover the problem only after a scan fails partway through.

Reproduction

export SKILLSPECTOR_PROVIDER=openai
export SKILLSPECTOR_MODEL=gpt-4o-mini-2024-07-18-preview
# Model ID typo — not in model_registry.yaml

skillspector scan ./my-skill/

Current behavior: Scan starts, uses 128k fallback context length. If the real model has 8k context, batches overflow and the API returns truncation errors or partial results mid-scan.

Expected behavior: Warning at startup that gpt-4o-mini-2024-07-18-preview is not in the model registry, showing the fallback values being used. Optionally, a strict mode that refuses to run with unvalidated models.

Proposed Implementation

Option A: Warn on startup (default behavior)

import logging
logger = logging.getLogger(__name__)

for slot, model in MODEL_CONFIG.items():
    ctx = _provider.get_context_length(model)
    if ctx is None:
        logger.warning(
            "Model '%s' (slot: %s) not found in model_registry.yaml. "
            "Using fallback context length (%d). Token budgeting may be "
            "inaccurate — add the model to the registry or verify the model ID.",
            model, slot, _FALLBACK_CONTEXT_LENGTH,
        )

Option B: Opt-in strict mode

if os.environ.get("SKILLSPECTOR_STRICT_MODEL_VALIDATION", "").lower() == "true":
    unknown = [
        f"  {slot}: {model}"
        for slot, model in MODEL_CONFIG.items()
        if _provider.get_context_length(model) is None
    ]
    if unknown:
        raise ValueError(
            "Strict model validation enabled. Unknown models:\n"
            + "\n".join(unknown)
            + "\nAdd them to model_registry.yaml or disable "
            "SKILLSPECTOR_STRICT_MODEL_VALIDATION."
        )

Both options can coexist: warn by default, fail-fast when strict mode is enabled.

Related

• models glm-5.2, glm-5.1, kimi-k2.7-code, deepseek-v4-flash fail #129 (OPEN): Models like glm-5.2, kimi-k2.7-code, deepseek-v4-flash fail at runtime. Upfront validation would catch many of these cases before the scan starts, giving users actionable feedback instead of mid-scan API errors.
• PR feat(report): add analysis_completeness field to JSON output #160 (MERGED): Added analysis_completeness field to report partial results when analyzers fail. Model validation would prevent some of these partial-result scenarios by catching bad configuration early.
• Token budget miscalculation from unvalidated models can also cause silent quality degradation — the semantic analyzer may truncate skill source code without warning, producing lower-quality findings.

Scope

• Add model validation loop after MODEL_CONFIG construction in constants.py (~15 lines)
• Add SKILLSPECTOR_STRICT_MODEL_VALIDATION env var support (~10 lines)
• Tests: known model passes, unknown model warns, strict mode raises
• Documentation: add model validation section to README

[a-thomas014488]

Running into silent fallbacks is a real pain; I've wasted hours debugging context errors that turned out to be simple typos in model names. I strongly support the "strict mode" idea, perhaps controlled by an environment variable like SKILLSPECTOR_STRICT_REGISTRY=true. It's much better to fail fast at startup than to have a scan crash halfway through due to context overflow. While Option A helps, warnings are often missed in logs, so a hard error for unvalidated models seems safer for production use.

---

🤖 若这条评论有帮助，欢迎点赞交流！

[kumayizxcvb]

Good catch — silent fallbacks to optimistic defaults are the worst kind of surprise, especially mid-scan.

For the implementation, one edge case worth considering: users with fine-tuned or locally deployed models won't ever match the registry. The warning message could suggest setting a manual context override via an additional env var, which would help those legitimate cases while still catching typos.

Also curious — is the registry currently static or fetched remotely? If strict mode depends on network access, it could break CI pipelines unexpectedly. A local cache with fallback to warn-only on fetch failures would make strict mode more reliable for automation.

Either way, surfacing this at startup instead of runtime failure is definitely the right call. The current behavior of discovering issues partway through a scan is a frustrating user experience.

---

🤖 若这条评论有帮助，欢迎点赞交流！