Skip to content

infra[notask]: fix the SDK e2e models cache key - #4592

Open
Victor-Rodzko wants to merge 1 commit into
mainfrom
infra/sdk-e2e-models-cache-key
Open

Victor-Rodzko wants to merge 1 commit into
mainfrom
infra/sdk-e2e-models-cache-key

Conversation

@Victor-Rodzko

@Victor-Rodzko Victor-Rodzko commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • hashFiles() in the SDK e2e cache steps points at packages/sdk/models/registry/models.ts. The registry moved to packages/inference in QVAC-21751 chore[bc]: use @qvac/inference for SDK #3595 and that path no longer exists. hashFiles() on a path that matches nothing returns an empty string, so the key has been the constant qvac-models- since June — visible verbatim in every run log as Cache restored from key: qvac-models-.

  • Registry changes therefore never invalidate the models cache.

  • desktop and electron share that single key. bootstrap:desktop pre-downloads 54 model constants; bootstrap:electron pre-downloads 36, a strict subset. The 18 desktop-only ones are the heavy models (FLUX_2_KLEIN_4B_Q4_0, UMT5_XXL_ENC_Q8_0, QWEN3_4B_Q4_K_M, the AUDIOGEN_* set, ABOT_WORLD_*, SD_V2_1_1B_Q8_0, GROOT_*, PI05_BASE_Q_AGGRESSIVE, SMOLVLA_LIBERO_VISION_Q8, REALESRGAN_X4PLUS_ANIME_6B, QWEN3_5_0_8B_MULTIMODAL_Q8_0).

  • Electron finishes bootstrap in ~9s against ~20min for desktop, so it always reserves the key first. Desktop is then served a hit and, because Save models cache is gated on cache-hit != 'true', never saves its own set — it re-downloads those 18 models inside Run tests (consumer + producer) on every run and throws them away.

  • In run 35338929076 all six jobs restored the same ~14.87 GB entry:

    job pre-download bootstrap
    desktop · win25 54 22m 15s
    desktop · ubuntu2204 54 23m 35s
    desktop · macos26 54 13m 39s
    electron · win25 36 12s
    electron · ubuntu2204 36 9s
    electron · macos26 36 8s
  • That bootstrap runs inside a step capped at timeout-minutes: 60. On qvac-win25-x64-gpu it consumed 22 of those 60 minutes and the job died on The action 'Run tests (consumer + producer)' has timed out after 60 minutes.

📝 How does it solve it?

  • Point hashFiles() at the registry's real location, packages/inference/src/models/registry/models.ts, so the key hashes actual content again.
  • Carry inputs.consumer in the shared key: desktop and electron keep separate entries and each saves its own complete set.
  • Fix the same dead path in the Snap cache steps. Snap already has its own key and path and never reaches the shared steps, so it needs no consumer segment — inputs.consumer only ever reaches the shared key as desktop or electron.
  • Changing the key also strands the current constant-key entry, so the first run after merge misses and repopulates correctly. With a miss, the download happens in the dedicated Bootstrap (download models) step, which is not under the 60-minute cap, instead of eating the test budget.
  • Not addressed here: suite/filter are still absent from the key, so a smoke run can save a narrower set over a full one; and Save models cache still only runs on a miss, so models a test pulls outside bootstrap (the skipPreDownload defs echo, sharded-embeddings, sharded-llm) never land in the cache.

