Repository navigation
Add MIGraphX and oneDNN execution provider support and improvements - #330
Conversation
Introduces MIGraphXExecutionProvider and DnnlExecutionProvider into the core provider flow by extending `IHVProvider`, runtime-kind classification, known provider lists, GPU classification, and ORT provider mapping. This lays the first implementation slice for EP expansion and includes Kiro spec artifacts (`requirements`, `design`, `tasks`) plus newly added Terraform workspace/state artifacts under `infra/terraform`.
This change adds AMD MIGraphX and Intel oneDNN execution-provider support across provider metadata and runtime classification. It updates alias and family mappings, GPU accelerator detection, recipe builder provider grouping, and task tracking/spec notes for the expansion pack work. It also adds steering guidance to avoid 'local-first' branding in user-facing copy and docs.
Introduces end-to-end support for MIGraphXExecutionProvider and DnnlExecutionProvider across Olive Studio. This includes provider catalog updates, IHV panel UX improvements (Qualcomm grouping, QNN ABI coercion notice, install/unavailable states), expanded pass compatibility and auto-coercion rules, and stronger provider conflict validation (including ROCm consumer vs datacenter differentiation). The change also extends hardware probing and server capability handling with oneDNN availability checks and MIGraphX install logic, adds MCP knowledge base EP selection guidance, and adds comprehensive unit/property tests for provider schema invariants, conflict detection, coercion behavior, hardware profile schema completeness, and provider classification/mapping.
There was a problem hiding this comment.
Sorry @tonythethompson, your pull request is larger than the review limit of 150000 diff characters
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 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 |
Qodana for JS77 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Greptile SummaryThe PR expands execution-provider support for AMD MIGraphX and Intel oneDNN while unifying QNN ABI selection and extending hardware probing and compatibility validation.
Confidence Score: 3/5The PR is not safe to merge because the hardware probe route cannot compile and MIGraphX Execute Live still fails after successful installation. The current hardware route contains malformed and out-of-scope probe fragments that prevent the server module from building, while MIGraphX readiness still maps to no capability slot and falls through to an error after installation. Files Needing Attention: src/server/routes/system.ts, src/server/services/venv/capabilityEnsure.ts, src/server/services/venv/status.ts
|
| Filename | Overview |
|---|---|
| src/server/routes/system.ts | Hardware-probe changes remain structurally corrupted with invalid syntax, duplicate declarations, and out-of-scope identifiers that prevent compilation. |
| src/server/services/venv/capabilityEnsure.ts | Adds MIGraphX installation and oneDNN registration checks, but still lacks a successful readiness path for MIGraphX after installation. |
| src/server/services/venv/status.ts | Classifies MIGraphX as having no capability slot without coordinating the required fallback registration check in capability ensure. |
| src/server/services/olive/migraphx.ts | Adds platform-gated MIGraphX package installation and import verification, but its successful result is rejected by the downstream capability flow. |
Reviews (3): Last reviewed commit: "Merge branch 'main' into ep-expandion-pa..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c92bf235f
ℹ️ 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".
|
/oc fix |
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
|
This session completed the remaining PR #330 work: Tests added (the last implementation gap):
Verification done this session:
Remaining: only the final lint/test/typecheck verification, which needs Working tree: 8 modified files + new |
|
CodeFactor found an issue: Complex Method It's currently on: |
|
CodeFactor found an issue: Expected an assignment or function call and instead saw an expression. It's currently on: |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summaries (4 snapshots, latest commit 76260dd)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 76260dd)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous review (commit c0a7cb4)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 545baf3)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (12 files)
Fix these issues in Kilo Cloud Previous review (commit a6a4a1d)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (12 files)
Reviewed by step-3.7-flash · Input: 111K · Output: 28.6K · Cached: 427.4K |
|
/oc review |
- capabilityEnsure: extract resolveMissingCapabilitySlot to fix CodeFactor Complex Method on ensureProviderCapability - pipInstall: replace ternary-as-statement with if/else (CodeFactor) - system: document gfx_target_version decimal encoding (kilocode finding invalid: valid GPUs are always >=5 digits)
|
Addressed the CodeFactor and kilocode findings in 9c674e5: CodeFactor
kilocode
Verified locally: targeted eslint (0 warnings), |
- system.ts: simplify [\\w] to \\w in rocminfo name regex (RegExpSimplifiable) - types.ts: replace unresolvable property-level @link targets with type-level links (JSValidateJSDoc)
|
Triage of the Qodana report (80 new problems), commit 545baf3: Fixed (PR-introduced, 3 findings)
Intentional, not fixed (13 findings)
Pre-existing repo debt, out of this PR's scope (64 findings)
Happy to take a separate cleanup pass on the repo-debt items if wanted, but they don't belong in this PR's diff. |
|
/oc review failing ci tests |
There was a problem hiding this comment.
Pull request overview
Adds first-class execution provider (EP) support for AMD MIGraphX (Instinct/CDNA) and Intel oneDNN (DNNL), plus QNN ABI UX unification and ROCm consumer-vs-datacenter differentiation across probe/validation/UI and MCP knowledge.
Changes:
- Register MIGraphX and DNNL providers end-to-end (types, catalog, runtime classification, venv/capability ensure, hardware probe, recipe mapping, and UI cards).
- Add ROCm ISA-family probing + validation guidance to distinguish RDNA (consumer) vs CDNA (Instinct) behaviors, and gate MIGraphX to Instinct hardware.
- Expand test coverage (including property-based tests) and extend the MCP knowledge base with new hardware profiles and EP selection guidance.
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Add MIGraphX + DNNL to IHVProvider union; doclink tweaks. |
| src/server/services/venv/status.ts | Treat MIGraphX/DNNL as “no capability slot” providers (resolved via ORT provider list). |
| src/server/services/venv/capabilityEnsure.ts | Add MIGraphX ensure/install + ORT-provider verification for MIGraphX/DNNL; refactor missing-slot handling. |
| src/server/services/venv/capabilityEnsure.test.ts | Add unit tests covering MIGraphX + DNNL ensure behaviors. |
| src/server/services/shared/pipInstall.ts | Add AbortSignal support to pip install helpers. |
| src/server/services/olive/migraphx.ts | New MIGraphX installer/ensure (Linux x64 gate + timeout + import verification). |
| src/server/services/olive/migraphx.test.ts | Tests for MIGraphX ensure gating, idempotency, timeout, and failures. |
| src/server/routes/system.ts | Add CPU ISA probe (AVX2/AVX-512/AMX) + ROCm ISA family probing; surface MIGraphX/DNNL notes/detection fields. |
| src/lib/vramEstimate.ts | Classify MIGraphX as GPU provider; DNNL as CPU provider. |
| src/lib/venvFamily.ts | Add provider aliases + include new EPs in known provider list; mandatory family mapping. |
| src/lib/providerRuntimeKind.ts | Classify MIGraphX/DNNL as local runtime providers. |
| src/lib/providerCatalog.ts | Add catalog entries for MIGraphX, DNNL, and QNN ABI; add grouping + workflow subtitle fields; tighten TensorRT copy. |
| src/lib/pipelineValidation.ts | Add DNNL conflicts, TensorRT-format conflicts, ROCm consumer/datacenter validation, and QNN pipeline rules. |
| src/lib/pipelineStateCommit.ts | Add auto-coercion rules for DNNL quant methods + QNN ABI enable/disable behaviors. |
| src/lib/passParameterValidation.ts | Label new providers in pass parameter validation UI. |
| src/lib/oliveRecipeHub.ts | Map new providers to short catalog device names. |
| src/lib/oliveRecipeBuilder.ts | Export GPU/NPU provider sets; add MIGraphX to GPU list. |
| src/lib/hardwareProbe.ts | Add isaFamily + Instinct detection; MIGraphX/DNNL ORT provider mapping; recommendation ordering updates. |
| src/lib/hardwareProbe.test.ts | Tests for MIGraphX Instinct gating + Instinct classifier. |
| src/lib/tests/venvFamily.test.ts | Add tests for MIGraphX/DNNL venv family mapping + known providers. |
| src/lib/tests/providerRuntimeKind.test.ts | Add tests for runtime kind classification. |
| src/lib/tests/providerCatalog.test.ts | Extend catalog coverage + add schema invariant property tests. |
| src/lib/tests/pipelineValidation.test.ts | Update QNN ABI tests + add EP expansion property tests. |
| src/lib/tests/oliveRecipeBuilder.test.ts | Add property tests for accelerator mapping; import GPU/NPU provider sets. |
| src/lib/tests/hardwareProfileSchema.test.ts | New property test validating MCP hardware profile schema completeness. |
| src/lib/tests/gpuClassification.test.ts | New tests for MIGraphX/DNNL GPU-vs-CPU classification/mapping. |
| src/lib/tests/epExpansionMigraphxConflicts.test.ts | New property tests validating MIGraphX conflict detection. |
| src/components/features/ihv/QnnAbiCoercionNotice.tsx | New transient notice for QNN ABI pass coercion. |
| src/components/features/ihv/ProviderCardGrid.tsx | Grouped provider grid rendering (e.g., Qualcomm Snapdragon section). |
| src/components/features/ihv/IHVIntegrationPanel.tsx | Track and display QNN ABI coercion notice on provider switch. |
| src/components/features/ihv/HardwareProviderCard.tsx | MIGraphX install-needed + DNNL AVX2-unavailable UI; workflow subtitle rendering; tooltip focusability tweaks. |
| src/components/features/ihv/hardwarePassCompatibility.ts | Expand pass compatibility definitions and quant-method mapping. |
| src/components/features/execute/recipe-graph/RecipeValidationPanel.tsx | Display strings for new providers in recipe validation panel. |
| olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json | Add Instinct + RDNA4 + oneDNN profiles; schema markers; minor formatting. |
| olive-mcp-server/olive_mcp_server/knowledge_base/ep_selection_guidance.json | New EP decision-tree guidance for MCP. |
| infra/terraform/.terraform.lock.hcl | Add Terraform provider lock file. |
| .kiro/steering/no-local-first-branding.md | Add steering rule forbidding “local-first” branding. |
| .kiro/specs/ep-expansion-pack/tasks.md | Add implementation plan/spec tasks. |
| .kiro/specs/ep-expansion-pack/requirements.md | Add feature requirements spec. |
| .kiro/specs/ep-expansion-pack/design.md | Add feature design spec. |
| .kiro/specs/ep-expansion-pack/.config.kiro | Add Kiro spec config metadata. |
| .gitignore | Ignore Terraform state and .terraform/ directory. |
Files not reviewed (1)
- infra/terraform/.terraform.lock.hcl: Generated file
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
/oc fix |
MIGraphX is not Instinct-only: ROCm 6.1+ supports RDNA3 (RX 7xxx) and ROCm 7.x supports RDNA4 (RX 9xxx). Rename isInstinctGpu to isMigraphxSupportedGpu with an ISA allowlist covering CDNA + RDNA3/4; RDNA1/2 and Vega remain excluded. Also fix the install hint wording (default project .venv, not 'ROCm venv') and update catalog copy + error messages.
- IHVIntegrationPanel: derive QNN ABI coercion notice during render via the prev-state pattern instead of setState in an effect (fixes the react-hooks/set-state-in-effect warning that fails pnpm lint) - pipelineValidation: remove unterminated JSDoc block that swallowed the ROCm differentiation comment banner - Rebuild shipped docs KB index after adding ep_selection_guidance.json (4,711 pairs) so CI's up-to-date check passes
|
Working tree clean, all 4 threads replied + resolved, CI re-running on
|
* Add MIGraphX/DNNL provider groundwork Introduces MIGraphXExecutionProvider and DnnlExecutionProvider into the core provider flow by extending `IHVProvider`, runtime-kind classification, known provider lists, GPU classification, and ORT provider mapping. This lays the first implementation slice for EP expansion and includes Kiro spec artifacts (`requirements`, `design`, `tasks`) plus newly added Terraform workspace/state artifacts under `infra/terraform`. * Add MIGraphX and oneDNN provider support This change adds AMD MIGraphX and Intel oneDNN execution-provider support across provider metadata and runtime classification. It updates alias and family mappings, GPU accelerator detection, recipe builder provider grouping, and task tracking/spec notes for the expansion pack work. It also adds steering guidance to avoid 'local-first' branding in user-facing copy and docs. * Add MIGraphX/oneDNN EP expansion support Introduces end-to-end support for MIGraphXExecutionProvider and DnnlExecutionProvider across Olive Studio. This includes provider catalog updates, IHV panel UX improvements (Qualcomm grouping, QNN ABI coercion notice, install/unavailable states), expanded pass compatibility and auto-coercion rules, and stronger provider conflict validation (including ROCm consumer vs datacenter differentiation). The change also extends hardware probing and server capability handling with oneDNN availability checks and MIGraphX install logic, adds MCP knowledge base EP selection guidance, and adds comprehensive unit/property tests for provider schema invariants, conflict detection, coercion behavior, hardware profile schema completeness, and provider classification/mapping. * Added MIGraphX tests + verified changes. CI pending. Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com> * fix(server): address CodeFactor and kilocode review findings on PR #330 - capabilityEnsure: extract resolveMissingCapabilitySlot to fix CodeFactor Complex Method on ensureProviderCapability - pipInstall: replace ternary-as-statement with if/else (CodeFactor) - system: document gfx_target_version decimal encoding (kilocode finding invalid: valid GPUs are always >=5 digits) * chore: fix Qodana findings introduced by PR #330 - system.ts: simplify [\\w] to \\w in rocminfo name regex (RegExpSimplifiable) - types.ts: replace unresolvable property-level @link targets with type-level links (JSValidateJSDoc) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * feat(hardware): widen MIGraphX gating to RDNA3/RDNA4 consumer GPUs MIGraphX is not Instinct-only: ROCm 6.1+ supports RDNA3 (RX 7xxx) and ROCm 7.x supports RDNA4 (RX 9xxx). Rename isInstinctGpu to isMigraphxSupportedGpu with an ISA allowlist covering CDNA + RDNA3/4; RDNA1/2 and Vega remain excluded. Also fix the install hint wording (default project .venv, not 'ROCm venv') and update catalog copy + error messages. * fix(ci): resolve pnpm version conflict in release workflows pnpm/action-setup@v4 with an explicit 'version: 11' conflicts with the packageManager field in package.json, failing both release workflows with 'Multiple versions of pnpm specified'. Align with ci.yml: use action-setup@v6 without a pinned version (resolved from packageManager) and bump checkout/setup-node to v7. * fix(release): use Xcode 26-compatible lipo argument order in macOS Node bundler macos-latest runners now ship Xcode 26.6 where lipo -verify_arch requires the input file before the architecture list, failing with 'unknown architecture specification flag'. * fix(release): use Xcode 26-compatible lipo argument order in macOS smoke test Same -verify_arch argument-order break as bundle-macos-node.sh, this time in the packaged .app smoke test. * fix(validation): remove duplicate ROCm declarations from merge conflict * fix(lint): add eslint disable annotation for setState in effect in IHVIntegrationPanel * ci(desktop): add macOS and Windows desktop package workflows Add desktop-macos.yml (macOS CI build + packaged-server smoke test, mirroring desktop-linux.yml) so Xcode/runner drift is caught on PRs, and desktop-windows.yml producing NSIS + MSI + MSIX artifacts with the bundled Node runtime. Remove the superseded tauri-build.yml, which never bundled the node-runtime resource required by tauri.conf.json, and bump upload-artifact to v7 in the dry-run workflow. * fix(ci): use valid 'app' bundle target in macOS desktop workflow Tauri rejects '--bundles macos'; the .app bundle target is named 'app'. * fix(release,ihv): address PR 336 review findings Bump desktop-release.yml checkout action to v7 to match the rest of the release workflows (Copilot). Clear the QNN ABI coercion notice when switching away from QnnAbiExecutionProvider so a later switch back without new coercions cannot resurface stale passes (Kilo Code). * test(ihv): add QNN ABI coercion notice transition tests Cover the three transition scenarios for the QNN coercion notice: 1. Shows notice when switching TO QnnAbiExecutionProvider with coerced passes 2. Clears notice when leaving QNN provider (stale-state fix) 3. Does not resurface stale passes on re-entry without new coercions Uses prop-driven state to drive provider transitions via rerender, avoiding the need for a mutable store mock. * build(release): drop MSI, add portable zip to Windows artifacts - Remove MSI from Windows bundle targets (redundant with NSIS) and from artifact validation/upload steps, cutting the ~1.2GB artifact down to NSIS + MSIX + portable zip - Add portable zip step that archives the release directory contents (exe + dist + node-runtime + scripts + mcp server + genai sidecar) - Upload portable zip alongside MSIX in desktop-release.yml - Pin tauri.conf.json bundle.targets to an explicit list excluding msi so tauri-action-based release flows stop publishing MSI * fix(desktop): strip Windows verbatim path prefix from sidecar cwd Installed Windows builds failed to open: Rust's canonicalize() returns \\\\?\-prefixed extended-length paths, and Node cannot resolve its entry script with such a cwd (EISDIR: lstat 'C:'), so the sidecar exited with code 1 and the Tauri setup hook panicked before the window ever showed. - Strip the \\\\?\ prefix (and map UNC\ back to \\\\server) after canonicalize in resolve_app_root - Drain the sidecar's stdout/stderr into bounded tails so the pipe buffer can never stall the server, and include the captured output in health-check failure messages for diagnosability Verified: NSIS install previously crashed in ~8s; with the fix the app starts its server and window in ~2s. * chore: repair lockfile entry for unplugin peer resolution * fix(desktop): grant loopback origin IPC so window controls work The desktop WebView navigates to http://127.0.0.1:PORT (the bundled Express server) rather than the tauri:// asset protocol, so it is a remote origin. Capabilities default to local-only, which meant every Tauri IPC call from the UI — including the custom title bar's minimize/maximize/close and drag-region — was denied and silently swallowed by TitleBar's catch block. Tauri 2 exposes remote access via the capability remote.urls field (URLPattern standard). Add http://127.0.0.1:*/* so the loopback-served UI gets the same permission set. Verified the pattern matches any loopback port/path against the urlpattern crate and confirmed it is embedded in the compiled capabilities. * Fixed TS2740 + unused eslint directive; lint & tests pass. Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com> --------- Co-authored-by: opencode-agent[bot] <opencode-agent[bot]@users.noreply.github.com> Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>



Summary by cubic
Adds first‑class MIGraphX (AMD GPU) and oneDNN (Intel CPU) execution providers and promotes QNN ABI to a provider with auto‑coercion to QairtPipeline. Providers now appear in the catalog, are gated by hardware/
onnxruntimeprobes, and QNN ABI replaces incompatible passes with a single pipeline and shows an inline notice.Catalog/UI: new cards for MIGraphX, oneDNN, and QNN ABI; grouped “Qualcomm Snapdragon” section and grouped grid rendering; QNN ABI coercion notice now derived during render; MIGraphX install/unavailable prompts (default venv wording).
Validation/commit: MIGraphX conflicts with OpenVINO/TensorRT; HQQ/RTN/KQuant allowed on MIGraphX; oneDNN blocks GPU‑only quant methods; ROCm consumer vs datacenter split via ISA families; accelerator classification/mapping updated.
Probe/ensure: CPU feature detection (AVX2/AVX‑512/AMX); MIGraphX soft‑detects on AMD Instinct (CDNA) and RDNA3/RDNA4 GPUs (RDNA1/2 and Vega excluded); capability ensure installs
migraphxon Linux x86_64 into the default venv with a 300s timeout and abort support; oneDNN availability verified via probedonnxruntimeproviders.Knowledge base/tests: MCP adds EP selection guidance and Instinct/RDNA4 profiles; shipped docs KB index rebuilt (4,711 pairs); unit/property tests cover catalog invariants, hardware profiles, conflicts, coercion behavior, and provider classification/mapping.
Refactor/chore: extracted
resolveMissingCapabilitySlot; added AbortSignal to pip installs; fixed JSDoc and React hooks lint warnings;.gitignoreignores Terraform state.MIGraphX: Linux x86_64 only; installs
migraphxinto the default venv; remains disabled if import fails; gated to AMD Instinct (CDNA) and RDNA3/RDNA4 GPUs.oneDNN: bundled with the default
onnxruntimewheel. Required action: use anonnxruntimewheel that reportsDnnlExecutionProviderin the probed provider list.Written for commit aa95a45. Summary will update on new commits.