studio: show the active run's saved config in the Training Progress popover - #7217
Merged
danielhanchen merged 9 commits intoJul 19, 2026
Merged
danielhanchen merged 9 commits into
danielhanchen merged 9 commits into
Conversation
…opover The Training Config popover on the live Training Progress page read the editable form store (useTrainingConfigStore), so it showed stale/static values whenever the form changed after the run started; only the History view read the run's saved config snapshot, which is why re-opening the same run from Recents showed the correct values (unslothai#6853). Wire the live view to the same authoritative source History already uses: - Extract History's field mapping into sections/run-config-override.ts (mapRunConfigToOverride) so both views share one mapper over GET /api/train/runs/{id} config. - LiveTrainingView fetches the run record as soon as the job id is known and passes the mapped override to ProgressSection; the fetched config is keyed by job id, and until it loads (or if the fetch fails) the form store remains the fallback. The run record is created at job start, so it is available while the run is live. - ProgressSection prefers configOverride whenever one is present instead of only when isHistorical, so the live override takes effect. Adds a source-level regression test pinning the wiring and the mapper's backend config keys. Fixes unslothai#6853
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e96943d02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ved method Two review fixes on the live Training Config popover source: 1. The backend creates the run row only on the first progress event, so the fetch issued as soon as the job id appeared commonly 404'd during model/dataset preparation and never retried -- leaving the popover on the form store for the whole run. The effect is now also keyed on firstStepReceived (and skips once resolved for the job), so it re-fetches exactly when the row is guaranteed to exist. 2. The popover's method label and LoRA-row visibility came from viewData.trainingMethod, still read from the editable form store; changing the form (e.g. LoRA -> Full) after starting a run relabeled it and hid its saved LoRA rows. The run-config mapper now derives trainingMethod from the snapshot's training_type/load_in_4bit (via parseBackendTrainingMethod, now exported from the feature index) and the live view prefers it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a877c87a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rst step The live config-popover fetch was keyed on firstStepReceived, which the runtime store sets only when step > 0. A run that fails or completes during preparation (before step 1) creates and finalizes its row from the terminal error/complete event, but neither the job id nor firstStepReceived changed, so the fetch never ran and the popover stayed on the editable form store -- showing the wrong config/method if the form was edited afterward (Configure re-enables on failure). Gate the fetch on a runRowReady signal = firstStepReceived OR a terminal phase (completed/error/stopped), the states in which the backend guarantees the row exists. This also stops the earlier fetch-then-404 churn during preparation and lets the effect depend only on values it reads (no lint suppression needed).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40a31877f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…-ready Two ways the popover could stay stuck on the editable form store for a whole run: - The backend publishes the progress event that reveals the run before create_run commits, so the first lookup can lose that race and 404. The catch changed neither runRowReady nor fetchedRunConfig, leaving every effect dependency identical, so no further attempt was ever made for that job. The failure path now schedules an explicit retry, bounded and keyed by job id, so a genuinely absent row falls back to the form store instead of polling. - A run recovered through status/metrics polling (SSE unavailable or blocked) has currentStep restored by applyStatus/applyMetrics but never firstStepReceived, and the phase stays training, so the row was treated as not ready even at step > 0. currentStep > 0 is now a readiness signal of its own.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3307b5f670
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
start_training() inserts the run row before the pump can consume any event -- deliberately, so the run appears in history during model loading -- and /status exposes the job id throughout the pre-step phases. Gating the lookup on a first step or a terminal phase therefore held the popover on the editable form store for the whole configuring/loading/downloading window, which on a long model or dataset load is minutes, and indefinitely for a run adopted from another client. The job id is now the entire readiness condition; the existing bounded retry still covers the instant before the insert commits.
… config; tighten popover comments
Member
|
Confirmed this fix live in Unsloth Studio. I started one training run (Qwen2.5-0.5B, QLoRA, 5 steps), then changed the editable Training form to different values, mimicking a user who edits the form after the run has started (#6853). Then I opened the Training Config popover on the live Training Progress page against both Before (
It is the same run in both (identical loss 3.5124 and grad norm 97.751); only the popover's data source differs: epochs 1 vs 3, batch size 8 vs 2, learning rate 0.05 vs 0.0003, max steps 999 vs 5, context length 256 vs 1024, warmup steps 42 vs 2, rank 8 vs 64, alpha 16 vs 128, variant lora vs rslora, method LoRA vs QLoRA. I also pushed a small follow-up to this branch: the History view now keeps showing blanks when a run has no saved config snapshot, instead of falling back to the live form store, and the popover comments were tightened. |
Member
|
Thanks! |
danielhanchen
pushed a commit
that referenced
this pull request
Jul 19, 2026
…opover (#7217) * studio: show the active run's saved config in the Training Progress popover The Training Config popover on the live Training Progress page read the editable form store (useTrainingConfigStore), so it showed stale/static values whenever the form changed after the run started; only the History view read the run's saved config snapshot, which is why re-opening the same run from Recents showed the correct values (#6853). Wire the live view to the same authoritative source History already uses: - Extract History's field mapping into sections/run-config-override.ts (mapRunConfigToOverride) so both views share one mapper over GET /api/train/runs/{id} config. - LiveTrainingView fetches the run record as soon as the job id is known and passes the mapped override to ProgressSection; the fetched config is keyed by job id, and until it loads (or if the fetch fails) the form store remains the fallback. The run record is created at job start, so it is available while the run is live. - ProgressSection prefers configOverride whenever one is present instead of only when isHistorical, so the live override takes effect. Adds a source-level regression test pinning the wiring and the mapper's backend config keys. Fixes #6853 * studio: retry the run-config fetch after the first step, carry the saved method Two review fixes on the live Training Config popover source: 1. The backend creates the run row only on the first progress event, so the fetch issued as soon as the job id appeared commonly 404'd during model/dataset preparation and never retried -- leaving the popover on the form store for the whole run. The effect is now also keyed on firstStepReceived (and skips once resolved for the job), so it re-fetches exactly when the row is guaranteed to exist. 2. The popover's method label and LoRA-row visibility came from viewData.trainingMethod, still read from the editable form store; changing the form (e.g. LoRA -> Full) after starting a run relabeled it and hid its saved LoRA rows. The run-config mapper now derives trainingMethod from the snapshot's training_type/load_in_4bit (via parseBackendTrainingMethod, now exported from the feature index) and the live view prefers it. * studio: fetch the run config on a terminal phase too, not just the first step The live config-popover fetch was keyed on firstStepReceived, which the runtime store sets only when step > 0. A run that fails or completes during preparation (before step 1) creates and finalizes its row from the terminal error/complete event, but neither the job id nor firstStepReceived changed, so the fetch never ran and the popover stayed on the editable form store -- showing the wrong config/method if the form was edited afterward (Configure re-enables on failure). Gate the fetch on a runRowReady signal = firstStepReceived OR a terminal phase (completed/error/stopped), the states in which the backend guarantees the row exists. This also stops the earlier fetch-then-404 churn during preparation and lets the effect depend only on values it reads (no lint suppression needed). * studio: retry the run-config lookup and accept a hydrated step as row-ready Two ways the popover could stay stuck on the editable form store for a whole run: - The backend publishes the progress event that reveals the run before create_run commits, so the first lookup can lose that race and 404. The catch changed neither runRowReady nor fetchedRunConfig, leaving every effect dependency identical, so no further attempt was ever made for that job. The failure path now schedules an explicit retry, bounded and keyed by job id, so a genuinely absent row falls back to the form store instead of polling. - A run recovered through status/metrics polling (SSE unavailable or blocked) has currentStep restored by applyStatus/applyMetrics but never firstStepReceived, and the phase stays training, so the row was treated as not ready even at step > 0. currentStep > 0 is now a readiness signal of its own. * studio: fetch the saved run config as soon as the job id exists start_training() inserts the run row before the pump can consume any event -- deliberately, so the run appears in history during model loading -- and /status exposes the job id throughout the pre-step phases. Gating the lookup on a first step or a terminal phase therefore held the popover on the editable form store for the whole configuring/loading/downloading window, which on a long model or dataset load is minutes, and indefinitely for a run adopted from another client. The job id is now the entire readiness condition; the existing bounded retry still covers the instant before the insert commits. * Fix Training Config popover fallback for history runs without a saved config; tighten popover comments --------- Co-authored-by: danielhanchen <unslothai@gmail.com>
VectorCipher
pushed a commit
to VectorCipher/unsloth
that referenced
this pull request
Jul 20, 2026
…opover (unslothai#7217) * studio: show the active run's saved config in the Training Progress popover The Training Config popover on the live Training Progress page read the editable form store (useTrainingConfigStore), so it showed stale/static values whenever the form changed after the run started; only the History view read the run's saved config snapshot, which is why re-opening the same run from Recents showed the correct values (unslothai#6853). Wire the live view to the same authoritative source History already uses: - Extract History's field mapping into sections/run-config-override.ts (mapRunConfigToOverride) so both views share one mapper over GET /api/train/runs/{id} config. - LiveTrainingView fetches the run record as soon as the job id is known and passes the mapped override to ProgressSection; the fetched config is keyed by job id, and until it loads (or if the fetch fails) the form store remains the fallback. The run record is created at job start, so it is available while the run is live. - ProgressSection prefers configOverride whenever one is present instead of only when isHistorical, so the live override takes effect. Adds a source-level regression test pinning the wiring and the mapper's backend config keys. Fixes unslothai#6853 * studio: retry the run-config fetch after the first step, carry the saved method Two review fixes on the live Training Config popover source: 1. The backend creates the run row only on the first progress event, so the fetch issued as soon as the job id appeared commonly 404'd during model/dataset preparation and never retried -- leaving the popover on the form store for the whole run. The effect is now also keyed on firstStepReceived (and skips once resolved for the job), so it re-fetches exactly when the row is guaranteed to exist. 2. The popover's method label and LoRA-row visibility came from viewData.trainingMethod, still read from the editable form store; changing the form (e.g. LoRA -> Full) after starting a run relabeled it and hid its saved LoRA rows. The run-config mapper now derives trainingMethod from the snapshot's training_type/load_in_4bit (via parseBackendTrainingMethod, now exported from the feature index) and the live view prefers it. * studio: fetch the run config on a terminal phase too, not just the first step The live config-popover fetch was keyed on firstStepReceived, which the runtime store sets only when step > 0. A run that fails or completes during preparation (before step 1) creates and finalizes its row from the terminal error/complete event, but neither the job id nor firstStepReceived changed, so the fetch never ran and the popover stayed on the editable form store -- showing the wrong config/method if the form was edited afterward (Configure re-enables on failure). Gate the fetch on a runRowReady signal = firstStepReceived OR a terminal phase (completed/error/stopped), the states in which the backend guarantees the row exists. This also stops the earlier fetch-then-404 churn during preparation and lets the effect depend only on values it reads (no lint suppression needed). * studio: retry the run-config lookup and accept a hydrated step as row-ready Two ways the popover could stay stuck on the editable form store for a whole run: - The backend publishes the progress event that reveals the run before create_run commits, so the first lookup can lose that race and 404. The catch changed neither runRowReady nor fetchedRunConfig, leaving every effect dependency identical, so no further attempt was ever made for that job. The failure path now schedules an explicit retry, bounded and keyed by job id, so a genuinely absent row falls back to the form store instead of polling. - A run recovered through status/metrics polling (SSE unavailable or blocked) has currentStep restored by applyStatus/applyMetrics but never firstStepReceived, and the phase stays training, so the row was treated as not ready even at step > 0. currentStep > 0 is now a readiness signal of its own. * studio: fetch the saved run config as soon as the job id exists start_training() inserts the run row before the pump can consume any event -- deliberately, so the run appears in history during model loading -- and /status exposes the job id throughout the pre-step phases. Gating the lookup on a first step or a terminal phase therefore held the popover on the editable form store for the whole configuring/loading/downloading window, which on a long model or dataset load is minutes, and indefinitely for a run adopted from another client. The job id is now the entire readiness condition; the existing bounded retry still covers the instant before the insert commits. * Fix Training Config popover fallback for history runs without a saved config; tighten popover comments --------- Co-authored-by: danielhanchen <unslothai@gmail.com>
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Training Config popover on the live Training Progress page read the
editable form store (useTrainingConfigStore), so it showed stale/static values
whenever the form changed after the run started; only the History view read the
run's saved config snapshot, which is why re-opening the same run from Recents
showed the correct values (#6853).
Wire the live view to the same authoritative source History already uses:
(mapRunConfigToOverride) so both views share one mapper over
GET /api/train/runs/{id} config.
passes the mapped override to ProgressSection; the fetched config is keyed
by job id, and until it loads (or if the fetch fails) the form store remains
the fallback. The run record is created at job start, so it is available
while the run is live.
only when isHistorical, so the live override takes effect.
Adds a source-level regression test pinning the wiring and the mapper's
backend config keys.
Fixes #6853