Skip to content

fix(llm): pin Hugging Face metadata revisions - #79

Open
pjb157 wants to merge 272 commits into
mainfrom
peter/pin-hf-model-card-revisions
Open

pjb157 wants to merge 272 commits into
mainfrom
peter/pin-hf-model-card-revisions

Conversation

@pjb157

@pjb157 pjb157 commented Aug 11, 2026

Copy link
Copy Markdown

Overview

Prevent model-card registration failures when a worker and frontend resolve different commits of the same mutable Hugging Face revision.

Root cause

A ModelDeploymentCard publishes checksums computed from the metadata files in the snapshot loaded by the worker, but its Hugging Face metadata locations identify only the repository or another mutable revision such as main.

The worker and frontend resolve that location independently and may have different cache states or resolve it at different times. If the mutable revision advances, the frontend can fetch bytes from a different commit. Checksum verification then correctly rejects the metadata, but the otherwise healthy worker cannot register.

This is a time-of-check/time-of-use problem: immutable checksums were paired with a mutable artifact address.

Details

  • Detect when a worker model path is an hf-hub snapshot and record its 40-character commit in a new optional source_revision model-card field.
  • Keep source_path as the canonical repository identifier and preserve the served model name.
  • Resolve checksummed metadata files directly from source_path@source_revision, bypassing mutable cache refs.
  • Download only the metadata files referenced by the card, using the existing cancellation-safe isolated hf-hub runtime.
  • Preserve the ModelExpress fallback for cards without an immutable revision and preserve local-path and self-hosted metadata behavior.

Why this belongs in Dynamo

Deployment-level mitigations can reduce the chance of a mismatch, but they do not make the model card self-consistent:

  • Requiring a commit in every configured model identifier shifts correctness to each operator and can couple an external model name to artifact versioning.
  • Pinning worker images or pre-populated caches does not guarantee that a separately running frontend resolves the same Hub commit.
  • DYN_SELF_HOST_METADATA is a useful transport when a reachable worker system-status server is available, but it deliberately degrades to shared-storage/Hub resolution when that server is absent and may be disabled. Correctness of the fallback should not depend on deployment topology.
  • Embedding metadata contents in NATS would also remove the second Hub lookup, but would turn discovery into a blob-transfer protocol with larger messages, retention/lifecycle questions, and a broader compatibility surface.

The model card already carries the metadata checksums and source location. Adding the immutable revision that identifies those bytes is the smallest protocol-level fix and applies to every deployment without imposing a particular transport.

Compatibility

The revision is intentionally separate from source_path:

  • New frontend with old worker: source_revision is absent, so resolution follows the existing legacy path.
  • Old frontend with new worker: the optional field is ignored and the unchanged source_path continues to use the legacy path.
  • source_revision alone does not change mdcsum, allowing mixed-version rolling upgrades when the checksummed metadata is otherwise identical.
  • Canonical model-ID consumers such as served-name handling and performance-model lookup continue to receive the original source path.

Once both sides understand source_revision, metadata is resolved from the exact worker commit.

Where should the reviewer start?

Start with lib/llm/src/local_model.rs, where the worker derives source_revision from the resolved cache snapshot. Then review lib/llm/src/model_card.rs, where the frontend applies that revision while resolving the card metadata.

The exact cache and network behavior is implemented in lib/llm/src/hub/huggingface.rs.

Validation

  • cargo fmt --all -- --check
  • cargo clippy -p dynamo-llm --no-default-features --lib --tests -- -D warnings
  • cargo test -p dynamo-llm --no-default-features --lib -- --test-threads=1 (1,744 passed; 3 ignored)
  • uvx pre-commit run --files lib/llm/src/hub.rs lib/llm/src/hub/huggingface.rs lib/llm/src/local_model.rs lib/llm/src/model_card.rs
  • git diff --check
  • Applied cleanly to current ai-dynamo/main (9f616aed66f1de3601ee8184bba53a61728467fa) with the new targeted tests and clippy passing there.

Regression coverage includes:

  • stale main pointing at a different cached snapshot;
  • empty-cache network download requesting the exact commit URL;
  • optional-field and legacy-mdcsum compatibility;
  • mutable or stale source revisions being replaced during exact resolution;
  • served-model-name preservation; and
  • cancellation of detached hf-hub chunk tasks.

