Studio: Route first-chat downloads through Download Manager - #7684
Studio: Route first-chat downloads through Download Manager#7684Etherll wants to merge 5 commits into
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98465f2e70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): "started" | "cancelling" | null { | ||
| const job = getState().jobs[jobKeyOf(req.kind, req.repoId, req.variant)]; | ||
| if (!job || !ACTIVE_STATES.has(job.state)) return null; | ||
| return job.state === "cancelling" ? "cancelling" : "started"; |
There was a problem hiding this comment.
Handle the new cancelling outcome in every caller
When an exact download is already stopping, requestStart() now returns "cancelling", but existing consumers were not updated for the expanded outcome. In particular, the pending model auto-load flow in chat-page.tsx:2497-2520 treats only "started" and "conflict" as retained states, so selecting a model whose download is cancelling immediately clears pendingHubAutoLoad; if that cancellation loses the race and the job completes, its onComplete event no longer loads the selected model. Either preserve the previous public "started" behavior or teach these callers to wait for the terminal event and retry/complete the selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 47fffc1: the pending selection now stays attached while the existing download is cancelling, so its terminal completion can still auto-load and cancellation can clear it.
| return false; | ||
| } | ||
|
|
||
| if (startOutcome === "cancelling" && terminal === "cancelled") continue; |
There was a problem hiding this comment.
Reclassify joined starts that are already cancelling
When a user cancels while the initial startJob()/apiStart() request is still pending and then sends another first chat, the new caller joins the exact pending promise at transport-conflict.ts:112-113. That promise still resolves to "started" at lines 199/207 because isJobActiveFor() includes the cancelling state, so after the shared onCancelled event this retry condition is false and the successor chat fails instead of restarting the transfer. The joined request needs to observe that the live job became cancelling, or otherwise identify successors independently of the shared start outcome.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 47fffc1: exact joiners now observe cancellation across the full shared-start lifetime and receive their own cancelling outcome while the original owner keeps its settled result. This matches shared Promise settlement semantics documented by MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then
| loadModel: async () => { | ||
| loadAttempts += 1; | ||
| return loadModel({ |
There was a problem hiding this comment.
Recheck the server model after the managed download
If another client or CLI loads a local model while the default download is running, the runtime store remains empty because Studio does not continuously reconcile /api/inference/status. After the download completes, this path checks only the local store and then calls /api/inference/load, replacing the model that finished loading later and should have won; before this change the default load request began before that later external load. Re-run the existing server-adoption/status probe after the managed transfer completes and before loading the default model.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 47fffc1: after the managed transfer completes, the first-chat path reruns the existing server-status adoption probe before reserving or loading the downloaded default model.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Route the automatic first-chat model download through Studio's existing Download Manager. Users can now see its progress and cancel it from the normal Downloads panel.
No new UI components or visual changes are introduced.
Motivation
When a user sent their first message without any downloaded models, the chat adapter fetched the default model directly. That transfer did not appear in Download Manager, so its progress and cancellation controls were unavailable.
Changes
downloadManager.requestStart()and subscribe to its terminal events before starting it (studio/frontend/src/features/chat/api/chat-adapter.ts:2144).studio/frontend/src/features/chat/api/chat-adapter.ts:1407).studio/frontend/src/features/hub/download-manager/pending-start.ts:23).studio/frontend/src/features/hub/download-manager/transport-conflict.ts:105).How to test
Run the focused frontend tests:
cd studio/frontend node --experimental-strip-types --test tests/download-manager-pending-start.test.ts tests/first-chat-managed-download.test.tsRelated
Replaces #7651 with a smaller change focused on routing first-chat downloads through the existing Download Manager.