🧪 How was it tested?

  • actionlint .github/workflows/test-node-sdk.yml — clean.

  • Verified packages/inference/src/models/registry/models.ts exists and holds the constants the bootstrap pulls (LLAMA_3_2_1B_INST_Q4_0, FLUX_2_KLEIN_4B_Q4_0, …); packages/sdk/src/models/registry/index.ts only re-exports it, so it is not a valid hash target.

  • Desktop-only e2e dispatch off this branch: run 35368887881. The key now carries a real hash, the entry misses, and — for the first time — the save step actually runs:

    key: qvac-models-desktop-902d32f07922c2e1d3e69917a7637352129a675e960408b92ba0799720f17051
    Cache not found for input keys: qvac-models-desktop-902d32f0...
    📥 Pre-downloading 54 unique model constant(s)
    🔧 Bootstrap completed in 1612712ms
    Cache saved with key: qvac-models-desktop-902d32f0...
    
  • Step timings, win25 and macos26:

    step win25 macos26
    Restore models cache miss, 0s miss, 0s
    Bootstrap (download models) 26m 54s 19m 23s
    Save models cache 8m 58s ✅ ran 10m 11s ✅ ran
    Run tests (consumer + producer) 37m 38s, success 20m 08s, success
  • The download now happens in its own step rather than inside the 60-minute test step: the harness's in-test bootstrap found everything already present and finished in 37s against 22min in run 35338929076. qvac-win25-x64-gpu, which previously died on has timed out after 60 minutes, is green.

  • That run was dispatched at 3313162cb, before the branch was rebased onto main. .github/workflows/test-node-sdk.yml and packages/inference/src/models/registry/models.ts are byte-identical at 3313162cb and the current head 5f8e588f7, so the key hash and the result carry over.

  • The saved entry is scoped to this branch. After merge, the first desktop run on main misses once and repopulates at main scope, where every branch can read it.

@Victor-Rodzko
Victor-Rodzko requested review from a team as code owners September 18, 2026 16:18
@github-actions

Copy link
Copy Markdown
Contributor

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

@github-actions

Copy link
Copy Markdown
Contributor

License compliance — clean

No new dependency license findings in this PR.

Warn-only (shadow) mode — this check does not block merges yet.

Updated automatically by the canonical license compliance workflow.

NOTICE presence (advisory)

Missing NOTICE (advisory, does not block):

  • ./docs/website
  • ./packages/fabric/test/integration
  • ./packages/llm-llamacpp/benchmarks/server
  • ./packages/llm-llamacpp/benchmarks/performance
  • ./packages/inference-addon-cpp/mobile
  • ./packages/asr-ggml/benchmarks/server
  • ./packages/embed-llamacpp/benchmarks/server
  • ./packages/embed-llamacpp/benchmarks/performance
  • ./packages/sdk/e2e
  • ./packages/vla-ggml/sim/server
  • ./.github/actions/release-merge-guard

@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Workflow security (shadow mode)

zizmor found 1124 finding(s) in .github/ (highest severity: high). This check is warn-only and does not block the merge.

Findings are annotated inline on the changed files and listed in the job summary.

Reproduce locally:

pipx run zizmor==1.27.0 --offline .github/

hashFiles() pointed at packages/sdk/models/registry/models.ts, which moved to
packages/inference in #3595. hashFiles() on a path that matches nothing returns
an empty string, so since June every SDK e2e job has restored from the constant
key `qvac-models-`: registry changes never invalidated it, and desktop and
electron shared a single entry.

bootstrap:desktop pre-downloads 54 model constants; bootstrap:electron
pre-downloads 36, a strict subset. Electron finishes bootstrap in ~9s against
~20min for desktop, so it always won the race for the key, and desktop — served
a hit — skipped its own save. Desktop then re-downloaded the missing 18 models
inside the test step on every run: 22 of the 60 minutes the step is allowed on
qvac-win25-x64-gpu in run 35338929076, which is what pushed that job into the
timeout.

Point hashFiles() at the registry's real location and carry the consumer in the
key. The Snap key had the same dead path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Victor-Rodzko
Victor-Rodzko force-pushed the infra/sdk-e2e-models-cache-key branch from 3313162 to 5f8e588 Compare September 18, 2026 18:07
@Victor-Rodzko
Victor-Rodzko changed the base branch from chore/sdk-0.20.0-changelog to main September 18, 2026 18:07
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.

1 participant