Related Issues

This PR is not linked to an issue:

  • Confirmed — no related issue

galletas1712 and others added 30 commits July 7, 2026 19:38
…amo#11359)

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
…o#10773) (ai-dynamo#11344)

Signed-off-by: cmdy <zhang_lin66@foxmail.com>
Co-authored-by: cmdy <zhang_lin66@foxmail.com>
)

Signed-off-by: xianlubird <xianlubird@gmail.com>
Co-authored-by: Xianlu Bird <xianlubird@gmail.com>
Co-authored-by: Dan Gil <dagil@nvidia.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
…11379)

Signed-off-by: Daniel Gil <dagil@nvidia.com>
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
Co-authored-by: Dan Gil <dagil@nvidia.com>
…i-dynamo#11340)

Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Co-authored-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
)

Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…of hardcoding True (ai-dynamo#10738) (ai-dynamo#11366)

Signed-off-by: nnshah1 <neelays@nvidia.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
…11325) [release/1.3.0] (ai-dynamo#11367)

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
…ge (DYN-3395) (ai-dynamo#11436)

Signed-off-by: Daniel Gil <dagil@nvidia.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
…d HTTP protocol [release/1.3.0] (ai-dynamo#11382)

Signed-off-by: Jie Hao <jihao@nvidia.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…dynamo#11249) (ai-dynamo#11420)

Signed-off-by: nnshah1 <neelays@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ai-dynamo#11425)

Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
)

Signed-off-by: tanmayv25 <tanmayv@nvidia.com>
Co-authored-by: Dan Gil <dagil@nvidia.com>
…48) (ai-dynamo#11402) (ai-dynamo#11404)

Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…11489)

Signed-off-by: Zhuangcheng(Jesse) Gu <zcgu@connect.hku.hk>
…mo#11455)

Signed-off-by: davilu-nvidia <134493379+davilu-nvidia@users.noreply.github.com>
Signed-off-by: PeaBrane <yanrpei@gmail.com>
Signed-off-by: Dan Gil <dagil@nvidia.com>
Co-authored-by: davilu-nvidia <davilu@nvidia.com>
Co-authored-by: PeaBrane <yanrpei@gmail.com>
…") (ai-dynamo#10248) [cherry-pick → release/1.3.0] (ai-dynamo#11555)

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…th (ai-dynamo#11511)

Signed-off-by: ssojrani@nvidia.com <ssojrani@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Co-authored-by: ssojrani-wq <ssojrani@nvidia.com>
…1591)

Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
)

Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…arser (ai-dynamo#11627)

Signed-off-by: Zhuangcheng(Jesse) Gu <zcgu@connect.hku.hk>
ai-dynamo#11621)

Signed-off-by: Indrajit Bhosale <iamindrajitb@gmail.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
…ai-dynamo#11652) - CP (ai-dynamo#11597)

Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Co-authored-by: Julien Mancuso <161955438+julienmancuso@users.noreply.github.com>
rushilbhat and others added 19 commits August 5, 2026 18:17
feat(router): add KV affinity routing across token and text backends
#71)

* fix(helm): give sub-10s PodMonitor endpoints an explicit scrapeTimeout

The 2026-08-04 upstream sync brought in the GMS shadow-failover worker
endpoints (system-0, system-1), the frontend-sidecar http endpoint, and the
dynamo-epp PodMonitor, all hardcoded at interval: 5s with no scrapeTimeout.
Grafana Alloy's default scrape timeout is 10s, and timeout > interval makes
the scrape config invalid — Alloy then rejects the entire PodMonitor, not
just the offending endpoint, so all worker engine metrics disappeared when
1.4.0 rolled out (and epp metrics never appeared). This is the same failure
mode #60 fixed for the primary endpoints.

Set scrapeTimeout: 5s on every 5s endpoint, keeping the 5s cadence the
failover dashboard relies on.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>

* chore: catch up generated operator docs for the checkpoint job controls

helm-docs and crd-ref-docs output for jobPriorityClassName and
maxConcurrentJobs, which landed without the operator check running.
Applied verbatim from the check's own regeneration diff so 'make check'
sees a clean tree.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>

---------

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…ht tracking

Record every dispatched request in a per-endpoint registry keyed by worker
instance for the lifetime of its response stream, carrying the request's
scheduling priority (nvext.agent_hints / PreprocessedRequest.routing),
admission order, and stream context. Covers all router modes through the
single dispatch choke point, with RAII release on completion, error, and
drop. Counts are exported as dynamo_frontend_worker_admission_inflight /
_total gauges so the accounting can be validated against worker-reported
load. Accounting only: no admission decision keys off it yet. Disable
with DYN_ADMISSION_TRACKING=0.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…ax_num_seqs

Read the scheduler's derived effective_max_running_requests_per_dp from
/get_server_info internal_states (falling back to the configured
max_running_requests server arg) and carry it on the worker's
ModelRuntimeConfig. The value is per DP rank, matching the engine's own
semantics. This gives the frontend a running-capacity number for
openai_backend workers, which previously advertised only KV capacity.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…st chunk

_handle_cancellation blocked on the request-id future, which was only
resolved from the first engine chunk's meta_info. A cancelled request
still sitting in the engine's waiting queue was therefore not aborted
until it started generating. Every async_generate call already passes an
explicit rid, and the tokenizer manager registers that rid at submission,
so the stream processors now pre-resolve the future with the dispatch
rid and the monitor arms immediately. The first-chunk path remains as
the fallback when no rid is supplied.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…helpers

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
… frontend

Bound each worker's engine queue length at the dispatch choke point,
opt-in via DYN_ADMISSION_QUEUE_MARGIN — one global margin, no per-model
tuning and no capacity knowledge: the engine's own scheduler bounds the
running set, the frontend only keeps waiting work shallow. The queue
signal is the worker's reported num_waiting_reqs (summed across dp
ranks), pushed into the admission state by the worker monitor from load
reports; between reports a burst can overshoot the margin by at most one
report-interval's arrivals.

