Enhance model selection UI and integrate Playground output - #262
Conversation
Deployed size (inferenceGb) was only visible after expanding a model's target list — the collapsed header showed just device badges and a target count. Add a compact "~X GB" (or "~min–max GB" when targets vary in size) label next to the title, reusing the RecipeRow.inferenceGb already computed for sorting, so it's visible before expanding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…model selected Nothing validated that a model was actually chosen. With no model, validation.statusLabel still said "Local checks passed" (green) next to an enabled-looking Execute Live button, and the panel piled model-specific tuning advisories (e.g. "NVIDIA prefers AWQ INT4 over PTQ INT8") plus an MCP diagnostic lookup on top of that — all meaningless without a model to apply them to. - pipelineValidation: add a critical "No model selected" issue (hasSelectedModel, now exported) so isBlocked/isRunnable correctly go false, and gate the trust_remote_code advisory on a model actually being set. - RecipeValidationPanel: when no model is selected, show only that one issue instead of merging in pass-parameter/compat/MCP warnings, and skip the MCP error-diagnostic fetch entirely (there's no error pattern to diagnose yet). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Local accelerators (the actual candidates on this machine) and export/ platform targets (QNN, WebGPU, CoreML, NNAPI, TFLite, WASM, ...) rendered as ~16 equal-weight cards with no distinction — export-only targets you'll almost never pick got the same visual weight as CUDA on a machine that has an NVIDIA GPU, turning step 02 into several screens of scrolling for what's usually a one-card decision. Collapse "Export & platform targets" behind a disclosure, showing just the count and a one-line description by default. Auto-expands if the currently selected provider happens to be one of them, so switching providers never hides your own active selection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two dead wires kept step 04 feeling like a separate tool instead of part
of the pipeline:
- PlaygroundPanel hardcoded `recipeJson={undefined}` into Browser Test, so
the "this test matches your configured recipe" hint could never show,
regardless of what was actually configured in steps 01-03. Now builds it
from live pipeline state via buildRecipeJsonFromState.
- Nothing pointed from a finished run back to Playground. Add a
"Test in Playground ->" button next to the Active Draft "Done" badge
once executionStatus is "completed", landing on Browser Test.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- ProviderCardGrid: sync disclosure state when selected provider changes to prevent hiding active export targets - RecipeCatalogBrowserList: format memory sizes before comparing to avoid redundant labels like "~1.0 GB–1.0 GB" - RecipeCatalogBrowserList: fix tooltip wording from "deployed model size" to "inference memory estimate" - pipelineValidation: add targeted unit tests for hasSelectedModel covering HF empty/whitespace, local files, and Azure paths Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- PlaygroundPanel: gate recipeJson on WebGPU provider to prevent false hints on CPU/QNN/DirectML - RecipeValidationPanel: reset issue-count ref when model deselected to avoid stale diagnostics Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reset prevIssueCountRef when criticalIssues count is 0 to ensure clean state for next issue detection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- HardwareCompatibilityMatrix: sticky header row + vertical scroll so column headers stay visible while scrolling through the pass x provider matrix
- ProviderCardGrid: collapse undetected local-accelerator cards behind "Show N other targets", mirroring the existing export/platform-targets disclosure pattern
- HardwareProviderCard: add progressive disclosure ("Show/Hide details") for plugin-install logs and hint paragraphs, collapsed by default (expanded for the active selection); quiet detected/compatible badges from emerald container to slate container + emerald icon
- CompatStatus: same slate-container/emerald-icon treatment for the "Compatible" tier pill
- App.tsx: soft incomplete-step indicator (amber dot) on nav items 02-04 when no model is selected yet, without gating navigation — free-scroll behavior is preserved
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
|
Warning Review limit reached
Next review available in: 11 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoBlock runs without a selected model and link Execute output to Playground
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
HardwareProviderCard,isExpandedis initialized fromisSelectedbut never updated when the selection changes; consider syncing expansion state withisSelectedso the newly active card auto-expands even if the user previously collapsed it. - The new undetected-local accelerator grouping in
ProviderCardGridrelies onisProviderDetectedLocallyandhardwareProbe; it may be worth guarding against anull/undefined probe or stale detection data to avoid misclassifying providers into the detected/undetected groups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HardwareProviderCard`, `isExpanded` is initialized from `isSelected` but never updated when the selection changes; consider syncing expansion state with `isSelected` so the newly active card auto-expands even if the user previously collapsed it.
- The new undetected-local accelerator grouping in `ProviderCardGrid` relies on `isProviderDetectedLocally` and `hardwareProbe`; it may be worth guarding against a `null`/undefined probe or stale detection data to avoid misclassifying providers into the detected/undetected groups.
## Individual Comments
### Comment 1
<location path="src/components/features/ihv/HardwareProviderCard.tsx" line_range="856" />
<code_context>
}: HardwareProviderCardProps) {
const isSelected = state.ihvProvider === p.id;
+ // Selected card starts open so the active target's details aren't hidden behind a click.
+ const [isExpanded, setIsExpanded] = useState(isSelected);
const Icon = p.icon;
const pConflicts = getProviderConflicts(p.id, state.passes);
</code_context>
<issue_to_address>
**question (bug_risk):** The expansion state is only initialized from selection and won’t respond to later selection changes.
`useState(isSelected)` only uses the initial `isSelected` value, so expansion won’t follow later selection changes. This can leave previously selected cards expanded while the newly selected card remains collapsed. If the active card should always be expanded, derive `isExpanded` from `isSelected` (or sync it via `useEffect`) instead of managing it as independent state.
</issue_to_address>
### Comment 2
<location path="src/components/features/ihv/HardwareProviderCard.tsx" line_range="970-977" />
<code_context>
+ const modelSelected = hasSelectedModel(pipelineState);
+ const isIncomplete = !modelSelected && id !== "input";
return (
<button
key={id}
type="button"
</code_context>
<issue_to_address>
**suggestion:** The details toggle button could be more accessible by tying it to the controlled content region.
The button’s `aria-expanded` is set correctly, but assistive technologies don’t have an explicit link to the content being toggled. Consider giving the collapsible container an `id` and referencing it with `aria-controls`, or using a native `details/summary` pattern so the control–content relationship is clearer for screen readers.
Suggested implementation:
```typescript
<button
type="button"
onClick={(e) => {
e.stopPropagation();
setIsExpanded((v) => !v);
}}
className="flex items-center gap-1 text-[11px] text-slate-500 hover:text-slate-400 transition-colors cursor-pointer"
aria-expanded={isExpanded}
aria-controls="hardware-provider-details"
>
```
To fully implement the accessibility improvement, the collapsible content container that is shown/hidden based on `isExpanded` should be given a matching `id="hardware-provider-details"` (or another consistent id if you prefer a different name). For example, wherever the expanded content is rendered, update the wrapping element to include this `id` so assistive technologies can correctly associate the toggle button with the controlled region.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Greptile SummaryThe PR improves model-selection validation, preserves the submitted execution recipe for Playground, and refines hardware-provider presentation and fallback behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/components/features/ihv/HardwareProviderCard.tsx | Synchronizes detail expansion with provider selection and removes the null-probe CPU hardware block, resolving the prior card-state and CPU fallback findings. |
| src/components/features/ihv/ProviderCardGrid.tsx | Groups undetected local providers while preserving all local targets when probe results are unavailable. |
| src/lib/hardwareProbe.ts | Treats CPU as locally detected regardless of probe availability, ensuring the fallback remains visible. |
| src/components/features/execute/ExecutionWorkspace.tsx | Captures the exact recipe from a successfully completed execution for subsequent Playground use. |
| src/components/features/playground/PlaygroundPanel.tsx | Prefers the captured execution recipe and passes it to Browser Test only when it targets WebGPU. |
| src/components/features/execute/recipe-graph/RecipeValidationPanel.tsx | Clears stale diagnostics after model deselection and resets diagnostic issue-count tracking. |
Reviews (6): Last reviewed commit: "perf: memoize captured WebGPU recipe che..." | Re-trigger Greptile
💡 Codex ReviewWhen the user follows the new button immediately after a successful Execute Live run, this condition always produces ℹ️ 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". |
…Selected) When clicking a different already-mounted provider card, isExpanded now syncs via useEffect so the newly active target opens to show install controls and hardware details, rather than staying collapsed. Matches existing pattern in ProviderCardGrid.tsx. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re: macroscope review on HardwareProviderCard.tsx:856 — Added useEffect to sync isExpanded when isSelected changes, so newly selected provider cards auto-open. Matches pattern in ProviderCardGrid. 🤖 Addressed by Claude Code |
When an Execute run reaches "completed", ExecutionWorkspace now builds the current recipe JSON and stores it in the playground Zustand store. The store adds `capturedRunRecipe` plus a setter, and clears that field during `resetPlayground` so session-scoped Playground state resets cleanly.
Code Review by Qodo
1.
|
Capture the exact recipe submitted for execution by using `runRecipeJsonRef` instead of rebuilding from potentially edited UI state after completion. Add `clearDiagnostic` to `useMcpDiagnostic` (aborts in-flight requests and resets state) and invoke it when no model is selected to prevent stale validation diagnostics. Also stop auto-injecting placeholder local files in batch job UI state hydration.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
…rowser Test - Ensure isProviderDetectedLocally always returns true for CPUExecutionProvider, preventing CPU from being marked as hardware-blocked when hardwareProbe is null. - Validate execution provider in PlaygroundPanel before passing capturedRunRecipe to InBrowserValidation so non-WebGPU run recipes do not trigger the WebGPU hint. - Export mapExecutionProviderFromRecipe from oliveRecipeHub for recipe provider inspection. - Add component and unit tests covering null probe CPU detection and PlaygroundPanel recipe provider filtering. Co-authored-by: Cursor <cursoragent@cursor.com>
Generate a per-provider details ID in `HardwareProviderCard` and wire both `aria-controls` and the details container `id` to it. This avoids duplicate static IDs across cards and keeps the expand/collapse button’s accessibility relationship accurate.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Pull request overview
This PR tightens the “model must be selected” invariant across the app, improves IHV/provider selection UX (including a11y affordances), and ensures the Playground Browser Test consumes the exact recipe that was executed (rather than rebuilding from potentially edited state).
Changes:
- Captures the submitted Execute recipe and routes it into the Playground store; Browser Test consumes it only when the recipe’s provider is WebGPU.
- Improves validation/diagnostics lifecycle (including abortable MCP diagnostics) and hardware/provider detection behavior (CPU always treated as locally detected).
- Enhances IHV UI/UX (collapsible provider cards, detected vs undetected grouping, sticky matrix header) plus nav “incomplete” indicators and batch hydration behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/stores/playgroundStore.ts | Adds capturedRunRecipe to persist the last successful Execute recipe for Playground use. |
| src/lib/oliveRecipeHub.ts | Exposes mapExecutionProviderFromRecipe for provider detection from recipe JSON. |
| src/lib/hooks/useMcpDiagnostic.ts | Adds clearDiagnostic() to abort/reset MCP diagnostic state. |
| src/lib/hardwareProbe.ts | Treats CPUExecutionProvider as locally detected even without a probe. |
| src/lib/hardwareProbe.test.ts | Adds unit coverage for isProviderDetectedLocally behavior. |
| src/components/features/playground/PlaygroundPanel.tsx | Prefers captured Execute recipe for Browser Test, with WebGPU-only gating. |
| src/components/features/playground/PlaygroundPanel.test.tsx | Tests captured recipe routing into InBrowserValidation only for WebGPU. |
| src/components/features/input/CompatStatus.tsx | Adjusts compatible pill styling and icon coloring. |
| src/components/features/ihv/ProviderCardGrid.tsx | Splits local providers into detected/undetected groups with a toggle for “other targets”. |
| src/components/features/ihv/ProviderCardGrid.test.tsx | Adds regression test ensuring CPU isn’t shown as hardware-blocked when probe is null. |
| src/components/features/ihv/HardwareProviderCard.tsx | Adds per-card collapsible “Show/Hide details” region with aria-controls. |
| src/components/features/ihv/HardwareCompatibilityMatrix.tsx | Adds vertical scroll with sticky header for the compatibility matrix. |
| src/components/features/execute/useOliveStream.ts | Exposes runRecipeJsonRef so callers can persist the submitted recipe. |
| src/components/features/execute/recipe-graph/RecipeValidationPanel.tsx | Clears MCP diagnostics on model deselect and resets issue counters when issues clear. |
| src/components/features/execute/ExecutionWorkspace.tsx | Captures executed recipe on completion into Playground store. |
| src/components/features/execute/BatchProcessingPanel.tsx | Preserves localFiles when hydrating UI from batch jobs. |
| src/App.tsx | Adds incomplete-step indicators/ARIA labels for steps when no model is selected. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const recipeJson = useMemo(() => { | ||
| if (capturedRunRecipe && isWebGpuRecipe(capturedRunRecipe)) return capturedRunRecipe; | ||
| return hasSelectedModel(state) && state.ihvProvider === "WebGpuExecutionProvider" | ||
| ? buildRecipeJsonFromState(state) | ||
| : undefined; | ||
| }, [capturedRunRecipe, state]); |
| // However, if probe is still loading or failed (null), show all locals together since we | ||
| // can't reliably distinguish detected from undetected (e.g., null probe would hide CPU fallback). | ||
| const { detectedLocal, undetectedLocal } = useMemo(() => { | ||
| const hardwareProbe = providerCardProps.hardwareProbe; | ||
| const detected: ProviderCatalogEntry[] = []; | ||
| const undetected: ProviderCatalogEntry[] = []; | ||
|
|
||
| // Only split if probe data is available; otherwise treat all as detected to avoid hiding CPU. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
CodeFactor found an issue: 'runRecipeJson' is assigned a value but never used. Allowed unused vars must match /^_/u. It's currently on: |
|
CodeFactor found an issue: Complex Method It's currently on: |
|
CodeFactor found an issue: Complex Method It's currently on: |
|
CodeFactor found an issue: Complex Method It's currently on: |
Summary by cubic
Blocks Execute until a model is chosen and pipes the exact executed recipe to Playground. Previously “Local checks passed” showed and MCP advisories ran with no model, and Playground rebuilt recipes from edited state; now a critical “No model selected” blocks Execute, diagnostics clear on deselect, and Browser Test only consumes a captured WebGPU run recipe.
hasSelectedModel; emit criticalmodel-source-not-set; gatetrust_remote_codeand MCP lookups until a model is set; addclearDiagnostic()to abort in‑flight requests and reset state; reset the issue counter when issues clear.runRecipeJsonRefand store ascapturedRunRecipeinplaygroundStore;PlaygroundPanelprefers this captured recipe and falls back to current state only when absent; pass to Browser Test only if its provider isWebGpuExecutionProvider(memoized check); show “Test in Playground →” after a successful run; clear on Playground reset.aria-controls/id; quiet the “Compatible” pill to a slate container with an emerald icon.isProviderDetectedLocally("CPUExecutionProvider")always true so CPU fallback isn’t hidden on a nullhardwareProbe; do not treat a missing probe as a block.Written for commit 7b7e8d2. Summary will update on new commits.
Note
Integrate captured Execute recipe into Playground and improve model selection UI
setCapturedRunRecipe; PlaygroundPanel passes it to the Browser Test only when it targets WebGPU.CPUExecutionProvideris always treated as detected locally inisProviderDetectedLocally, even when probe data is absent; the provider card no longer shows as hardware-blocked in this case.Macroscope summarized 7b7e8d2.