audit: fix 12 code-quality issues, extract 4 large components, lint-clean sweep - #156
Conversation
…lean sweep - Replaced 8 `any` types with `unknown` in oliveRecipeHub.ts, removing all eslint-disable comments (#143) - Merged duplicate imports: credentials.ts (#149), familyEnsure.ts (#146), registry.test.ts (#145) - Removed unused `beforeEach` in ndjsonInstall.test.ts (#148) - Renamed ambiguous `l` → `lat_lower` in strategy_advisor.py (#147) - Replaced bare `except Exception: continue` with typed error handling in docs_search.py (#151) - Renamed unused `reject` → `_reject` in arenaOliveOutputs.test.ts (#144) - Added eslint-disable comments for 6 legitimate set-state-in-effect hooks - Extracted `buildProbeDiagnostics` (160 lines) from probeSystemHardware in system.ts — reduced function from 395 to ~250 lines (#140) - Extracted `OwrExportOverlay` (301 lines, 14 typed props) into OwrExportOverlay.tsx — reduced ExecutionWorkspace.tsx by 233 lines (#120) - Extracted `BatchJobList` + `BatchJobCard` as module-level helpers in BatchProcessingPanel.tsx (#122) - Assessed IHVIntegrationPanel.tsx extraction candidates (#121) - Updated docs/Tech Debt & Issues.md with full audit: 23 closed, 31 open - Validated: tsc --noEmit clean, ESLint zero warnings, all 872 tests pass 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 26 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 (15)
📝 WalkthroughWalkthroughThe change extracts execution, export, batch-processing, and hardware UI components, centralizes Olive streaming and probe diagnostics, replaces unsafe recipe typing, narrows knowledge-base error handling, and updates technical-debt documentation. ChangesApplication refactors and cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. Analyzed 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 QodoAudit sweep: type-safety, lint cleanup, and large-component extractions
AI Description
Diagram
High-Level Assessment
Files changed (19)
|
Greptile SummaryThe PR performs a broad maintainability cleanup while extracting several large UI and server-side units without an established blocking regression.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/components/features/ExecutionWorkspace.tsx | Delegates execution lifecycle and OWR export rendering to extracted modules while retaining the workspace orchestration and public props. |
| src/components/features/useOliveStream.ts | Encapsulates execution, SSE reconnection, cancellation, completion, and cleanup behavior with generation-based stale-callback protection. |
| src/components/features/OwrExportOverlay.tsx | Extracts the OWR export dialog, copy/download controls, and focus-management behavior into a typed component. |
| src/server/routes/system.ts | Extracts hardware-probe diagnostics into typed helpers while preserving consumption of the resulting notes and provider capability state. |
| src/components/features/IHVIntegrationPanel.tsx | Delegates compatibility and pass-card rendering through typed, memoized prop bundles. |
| src/components/features/HardwareCompatibilityMatrix.tsx | Extracts the interactive hardware compatibility matrix and improves its keyboard-accessible controls. |
| src/components/features/HardwarePassCards.tsx | Extracts hardware pass cards while retaining compatibility and quantization activation checks. |
| src/components/features/BatchProcessingPanel.tsx | Extracts batch-list and batch-card rendering and provides dedicated accessible selection and deletion controls. |
| olive-mcp-server/olive_mcp_server/tools/docs_search.py | Narrows knowledge-base loading failures to expected file, decoding, and JSON errors while retaining valid sibling files. |
| src/lib/oliveRecipeHub.ts | Replaces explicit any usage with unknown and record types as part of the lint-clean type-safety sweep. |
Reviews (12): Last reviewed commit: "fix: use const for finalizeExecution exi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/Tech` Debt & Issues.md:
- Around line 139-151: The GitHub issue audit contains inconsistent counts,
duplicate closure references, and closed issues in the remaining-open list.
Reconcile the Summary and “Remaining Open” table against a single GitHub
snapshot: make each category’s count match its listed IDs, remove or correctly
identify the duplicate `#145` note, and exclude closed issues `#120`, `#121`, `#122`,
and `#140` from the open list while preserving the accurate total of 31 open
issues.
In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py`:
- Around line 82-84: Update the per-file exception handler in _load_kb_text() to
include UnicodeDecodeError alongside OSError and json.JSONDecodeError,
preserving the existing debug logging and continue behavior so malformed UTF-8
files do not prevent later files from loading.
In `@src/components/features/BatchProcessingPanel.tsx`:
- Around line 477-590: Update the selectable job-card container around the job
status and details to support keyboard activation, including an appropriate
interactive role, tab focusability, and Enter/Space handlers that invoke
onSelect while preserving click behavior. Add an accessible name to the delete
button, such as an aria-label describing deletion of the current job, while
keeping its existing stopPropagation and onDelete behavior.
In `@src/components/features/IHVIntegrationPanel.tsx`:
- Around line 473-476: Update the scoped eslint suppression in the useEffect
invoking runHardwareProbe to include an inline explanation that the synchronous
state updates are intentional for the mount hardware probe, while
runHardwareProbe is also shared by the rescan button and install handlers.
In `@src/components/features/OwrExportOverlay.tsx`:
- Line 2: Replace the barrel UI imports in OwrExportOverlay.tsx (line 2) and
ExecutionWorkspace.tsx (line 16) with direct imports from each component’s
defining module, covering every imported UI component and preserving their
existing usage.
- Around line 87-94: Add accessible names in OwrExportOverlay: give the
icon-only Button invoking onClose an aria-label describing its close action, and
provide a programmatic label for the read-only textarea using the existing label
mechanism or an associated labeling attribute.
- Around line 74-78: Update handleCopyActiveCode so setIsOwrCopied(true) runs
only after navigator.clipboard.writeText(fileContent) resolves successfully;
preserve the existing two-second reset and avoid showing the copied state when
the write rejects.
In `@src/lib/oliveRecipeHub.ts`:
- Around line 504-509: Add runtime validation in deriveUiStateFromOliveRecipe
for external recipe fields before populating or applying UIState. Use shared
record and string-array guards to validate hf_config.model_name and local_files,
rejecting malformed values rather than assigning them to state or calling
setState; add coverage for an invalid model_name and non-string local_files
entries.
In `@src/server/routes/system.ts`:
- Around line 230-371: Split buildProbeDiagnostics into the requested domain
helpers: buildOrtProviderNotes, buildTensorRtNotes, buildCudaNotes,
buildOpenVinoNotes, and buildQnnNotes. Move each corresponding note-generation
block into its helper, preserving deterministic ordering and existing messages,
while keeping nvidiaTensorRtFamilyCapable and qnnHostMode available for the
returned ProbeDiagnosticOutput and downstream
mergeDetectedProviders/pickRecommendedProvider behavior.
- Line 310: Update ProbeDiagnosticInput to include a platformOs field, populate
it with process.platform in probeSystemHardware, and pass input.platformOs to
resolveQnnHostMode within buildProbeDiagnostics instead of reading
process.platform directly. Keep platform and architecture resolution driven by
the input values so the helper remains pure and testable.
- Around line 358-368: Add focused route-level tests for probeSystemHardware
covering NVIDIA TensorRT-family capability at the compute-capability floor and
below it, including zero GPUs, and verifying the resulting diagnostics and
provider recommendations. Also cover qnnHostMode output and its effects on
detectedProviders and recommendedProvider, exercising the private
buildProbeDiagnostics behavior through the public route API.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cf12896b-f1fe-4c09-a8d7-90e2ce451683
📒 Files selected for processing (19)
docs/Tech Debt & Issues.mdolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/strategy_advisor.pysrc/components/TitleBar.tsxsrc/components/features/ArenaConvenience.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/LocalModelManager.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/gemini/SettingsPanel.tsxsrc/components/features/gemini/useLocalEngineSetup.tssrc/lib/devin/credentials.tssrc/lib/ndjsonInstall.test.tssrc/lib/oliveRecipeHub.tssrc/server/routes/arenaOliveOutputs.test.tssrc/server/routes/system.tssrc/server/services/ai/registry.test.tssrc/server/services/venv/familyEnsure.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
src/**/*.{ts,tsx}: Match existing naming, file layout, and TypeScript patterns insrc/.
Put shared recipe logic insrc/lib/, especiallypipelineValidation.ts,oliveRecipeBuilder.ts, andrecipePipeline.ts.
src/**/*.{ts,tsx}: Keep validation logic in shared libraries rather than duplicating it in UI cell helpers or inspectors.
Split theInputEnvironmentPanel,IHVIntegrationPanel, andExecutionWorkspacemega-panels into feature folders with colocated hooks and tests.
Keep server and UI AI provider catalogs synchronized, preferably through a shared provider ID list or synchronization test; register new providers in both catalogs.
Add test coverage forrecipe-graph/,passCatalog,oliveRecipeHub,jobHistoryStore, andvramEstimate, and strengthen component tests for the large panels.
src/**/*.{ts,tsx}: All UI state mutations must go throughcommitUiStateUpdateinsrc/lib/pipelineValidation.tsso invariants are enforced; useusePipelineState()for state access andreplaceStatefor recipe imports or preset loads.
Avoidexport *barrel imports; import directly from the actual module file to preserve Vite tree-shaking and component-test isolation.
src/**/*.{ts,tsx}: In React/TypeScript source, avoid barrel imports; import from the specific module/file instead of re-export index files.
In React/TypeScript source, eliminate waterfalls in data loading and rendering flows.
In React/TypeScript source, defer non-critical third-party libraries instead of loading them eagerly.
Files:
src/components/features/IHVIntegrationPanel.tsxsrc/server/routes/arenaOliveOutputs.test.tssrc/lib/devin/credentials.tssrc/components/features/ArenaConvenience.tsxsrc/components/TitleBar.tsxsrc/server/routes/system.tssrc/components/features/LocalModelManager.tsxsrc/lib/ndjsonInstall.test.tssrc/components/features/gemini/useLocalEngineSetup.tssrc/server/services/ai/registry.test.tssrc/server/services/venv/familyEnsure.tssrc/components/features/ExecutionWorkspace.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/components/features/OwrExportOverlay.tsxsrc/components/features/gemini/SettingsPanel.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx}: Place imports at the top of modules; use inline imports only for a documented circular dependency.
Run linting and ensure typecheck-related CI checks pass before submitting changes.
For UI or server changes, manually smoke-test development startup, recipe loading/building, validation banners, and live execution when execution behavior is touched.
Files:
src/components/features/IHVIntegrationPanel.tsxsrc/server/routes/arenaOliveOutputs.test.tssrc/lib/devin/credentials.tssrc/components/features/ArenaConvenience.tsxsrc/components/TitleBar.tsxsrc/server/routes/system.tssrc/components/features/LocalModelManager.tsxsrc/lib/ndjsonInstall.test.tssrc/components/features/gemini/useLocalEngineSetup.tssrc/server/services/ai/registry.test.tssrc/server/services/venv/familyEnsure.tssrc/components/features/ExecutionWorkspace.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/components/features/OwrExportOverlay.tsxsrc/components/features/gemini/SettingsPanel.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When working with React 19 or Vite 8 APIs, consult current Context7 documentation instead of assuming conventions from earlier major versions.
Files:
src/components/features/IHVIntegrationPanel.tsxsrc/server/routes/arenaOliveOutputs.test.tssrc/lib/devin/credentials.tssrc/components/features/ArenaConvenience.tsxsrc/components/TitleBar.tsxsrc/server/routes/system.tssrc/components/features/LocalModelManager.tsxsrc/lib/ndjsonInstall.test.tssrc/components/features/gemini/useLocalEngineSetup.tssrc/server/services/ai/registry.test.tssrc/server/services/venv/familyEnsure.tssrc/components/features/ExecutionWorkspace.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/components/features/OwrExportOverlay.tsxsrc/components/features/gemini/SettingsPanel.tsx
src/server/routes/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each API route module must export a
mountXxxRoutes(router)function and be wired intoserver.ts.
Files:
src/server/routes/arenaOliveOutputs.test.tssrc/server/routes/system.ts
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Pin the Python
mcpdependency to a version below 2 because version 2.x removesmcp.server.fastmcpand breaks imports.
Files:
olive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/strategy_advisor.py
src/server/services/ai/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep AI provider integrations organized as one provider implementation per file; provider detection belongs in
detect.ts.
Files:
src/server/services/ai/registry.test.ts
src/server/services/venv/**/*.{ts,js}
📄 CodeRabbit inference engine (REVIEW.md)
Pin the runtime
olive-aiinstallation to a supported version or range and document the supported Olive versions.
Files:
src/server/services/venv/familyEnsure.ts
src/lib/oliveRecipeHub.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing UI-to-Olive JSON mapping, update related import/export logic in
oliveRecipeHub.ts.
Files:
src/lib/oliveRecipeHub.ts
🧠 Learnings (1)
📚 Learning: 2026-08-04T12:36:02.655Z
Learnt from: tonythethompson
Repo: tonythethompson/Olive-Studio PR: 97
File: src/components/features/BatchProcessingPanel.tsx:0-0
Timestamp: 2026-08-04T12:36:02.655Z
Learning: When updating pipeline state through usePipelineState().setState in React components, do not wrap the update in another commitUiStateUpdate call. PipelineStore.setState already invokes commitUiStateUpdate(store.state, partial) to enforce UI state invariants; a second commit can duplicate the operation and merge against a stale component state snapshot.
Applied to files:
src/components/features/IHVIntegrationPanel.tsxsrc/components/features/ArenaConvenience.tsxsrc/components/TitleBar.tsxsrc/components/features/LocalModelManager.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/gemini/SettingsPanel.tsx
🪛 GitHub Check: CodeFactor
src/server/routes/system.ts
[notice] 230-371: src/server/routes/system.ts#L230-L371
Complex Method
🪛 LanguageTool
docs/Tech Debt & Issues.md
[uncategorized] ~175-~175: The official name of this software platform is spelled with a capital “H”.
Context: ...jectField). GET-only routes (system.ts, github.ts) don't need it. |
| #153 | Expa...
(GITHUB)
[style] ~221-~221: To form a complete sentence, be sure to include a subject.
Context: ...ion issues | Older CodeFactor findings. May be partially addressed by prior refacto...
(MISSING_IT_THERE)
🪛 React Doctor (0.9.3)
src/components/features/BatchProcessingPanel.tsx
[warning] 477-477: Screen reader users can't tell this click handler is interactive because it has no role, so add a role or use a button or link.
Give clickable static elements a role, or use a button or link.
(no-static-element-interactions)
[warning] 530-530: Your users can see & submit the wrong data when this list reorders or filters, so use a stable id like key={item.id}, not the array index "idx".
Use a stable id from the item, like key={item.id} or key={item.slug}. Index keys break when the list reorders or filters.
(no-array-index-as-key)
[warning] 581-581: Blind users can't tell what this control does because screen readers find no label, so add visible text, aria-label, or aria-labelledby.
Give every interactive control a label screen readers can read.
(control-has-associated-label)
src/components/features/OwrExportOverlay.tsx
[warning] 190-190: ' in JSX text can read as markup & confuse readers.
Replace bare ' / " / > / } characters with HTML entities so literal UI text is encoded consistently.
(no-unescaped-entities)
[warning] 255-255: Blind users can't tell what this control does because screen readers find no label, so add visible text, aria-label, or aria-labelledby.
Give every interactive control a label screen readers can read.
(control-has-associated-label)
🔍 Remote MCP GitHub Copilot
Relevant review context
- PR
#156is a single commit (eb5a81e) and currently has a successful Vercel deployment, while the CodeRabbit status remains pending. The validation claims in the commit message are not independently represented by repository check results. deriveUiStateFromOliveRecipeis consumed byInputEnvironmentPanel, the recipe-builder validation script, and recipe-builder tests. The newunknownsignature is therefore on an active import/parsing path.- The
unknownmigration primarily uses type assertions (as Record<string, unknown>) rather than runtime schema validation. In particular, recipe fields andlocal_filescontents are still trusted at runtime; malformed external recipe JSON remains a review area. buildProbeDiagnosticsis private, called once byprobeSystemHardware, and its output directly feeds detected-provider calculation and recommended-provider selection. The route directory contains policy tests but no dedicatedsystem.test.ts, so helper behavior is not visibly covered by a focused route test.- The issue-audit count of 31 remaining open issues matches the repository’s current issue list. However, the document’s “Remaining Open” section also repeats issues
#120,#121,#122, and#140, which are closed, making that section internally inconsistent.
🔇 Additional comments (16)
src/server/routes/system.ts (2)
202-228: LGTM!
562-584: LGTM!olive-mcp-server/olive_mcp_server/tools/strategy_advisor.py (1)
54-59: LGTM!src/components/TitleBar.tsx (1)
16-17: LGTM!src/components/features/ArenaConvenience.tsx (1)
74-75: LGTM!src/components/features/LocalModelManager.tsx (1)
182-183: LGTM!src/components/features/gemini/SettingsPanel.tsx (1)
125-126: LGTM!src/components/features/gemini/useLocalEngineSetup.ts (1)
173-174: LGTM!src/lib/devin/credentials.ts (1)
7-7: LGTM!src/lib/ndjsonInstall.test.ts (1)
1-1: LGTM!src/server/routes/arenaOliveOutputs.test.ts (1)
441-441: LGTM!src/server/services/ai/registry.test.ts (1)
30-31: LGTM!src/server/services/venv/familyEnsure.ts (1)
10-10: LGTM!Also applies to: 35-35
src/components/features/ExecutionWorkspace.tsx (1)
1053-1066: 🎯 Functional CorrectnessProvide validation evidence for both extracted UI flows.
The stated validation results are not independently represented by repository checks. Run the relevant component tests and manually verify development startup, recipe loading/building, validation banners, OWR platform/file selection and bundle download, plus batch empty-state, selection, and deletion behavior.
src/components/features/ExecutionWorkspace.tsx#L1053-L1066: verify controlled OWR overlay interaction and bundle download behavior.src/components/features/BatchProcessingPanel.tsx#L1024-L1029: verify queue rendering and delegated selection/deletion behavior.As per coding guidelines, “For UI or server changes, manually smoke-test development startup, recipe loading/building, validation banners, and live execution when execution behavior is touched.”
Sources: Coding guidelines, MCP tools
src/lib/oliveRecipeHub.ts (1)
155-157: LGTM!Also applies to: 216-217, 310-311, 322-365
docs/Tech Debt & Issues.md (1)
3-11: LGTM!Also applies to: 42-44
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
Reconcile GitHub issue audit counts, harden KB/docs and recipe import validation, improve batch/OWR accessibility, split probe diagnostics helpers with platformOs, and add focused tests. Co-authored-by: Anthony Thompson <github@trackdub.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Regression for Qodo finding: invalid UTF-8 KB file must not abort loading sibling valid JSON entries. Co-authored-by: Anthony Thompson <github@trackdub.com>
…e audit doc - Extracted HardwareCompatibilityMatrix (359 lines, 7 props) from IHVIntegrationPanel.tsx — interactive heatmap table + footer legend. IHV panel reduced from 1592 to 1282 lines (−310, 19%). (#121) - Extracted useOliveStream hook (398 lines) from ExecutionWorkspace.tsx — full SSE lifecycle: connectSSE with log/metrics/done listeners, exponential backoff reconnect (10 att / 30s max), graceful cancel handling, job history persistence. Component reduced from 1515 to 1174 lines (−341, 22%). Cumulative reduction with prior OwrExportOverlay extraction: 1753 → 1174 (−579, 33%). (#120) - Removed 11 unused imports, 7 state/ref declarations across both files. - Exported OptimizationPassValidation interface from IHV panel. - Closed #139 (PluginInstallBlock verified DRY — reused 6×). - Updated docs/Tech Debt & Issues.md: 24 closed, 30 open, all 4 complexity extractions complete with final line counts. - Full CI validated: tsc clean, ESLint zero warnings, 1773/1774 tests pass. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/server/routes/system.ts (1)
268-300: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not report incompatible GPUs as TensorRT compatible.
Lines 270-285 add install guidance that states “GPU is compatible” for any detected NVIDIA GPU. Lines 288-299 then hide TensorRT and TensorRT-RTX providers when every GPU is below SM 7.5.
Calculate
nvidiaTensorRtFamilyCapablebefore these messages. Only show compatibility and install guidance when it is true. Add a Pascal test assertion that rejects the compatible/install message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/routes/system.ts` around lines 268 - 300, Move the nvidiaTensorRtFamilyCapable calculation before the TensorRT and TensorRT RTX note-generation branches, and gate their compatibility/install guidance on it so below-SM-7.5 GPUs are never described as compatible. Preserve the existing verified-runtime messages where applicable, keep the final low-capability warning behavior, and add a Pascal test assertion confirming no compatible/install message is emitted.src/components/features/ExecutionWorkspace.tsx (1)
1054-1067: 📐 Maintainability & Code Quality | 🔵 TrivialComplete the required validation before merge.
This change moves a user-facing export flow across a component boundary. The current status still lists
validate,python-tests,security,docker-build,olive-pass-availability, and Greptile as in progress. Complete linting, typecheck-related checks, and the required UI smoke tests before treating this stack as ready.As per coding guidelines, run linting and typecheck checks and manually smoke-test UI changes before submission.
The supplied external-tool status lists these checks as in progress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/features/ExecutionWorkspace.tsx` around lines 1054 - 1067, Complete validation for the ExecutionWorkspace export-flow change before merge: run linting and all typecheck-related checks, resolve any failures, and manually smoke-test the OwrExportOverlay flow including opening, closing, platform/file/thread/VRAM changes, and bundle download. Also complete the required validate, python-tests, security, docker-build, olive-pass-availability, and Greptile checks.Sources: Coding guidelines, MCP tools
src/components/features/OwrExportOverlay.tsx (1)
84-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAdd modal focus trapping and dialog semantics to
OwrExportOverlay.The full-screen export overlay does not expose
role="dialog"oraria-modal, and it does not trap/focus-manage keyboard input when open. This leaves keyboard users able to Tab into background app controls while the overlay is in front. Use the project’s modal/dialog primitive, or add focus entry, focus restoration, focus containment, and Escape handling before rendering the overlay.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/features/OwrExportOverlay.tsx` around lines 84 - 85, Update OwrExportOverlay to use the project’s modal/dialog primitive, or implement equivalent dialog behavior around the overlay: expose role="dialog" and aria-modal, move focus into the dialog when opened, trap Tab navigation within it, restore focus when closed, and close on Escape while preserving the existing export content and styling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/Tech` Debt & Issues.md:
- Line 141: Update the audit verification around the `gh issue list --state
open` command in the documentation to specify an explicit `--limit` greater than
31, and record the resulting issue-number list alongside the snapshot so the
31-open-issue count is reproducible.
In `@src/components/features/BatchProcessingPanel.tsx`:
- Around line 477-487: The card’s keyboard handler around the selectable
`role="button"` must not intercept Enter or Space events originating from the
nested delete control. Keep selection activation for the card itself, ensure the
delete control independently handles Enter and Space to invoke deletion, and
prevent the card selection handler from suppressing the delete button’s native
activation.
In `@src/components/features/OwrExportOverlay.tsx`:
- Around line 76-80: Update handleCopyActiveCode to handle rejected
navigator.clipboard.writeText(fileContent) promises, ensuring the success
callback sets isOwrCopied true only after a successful write and the rejection
path leaves it false without an unhandled rejection.
In `@src/lib/__tests__/oliveRecipeHub.deriveUiState.test.ts`:
- Around line 4-42: Extend the deriveUiStateFromOliveRecipe validation suite
with valid and malformed runtime-shape cases for execution-provider,
catalog-device, quantization, pruning, pass-config, and model_path inputs.
Assert valid values produce the expected UI state and malformed values are
ignored or unset rather than accepted through casts, preserving existing
hf_config.model_name and local_files coverage.
---
Outside diff comments:
In `@src/components/features/ExecutionWorkspace.tsx`:
- Around line 1054-1067: Complete validation for the ExecutionWorkspace
export-flow change before merge: run linting and all typecheck-related checks,
resolve any failures, and manually smoke-test the OwrExportOverlay flow
including opening, closing, platform/file/thread/VRAM changes, and bundle
download. Also complete the required validate, python-tests, security,
docker-build, olive-pass-availability, and Greptile checks.
In `@src/components/features/OwrExportOverlay.tsx`:
- Around line 84-85: Update OwrExportOverlay to use the project’s modal/dialog
primitive, or implement equivalent dialog behavior around the overlay: expose
role="dialog" and aria-modal, move focus into the dialog when opened, trap Tab
navigation within it, restore focus when closed, and close on Escape while
preserving the existing export content and styling.
In `@src/server/routes/system.ts`:
- Around line 268-300: Move the nvidiaTensorRtFamilyCapable calculation before
the TensorRT and TensorRT RTX note-generation branches, and gate their
compatibility/install guidance on it so below-SM-7.5 GPUs are never described as
compatible. Preserve the existing verified-runtime messages where applicable,
keep the final low-capability warning behavior, and add a Pascal test assertion
confirming no compatible/install message is emitted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f678c1fb-989c-4171-bc7a-84d50a10570e
📒 Files selected for processing (10)
docs/Tech Debt & Issues.mdolive-mcp-server/olive_mcp_server/tools/docs_search.pysrc/components/features/BatchProcessingPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/OwrExportOverlay.tsxsrc/lib/__tests__/oliveRecipeHub.deriveUiState.test.tssrc/lib/oliveRecipeHub.tssrc/server/routes/system.probeDiagnostics.test.tssrc/server/routes/system.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
- GitHub Check: validate
- GitHub Check: security
- GitHub Check: olive-pass-availability
- GitHub Check: docker-build
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
src/**/*.{ts,tsx}: Match existing naming, file layout, and TypeScript patterns insrc/.
Put shared recipe logic insrc/lib/, especiallypipelineValidation.ts,oliveRecipeBuilder.ts, andrecipePipeline.ts.
src/**/*.{ts,tsx}: Keep validation logic in shared libraries rather than duplicating it in UI cell helpers or inspectors.
Split theInputEnvironmentPanel,IHVIntegrationPanel, andExecutionWorkspacemega-panels into feature folders with colocated hooks and tests.
Keep server and UI AI provider catalogs synchronized, preferably through a shared provider ID list or synchronization test; register new providers in both catalogs.
Add test coverage forrecipe-graph/,passCatalog,oliveRecipeHub,jobHistoryStore, andvramEstimate, and strengthen component tests for the large panels.
src/**/*.{ts,tsx}: All UI state mutations must go throughcommitUiStateUpdateinsrc/lib/pipelineValidation.tsso invariants are enforced; useusePipelineState()for state access andreplaceStatefor recipe imports or preset loads.
Avoidexport *barrel imports; import directly from the actual module file to preserve Vite tree-shaking and component-test isolation.
src/**/*.{ts,tsx}: In React/TypeScript source, avoid barrel imports; import from the specific module/file instead of re-export index files.
In React/TypeScript source, eliminate waterfalls in data loading and rendering flows.
In React/TypeScript source, defer non-critical third-party libraries instead of loading them eagerly.
Files:
src/lib/__tests__/oliveRecipeHub.deriveUiState.test.tssrc/server/routes/system.probeDiagnostics.test.tssrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/server/routes/system.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx}: Place imports at the top of modules; use inline imports only for a documented circular dependency.
Run linting and ensure typecheck-related CI checks pass before submitting changes.
For UI or server changes, manually smoke-test development startup, recipe loading/building, validation banners, and live execution when execution behavior is touched.
Files:
src/lib/__tests__/oliveRecipeHub.deriveUiState.test.tssrc/server/routes/system.probeDiagnostics.test.tssrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/server/routes/system.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When working with React 19 or Vite 8 APIs, consult current Context7 documentation instead of assuming conventions from earlier major versions.
Files:
src/lib/__tests__/oliveRecipeHub.deriveUiState.test.tssrc/server/routes/system.probeDiagnostics.test.tssrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/lib/oliveRecipeHub.tssrc/server/routes/system.ts
src/server/routes/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Each API route module must export a
mountXxxRoutes(router)function and be wired intoserver.ts.
Files:
src/server/routes/system.probeDiagnostics.test.tssrc/server/routes/system.ts
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Pin the Python
mcpdependency to a version below 2 because version 2.x removesmcp.server.fastmcpand breaks imports.
Files:
olive-mcp-server/olive_mcp_server/tools/docs_search.py
src/lib/oliveRecipeHub.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing UI-to-Olive JSON mapping, update related import/export logic in
oliveRecipeHub.ts.
Files:
src/lib/oliveRecipeHub.ts
🧠 Learnings (1)
📚 Learning: 2026-08-04T12:36:02.655Z
Learnt from: tonythethompson
Repo: tonythethompson/Olive-Studio PR: 97
File: src/components/features/BatchProcessingPanel.tsx:0-0
Timestamp: 2026-08-04T12:36:02.655Z
Learning: When updating pipeline state through usePipelineState().setState in React components, do not wrap the update in another commitUiStateUpdate call. PipelineStore.setState already invokes commitUiStateUpdate(store.state, partial) to enforce UI state invariants; a second commit can duplicate the operation and merge against a stale component state snapshot.
Applied to files:
src/components/features/IHVIntegrationPanel.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/OwrExportOverlay.tsxsrc/components/features/BatchProcessingPanel.tsx
🔍 Remote MCP DeepWiki, GitHub Copilot
Additional review context
- PR
#156’s current checks are not yet complete:validate,python-tests,security,docker-build, andolive-pass-availabilityare still in progress. Vercel and CodeFactor succeeded; Greptile is also in progress. BatchProcessingPanelnow adds keyboard selection to job cards and an accessible delete label while preserving the existing click/delete handlers. The related component tests only verify rendering and empty-state behavior, not keyboard activation or deletion.OwrExportOverlayowns the copied-state timer and invokesnavigator.clipboard.writeText(...).then(...); the promise is intentionally discarded withvoid, but no rejection handler is present.deriveUiStateFromOliveRecipenow acceptsunknown, but several recipe substructures still use type assertions toRecord<string, unknown>rather than recursively validating their shape. Focused tests cover onlyhf_config.model_nameandlocal_files.buildProbeDiagnosticsis now pure and receivesplatformOs/platformArchexplicitly. Its focused tests cover TensorRT capability thresholds, zero-GPU handling, and QNN host modes, but the fullprobeSystemHardwareroute integration remains indirectly tested.- DeepWiki could not retrieve repository context because
tonythethompson/Olive-Studiowas not indexed/found; no DeepWiki architectural facts were available.
🔇 Additional comments (22)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (1)
82-83: LGTM!src/server/routes/system.ts (3)
202-261: LGTM!
303-434: LGTM!
625-645: LGTM!src/server/routes/system.probeDiagnostics.test.ts (1)
1-86: LGTM!src/components/features/IHVIntegrationPanel.tsx (1)
473-478: 📐 Maintainability & Code QualityComplete required validation before merge.
validate,python-tests,security,docker-build, andolive-pass-availabilityare still in progress. Confirm lint and typecheck completion. Smoke-test startup, recipe loading or building, and validation banners.As per coding guidelines, “Run linting and ensure typecheck-related CI checks pass before submitting changes.”
Sources: Coding guidelines, MCP tools
src/components/features/OwrExportOverlay.tsx (5)
1-38: LGTM!
40-75: LGTM!
86-197: LGTM!
200-267: LGTM!
269-307: LGTM!src/components/features/ExecutionWorkspace.tsx (2)
16-17: LGTM!Also applies to: 40-42, 67-67, 475-475
1054-1067: 🗄️ Data Integrity & IntegrationNo export-settings stale-bundle issue found.
src/components/features/BatchProcessingPanel.tsx (2)
426-463: LGTM!
1034-1039: LGTM!src/lib/oliveRecipeHub.ts (4)
63-70: LGTM!
517-523: LGTM!Also applies to: 525-530, 548-550
575-576: LGTM!
163-165: 🗄️ Data Integrity & IntegrationNo remaining runtime validation issue.
src/lib/__tests__/oliveRecipeHub.deriveUiState.test.ts (1)
1-3: LGTM!docs/Tech Debt & Issues.md (2)
3-28: LGTM!Also applies to: 42-44, 78-86, 96-98, 112-114, 124-124, 136-140
142-265: LGTM!
|
Note Docstrings generation - SUCCESS |
|
CodeFactor found multiple issues: Complex Method
Error: Calling setState synchronously within an effect can trigger cascading rendersEffects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /app/src/components/features/ExecutionWorkspace.tsx:563:7 |
Docstrings generation was requested by @tonythethompson. The following files were modified: * `olive-mcp-server/olive_mcp_server/tools/docs_search.py` * `olive-mcp-server/olive_mcp_server/tools/strategy_advisor.py` * `src/components/TitleBar.tsx` * `src/components/features/ArenaConvenience.tsx` * `src/components/features/BatchProcessingPanel.tsx` * `src/components/features/ExecutionWorkspace.tsx` * `src/components/features/HardwareCompatibilityMatrix.tsx` * `src/components/features/OwrExportOverlay.tsx` * `src/components/features/useOliveStream.ts` * `src/lib/oliveRecipeHub.ts` * `src/server/routes/system.ts` These files were kept as they were: * `olive-mcp-server/tests/test_docs_search_semantic.py` * `src/components/features/IHVIntegrationPanel.tsx` * `src/components/features/LocalModelManager.tsx` * `src/components/features/gemini/SettingsPanel.tsx` * `src/components/features/gemini/useLocalEngineSetup.ts` These files were ignored: * `src/lib/__tests__/oliveRecipeHub.deriveUiState.test.ts` * `src/lib/ndjsonInstall.test.ts` * `src/server/routes/arenaOliveOutputs.test.ts` * `src/server/routes/system.probeDiagnostics.test.ts` * `src/server/services/ai/registry.test.ts` These file types are not supported: * `docs/Tech Debt & Issues.md`
|
CodeFactor analysis of the latest commits — all 7 findings assessed: From this PR (addressable)
From older commits (pre-existing, not in this PR)
Bottom line
|
CodeFactor: exitCode is never reassigned in the terminal finalizer. Co-authored-by: Anthony Thompson <github@trackdub.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Summary
Full GitHub issue audit and code-quality sweep across the Olive Studio codebase. 22 issues closed, 33 remain open.
Code-quality fixes (10 issues)
anytypes withunknown/Record<string, unknown>inoliveRecipeHub.ts, removing alleslint-disablecommentsfamilyEnsure.ts,registry.test.ts, andcredentials.tsbeforeEachfromndjsonInstall.test.tsl→lat_lowerinstrategy_advisor.pyexcept Exception: continuewith typed error handling indocs_search.pyreject→_rejectinarenaOliveOutputs.test.tsLint cleanup (8 warnings)
All ESLint warnings in
src/andserver.tsresolved —eslint --max-warnings 20exits clean.Component extraction (4 issues)
buildProbeDiagnostics(~160 lines) fromprobeSystemHardwareinsystem.ts— reduced from 395 → ~250 lines (37%)OwrExportOverlay(301 lines, 14 typed props) into new file —ExecutionWorkspace.tsxreduced from 1753 → 1520 lines (13%)BatchJobList+BatchJobCardas module-level helpers inBatchProcessingPanel.tsxIHVIntegrationPanel.tsxVerified complete (7 issues)
#124, #134, #136, #138, #153, #154, #155 — all verified as complete against the codebase.
Validation
tsc --noEmit— cleaneslint --max-warnings 20 src/ server.ts— zero warnings