Below the margin requests admit normally; in selection-free modes a
full-queue worker retargets to one with queue headroom; at the margin
everywhere, a strictly-lower-priority in-flight request — running or
queued, from the worker's whole in-flight set — is evicted (lowest
priority first, most-recently-admitted tie-break, which naturally picks
engine-queued work) and its stream synthesizes a non-migratable
ResourceExhausted overload frame; with no victim the request is rejected
with a typed AdmissionRejection that the HTTP layer maps to the overload
status with a retry hint.

Response bodies stay free of scheduling internals: no priorities, queue
depths, or margins — only the generic overload message and
retry_after_ms. Victim details go to logs and per-worker eviction/
rejection counters. Mid-stream structured overload errors now surface
with their real code instead of the generic internal shape. Workers that
have never reported a queue depth are unenforced; everything is off by
default and unset knobs are byte-identical to before.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…stry

feat(runtime): frontend admission registry for per-worker in-flight tracking
feat(runtime): priority-aware engine-queue bound with eviction at the frontend
…-advertise

feat(openai-backend): advertise the engine running-request limit as max_num_seqs
…t-chunk

fix(sglang): arm the abort monitor with the dispatch rid, not the first chunk
The bridge only knew how to register Chat|Completions and only resolved an
upstream path for `messages` or `prompt`, so a site that attaches engines
through it could not offer an embedding model at all.

`--embedding-worker` registers ModelType.Embedding and routes requests
carrying `input` to the upstream's /v1/embeddings. That path skips the
generation machinery outright: no SSE, no stream flags, no tool-call
coalescing, no request id, and no abort scheduling, none of which mean
anything for a single pooling forward pass.

Two details the frontend imposes. The worker hop is always base64, because
the frontend decodes base64 to float at the HTTP boundary but never encodes
in the other direction — forwarding floats would hand a client that asked
for base64 the wrong shape. And the response is deserialized straight into
CreateEmbeddingResponse, whose fields are all required, so a missing key is
named here rather than surfacing as an opaque Rust deserialization failure.

Capacity probing and load reporting are skipped for embedding workers: both
key off a KV-usage gauge that a pooling engine never exports.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
Both embedding branches passed the frontend's configured router mode
straight into the PushRouter, and PushRouter::generate bails outright in
KV mode. A frontend started with `--router-mode kv` therefore failed every
/v1/embeddings request with a 500 — "KV routing should not call generate
on PushRouter" — no matter which backend served the model. Nothing caught
it because no embedding model had been registered against a KV frontend.

