Skip to content

feat(cli): -m/--model cannot select a revision, while mlxcel download can #1113

Closed

Description

@rapsealk

Summary

mlxcel download accepts --revision (branch, tag, or commit hash), but the -m/--model resolver used by generate, run, serve and inspect has no equivalent and always resolves against main. A user can fetch a pinned revision but cannot then run it through the resolver by repo-id. The plumbing already threads a revision argument end to end; only the two public entry points hardcode None.

Background

  • mlxcel download defines --revision at src/downloader/cli.rs:85-88 ("Repository revision (branch, tag, or commit hash). Defaults to main."), with an example at :50-51.
  • src/downloader/resolver.rs:153-155 records the gap deliberately: "The CLI subcommands do not currently expose a --revision flag, so they pass None, matching mlxcel download's default."
  • resolve_repo_id (src/downloader/resolver.rs:160-164) takes revision: Option<&str> and threads it into store::hf_cache_snapshot (:247) and download_options (:291-305, setting revision at :302).
  • The only callers passing None are the two public entry points resolve_model_source (:100) and resolve_model_source_with_override (:111). Wiring a flag means adding the argument to those and threading it from the subcommands, with no new resolution logic.

Proposed Solution

Add --revision <REV> to the subcommands that take -m/--model, matching mlxcel download's flag name and help text, and thread it into resolve_model_source_with_override.

Implementation Notes

Two behaviours to decide and document:

  1. What a revision means when -m names an existing local path. Step 1 of the resolver (src/downloader/resolver.rs:118-120) returns local paths verbatim and never consults a revision. Erroring on --revision with a local path is probably clearer than silently ignoring it.
  2. Whether the mlxcel global store is revision-aware. store::model_dir_with_override stores snapshots at <owner>/<name> with no revision component (src/downloader/store.rs, model_dir_under), so two revisions of one repo would collide there; the read-only HF-cache probe (store::hf_cache_snapshot) is revision-aware. The simplest first version supports --revision only where it can be honoured correctly and says so.

Point 2 decides how big this is. Raise it on the issue before writing code; if store layout has to change, this stops being a good first issue and should be split.

Acceptance Criteria

  • mlxcel run owner/name --revision <REV> resolves that revision.
  • --revision help text matches mlxcel download's wording.
  • The local-path interaction is either an error or documented as ignored.
  • The store-collision question from point 2 is answered in the PR description, not left implicit.
  • src/downloader/resolver.rs:153-155 is updated, since it documents the absence.

Original Suggestion

Title: feat(cli): -m/--model cannot select a revision, while mlxcel download can

Problem / Background

mlxcel download accepts --revision (src/downloader/cli.rs:85-88), documented in its
own help as "Repository revision (branch, tag, or commit hash)" with an example at
src/downloader/cli.rs:50-51:

# Specific revision (branch, tag, or commit hash):
mlxcel download mlx-community/Qwen3-4B-4bit --revision main

The -m/--model resolver used by generate, run, serve and inspect has no
equivalent. src/downloader/resolver.rs:153-155 records this deliberately:

revision selects the HF-cache snapshot revision (branch / tag / commit); None means
main. The CLI subcommands do not currently expose a --revision flag, so they pass
None
, matching mlxcel download's default.

So a user can fetch a pinned revision but cannot then run it through the resolver by
repo-id: mlxcel run owner/name always resolves against main.

Why this is small

The plumbing is already in place and already threaded end to end. resolve_repo_id
(src/downloader/resolver.rs:160-164) takes revision: Option<&str> and passes it to both
store::hf_cache_snapshot (:247) and download_options (:291-305). The only callers
that pass None are the two public entry points,
resolve_model_source (:100) and resolve_model_source_with_override (:111). Wiring a
flag means adding the argument to those entry points and threading it from the subcommands
— no new resolution logic.

Proposed solution

Add --revision <REV> to the subcommands that take -m/--model, matching mlxcel download's
flag name and help text, and thread it into resolve_model_source_with_override.

