Skip to content

fix(serve): refuse to boot a pinned-weights package over an incomplete cache - #136

Merged
ezietlowTT merged 2 commits into
mainfrom
fix/serve-incomplete-weights-gate
Sep 22, 2026
Merged

ezietlowTT merged 2 commits into
mainfrom
fix/serve-incomplete-weights-gate

Conversation

@jashansinghTT

@jashansinghTT jashansinghTT commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to DEVSTACK-470.

Nothing about how a package serves changes here. This is about guiding the user when the weights on the host are incomplete and serve cannot fix that itself (--local-only, or the prefetch failed on a gated repo / offline Hub).

Some manifests pin the weights snapshot path straight into the serve env (the reporter's ndaly/Mistral-Small-4-119B-2603-tt-p300x2 sets MISTRAL4_WEIGHTS_DIR=/hf/hub/models--…/snapshots/…) and the loader opens that path directly. For those, our note "the model will download them inside the container" was wrong: the boot went through ~30s of device init and died in the engine on the first missing shard. The same failure now happens before device init, as a card that says what is going on and what to do:

  • whether the download is absent or was interrupted, and how much is already on disk
  • the exact hf download <repo> --revision <rev> that resumes it, and tt-model pull <id> --with-weights as the managed equivalent
  • --no-weights as the escape hatch, which warns and boots exactly as before

Packages that do not pin a path keep the advisory note, and pull never refuses. The note itself now also tells an interrupted download from an absent one, since the fix differs (resume, not start over).

Tests

  • 7 new tests in tests/test_container_cli.py, 6 of which fail on main.
  • Full offline suite: 919 passed. The 3 rootless-docker tests in test_container_run.py fail identically on untouched main on my box.
  • Not exercised end to end on hardware: all four chips here were busy, and the capacity check runs first.

To try it (a box with free TT chips + docker)

git clone https://github.com/tenstorrent/tt-model-manager && cd tt-model-manager
git checkout fix/serve-incomplete-weights-gate
python -m venv .venv && . .venv/bin/activate && pip install -e .

tt-model pull ndaly/Mistral-Small-4-119B-2603-tt-p300x2        # image only

# fake an interrupted weights download (instant, 1 GB sparse file)
B=~/.cache/huggingface/hub/models--mistralai--Mistral-Small-4-119B-2603/blobs
mkdir -p "$B" && fallocate -l 1G "$B/0000deadbeef.incomplete"

tt-model serve ndaly/Mistral-Small-4-119B-2603-tt-p300x2 --local-only

Expected: a red card "weights download is incomplete" naming MISTRAL4_WEIGHTS_DIR and "1.0 GB is on disk", exit 1, nothing new in docker ps. Add --no-weights and it prints a ⚠ "a download was interrupted" note and boots instead (Ctrl-C, then tt-model stop …). Clean up with rm "$B/0000deadbeef.incomplete".

🤖 Generated with Claude Code

…e cache

Some container manifests pin the weights snapshot path straight into the
serve env (MISTRAL4_WEIGHTS_DIR=/hf/hub/models--org--w/snapshots/...) and the
loader opens it directly. For those packages the advisory "the model will
download them inside the container" is false: with --local-only, or when the
prefetch itself fails (gated repo, offline), serve warned and booted anyway,
paid ~30s of device init, and died in the engine on the first missing shard.

Pass the merged serve profile into ensure_weights (serve only; pull never
refuses). When its env pins a path into the weights' HF cache dir and the
cache is not whole after the point where serve could have fixed it, raise a
diagnosis card instead of the note: it tells an interrupted download from an
absent one, says how much is already on disk, and names the exact
`hf download --revision` that resumes it plus the managed
`tt-model pull --with-weights`. --no-weights stays the escape hatch. The
advisory note itself now also distinguishes a partial download from an
absent one, since the fix differs (resume, not restart).

DEVSTACK-470

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jashansinghTT
jashansinghTT marked this pull request as ready for review September 21, 2026 18:10
@jashansinghTT
jashansinghTT requested review from a team and ezietlowTT as code owners September 21, 2026 18:10

@anirudTT anirudTT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question : on the implementation, do we also help the user re download the model weights ?, this can ofc be a follow up PR

@ezietlowTT ezietlowTT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approve. Reviewed and tested (container_cli tests 182 passed; full offline suite 922 passed; verified 6 of the new tests fail against pre-fix main).

The core insight is right and the fix is well-scoped. Two kinds of package declare weights: one calls from_pretrained on the HF id inside the container (a missing cache is just a slow first boot), the other pins the snapshot path into the serve env and the loader opens it directly (a missing shard is fatal, in the engine, after ~30s of device init). The old advisory "the model will download them inside the container" is simply false for the second kind, and this moves that failure to a card before device init.

Verified against the code:

  • _pinned_weights_env keys off the HF cache dir name (models--org--repo) appearing in any serve-env value — the one stable marker that the model reads from the cache rather than fills it. Reasonable and correct.
  • The refusal fires only for serve (which passes profile) with a pinned path over an incomplete cache that serve can't fix (--local-only, or the prefetch failed); pull passes no profile and so never refuses — matches the stated contract, and there's a test for it.
  • --no-weights stays the escape hatch (a warn, not a refusal) — pinned and not no_weights. Correct.
  • A complete cache still serves offline (tested), so the gate doesn't over-fire.
  • Interrupted vs absent is told apart with bytes-on-disk, so the remedy is resume (not delete-and-restart), and a failed prefetch's own classification (gated/offline/no-repo) is folded into the card's actions.

Not hardware-exercised (author notes the chips were busy and the capacity check runs first) — acceptable here: the whole path is offline-testable and the new tests cover the refuse/warn/escape-hatch/pull branches. community-labeling red is the known #80 gating.

# Conflicts:
#	tests/test_container_cli.py
@ezietlowTT

Copy link
Copy Markdown
Contributor

Resolved the merge conflict for you. It was a purely additive collision in tests/test_container_cli.py: #134 (card template fields) landed on main and appended its push-card-gate tests at the end of the same file where this PR appends its pinned-weights-gate tests. I merged current main in and kept both test blocks (nothing else conflicted).

Non-destructive (fast-forward merge commit, no history rewrite). Verified: full offline suite 986 passed, and this branch's diff vs main is exactly this PR's feature (the weights gate + its tests + docs) — nothing from #134 leaked in as new, nothing dropped. My approval still stands; it's mergeable now.

@ezietlowTT
ezietlowTT merged commit 5caec6c into main Sep 22, 2026
7 checks passed
@ezietlowTT
ezietlowTT deleted the fix/serve-incomplete-weights-gate branch September 22, 2026 18:32
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.

3 participants