Pooling models hold no KV cache, so there is no prefix overlap for the KV
router to score; the branches already say as much in a comment without
acting on it. Fall back to round-robin, which is what uniform-cost
single-pass requests want anyway, and log the substitution so an operator
who set kv deliberately can see it did not apply here.

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
…ghten comment

Signed-off-by: fergus barratt <fergus.barratt00@gmail.com>
feat(openai-backend): serve embeddings from a pooling upstream
… can route to them

A rank only became a selectable KV-routing candidate after its first
ActiveLoad event arrived, but load events are non-durable and deduped at
the worker, so a frontend that subscribed after a rank's bootstrap never
learned the rank existed: it received no traffic, so its load never
changed, so it never published again. In production this pinned 100% of
a dp=8 worker's traffic to the single visible rank.

Seed kv_used_blocks and num_waiting_reqs to zero for every configured
rank when the runtime config is applied, so a configured rank with no
report yet is treated as idle rather than nonexistent. or_insert keeps
config re-application from clobbering live reported values.

Signed-off-by: Rushil Bhat <bhatrushil@gmail.com>
The worker metrics publisher deduped unchanged per-rank load reports
forever, so an idle rank published exactly once in its lifetime on a
non-durable event plane. Any subscriber that missed that message (a
redeployed frontend, or a bootstrap lost to the discovery window) could
never converge on the full rank set.

Keep the dedupe for change-driven publishing, and additionally
re-broadcast the full rank set every DYN_WORKER_METRICS_HEARTBEAT_SECS
(default 30s, 0 disables), so any subscriber converges within one
heartbeat of joining. A report whose NATS publish failed is also
retried on the next beat.

Adds e2e coverage for both halves of the fix: a late-subscribing
frontend must not pin traffic to the only rank it saw a report for, and
the heartbeat must deliver pre-subscription loads.

Signed-off-by: Rushil Bhat <bhatrushil@gmail.com>
fix(router): make every dp rank routable despite missed load reports
@pjb157
pjb157 temporarily deployed to external_collaborator August 11, 2026 14:21 — with GitHub Actions Inactive
@github-actions github-actions Bot added the fix label Aug 11, 2026
@pjb157
pjb157 marked this pull request as ready for review August 11, 2026 14:23
Copilot AI lite review requested due to automatic review settings August 11, 2026 14:23

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.

Pull request overview

This PR hardens Hugging Face–backed model-card metadata resolution by pinning “hf://” sources to an immutable commit SHA derived from the worker’s resolved hf-hub cache snapshot, preventing frontend/worker registration mismatches when a mutable revision (e.g. main) advances.

Changes:

  • Worker-side: record repo@<commit-sha> in the model card when the local model path is an hf-hub snapshot, while keeping the served/display name stable.
  • Frontend-side: when resolving model-card metadata for hf://repo@<commit>/*, fetch/validate metadata directly from that commit snapshot (cached or downloaded), avoiding mutable cache refs.
  • Add regression tests for stale main refs, revision preservation in URIs, and served-name compatibility.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
lib/llm/src/model_card.rs Resolve hf:// metadata via pinned repo@commit snapshots when available; add regression tests.
lib/llm/src/local_model.rs Pin worker-emitted source_path to the snapshot commit SHA without altering served name; add unit tests.
lib/llm/src/hub/huggingface.rs Add helpers to resolve only pinned metadata files from a commit revision (cache-first, offline-aware) and tests.
lib/llm/src/hub.rs Add from_pinned_hf_metadata entrypoint and wire it into hub exports.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Signed-off-by: pjb157 <84070455+pjb157@users.noreply.github.com>
@pjb157
pjb157 force-pushed the peter/pin-hf-model-card-revisions branch from 596faa6 to b106a73 Compare August 12, 2026 11:47
@pjb157
pjb157 had a problem deploying to external_collaborator August 12, 2026 11:47 — with GitHub Actions Failure
@rst0git
rst0git force-pushed the main branch 2 times, most recently from b74f1e6 to cafd486 Compare September 1, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.