Skip to content

don't answer a bad model path from the cache - #174

Merged
chiruu12 merged 3 commits into
devfrom
fix/model-path-substitution-149
Aug 31, 2026
Merged

chiruu12 merged 3 commits into
devfrom
fix/model-path-substitution-149

Conversation

@chiruu12

Copy link
Copy Markdown
Collaborator

Closes #149.

resolve_spec_path fell through to resolve_local_path(tier) whenever the configured checkpoint failed validation, so a pinned path that went missing was answered with whatever the cache held, silently. require_ml=True did not help, because the substituted model satisfied it.

The cache is now only consulted when nobody named a checkpoint. An explicit path or UNPLUG_MODEL_PATH that is unusable stays unusable and warns, and the caller decides what happens next: download it when auto_download_model is set, degrade when require_ml is false, raise when it is true. Auto-download is unaffected, since it already runs after this and keys off the same validity check.

Three tests. They build a populated cache under tmp_path rather than relying on the machine having model weights, so they fail on a runner too. Reverting store.py turns four tests red, including the two on dev that were passing in CI for the wrong reason.

1255 passed, 3 skipped, 30 deselected
ruff, ruff format, mypy: clean

Copilot AI lite review requested due to automatic review settings August 29, 2026 12:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 29, 2026 •

Copy link
Copy Markdown

Greptile Summary

The PR prevents invalid explicitly configured model paths from being silently replaced by cached or automatically downloaded checkpoints.

  • Preserves invalid explicit and UNPLUG_MODEL_PATH values for downstream handling.
  • Improves required-ML errors by naming the unusable checkpoint path.
  • Adds deterministic cache, auto-download, and error-message regression tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
sdk/src/unplug/core/runtime/model_runtime.py Stops auto-download from replacing a named invalid checkpoint and reports the offending path when ML is required.
sdk/src/unplug/ml/store.py Restricts cache fallback to specifications without an explicit or environment-provided checkpoint path.
sdk/tests/unit/ml/test_model_store.py Adds isolated regression coverage for explicit paths, environment overrides, cache resolution, auto-download, and required-ML errors.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve active model specification] --> B{Explicit or environment path set?}
  B -->|Yes| C{Valid checkpoint?}
  C -->|Yes| D[Load configured checkpoint]
  C -->|No| E[Preserve unusable path]
  E --> F{ML required?}
  F -->|Yes| G[Fail model setup]
  F -->|No| H[Continue with degraded ML coverage]
  B -->|No| I{Cached checkpoint available?}
  I -->|Yes| J[Load cached checkpoint]
  I -->|No| K{Automatic download enabled?}
  K -->|Yes| L[Download catalog tier]
  K -->|No| F
Loading

Reviews (3): Last reviewed commit: "name the rejected path in the require_ml..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown

coverage

SDK Coverage •
FileStmtsMissCoverMissing
src/unplug/core/runtime
   model_runtime.py1112875%49, 59, 61, 63, 72, 94–97, 100, 105, 112, 128, 130, 157–165, 170, 172–175
src/unplug/ml
   store.py2042389%52–53, 61–62, 68, 100, 144–145, 148, 160, 163, 196, 201–202, 214, 229, 233, 244, 262–265, 326
TOTAL740489388% 

Tests Skipped Failures Errors Time
1290 25 💤 0 ❌ 0 🔥 41.321s ⏱️

@chiruu12
chiruu12 merged commit 46e277e into dev Aug 31, 2026
7 checks passed
@chiruu12
chiruu12 deleted the fix/model-path-substitution-149 branch August 31, 2026 21:09
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.

Invalid configured model path silently falls back to the cache

2 participants