fix(llmodels): keep the scaling baseline in sync with list replica actions - #1413
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A zero baseline is incorrectly treated as missing during Start, contradicting the stated behavior.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Synchronizes scheduled-scaling baselines with model-list replica actions to address #6257.
Changes:
- Updates baselines during inline edit and start/stop actions.
- Adds scheduled-scaling context to list rows.
- Adds localized schedule indicators and tooltips.
| File | Description |
|---|---|
src/pages/llmodels/components/table-list.tsx |
Synchronizes replica and baseline updates. |
src/pages/llmodels/config/types.ts |
Adds schedule data to list rows. |
src/pages/llmodels/hooks/use-models-columns.tsx |
Displays the scheduled-scaling marker. |
src/locales/en-US/models.ts |
Adds English labels. |
src/locales/ja-JP/models.ts |
Adds Japanese labels. |
src/locales/ru-RU/models.ts |
Adds Russian labels. |
src/locales/tr-TR/models.ts |
Adds Turkish labels. |
src/locales/zh-CN/models.ts |
Adds Chinese labels. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| const baseline = record.scaling_schedule?.enabled | ||
| ? record.scaling_schedule.baseline_replicas | ||
| : null; | ||
| return baseline || 1; |
There was a problem hiding this comment.
Code Review
This pull request introduces support for scheduled scaling in the model list view. It adds necessary localization strings, updates the ListItem type definition, and implements helper functions in table-list.tsx to ensure that replica updates and start/stop actions correctly handle the baseline replica count when a schedule is enabled. The UI is also updated to display a 'Scheduled' tag with a tooltip explaining the scaling behavior. The reviewer suggested improving type safety by replacing the any type with ListItem in the newly added helper functions, which is a valid improvement opportunity.
| const getReplicasUpdate = (record: any, replicas: number) => { | ||
| const schedule = record.scaling_schedule; | ||
| if (!schedule?.enabled) { | ||
| return { replicas }; | ||
| } | ||
| return { | ||
| replicas, | ||
| scaling_schedule: { ...schedule, baseline_replicas: replicas } | ||
| }; | ||
| }; | ||
|
|
||
| // A stopped row reports `replicas: 0`, so the fallback is what actually decides | ||
| // the count on start. Under a schedule that is the baseline the user declared, | ||
| // not a hardcoded 1 — which is all the row's own `replicas` could ever give us. | ||
| const getStartReplicas = (record: any) => { | ||
| if (record.replicas) { | ||
| return record.replicas; | ||
| } | ||
| const baseline = record.scaling_schedule?.enabled | ||
| ? record.scaling_schedule.baseline_replicas | ||
| : null; | ||
| return baseline || 1; | ||
| }; |
There was a problem hiding this comment.
To improve type safety and maintainability, avoid using any for the record parameter in getReplicasUpdate and getStartReplicas. Since ListItem is already imported in this file, you can type record as ListItem directly.
const getReplicasUpdate = (record: ListItem, replicas: number) => {
const schedule = record.scaling_schedule;
if (!schedule?.enabled) {
return { replicas };
}
return {
replicas,
scaling_schedule: { ...schedule, baseline_replicas: replicas }
};
};
// A stopped row reports replicas: 0, so the fallback is what actually decides
// the count on start. Under a schedule that is the baseline the user declared,
// not a hardcoded 1 — which is all the row's own replicas could ever give us.
const getStartReplicas = (record: ListItem) => {
if (record.replicas) {
return record.replicas;
}
const baseline = record.scaling_schedule?.enabled
? record.scaling_schedule.baseline_replicas
: null;
return baseline || 1;
};
…tions The inline replica edit and start/stop sent only `replicas`, leaving `baseline_replicas` stale, so the scheduler reconciled the count straight back and the action silently did nothing. Start also fell back to a hardcoded 1 instead of the declared baseline. Refs #6257
8b0020e to
84a6c64
Compare

With scheduled scaling on,
replicasis the live count the scheduler writes andscaling_schedule.baseline_replicasis the idle count the user declared. The deploy form keeps the two equal, but the list page never learned the contract: the inline replica edit and start/stop spread the row and overrodereplicasalone, so the baseline went out with its old value andcompute_desired_replicasdrove the count straight back to it on the next tick. The request returned 200 and the UI showed a success toast, so the action looked like it had simply done nothing.getReplicasUpdatenow carries the baseline along with every replica change on a schedule-enabled row; rows without a schedule send the same payload as before. All three entry points route through it, and both batch start/stop reuse the same handlers.Start no longer falls back to a hardcoded 1. A stopped row reports
replicas: 0, so the fallback is what actually picks the count — it is now the declared baseline, with 1 only when there is none. A baseline of 0 is deliberately excluded: stopping from this list writes exactly that, and honouring it would make the next start a no-op.scaling_schedulewas added toListItem; the API already returns it on the row.No UI content changes — this is the bug fix only.
Not covered, pending a backend decision: inside an active window
compute_desired_replicasreturns the rule's replica count without consulting the baseline at all, so an explicit stop there is still reverted. That is questions 1 and 3 in gpustack/gpustack#6257 (comment) — either the backend suppresses the window on an explicit stop /baseline_replicas: 0, or the UI withholds the stop action while a window is active.ref #6257
🤖 Generated with Claude Code