Two behaviours to decide and then document:

  1. What a revision means when -m names an existing local path. Step 1 of the resolver
    (src/downloader/resolver.rs:118-120) returns local paths verbatim and never consults a
    revision. Erroring on --revision with a local path is probably clearer than silently
    ignoring it.
  2. Whether the mlxcel global store (store::model_dir_with_override) is revision-aware. It
    stores snapshots at <owner>/<name> with no revision component, so two revisions of one
    repo would collide. The read-only HF-cache probe (store::hf_cache_snapshot) is
    revision-aware. The simplest first version is to support --revision only where it can
    be honoured correctly, and say so.

Point 2 is the one that decides how big this is. Raise it on the issue before writing code;
if store layout has to change, this stops being a good first issue and should be split.

Acceptance criteria

  • mlxcel run owner/name --revision <REV> resolves that revision.
  • --revision help text matches mlxcel download's wording.
  • The local-path interaction is either an error or documented as ignored.
  • The store-collision question from point 2 is answered in the PR description, not left
    implicit.
  • src/downloader/resolver.rs:153-155 is updated, since it documents the absence.

Activity

  1. inureyes commented on Aug 13, 2026

    @inureyes
    Member

    Answering point 2 first, since the issue asks for it before any code is written.

    Point 2: the mlxcel store is not revision-aware, and making it so is a store-layout change

    Confirmed against main at 5d082899:

    • store::model_dir_under composes <models_root>/<owner>/<name>. There is no revision component, so two revisions of one repo occupy the same directory.
    • This is already documented in two places rather than being an oversight. src/downloader/store.rs:550-551 says revision "is only used for the HF-cache probe (the mlxcel store is not revision-namespaced)", and mlxcel rm already ships a --revision flag whose help text in src/main.rs:251-253 says the same thing: "Repository revision used only to locate an HF-cache snapshot when the model is not in the mlxcel store (defaults to main). The mlxcel store itself is not revision-namespaced."
    • The collision is not theoretical, and it already affects mlxcel download --revision today. download_repo_blocking (src/downloader/mod.rs:792-800) checks snapshot_complete(&local_dir, &wanted) against the file list of the requested revision and, when every wanted filename is already present and non-zero, prints "all expected files already present at ..., skipping (use --force to refresh)" and returns without fetching anything. Two revisions of a repo normally share filenames (config.json, model.safetensors, ...), so requesting revision B into a store directory populated from revision A silently yields revision A.

    So making -m --revision fully general requires revision-namespacing the store, which touches model_dir_under, list_models_under, remove_model_under, and the download verb's destination. Per this issue's own instruction, that is where it stops being a good first issue.

    Conclusion: not splitting the CLI plumbing out, but splitting the store-layout work off. This issue keeps the part that can be honoured correctly with no layout change; a follow-up issue covers revision-namespacing the store and, with it, the pre-existing mlxcel download --revision collision described above, which is a defect in its own right and is not created by this change.

    What this issue will implement

    --revision is threaded through the resolver entry points and the -m/--model-taking subcommands, and is honoured only along revision-aware paths. Concretely, when a revision is requested:

    • the HuggingFace cache probe (store::hf_cache_snapshot, revision-aware) is consulted and is a correct hit;
    • the two reuse locations that carry no revision provenance, the legacy per-CWD ./models/<basename> directory and the mlxcel store at <owner>/<name>, are not treated as hits, because a hit there could be any revision and serving it would be exactly the silent-wrong-revision failure the flag exists to avoid;
    • on a genuine miss the requested revision is fetched, and when a snapshot for that repo already occupies the non-namespaced store directory the resolver fails with an explanation and the workaround rather than fetching into it, since the fetch would either be skipped as "already complete" or interleave two revisions in one directory.

    This keeps the flag's promise exact: if -m <repo> --revision <rev> returns a path, that path is <rev>. Where it cannot promise that, it says so instead of guessing.

    Point 1: --revision with an existing local path is an error

    Step 1 of the resolver returns an existing path verbatim and never consults a revision, so a revision passed alongside a local path can only be a mistaken expectation. Erroring names the mistake; ignoring it would let a user believe they had pinned something. This also matches the flag's behaviour above: it either honours the revision or refuses, and never silently ignores it.

    Both answers will be repeated in the PR description, per the acceptance criteria.

  2. added and removed on Aug 13, 2026
  3. added a commit that references this issue on Aug 20, 2026
    25f8eb5
  4. added a commit that references this issue on Oct 27, 2026
    1b9d9d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions