fix: apply CodeRabbit cleanup and harden normalization - #12
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughThe MCP server centralizes tool registration, knowledge-base loading uses shared public helpers, normalization gains stricter matching and caching, pass-chain validation consumes canonical frameworks, troubleshooting and template assembly are adjusted, and normalization tests are added. ChangesMCP tooling updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
1 of 5 tasks
tonythethompson
added a commit
that referenced
this pull request
Aug 4, 2026
CUDA install UX (#12, #13, #16, #17, #18, #19, #21, #22, #25) - hardwareProbe.mergeDetectedProviders: simplify cudaOk ternary to `input.cudaLoadable !== false` — same semantic, half the surface. - hardwareProbe's pre-Maxwell box reason + recipeHardwareCompatibility's CUDA-floor reason now name Maxwell SM 5.0 as 'GeForce GTX 750 Ti or GTX 9xx series' instead of mistakenly borrowing 'GeForce RTX 20xx+' (RTX 20xx is Turing SM 7.5 — the floor above — which previously implied a Pascal SM 6.x owner had an unsupported card). - recipeHardwareCompatibility widens tensorRtInstallHint to ePInstallHint and routes the onnxruntime-gpu install hint through it so the probe's detail string (driver/wheel mismatch, missing module, etc.) is preserved end-to-end. - IHVIntegrationPanel gates the CUDA toolkit download-link paragraph on `cudaEpInVenv` so it only renders when the EP is actually registered — removes the contradictory 'CUDA EP detected / install button shown' state. - InputEnvironmentPanel renders the requiresInstall hint directly so the label and the pip-install command can never disagree again (the old code branched on `kind` and silently printed the TensorRT label next to the onnxruntime install command). Drop the now-dead pinnedTensorRtLabel + tensorrtRtxEpAbiLabel imports. - system.ts hardens the ORT GPU probe with an explicit 30 s timeout (a broken driver install can leave the onnxruntime import hanging and bind the HTTP request) and derives the pinned-version string from pinnedOrtGpuLabel() instead of hardcoding '1.26.0' — a wheel bump in oliveGpuRuntime now propagates to every error message and install hint without further edits. - system.ts notes generator now derives its suggested pip command from pinnedOrtGpuInstallCommand() instead of a literal 'pip install onnxruntime-gpu==1.26.0' string. Shared install helpers (#23, #24, #26) - src/server/services/shared/pipInstall.ts is the single NDJSON-aware pip install helper used by every install route; the local copies in cuda.ts + tensorrt-rtx.ts deleted (kept the exact byte-shape so the UIs NDJSON parser keeps working unchanged). - cuda.ts exports ensureOnnxRuntimeGpu; tensorrt-rtx.ts calls it directly instead of carrying a private duplicate (mirror drift was the actual bug — they were diverging in subtle ways). - cuda.ts ensureOnnxRuntimeGpu explicitly returns libsDir: null on success so the documented return-type contract is honoured and external callers can distinguish "no library directory available" from "field not set". - tensorrt-rtx.ts uses platform-appropriate native-lib extensions: onnxruntime_providers_nv_tensorrt_rtx.dll on win32, libonnxruntime_providers_nv_tensorrt_rtx.so on Linux, .dylib on Darwin (a hardcoded .dll hides the real reason an import fails on non-Windows platforms). The probe script picks the right extension at runtime. Manifest / dedupe (#5, #20, #6) - cudaDeps.cudaDownloadUrlForOs routes darwin to the archive landing page (Apple dropped CUDA toolkit support after CUDA 11.6) and uses word-boundary regexes so 'darwin' no longer accidentally hits the Windows branch via the substring 'win'. - tensorrtRtxDeps.tensorrtRtxEpAbiInstallCommand now derives the manual pip command from the args list (same delegation pattern pinnedTensorRtInstallCommand uses), so an index/version bump cannot desync the server-side install and the user-facing fallback hint. - src/server/shared/anyDotVenvDir.ts is the single chokidar ANY_DOT_VENV_DIR regex; vite.config.ts and server.ts both import from here so a rename/back-up of the venv directory is filtered out by both Vite watchlists in lockstep. Test fixtures (#14, #15) - cudaDeps.test isPreMaxwellNvidiaBox describe block: split the mislabeled 'every card is at or above the floor' assertion into two focused tests (every-card-above-floor / every-card-below-floor); add a darwin routing suite (asserts darwin does NOT hit the Windows branch via substring 'win' and lands on the archive landing page). - providerCatalog.test cross-family lockstep test title now uses a template literal so the floor number interpolates into the test name (was a verbatim '$floor' string). e2e/scroll-bounds-guardrail.spec.ts (#7, #8, #9) - Page.evaluate wraps Radix Tooltip elements in the full Provider/Root/Trigger/Portal/Content hierarchy using React.createElement so the elements go through the JSX reconciler (plain RdxTooltip.Portal(...) / Content(...) function calls were missing Radix's Provider context and the sentinel never mounted). - DOM-fallback branch preserves the actual import-error string so a future failure is filed with the real reason, not a synthetic 'Radix bare imports did not resolve' placeholder. - Added paint-time hit-test: document.elementFromPoint at the bbox centre must resolve to the sentinel (or one of its ancestors up to #root). getClientRects alone cannot detect overflow:hidden clipping because clip preserves the rect coordinates; the hit-test is the actual guardrail. - Restrict the lint-disable comment to the placeholder line so the prettier/sonar warnings stay clean. Verified: tsc clean, 734/734 unit tests + 229/229 server tests pass, pnpm validate:recipe ok, pnpm lint 9 pre-existing warnings / 0 new errors, live /api/system/hardware-probe?refresh=1 reflects the updated probe fields.
tonythethompson
added a commit
that referenced
this pull request
Aug 4, 2026
* feat: CUDA+TensorRT install UX, lock SM-floor drift, bound scroll overshoot Provider compat & install - CUDA gets the same one-click install UX TRT/TRT-RTX already have: dedicated /api/env/install-onnxruntime-gpu route pip-installs the pinned 1.26.0 wheel into .venv with NDJSON progress, alongside the existing tensorrt / tensorrt- rtx routes (src/server/routes/env.ts, src/server/services/olive/cuda.ts). - IHV panel surfaces the right CTA per state: external link to NVIDIA's CUDA Toolkit archive (system-level install), pip button for onnxruntime-gpu, or rose terminator for pre-Maxwell GPUs that no install can recover. - Probe split into 4 user-facing states: no GPU / pre-Maxwell / driver+wheel missing / driver+toolkit+EP mismatch — the hidden one-line "NVIDIA CUDA was not detected" is gone. Hardware probe + recipe compat - Add cudaToolkit? and cuda? fields to HardwareProbeResult, with cudaLoadable gate on mergeDetectedProviders so recipe compat and IHV panel install button fire in lockstep. - Pre-Maxwell SM 5.0 short-circuit on CUDA recipes, mirroring the existing pre-Turing SM 7.5 short-circuit: never advertise an install on a card that cannot run the EP. - Center the SM floors in cudaDeps.ts and tensorrtRtxDeps so they're the single source of truth and importable by tests. SM-floor drift guards (NEW) - providerCatalog.ts TRT-RTX chip: chip + tooltip.requirements both cite 7.5 numerically, name Turing / RTX 20xx, call out Maxwell/Pascal/Kepler. - providerCatalog.ts full-TensorRT chip: same treatment — previously had no numeric anchor, just "Turing or newer (GeForce RTX 20xx+)". - recipeHardwareCompatibility pre-Turing reason: drift-guard describe block imports the constant and asserts it appears in BOTH TRT and TRT-RTX reasons; locks the "Turing / RTX 20xx+" tie-in phrase and ensures the install hint stays undefined on pre-Turing boxes. - providerCatalog.test.ts: full sibling describe block for the full-TensorRT half of the family, including a cross-family lockstep assertion (both halves must contain the same numeric constant). - Future Bump of TENSORRT_FAMILY_MIN_COMPUTE_CAPABILITY fails CI in 3 test files (hardwareProbe / providerCatalog / recipeHardwareCompatibility) and forces all 4 user-facing surfaces to update atomically. Scroll bound + Vite watcher - App-level, IHV panel, and InputEnvironmentPanel clipped to min-h-0 overflow-hidden under a properly bounded scroll container so the page can't be scrolled past the end into empty space (CSS-only, no JS). - e2e/scroll-bounds-guardrail.spec.ts mounts a Radix portal at #root and asserts it isn't clipped — guards the next person from re-applying the #root overflow lock that would break portals. - vite.config.ts: ignore .venv*, .venv.bak, .venv.old, .venv-renamed so venv tooling (renames, swaps) no longer triggers page reloads. Tests - src/lib/cudaDeps.test.ts (18) — SM 5.0 floor lock + pre-Maxwell classification + pinned install command. - src/lib/tensorrtRtxDeps.test.ts — RTF EP-ABI install command shape. - src/lib/__tests__/providerCatalog.test.ts (16) — SM 7.5 lockstep across catalog chip and hardware-probe reason for BOTH halves of the family. - src/lib/__tests__/recipeHardwareCompatibility.test.ts (23) — full SM-floor drift-guard describe block plus install-needed scenarios for CUDA/TRT/ TRT-RTX on supported and pre-floor hardware. - src/lib/hardwareProbe.test.ts + tensorrtDeps.test.ts — extended for CUDA 4-state branching and TRT/TRT-RTX install-hint gating. Verified - pnpm exec tsc --noEmit: clean - pnpm lint: 9 pre-existing warnings, 0 new, 0 errors - pnpm vitest run --config vitest.config.ts: 731/731 passing - Live preview at http://127.0.0.1:3000/ verified wiring (cudaToolkit field now surfaces on /api/system/hardware-probe). * fix: Use .venv-scoped CUDA readiness * fix: Compute scroll position relative to container * feat: OpenVINO stack install button mirroring TensorRT UX - Add openvino + optimum-intel[openvino] install via NDJSON /api/env/install-openvino into .venv - Probe openvino version, Core().available_devices and optimum.intel availability - Wire requiresInstall/openvinoNeedsInstall badge and install button in IHVIntegrationPanel - Link to Intel GPU/NPU driver docs when discrete devices are absent - Update recipe inference and hardwareProbe/pickRecommendedProvider for OpenVINO - Add unit test for openvino stack install args * refactor(openvino): split probe script, parse and install logic to reduce complexity * fix: 2 findings — Unify shared venv install mutex; Rate-limit OpenVINO i - Unify shared venv install mutex - Rate-limit OpenVINO installation * fix: 2 findings — Preserve partial OpenVINO probe results; Parallelize h - Preserve partial OpenVINO probe results - Parallelize hardware probe calls * fix: address Greptile review feedback - Add optimum.intel import check in recipe inference - Restrict OpenVINO hardware detection to Intel-branded CPUs - Surface optimum.intel import errors in probe detail - Apply heavyCommandRateLimit to all venv install routes * Update src/server/services/olive/openvino.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/lib/cudaDeps.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * fix: apply CodeRabbit auto-fixes Fixed 6 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix: install and verify onnxruntime-openvino for OpenVINO EP Codex P1: openvino + optimum-intel alone do not register OpenVINOExecutionProvider. Install onnxruntime-openvino, remove conflicting ORT wheels with a warning, and gate readiness on the EP. P2 (shared venv pip mutex) was already unified on this branch. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address PR #106 review feedback across 23 outstanding threads CUDA install UX (#12, #13, #16, #17, #18, #19, #21, #22, #25) - hardwareProbe.mergeDetectedProviders: simplify cudaOk ternary to `input.cudaLoadable !== false` — same semantic, half the surface. - hardwareProbe's pre-Maxwell box reason + recipeHardwareCompatibility's CUDA-floor reason now name Maxwell SM 5.0 as 'GeForce GTX 750 Ti or GTX 9xx series' instead of mistakenly borrowing 'GeForce RTX 20xx+' (RTX 20xx is Turing SM 7.5 — the floor above — which previously implied a Pascal SM 6.x owner had an unsupported card). - recipeHardwareCompatibility widens tensorRtInstallHint to ePInstallHint and routes the onnxruntime-gpu install hint through it so the probe's detail string (driver/wheel mismatch, missing module, etc.) is preserved end-to-end. - IHVIntegrationPanel gates the CUDA toolkit download-link paragraph on `cudaEpInVenv` so it only renders when the EP is actually registered — removes the contradictory 'CUDA EP detected / install button shown' state. - InputEnvironmentPanel renders the requiresInstall hint directly so the label and the pip-install command can never disagree again (the old code branched on `kind` and silently printed the TensorRT label next to the onnxruntime install command). Drop the now-dead pinnedTensorRtLabel + tensorrtRtxEpAbiLabel imports. - system.ts hardens the ORT GPU probe with an explicit 30 s timeout (a broken driver install can leave the onnxruntime import hanging and bind the HTTP request) and derives the pinned-version string from pinnedOrtGpuLabel() instead of hardcoding '1.26.0' — a wheel bump in oliveGpuRuntime now propagates to every error message and install hint without further edits. - system.ts notes generator now derives its suggested pip command from pinnedOrtGpuInstallCommand() instead of a literal 'pip install onnxruntime-gpu==1.26.0' string. Shared install helpers (#23, #24, #26) - src/server/services/shared/pipInstall.ts is the single NDJSON-aware pip install helper used by every install route; the local copies in cuda.ts + tensorrt-rtx.ts deleted (kept the exact byte-shape so the UIs NDJSON parser keeps working unchanged). - cuda.ts exports ensureOnnxRuntimeGpu; tensorrt-rtx.ts calls it directly instead of carrying a private duplicate (mirror drift was the actual bug — they were diverging in subtle ways). - cuda.ts ensureOnnxRuntimeGpu explicitly returns libsDir: null on success so the documented return-type contract is honoured and external callers can distinguish "no library directory available" from "field not set". - tensorrt-rtx.ts uses platform-appropriate native-lib extensions: onnxruntime_providers_nv_tensorrt_rtx.dll on win32, libonnxruntime_providers_nv_tensorrt_rtx.so on Linux, .dylib on Darwin (a hardcoded .dll hides the real reason an import fails on non-Windows platforms). The probe script picks the right extension at runtime. Manifest / dedupe (#5, #20, #6) - cudaDeps.cudaDownloadUrlForOs routes darwin to the archive landing page (Apple dropped CUDA toolkit support after CUDA 11.6) and uses word-boundary regexes so 'darwin' no longer accidentally hits the Windows branch via the substring 'win'. - tensorrtRtxDeps.tensorrtRtxEpAbiInstallCommand now derives the manual pip command from the args list (same delegation pattern pinnedTensorRtInstallCommand uses), so an index/version bump cannot desync the server-side install and the user-facing fallback hint. - src/server/shared/anyDotVenvDir.ts is the single chokidar ANY_DOT_VENV_DIR regex; vite.config.ts and server.ts both import from here so a rename/back-up of the venv directory is filtered out by both Vite watchlists in lockstep. Test fixtures (#14, #15) - cudaDeps.test isPreMaxwellNvidiaBox describe block: split the mislabeled 'every card is at or above the floor' assertion into two focused tests (every-card-above-floor / every-card-below-floor); add a darwin routing suite (asserts darwin does NOT hit the Windows branch via substring 'win' and lands on the archive landing page). - providerCatalog.test cross-family lockstep test title now uses a template literal so the floor number interpolates into the test name (was a verbatim '$floor' string). e2e/scroll-bounds-guardrail.spec.ts (#7, #8, #9) - Page.evaluate wraps Radix Tooltip elements in the full Provider/Root/Trigger/Portal/Content hierarchy using React.createElement so the elements go through the JSX reconciler (plain RdxTooltip.Portal(...) / Content(...) function calls were missing Radix's Provider context and the sentinel never mounted). - DOM-fallback branch preserves the actual import-error string so a future failure is filed with the real reason, not a synthetic 'Radix bare imports did not resolve' placeholder. - Added paint-time hit-test: document.elementFromPoint at the bbox centre must resolve to the sentinel (or one of its ancestors up to #root). getClientRects alone cannot detect overflow:hidden clipping because clip preserves the rect coordinates; the hit-test is the actual guardrail. - Restrict the lint-disable comment to the placeholder line so the prettier/sonar warnings stay clean. Verified: tsc clean, 734/734 unit tests + 229/229 server tests pass, pnpm validate:recipe ok, pnpm lint 9 pre-existing warnings / 0 new errors, live /api/system/hardware-probe?refresh=1 reflects the updated probe fields. * fix: add missing useOpenVinoInstall hook for CI typecheck CodeRabbit's auto-fix imported @/components/features/useOpenVinoInstall but never added the module, so pnpm lint (tsc) failed with TS2307. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address follow-up review on tensorrt.ts pipInstall, install mutex, state-4 test - tensorrt.ts: extract private pipInstall to ../shared/pipInstall.ts; local spawn-based copy removed, all 4 install call-sites now route through the shared helper so cuda/tensorrt/tensorrt-rtx agree on error contract and the '[deps]' output prefix. - IHVIntegrationPanel.tsx: replace installingTrt | installingTrtRtx | installingOrtGpu trio of mutex flags with a single installInProgress derived from all three. The three handler guards, setInstalling(true) cleanup, and React Button disabled props all use the unified flag so a concurrent pip install can no longer race the shared .venv. - hardwareProbe.test.ts: state-4 fixture now passes cuda.loadable:true so the install-hint branch (state 3) is bypassed and the cascade reaches the state-4 driver/wheel mismatch reason. Without cuda.loadable the fixture was exercising state 3 instead of state 4, so the assertion originally committed failed to validate the intended branch. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com> * Update src/components/features/IHVIntegrationPanel.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: extract HardwareProviderCard to clear CodeFactor complexity Move the provider-card map (Very Complex Method) out of IHVIntegrationPanel into a dedicated component with chrome helpers and install/conflict subcomponents so CodeFactor can pass on PR 108. Co-authored-by: Anthony Thompson <github@trackdub.com> * fix: address CodeRabbit OpenVINO review findings Share NDJSON install helper with residual-frame flush, add hook tests, require --upgrade with eager strategy, tighten Intel hardware detection via computeOpenVinoCompatibleHardware + lspci/Win32 probe, and gate ORT wheel uninstall with install-failure recovery messaging. Co-authored-by: Anthony Thompson <github@trackdub.com> --------- Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Codebuff <noreply@codebuff.com>
1 of 2 tasks
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
1 participant
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.
Merges the CodeRabbit simplification branch with the registration/KB cleanup and fixes normalization.py before merging.
All 44 Olive MCP tests pass.
Generated with Devin
Summary by cubic
Hardened normalization for frameworks, models, and hardware to produce accurate, consistent matches and avoid false positives. Cleaned up MCP tool registration and shared KB access; added tests for normalization.
Bug Fixes
Refactors
KB_DIR) and updated docs search to use them.TOOLSlist.Written for commit 7eb8fba. Summary will update on new commits.