fix(onboarding): harmonise outbound model alias with rendered payload#5
Merged
Merged
Conversation
The background 'Hello World' batch and the Run-Now handler were both
reading from a single `modelAlias` value that fell back to a hard-coded
`medgemma-4b` while the catalog query was still loading. Because the
background effect fired immediately after auth resolved (well before
`useModels` returned), the visible payload would later re-render with
the catalog alias (e.g. 'deepseek') while the backend received
'medgemma-4b' — producing 'Model medgemma-4b has not been configured or
is not available to user' on every onboarding load in environments
without that model.
Split the alias into two values:
- displayModelAlias: always populated (real alias or placeholder) and
used only for rendering code samples so the UI is never blank.
- runnableModelAlias: undefined while the catalog is loading and when
the user has no accessible chat model. Used for outbound requests.
The background batch now waits for the catalog to resolve and bails
silently (no toast, no request) when there's no entitled chat model.
Run Now does the same in its IIFE so the simulated success timer still
fires while we skip the doomed network call.
Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
The previous split of displayModelAlias / runnableModelAlias still left 'medgemma-4b' visible in the rendered code samples for the brief window between mount and the useModels query resolving. Anyone reading the payload during that window would reasonably expect medgemma to be the model the backend is asked to run — exactly the divergence we set out to remove. Replace the hard-coded fallback with an obviously-non-runnable '<your-model-alias>' placeholder, and disable the Run Now button until runnableModelAlias is populated. The visible payload and the outbound payload now share a single source of truth: when there's a real alias, both reference it; when there isn't, the placeholder is shown and the button is disabled. Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
Previously the post-success auto-redirect always landed users on
/models — fine as a generic 'where would you like to go next' but it
hides the workload they just queued. Send them instead to the route
that will surface their job:
- /async when workloadType is 'async'
- /batches when workloadType is 'batch'
Both routes are config-gated by batches.enabled (and async further
gated by batches.async_requests.enabled), so we consult
useAuthorization().canAccessRoute and fall back to /models when the
preferred destination isn't accessible on this deployment, to avoid
ProtectedRoute bouncing the user mid-redirect.
The 'Skip to Dashboard' header button still lands on /models per the
original spec. The CLI listener path also stays on /models — that path
is the 'click to continue' simulation, not an actual workload, so
there's no specific job for the user to view.
Inline success-strip copy now reflects the destination ('Taking you to
your batch/async request…') so the redirect isn't surprising.
Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f134ea9. Configure here.
| !runnableModelAlias | ||
| ? "Loading available models…" | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
Tooltip says "loading" even when no models exist
Low Severity
The title tooltip checks only !runnableModelAlias and always shows "Loading available models…". Once modelsLoading becomes false and no CHAT model is found, runnableModelAlias stays undefined permanently, so the button remains disabled with a "loading" message that will never resolve. The condition conflates the "still loading" state with the "loaded but empty" state. Users with no accessible chat models see a perpetual loading message instead of an explanation that no models are available.
Reviewed by Cursor Bugbot for commit f134ea9. Configure here.
Including browserSuccessRoute in the auto-redirect effect's deps caused a lost-redirect bug: if the value's identity changed during the 2s window (workloadType toggle, useAuthorization refetch race, config invalidation, …) the cleanup function ran clearTimeout, the effect re-ran, and the idempotency ref short-circuited it before a new timer could be scheduled. The user would see 'Taking you to your batch…' indefinitely without ever navigating. Move the destination into a ref that's updated on every render so the trigger effect captures the latest value at success-time without declaring a dep. Deps are now only the success triggers + navigate, none of which churn between idle and success, so the cleanup-cancel path is unreachable by construction. Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
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
2 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.
Bug
Reported on staging:
The visible payload rendered against
deepseek-ai/DeepSeek-V4-Pro(the user's actual catalog model), but the backend was being asked to runmedgemma-4b.Root cause
Onboarding.tsxhad a singlemodelAliasvalue that read fromuseModels({ accessible: true })and fell back to a hard-coded"medgemma-4b"while the catalog query was still loading. The background "Hello World"useEffectfired the moment auth resolved (almost always beforeuseModelsreturned), captured the placeholder via closure, and POSTedmedgemma-4bto/ai/v1/files+/ai/v1/batches. By the time the visible payload re-rendered with the real catalog alias the request had already gone out with the wrong model.handleRunNowhad the same latent bug if a user clicked "Run Now" before the catalog query resolved.Fix
Single source of truth for the model alias
runnableModelAliasundefineddisplayModelAlias<your-model-alias>placeholder"medgemma-4b"constant is gone. The display fallback is"<your-model-alias>"— obviously non-runnable, so it can't be mistaken for a real model.modelsLoadingto clear, then either fires the toast + real batch usingrunnableModelAlias, or skips silently (no toast, no doomed POST) if the user has no entitled chat model.runnableModelAliasis populated, with atitletooltip explaining the wait.handleRunNowalso defends against the disabled state being bypassed.displayModelAlias, so as soon as the catalog returns the user sees the same alias the backend will receive.Post-success redirect goes to the job route
Previously the auto-redirect after a successful run landed users on
/modelsregardless of which workload they ran. Now:/asyncwhenworkloadType === "async"./batcheswhenworkloadType === "batch"./modelsfallback when the preferred route isn't accessible on this deployment (both routes are config-gated bybatches.enabled/batches.async_requests.enabled; the destination is filtered throughuseAuthorization().canAccessRouteto avoidProtectedRoutebouncing the user mid-redirect).The "Skip to Dashboard" header button still goes to
/modelsper the original spec. The CLI listener path also stays on/modelsbecause it's the "click to continue" simulation, not an actual workload — there's no specific job for the user to view there.Inline success-strip copy now reflects the destination ("Taking you to your batch / async request…") so the redirect isn't surprising.
Out of scope
The other lines in the original console log (
GET /authentication/register 404,POST /ai/v1/files 403) are environment / permissions issues unrelated to this PR.QA
pnpm run lintclean.pnpm exec tsc -b tsconfig.app.jsonclean.pnpm test -- --run: 498 / 498 passing.