perf: cut dist size 56MB->8MB, dedupe hardware-probe fetches via React Query - #161
Conversation
- Serve precompressed assets via express-static-gzip with immutable cache headers on hashed assets, no-cache on index.html - Strip onnxruntime-web's unused .wasm binaries from dist at build time (all call sites load wasm from the jsdelivr CDN, never the local copy) - Move @openai/codex-sdk to optionalDependencies (saves ~408MB install for consumers who skip optional deps) - Delete dead BatchComparisonChart.tsx and the recharts dependency (zero importers) - Resize logo.png from 966x966/688KB to 128x128/24KB, add explicit width/height to both <img> usages, delete unreferenced app.ico - Lazy-load jszip in ExecutionWorkspace's export handler instead of bundling it into the eager entry chunk
…React Query The dashboard mounts all pipeline panels simultaneously (single scroll page, not tab-switched), so 7 components each independently calling fetchHardwareProbe() on mount meant up to 7 duplicate probe requests per page load with no cache reuse across navigation. - Add useHardwareProbe()/useRefreshHardwareProbe() sharing one query key across BatchProcessingPanel, ExecutionWorkspace, IHVIntegrationPanel, InputEnvironmentPanel, ProviderInspector, StepInspector, and VramEstimateBanner. React Query collapses concurrent mounts into a single request instead of firing one per component. - IHVIntegrationPanel keeps its install-triggered "refresh" and first-probe auto-apply-recommended-provider behavior, now driven by the shared query instead of local-only state. - VramEstimateBanner's prop-driven fallback (force-refresh when the passed-in probe is missing RAM info) preserved as-is, just reading from the shared cache when no prop is given. - Migrate useKbSync's status polling to useQuery (interval + focus refetch replace the old setInterval + visibilitychange listener) and its sync action to useMutation. - Migrate InputEnvironmentPanel's HF token status/save/delete from hand-rolled fetch+state to useQuery + two useMutations. - Add renderWithProviders test helper (QueryClientProvider via RTL's `wrapper` option, so it survives rerender()) for the 4 component test files that now exercise React Query hooks.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR centralizes hardware probing and knowledge-base synchronization with React Query. It adds gzip-aware asset serving, removes unused build output, lazy-loads JSZip, improves optional SDK errors, and adds intrinsic logo dimensions. ChangesAsset delivery and bundle output
Shared hardware probe state
Knowledge-base synchronization
Runtime loading and UI cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✨ Finishing Touches 💡 1📝 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 QodoPerf: shrink dist/install and dedupe hardware probe fetches via React Query
AI Description
Diagram
High-Level Assessment
Files changed (20)
|
Greptile SummaryThe PR substantially reduces distribution and installation size, adds compressed static serving with differentiated cache policies, and consolidates repeated hardware and status requests through React Query.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| server.ts | Adds precompressed static serving and correctly distinguishes index, hashed build outputs, and stable public assets for caching. |
| src/components/features/VramEstimateBanner.tsx | The follow-up fix ensures a complete parent probe supersedes an older forced probe. |
| src/lib/hooks/useHardwareProbe.ts | Centralizes hardware probing under a shared React Query key with refresh support. |
| src/lib/hooks/useKbSync.ts | Migrates synchronization polling to React Query. |
| src/components/features/InputEnvironmentPanel.tsx | Migrates Hugging Face token status and update operations to queries and mutations. |
| vite.config.ts | Adds compressed output generation and strips unused ONNX Runtime WebAssembly assets. |
| package.json | Adds compressed-static serving, makes the Codex SDK optional, and removes the unused charting dependency. |
Reviews (6): Last reviewed commit: "Merge branch 'main' into perf/vite-bundl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo Fixer✅ Committed (1) · ☑ Fixed (1) Commits pushed directly to this PR — no separate fix PR opened. Process — 1 fixed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/features/ExecutionWorkspace.tsx (1)
386-433: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle JSZip chunk-load failures.
await import("jszip")runs before the existingtry, so a failed lazy chunk load escapesZIP Generation failed. Move the dynamic import into thattryblock before creating the Zip file.🤖 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 386 - 433, Move the dynamic import of JSZip inside the existing try block in the bundle-generation flow, before constructing the Zip instance. Ensure failures from await import("jszip") are caught by the existing “ZIP Generation failed” handler, while preserving the current archive creation and download behavior.
🤖 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 `@server.ts`:
- Around line 214-218: Update the SPA fallback flow around
res.sendFile(indexHtml) to set the response Cache-Control header to no-cache
before sending index.html. Keep the existing setHeaders callback for
expressStaticGzip responses unchanged.
- Around line 207-221: Update the production server listener configuration near
the Express startup call to bind to 127.0.0.1 instead of 0.0.0.0 by default.
Preserve the existing port and startup behavior, and do not add remote binding
unless an authenticated explicit mode already exists.
In `@src/components/features/InputEnvironmentPanel.tsx`:
- Around line 136-181: Update the hfTokenStatusQuery queryFn and
clearTokenMutation mutationFn to throw when their fetch responses are not ok,
checking r.ok before parsing status data or completing deletion. Ensure the
component renders a distinct query-error state rather than treating failed
status requests as “none,” while preserving cache updates only after successful
operations; add tests covering failed status and delete responses.
In `@src/components/features/VramEstimateBanner.tsx`:
- Around line 39-60: Update useHardwareProbe to accept an enabled option, and
call it from VramEstimateBanner with the query disabled whenever
hardwareProbeProp !== undefined. Preserve the existing shared-query behavior
when no prop is supplied, while retaining the explicit refreshHardwareProbe flow
for props with missing system RAM.
In `@src/lib/hooks/useHardwareProbe.ts`:
- Around line 14-19: Update useHardwareProbe’s useQuery configuration to disable
automatic retries by adding retry: false, preserving the existing query key,
fetcher, and stale-time behavior.
In `@src/lib/hooks/useKbSync.ts`:
- Around line 55-68: Update postKbSync to require data.ok === true before
returning, rejecting 2xx responses with ok false, a missing ok field, or
malformed JSON so React Query receives the mutation error. Preserve the existing
HTTP-status error and rate-limit messaging, and add hook tests covering failed
2xx bodies and malformed 2xx responses.
- Around line 125-129: Update the useQuery configuration in the statusQuery
definition to set retry: false, preserving single-attempt polling when
fetchKbStatus receives any non-2xx response. Add a useKbSync hook test covering
a 429 response and assert that the status endpoint is requested exactly once.
In `@vite.config.ts`:
- Around line 22-30: The closeBundle cleanup currently races with compression
and can delete WASM files before their .gz artifacts are written. Update the
stripUnusedOrtWasm cleanup flow to run as an explicit post-build step after
compression and final asset writes complete, then assert that assetsDir contains
no .wasm or .wasm.gz files.
---
Outside diff comments:
In `@src/components/features/ExecutionWorkspace.tsx`:
- Around line 386-433: Move the dynamic import of JSZip inside the existing try
block in the bundle-generation flow, before constructing the Zip instance.
Ensure failures from await import("jszip") are caught by the existing “ZIP
Generation failed” handler, while preserving the current archive creation and
download behavior.
🪄 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: b6acd2e1-7fba-4b7a-bee5-43f7c3ac65f3
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/assets/app.icois excluded by!**/*.icopublic/assets/logo.pngis excluded by!**/*.png
📒 Files selected for processing (20)
package.jsonserver.tssrc/App.tsxsrc/components/TitleBar.tsxsrc/components/features/BatchComparisonChart.tsxsrc/components/features/BatchProcessingPanel.test.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/components/features/ExecutionWorkspace.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/IHVIntegrationPanel.test.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/VramEstimateBanner.tsxsrc/components/features/__tests__/testUtils.tsxsrc/components/features/recipe-graph/StepInspector.tsxsrc/components/features/recipe-graph/inspectors/ProviderInspector.tsxsrc/lib/hooks/useHardwareProbe.tssrc/lib/hooks/useKbSync.tsvite.config.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)
💤 Files with no reviewable changes (1)
- src/components/features/BatchComparisonChart.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
- GitHub Check: validate
🧰 Additional context used
📓 Path-based instructions (4)
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/App.tsxsrc/components/TitleBar.tsxsrc/components/features/ExecutionWorkspace.test.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/VramEstimateBanner.tsxsrc/components/features/IHVIntegrationPanel.test.tsxsrc/components/features/recipe-graph/inspectors/ProviderInspector.tsxsrc/components/features/BatchProcessingPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/__tests__/testUtils.tsxsrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/components/features/recipe-graph/StepInspector.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/lib/hooks/useKbSync.tssrc/components/features/InputEnvironmentPanel.test.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/App.tsxsrc/components/TitleBar.tsxsrc/components/features/ExecutionWorkspace.test.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/VramEstimateBanner.tsxserver.tssrc/components/features/IHVIntegrationPanel.test.tsxsrc/components/features/recipe-graph/inspectors/ProviderInspector.tsxsrc/components/features/BatchProcessingPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/__tests__/testUtils.tsxsrc/components/features/InputEnvironmentPanel.tsxvite.config.tssrc/components/features/BatchProcessingPanel.tsxsrc/components/features/recipe-graph/StepInspector.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/lib/hooks/useKbSync.tssrc/components/features/InputEnvironmentPanel.test.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/App.tsxsrc/components/TitleBar.tsxsrc/components/features/ExecutionWorkspace.test.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/VramEstimateBanner.tsxserver.tssrc/components/features/IHVIntegrationPanel.test.tsxsrc/components/features/recipe-graph/inspectors/ProviderInspector.tsxsrc/components/features/BatchProcessingPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/__tests__/testUtils.tsxsrc/components/features/InputEnvironmentPanel.tsxvite.config.tssrc/components/features/BatchProcessingPanel.tsxsrc/components/features/recipe-graph/StepInspector.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/lib/hooks/useKbSync.tssrc/components/features/InputEnvironmentPanel.test.tsx
server.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep Olive spawning, dependency installation, and PATH handling in
server.ts.
server.ts: Default the Express API bind address to127.0.0.1; do not expose it to a LAN or public internet without authentication and binding fixes.
Add global Express error middleware so unhandled errors are consistently handled without leaking stack traces.
Files:
server.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/TitleBar.tsxsrc/components/features/ExecutionWorkspace.test.tsxsrc/components/features/VramEstimateBanner.tsxsrc/components/features/IHVIntegrationPanel.test.tsxsrc/components/features/recipe-graph/inspectors/ProviderInspector.tsxsrc/components/features/BatchProcessingPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/components/features/__tests__/testUtils.tsxsrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/BatchProcessingPanel.tsxsrc/components/features/recipe-graph/StepInspector.tsxsrc/components/features/IHVIntegrationPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsx
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Relevant review context
- The actual change is Olive-Studio PR
#161, notmta1124-1629472/Babel-Player; DeepWiki could not index the referenced repositories, so no architectural context was available there. Appcreates oneQueryClientper app instance and wraps the dashboard inQueryClientProvider, so the new hardware and KB hooks have a shared cache in production.useHardwareProbeuses one query key with a five-minutestaleTime; forced probes update that same cache withsetQueryData. TanStack Query documents these as the standard caching and cache-update mechanisms.- Both
useHardwareProbeand the new KB status query omitretry; TanStack Query’s documented client default is three retries, whereas the previous manual fetch paths attempted once. The shared test utility explicitly disables retries only in tests. VramEstimateBannercallsuseHardwareProbe()unconditionally even when a probe prop is supplied. When the prop lacks RAM, this can trigger both the shared normal query and the explicit forced refresh; previously only the forced refresh occurred.useKbSyncnow invalidates the KB-status query after successful sync, matching TanStack Query’s documented mutation/invalidation pattern. Existing tests cover freshness helpers but do not directly exercise the hook’s query/mutation behavior.- The Vite cleanup plugin uses
closeBundle; Vite documents that this runs after output writing, but itscloseBundlehooks are executed in parallel, so interaction with the compression plugin should be verified. - PR checks currently show CodeQL, CodeFactor, security, and Vercel passing; validation, Docker, Python tests, Olive availability, and Greptile review were still in progress when queried.
🔇 Additional comments (20)
src/App.tsx (1)
203-204: LGTM!src/components/TitleBar.tsx (1)
5-8: LGTM!Also applies to: 67-73
src/lib/hooks/useHardwareProbe.ts (1)
8-12: LGTM!Also applies to: 26-32
src/components/features/__tests__/testUtils.tsx (1)
8-30: LGTM!src/components/features/BatchProcessingPanel.tsx (1)
17-18: LGTM!Also applies to: 667-694
src/components/features/BatchProcessingPanel.test.tsx (1)
2-4: LGTM!src/components/features/IHVIntegrationPanel.tsx (1)
20-21: LGTM!Also applies to: 85-138
src/components/features/IHVIntegrationPanel.test.tsx (1)
2-3: LGTM!src/components/features/InputEnvironmentPanel.tsx (1)
2-2: LGTM!Also applies to: 44-44, 122-122, 218-218, 1608-1611
src/components/features/InputEnvironmentPanel.test.tsx (1)
1-132: LGTM!src/components/features/VramEstimateBanner.tsx (1)
4-5: LGTM!src/components/features/recipe-graph/StepInspector.tsx (1)
3-3: LGTM!Also applies to: 28-28
src/components/features/recipe-graph/inspectors/ProviderInspector.tsx (1)
1-4: LGTM!Also applies to: 24-24
src/components/features/ExecutionWorkspace.test.tsx (1)
2-4: LGTM!src/lib/hooks/useKbSync.ts (3)
1-2: LGTM!
26-37: LGTM!
139-169: LGTM!vite.config.ts (1)
108-132: LGTM!src/components/features/ExecutionWorkspace.tsx (1)
53-53: LGTM!Also applies to: 351-351
package.json (1)
75-77: 🩺 Stability & AvailabilityCodex SDK boundary is intact.
Runtime access to
@openai/codex-sdkis behind a lazy import, and the server startup path does not load it directly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@server.ts`:
- Around line 217-219: Update the cache-control selection around the filePath
check so the one-year immutable policy applies only to fingerprinted build
assets, not every non-index.html file. Keep index.html non-cached, and use a
revalidating cache policy for stable asset URLs such as /assets/logo.png.
🪄 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: 283a2b9c-5f9e-4d24-9a3e-c3521aef7c18
📒 Files selected for processing (1)
server.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
⚠️ CI failures not shown inline (1)
GitHub Check: Greptile Review: Confidence 3/5 — below your required 4/5
Conclusion: failure
Greptile reviewed this pull request successfully — this check reflects your team's confidence threshold, not a review failure. The review scored 3/5, below the 4/5 this repository requires for the check to pass.
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
server.ts
server.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep Olive spawning, dependency installation, and PATH handling in
server.ts.
server.ts: Default the Express API bind address to127.0.0.1; do not expose it to a LAN or public internet without authentication and binding fixes.
Add global Express error middleware so unhandled errors are consistently handled without leaking stack traces.
Files:
server.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:
server.ts
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Additional review context
- Potential stale VRAM data:
VramEstimateBanneralways prefersforcedProbeover laterhardwareProbePropupdates, so a forced result can remain authoritative after a parent rescan. - Cache policy scope:
server.tsapplies one-year immutable caching to every non-index.htmlfile, including stable URLs such as/assets/logo.png; only hashed build assets are generally safe for immutable caching. - Optional Codex dependency: The PR makes
@openai/codex-sdkoptional, while repository code still dynamically imports it without handling module absence. This may expose a raw module-resolution failure when the optional package is unavailable. - React Query behavior: TanStack Query v5 defaults client queries to three retries and five-minute garbage collection.
staleTime,setQueryData,invalidateQueries, andrefetchIntervalbehave as used by the new hooks; invalidation refetches active queries by default. - Build cleanup ordering: Vite documents that
closeBundleruns after output files are written, but itscloseBundlehooks run in parallel. The cleanup plugin therefore shares a concurrent lifecycle with compression-related plugins. - Validation status: CodeQL, validation, Docker, Python tests, security, CodeFactor, Olive availability, and Vercel checks passed. Greptile failed, and an automated review reported the stale-probe, cache-policy, and optional-dependency concerns above.
- Repository search:
useKbSync()has one production consumer,KbSyncIndicator.tsx. - DeepWiki was attempted for repository architecture but was rate-limited; no additional architectural facts were obtained.
🔇 Additional comments (2)
server.ts (2)
2-2: LGTM!
224-229: LGTM!
… noise, race, error UX)
- VramEstimateBanner: a forced RAM-refresh probe permanently shadowed
later hardwareProbeProp updates since it was never cleared. Now only
overrides the prop while the prop itself is still missing RAM info.
- server.ts: only mark Vite's Rollup-emitted, content-hashed JS/CSS as
immutable-cached. Public assets copied verbatim (logo.png, fonts,
favicon) keep a stable URL, so a content change without a filename
change could otherwise be pinned client-side for a year.
- useHardwareProbe/useKbSync status query: disable React Query's
default 3x retry. fetchHardwareProbe already retries once internally
(shells out to GPU/TensorRT detection — expensive to triple); the KB
status endpoint is rate-limited, so retrying failures adds noise for
no benefit.
- useKbSync's postKbSync now rejects on a 2xx response with
`{ ok: false }`, a missing `ok` field, or malformed JSON instead of
resolving as if the sync succeeded.
- InputEnvironmentPanel's hf-token-status query now checks res.ok
before parsing — fetch() doesn't reject on 4xx/5xx.
- ExecutionWorkspace: the jszip dynamic import ran before the
try/catch that was supposed to guard "ZIP Generation failed" —
a failed chunk load bypassed it as an unhandled rejection.
- codexAgent: a missing @openai/codex-sdk (now optionalDependencies)
surfaced as a raw module-resolution error, and the failed import
promise was cached forever, requiring a server restart even after
installing the package. Now throws a clear message and resets the
cached promise so a later install works without a restart.
- vite.config.ts: moved the ORT wasm strip from closeBundle (a
parallel hook — races with vite-plugin-compression's own closeBundle,
could delete files mid-write or miss them) to generateBundle, which
runs before anything is written to disk. Compression's hook now
never sees the wasm assets to begin with.
Not changed: server.ts's 0.0.0.0 bind. A bot flagged this as a
security issue, but binding to 127.0.0.1 by default would break
legitimate LAN/WSL2-forwarding access patterns for a tool that's
explicitly meant to run locally. No in-repo policy backs the claimed
"coding guideline" this cited. Left as-is pending an explicit call
from the repo owner.
…e, tests) Follow-up on the earlier hf-token fix — coderabbit correctly flagged it as partial: - clearTokenMutation now checks r.ok before treating a DELETE as success; it previously wrote "none" to the cache on any response, including a failed delete. - handleClearToken wraps the mutation call in try/catch, matching handleSubmitToken's existing pattern — clearTokenMutation.mutateAsync can now reject and an onClick handler can't let that escape as an unhandled rejection. - hfTokenStatus gets a distinct "error" state instead of collapsing a failed status query into "none" (which read as "no token configured" rather than "couldn't check"). - Added test coverage for both failure paths.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
… drives it - server.ts: the immutable-cache check matched any .js/.css by extension alone. A future stable-URL script copied verbatim from public/ would have been misclassified as content-hashed. Now requires the actual Vite hash suffix (name-<8-char-hash>.ext). - useHardwareProbe gains an `enabled` option. VramEstimateBanner now disables the shared query when a hardwareProbe prop is supplied — previously it fired an unconditional fetchHardwareProbe(false) query in parallel with the forced-refresh effect's fetchHardwareProbe(true) whenever the prop was missing RAM info, doubling probe invocations.
|
CodeFactor found an issue: Very Complex Method It's currently on: |
|
Pre-existing — VramEstimateBanner.tsx was already ~348 lines before this PR touched it; this PR's changes add ~10 net lines (the forced-probe staleness fix and the enabled-query fix). A full complexity refactor of the component is real work unrelated to this PR's scope (bundle size + query dedup), and would create merge friction with #162, which already relocated this file. Leaving it out of scope here — happy to take it on as its own PR if wanted. 🤖 Addressed by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@server.ts`:
- Around line 215-226: Update the cache-control check in the static asset header
logic to treat both index.html and its compressed index.html.gz path as
non-cacheable, covering express-static-gzip’s rewritten request path while
preserving the existing hashed JS/CSS caching behavior.
In `@src/components/features/ExecutionWorkspace.tsx`:
- Around line 386-393: Update the ZIP-loading catch in the export flow around
the jszip import and the OwrExportOverlay state to store a downloadable error
for OwrExportOverlayProps instead of returning silently. Render that error with
role="alert" in OwrExportOverlay, and clear the error state before each new
download attempt.
In `@src/components/features/InputEnvironmentPanel.tsx`:
- Around line 187-192: Update handleClearToken and the Clear control in
InputEnvironmentPanel so failed clearTokenMutation requests produce a
clear-specific user-visible error instead of being silently ignored, using
clearTokenMutation.error. Disable the Clear action while
clearTokenMutation.isPending, while preserving the existing successful-clear
behavior.
In `@src/lib/codex/codexAgent.ts`:
- Around line 60-62: Update the error message in the Codex provider loading path
to recommend `pnpm add --save-optional `@openai/codex-sdk`` instead of `pnpm add
`@openai/codex-sdk``, preserving the optional dependency declaration.
- Around line 56-63: Update the catch around the Codex module initialization and
codexModulePromise so only an explicit missing-@openai/codex-sdk resolution
failure produces the optional-dependency installation message. For module
evaluation, export, or other load failures, throw a generic Codex provider load
error while preserving the original error as its cause; retain resetting
codexModulePromise for both paths.
🪄 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: a5cd736b-c0f9-4b67-84f4-632da734fd4a
📒 Files selected for processing (9)
server.tssrc/components/features/ExecutionWorkspace.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/VramEstimateBanner.tsxsrc/lib/codex/codexAgent.tssrc/lib/hooks/useHardwareProbe.tssrc/lib/hooks/useKbSync.tsvite.config.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. (5)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
- GitHub Check: olive-pass-availability
- GitHub Check: validate
- GitHub Check: docker-build
🧰 Additional context used
📓 Path-based instructions (4)
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/codex/codexAgent.tssrc/components/features/VramEstimateBanner.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/lib/hooks/useKbSync.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/codex/codexAgent.tsvite.config.tssrc/components/features/VramEstimateBanner.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/lib/hooks/useKbSync.tsserver.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/codex/codexAgent.tsvite.config.tssrc/components/features/VramEstimateBanner.tsxsrc/lib/hooks/useHardwareProbe.tssrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsxsrc/lib/hooks/useKbSync.tsserver.ts
server.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep Olive spawning, dependency installation, and PATH handling in
server.ts.
server.ts: Default the Express API bind address to127.0.0.1; do not expose it to a LAN or public internet without authentication and binding fixes.
Add global Express error middleware so unhandled errors are consistently handled without leaking stack traces.
Files:
server.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/VramEstimateBanner.tsxsrc/components/features/InputEnvironmentPanel.tsxsrc/components/features/InputEnvironmentPanel.test.tsxsrc/components/features/ExecutionWorkspace.tsx
🪛 React Doctor (0.9.3)
src/components/features/InputEnvironmentPanel.tsx
[warning] 1601-1601: ' 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)
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Additional review context
- DeepWiki: Repository lookup failed (
tonythethompson/Olive-Studiowas not indexed), so no architectural context was obtained. - KB sync:
useKbSync()is consumed byKbSyncIndicator.tsx; the current implementation performs status fetching, a five-minute interval refresh, visibility refreshes, and one stale-status auto-sync. A React Query migration should preserve these behaviors and avoid duplicate syncs. - Codex callers: Codex functionality is reached through both
codexRoutes.tsandproviderRoutes.ts, while the SDK loader is implemented insrc/lib/codex/codexAgent.ts. Missing optional-dependency errors therefore affect multiple API paths. - Current static serving: The baseline server uses
express.static(distPath)followed by an uncached SPA fallback viares.sendFile(indexHtml). Cache-header changes should retain the distinction between actual assets and fallback HTML. - Context7: No dedicated
express-static-gzipdocumentation library was found; only Express documentation matches were returned, so no additional package-specific validation was available.
🔇 Additional comments (6)
src/lib/hooks/useHardwareProbe.ts (1)
14-23: LGTM!src/components/features/InputEnvironmentPanel.tsx (2)
122-122: LGTM!Also applies to: 136-176, 229-229, 1624-1627
1600-1604: 📐 Maintainability & Code QualityVerify the JSX entity lint result.
React Doctor reports
no-unescaped-entitiesforCouldn't. If the configured lint rule is enforced, replace it withCouldn'tand confirm the lint check passes.Sources: Coding guidelines, Linters/SAST tools
src/components/features/InputEnvironmentPanel.test.tsx (1)
1-1: LGTM!Also applies to: 133-192
src/components/features/VramEstimateBanner.tsx (1)
51-58: 🎯 Functional CorrectnessNo stale forced-probe response to address.
The component always resolves to the most recent incomplete
hardwareProbePropbefore applying the pending async result, so a newer prop cannot be shadowed by an older refresh response.> Likely an incorrect or invalid review comment.src/lib/hooks/useKbSync.ts (1)
68-70: LGTM!Also applies to: 132-132
| if (filePath.endsWith("index.html")) { | ||
| res.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); | ||
| return; | ||
| } | ||
| // Only Vite's Rollup-emitted JS/CSS carry a content hash in the | ||
| // filename (name-<8-char-hash>.js) — safe to cache forever. | ||
| // Matching on extension alone would also catch any stable-URL | ||
| // .js/.css copied verbatim from public/, so require the actual | ||
| // hash suffix. Everything else under dist/ (logo.png, fonts, | ||
| // favicon) has a stable URL and must be revalidated, not served | ||
| // from a 1-year cache untouched. | ||
| const isHashedBuildOutput = /-[\w-]{8}\.(js|css)(\.gz)?$/.test(filePath); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact declared middleware version for its compressed-request rewrite.
tarball="$(
curl -fsSL https://registry.npmjs.org/express-static-gzip/3.0.1 |
python3 -c 'import json, sys; print(json.load(sys.stdin)["dist"]["tarball"])'
)"
curl -fsSL "$tarball" |
tar -xzO package/index.js |
grep -nE 'convertToCompressedRequest|req\.url|serveStatic'
# Confirm that server.ts classifies both index.html and index.html.gz as non-cacheable.
rg -n -C 4 'endsWith\("index\.html"|Cache-Control' server.tsRepository: tonythethompson/Olive-Studio
Length of output: 2273
Keep compressed index.html non-cacheable.
express-static-gzip rewrites /index.html requests to /index.html.gz, but setHeaders only checks index.html. Compressed SPA entry documents can then get public, max-age=3600, so stale documents may still reference removed hashed assets.
Proposed fix
- if (filePath.endsWith("index.html")) {
+ if (filePath.endsWith("index.html") || filePath.endsWith("index.html.gz")) {
res.setHeader("Cache-Control", "no-cache, no-store, must-revalidate");
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (filePath.endsWith("index.html")) { | |
| res.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); | |
| return; | |
| } | |
| // Only Vite's Rollup-emitted JS/CSS carry a content hash in the | |
| // filename (name-<8-char-hash>.js) — safe to cache forever. | |
| // Matching on extension alone would also catch any stable-URL | |
| // .js/.css copied verbatim from public/, so require the actual | |
| // hash suffix. Everything else under dist/ (logo.png, fonts, | |
| // favicon) has a stable URL and must be revalidated, not served | |
| // from a 1-year cache untouched. | |
| const isHashedBuildOutput = /-[\w-]{8}\.(js|css)(\.gz)?$/.test(filePath); | |
| if (filePath.endsWith("index.html") || filePath.endsWith("index.html.gz")) { | |
| res.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); | |
| return; | |
| } | |
| // Only Vite's Rollup-emitted JS/CSS carry a content hash in the | |
| // filename (name-<8-char-hash>.js) — safe to cache forever. | |
| // Matching on extension alone would also catch any stable-URL | |
| // .js/.css copied verbatim from public/, so require the actual | |
| // hash suffix. Everything else under dist/ (logo.png, fonts, | |
| // favicon) has a stable URL and must be revalidated, not served | |
| // from a 1-year cache untouched. | |
| const isHashedBuildOutput = /-[\w-]{8}\.(js|css)(\.gz)?$/.test(filePath); |
🤖 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 `@server.ts` around lines 215 - 226, Update the cache-control check in the
static asset header logic to treat both index.html and its compressed
index.html.gz path as non-cacheable, covering express-static-gzip’s rewritten
request path while preserving the existing hashed JS/CSS caching behavior.
| let zip: InstanceType<typeof import("jszip")>; | ||
| try { | ||
| const { default: JSZip } = await import("jszip"); | ||
| zip = new JSZip(); | ||
| } catch (e) { | ||
| console.error("Failed to load ZIP module", e); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the existing OWR overlay error and alert conventions before adding the error prop.
rg -n -C 3 'OwrExportOverlay|role="alert"|set.*Error|Failed to load ZIP module' \
src/components/features/ExecutionWorkspace.tsx src/components/features
# Locate adjacent component tests for the export flow.
rg -n -C 3 'OWR|DownloadOwr|Export for OWR|jszip' \
src/components/features/ExecutionWorkspace.test.tsx src/components/featuresRepository: tonythethompson/Olive-Studio
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ExecutionWorkspace OWR-related state and overlay call =="
sed -n '200,260p' src/components/features/ExecutionWorkspace.tsx
sed -n '350,410p' src/components/features/ExecutionWorkspace.tsx
sed -n '700,730p' src/components/features/ExecutionWorkspace.tsx
echo "== OwrExportOverlayProps and render =="
sed -n '1,180p' src/components/features/OwrExportOverlay.tsx
echo "== package/jszip dependency =="
rg -n '"jszip"|exportOwrBundle|handleOwrDownload|ondownloadbundle|download' package.json public src/components/features/ExecutionWorkspace.tsx src/components/features/OwrExportOverlay.tsxRepository: tonythethompson/Olive-Studio
Length of output: 12183
Show the ZIP load failure in the OWR export UI.
The jszip import constructor catch returns silently, leaving OwrExportOverlay open with no download. Pass a downloadable error to OwrExportOverlayProps, render it with role="alert", and clear it before the next download attempt.
🤖 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 386 - 393,
Update the ZIP-loading catch in the export flow around the jszip import and the
OwrExportOverlay state to store a downloadable error for OwrExportOverlayProps
instead of returning silently. Render that error with role="alert" in
OwrExportOverlay, and clear the error state before each new download attempt.
| const handleClearToken = async () => { | ||
| await fetch("/api/env/hf-token", { method: "DELETE" }); | ||
| setHfTokenStatus("none"); | ||
| try { | ||
| await clearTokenMutation.mutateAsync(); | ||
| } catch { | ||
| /* ignore — clearTokenMutation.error surfaces via mutation state */ | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose failed token deletion.
Line 190 suppresses the rejected mutation. The component does not render clearTokenMutation.error, so a failed Clear action has no user-visible result. Render a clear-specific error and disable Clear while clearTokenMutation.isPending.
🤖 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/InputEnvironmentPanel.tsx` around lines 187 - 192,
Update handleClearToken and the Clear control in InputEnvironmentPanel so failed
clearTokenMutation requests produce a clear-specific user-visible error instead
of being silently ignored, using clearTokenMutation.error. Disable the Clear
action while clearTokenMutation.isPending, while preserving the existing
successful-clear behavior.
| ({ Codex } = await codexModulePromise); | ||
| } catch (err) { | ||
| // Reset so a later install doesn't require a server restart. | ||
| codexModulePromise = null; | ||
| throw new Error( | ||
| "Codex provider unavailable: @openai/codex-sdk (optionalDependencies) is not installed. Run `pnpm add @openai/codex-sdk` to enable it.", | ||
| { cause: err }, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)codexAgent\.ts$|(^|/)codexRoutes\.ts$|package\.json$' || true
echo "== codexAgent.ts outline =="
ast-grep outline src/lib/codex/codexAgent.ts | head -120 || true
echo "== codexAgent relevant content =="
cat -n src/lib/codex/codexAgent.ts | sed -n '1,130p'
echo "== codexRoutes relevant content =="
cat -n src/server/routes/ai/codexRoutes.ts | sed -n '1,220p'
echo "== package deps "
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
if p.exists():
data=json.loads(p.read_text())
for sec in ('dependencies','devDependencies','optionalDependencies','peerDependencies'):
deps=data.get(sec,{})
if '`@openai/codex-sdk`' in deps:
print(sec, '`@openai/codex-sdk`', deps['`@openai/codex-sdk`'])
PY
rg -n '`@openai/codex-sdk`|codexModulePromise|Codex provider unavailable|codexRoutes' src package.json pnpm-lock.yaml 2>/dev/null || trueRepository: tonythethompson/Olive-Studio
Length of output: 10944
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
echo "== Node error shape probe =="
node - <<'JS'
function makeFetchError() {
try {
fetch('doesnt_exist.mjs').catch(() => {});
} catch (err) {
return err;
}
}
const error = makeFetchError();
console.log(JSON.stringify({
constructor: error?.constructor && error.constructor.name,
message: error?.message,
code: error?.code,
cause: Boolean(error?.cause),
causeMessage: error?.cause?.message ?? null,
causeCode: error?.cause?.code ?? null,
causeConstructor: error?.cause?.constructor && error.cause.constructor.name ?? null,
}, null, 2));
JS
echo "== package availability probe =="
node - <<'JS'
(async () => {
const pkg = '`@openai/codex-sdk`';
try {
const module = await import(pkg);
console.log(JSON.stringify({
pkg,
available: true,
constructor: module?.Codex?.constructor && module.Codex.constructor.name,
hasImport: Boolean(module && module.Codex),
}, null, 2));
} catch (cause) {
const err = new Error(`${pkg} unavailable`);
err.cause = cause;
console.log(JSON.stringify({
pkg,
available: false,
message: err.message,
code: err.code ?? null,
cause: Boolean(err.cause),
causeMessage: err.cause?.message ?? null,
causeCode: err.cause?.code ?? null,
causeConstructor: err.cause?.constructor && err.cause.constructor.name ?? null,
}, null, 2));
}
})();
JS
Length of output: 942
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
echo "== Node version =="
node --version
echo "== Node error shape probe =="
node - <<'JS'
function makeFetchError() {
try {
fetch('file://' + process.cwd() + '/doesnt_exist_invalid_module.mjs').catch(() => {});
} catch (err) {
return err;
}
}
const error = makeFetchError();
console.log(JSON.stringify({
message: (error && error.message) || null,
code: (error && error.code) || null,
hasCause: Boolean(error && error.cause),
causeMessage: (error && error.cause && error.cause.message) || null,
causeCode: (error && error.cause && error.cause.code) || null,
}, null, 2));
JS
echo "== Package availability probe =="
node - <<'JS'
(async () => {
const pkg = '`@openai/codex-sdk`';
try {
const module = await import(pkg);
console.log(JSON.stringify({
pkg,
available: true,
constructorName: module && module.Codex && module.Codex.name ? module.Codex.name : null,
hasImport: Boolean(module && module.Codex),
}, null, 2));
} catch (cause) {
const err = new Error(pkg + ' unavailable');
err.cause = cause;
console.log(JSON.stringify({
pkg,
available: false,
errorHasCode: Boolean(err.code),
errorHasCause: Boolean(err.cause),
causeMessage: (cause && cause.message) || null,
causeCode: (cause && cause.code) || null,
}, null, 2));
}
})();
JS
Length of output: 710
🌐 Web query:
Node.js import ERR_MODULE_NOT_FOUND message package installed module evaluates syntax error error shape
💡 Result:
The ERR_MODULE_NOT_FOUND error is specific to Node.js ECMAScript Modules (ESM) [1]. It occurs when the ESM resolver fails to locate a requested module, package, or file [2][3]. Unlike the traditional CommonJS MODULE_NOT_FOUND error, ESM resolution follows strict rules, and the error shape typically includes the code ERR_MODULE_NOT_FOUND and an indication of the import context [4][1][3]. Common causes and solutions include: 1. Missing File Extensions: In ESM, you must explicitly include file extensions (e.g.,.js) in import specifiers [1][5]. Unlike CommonJS, Node.js will not automatically append.js or resolve directory indexes (like index.js) [1][5][6]. - Incorrect: import { func } from './utils'; - Correct: import { func } from './utils.js'; 2. Directory Import Restrictions: ESM does not support importing directories to resolve an index file by default [1][3]. You must specify the full path to the file [1][5]. 3. Misconfigured package.json Exports: If you are importing from a package, the package's exports field in its package.json may explicitly restrict access to the file you are trying to reach, leading to this error even if the file exists [1][2]. 4. Global vs. Local Installation: If you installed a package globally (npm install -g) but are trying to import it in a project, Node.js will not find it [7]. Node.js looks for packages in the node_modules folder relative to the current project [7][8]. Ensure the dependency is listed in your package.json and installed locally [8]. 5. TypeScript/Path Aliases: If using TypeScript, path aliases defined in tsconfig.json (e.g., @app/utils) are not natively resolved by Node.js at runtime [9]. Node.js only sees the compiled JavaScript and does not understand these custom mappings unless you use a loader or build step that resolves them [9]. 6. Module System Mismatch: If your project is configured with "type": "module" in package.json, Node.js expects ESM imports [6]. If you try to import a package that is CommonJS-only or incorrectly set up, it may fail to resolve properly [6][9]. To debug, verify that the path/specifier exists exactly as written, check that all local dependencies are installed in the project's node_modules, and ensure you are providing full file extensions for all relative imports [1][2][8].
Citations:
- 1: https://fixdevs.com/blog/node-err-module-not-found/
- 2: https://www.erroratlas.net/errors/nodejs-err-module-not-found
- 3: http://nodejs.org/docs/latest/api/esm.html
- 4: doc: fix documentation for
MODULE_NOT_FOUNDandERR_MODULE_NOT_FOUNDnodejs/node#41645 - 5: https://stackoverflow.com/questions/70938102/why-im-getting-error-err-module-not-found-cannot-find-module-when-running-th
- 6: https://stackoverflow.com/questions/78229576/node-js-import-results-in-err-module-not-found
- 7: https://nodejs-errors.dev/MODULE_NOT_FOUND/
- 8: https://sentry.io/answers/how-do-i-resolve-cannot-find-module-error-using-node-js/
- 9: https://nodewire.net/fix-cannot-find-module/
🌐 Web query:
Node v24 import error cause ERR_MODULE NOT FOUND syntax error caught ErrorModuleFail cause
💡 Result:
In Node.js v24, the ERR_MODULE_NOT_FOUND error is the standard error code produced by the ES Module (ESM) loader when it fails to resolve a module specifier [1][2][3]. Unlike the classic CommonJS MODULE_NOT_FOUND error, which is often more permissive, ERR_MODULE_NOT_FOUND indicates that the ESM resolver could not locate the exact file requested [1][2]. Common causes for this error include: 1. Missing File Extensions: Unlike CommonJS, ESM requires explicit file extensions in import specifiers (e.g., import './utils.js' instead of import './utils') [1][4][3]. 2. No Directory Index Lookup: ESM does not automatically resolve directory imports to an index file (e.g., importing a folder will not automatically find index.js) [1][3]. 3. TypeScript Integration Issues: When using tools like ts-node or tsx with TypeScript, the error often arises because the code is attempting to import a.ts file as if it were a.js file, or because path aliases defined in tsconfig.json are not being resolved at runtime [5][6][4]. 4. Case Sensitivity: Import paths are case-sensitive on Linux-based filesystems [4][3]. An import path that works in a local development environment (e.g., macOS or Windows) may fail in production (e.g., Linux) if the case does not exactly match the file on disk [4][3]. 5. Misconfigured Exports: If the module exists but is not properly exposed via the exports field in package.json, Node.js may report it as not found, though this is sometimes accompanied by specific ERR_PACKAGE_PATH_NOT_EXPORTED errors [1][3]. Regarding your specific mention of "syntax error caught ErrorModuleFail," this is likely a misinterpretation of stack trace output or an error handling pattern where a surrounding try/catch block intercepts the module resolution failure [7]. When you wrap an ESM import in a try/catch block, you can inspect the err.code property to confirm it is ERR_MODULE_NOT_FOUND [7]. If you are encountering this in a TypeScript context, verify that your loader (e.g., tsx or ts-node) is correctly configured to handle ESM and that you are using the correct module resolution settings in your tsconfig.json [5][6][7].
Citations:
- 1: https://fixdevs.com/blog/node-err-module-not-found/
- 2: https://nodejs-errors.dev/MODULE_NOT_FOUND/
- 3: https://esmodules.com/errors/
- 4: https://nodewire.net/fix-cannot-find-module/
- 5:
ERR_MODULE_NOT_FOUNDwhen usingnodeto run TypeScript that imports an ES Module with.jsor no extension nodejs/node#61229 - 6: tsImport() fails with ERR_MODULE_NOT_FOUND on Node 24 when a CommonJS-context .ts file imports an ESM dependency privatenumber/tsx#801
- 7: ERR_MODULE_NOT_FOUND error has undefined code TypeStrong/ts-node#1862
🌐 Web query:
GitHub Node.js lib internal errors ESM load ERR_MODULE_NOT_FOUND ModuleNotFoundError
💡 Result:
The ERR_MODULE_NOT_FOUND error in Node.js ESM (ECMAScript Modules) projects typically occurs because Node.js strictly adheres to the ESM specification, which differs from the legacy CommonJS resolution behavior [1][2][3]. Unlike CommonJS, Node.js ESM does not guess file extensions or automatically resolve directory index files [1][3][4]. Common causes and solutions include: 1. Missing File Extensions: ESM requires explicit file extensions in relative import specifiers [5][3]. For example, importing './utils' will fail; you must use './utils.js' [1][2][3]. This requirement applies even in TypeScript projects, where you must use the extension of the compiled JavaScript file (e.g.,.js) rather than the source file (e.g.,.ts) [2][3]. 2. Directory Import Limitations: ESM does not automatically resolve index files in directories [1][3]. If you have a file at './components/index.js', you must explicitly import it as './components/index.js' rather than just './components' [1][3]. 3. TypeScript Configuration: If you use TypeScript, ensure your tsconfig.json is configured to produce compliant ESM output [2][6]. Using "moduleResolution": "bundler" allows extensionless imports in TypeScript but will cause runtime ERR_MODULE_NOT_FOUND errors in Node.js [6]. Changing this to "node" or "nodenext" enforces stricter resolution that aligns with Node.js requirements [6][4]. 4. Missing Exports Map: If you are encountering this error when importing a dependency that is not properly configured, it may be because that package lacks an "exports" map in its package.json [7][8]. When a package does not define an "exports" map, Node.js may fail to resolve subpath imports (like 'pkg/subpath') if they lack the required file extension [7][8]. To troubleshoot, always verify the exact file path and ensure it exists on the filesystem with the correct case, as Node.js resolution is case-sensitive on most filesystems [1][2]. If you are using a monorepo or build tool, ensure that the build process has correctly outputted the files to the expected destination before execution [9][2].
Citations:
- 1: https://fixdevs.com/blog/node-err-module-not-found/
- 2: https://nodewire.net/fix-cannot-find-module/
- 3: https://errornotes.dev/en/errors/nodejs/fixing-nodejs-errmodulenotfound-the-missing-extension-problem
- 4: https://deadends.dev/node/err-module-not-found-esm/
- 5: ERR_MODULE_NOT_FOUND for project-level module nodejs/help#3163
- 6: https://stackoverflow.com/questions/79424592/error-err-module-not-found-cannot-find-module-when-using-bundler-resolution
- 7: ERR_MODULE_NOT_FOUND in 0.1.31: session.js missing .js extension on vscode-jsonrpc/node import github/copilot-sdk#707
- 8: @github/copilot-sdk fails on Node 24+ due to vscode-jsonrpc ESM resolution github/copilot-sdk#710
- 9: NodeJs app with ESM enabled cannot import from shared library nrwl/nx#22104
Preserve missing-SDK errors from generic module loads.
This catch treats failed resolution, module evaluation, and export failures the same as a missing optional dependency. src/server/routes/ai/codexRoutes.ts returns only err.message, so API callers can get the false “is not installed” diagnosis when @openai/codex-sdk is present but broken. Detect the missing package explicitly, and use a generic load error for other failures.
🤖 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/lib/codex/codexAgent.ts` around lines 56 - 63, Update the catch around
the Codex module initialization and codexModulePromise so only an explicit
missing-@openai/codex-sdk resolution failure produces the optional-dependency
installation message. For module evaluation, export, or other load failures,
throw a generic Codex provider load error while preserving the original error as
its cause; retain resetting codexModulePromise for both paths.
Source: MCP tools
| throw new Error( | ||
| "Codex provider unavailable: @openai/codex-sdk (optionalDependencies) is not installed. Run `pnpm add @openai/codex-sdk` to enable it.", | ||
| { cause: err }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Package manager files:\n'
git ls-files | grep -E '(^|/)(package.json|pnpm-lock.yaml|package-lock.json|yarn.lock)$' || true
printf '\npackage.json optionalDependencies and dependencies mentioning codex:\n'
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
if p.exists():
data=json.loads(p.read_text())
for key in ['dependencies','optionalDependencies','devDependencies','peerDependencies']:
deps=data.get(key,{})
if '`@openai/codex-sdk`' in deps:
print(key, deps['`@openai/codex-sdk`'])
PY
printf '\ncodexAgent relevant lines:\n'
fd -a 'codexAgent.ts' . | sed 's#^\./##' | while read -r f; do
echo "--- $f"
nl -ba "$f" | sed -n '1,120p'
done
printf '\nSearch for add --save-optional or optionalDependencies install guidance:\n'
rg -n "save-optional|--save-optional|optionalDependencies|`@openai/codex-sdk`" .Repository: tonythethompson/Olive-Studio
Length of output: 437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'codexAgent.ts relevant lines:\n'
if [ -f src/lib/codex/codexAgent.ts ]; then
awk 'NR<=120 {printf "%6d\t%s\n", NR, $0}' src/lib/codex/codexAgent.ts
fi
printf '\nSearch for pnpm add guidance:\n'
rg -n "pnpm add|--save-optional|save-optional|optionalDependencies|`@openai/codex-sdk`" .
printf '\nRepository pnpm script snippets:\n'
python3 - <<'PY'
import json
from pathlib import Path
data=json.loads(Path('package.json').read_text())
for name, value in (data.get('scripts') or {}).items():
if 'pnpm' in str(value):
print(f"{name}: {value}")
PY
printf '\nCheck local pnpm save-optional behavior from docs/config without installing:\n'
# pnpm 8+ already has --save-optional; output command is only valid if pnpm is installed and supports it.
if command -v pnpm >/dev/null 2>&1; then
pnpm add --help 2>/dev/null | rg -n "save-optional|--save-optional|optionalDependencies|option" | head -20 || true
else
echo "pnpm is not installed in the sandbox"
fiRepository: tonythethompson/Olive-Studio
Length of output: 10736
Keep pnpm optional dependency installs optional.
pnpm add @openai/codex-sdk installs a regular dependency, while `@openai/codex-sdk` is declared under `optionalDependencies`. Use `pnpm add --save-optional `@openai/codex-sdk in this message to preserve the optional-provider boundary.
🤖 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/lib/codex/codexAgent.ts` around lines 60 - 62, Update the error message
in the Codex provider loading path to recommend `pnpm add --save-optional
`@openai/codex-sdk`` instead of `pnpm add `@openai/codex-sdk``, preserving the
optional dependency declaration.
This branch was successfully deployed
1 participant
Summary
Test plan
tsc --noEmitcleaneslint0 errors (1 pre-existing warning, unrelated)pnpm buildsucceeds, dist verified wasm-free🤖 Generated with Claude Code