refactor: tech-debt passes 1-6 (validation perf, rule tables, log caps, ai.ts split) - #115
Merged
Merged
Conversation
Pass 1 (quick fixes): queryClient scoped inside App; OLIVE_SERVE_STATIC env switch for static serving; vite dynamically imported in dev branch and moved to devDependencies; types.ts OliveRecipe.systems/PassConfig.config to Record<string, unknown>. Pass 2 (validation perf): ref-equality memo on buildOliveRecipe; buildRecipeFromState reuses validation.recipe and derives advisories inline; StepInspector drops duplicate validation; ExecutionWorkspace defers the pipeline derivation (useDeferredValue) and rebuilds fresh at Execute/Queue time so submissions are never stale. Pass 3 (hygiene): removed dead @mendable/firecrawl-js dependency and its allowBuilds entry; confirmed onnxruntime-web/@huggingface/transformers already ship as lazy chunks. Pass 4 (rule tables): CROSS_PASS_RULES is the single source of truth for coercePassFields and getCrossPassIssues (autoCoerce flag per rule); inferHfTask/inferModelType rewritten as explicit ordered lookup tables. Pass 5 (job robustness): job logs capped at 1000 lines with batched trim (1250 watermark), OliveJob.logsTruncated flag, SSE reconnect replay emits a trim marker, /olive/status exposes logsTruncated. Adds gpu.test.ts and a truncated-replay stream test. Pass 6 (ai.ts split): routes/ai.ts (2,120 lines) split into routes/ai/ sub-modules (provider, chat, lmStudio, ollama, installEngine, codex, devin, cloudflare, localEngines, modelCatalog, streamHelpers, index); all local-engine mutable state moved to services/ai/localEngineState.ts with resetLocalEngineRuntime() for tests; dropped dead registerAiRoutes export. Also includes accumulated work-in-progress changes across the tree.
|
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}
Contributor
|
Important Review skippedToo many files! This PR contains 229 files, which is 129 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (229)
You can disable this status message by setting the 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 |
Owner
Author
CI verification — all green ✅GitHub Actions run #30966708317 on the PR head:
Lint reports the 12 pre-existing warnings (under the project's Local full-suite verification on identical tree (WSL, forks pool serial): lint clean, unit 54/54 files (794 tests), server 22/22 (232), component 16/16 (103), integration 41/41, build clean — all green. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fea0e20e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
PR Summary by QodoTech-debt passes 1–6: validation perf, rule tables, log caps, ai.ts split
AI Description
Diagram
High-Level Assessment
Files changed (29)
|
Contributor
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
Qodo Fixer✅ Committed (3) · ☑ Fixed (3) Commits pushed directly to this PR — no separate fix PR opened. Process — 3 fixed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The env-read refactor (1a6f82f) moved readEnvApiKey from lib/aiResponse.ts to services/ai/env.ts, but registry.test.ts still mocked the old path, so the mock never intercepted and provider-detection assertions saw undefined. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Owner
Author
|
Re-verified green on final head ( |
Addresses the Codex review comment on PR #115: the export handlers serialized the deferred display recipe, which could lag the latest keystroke. Export artifacts now rebuild fresh via buildRecipeJsonFromState(state), matching the Execute/Queue handlers. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Owner
Author
|
@greptile-apps review |
Contributor
Greptile SummaryThe PR completes six tech-debt passes spanning recipe-validation performance, declarative validation rules, bounded job logs, production static-serving selection, dependency cleanup, and decomposition of the AI route module.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code failure remaining after review. Live-state rebuilds protect execution boundaries, log retention and replay remain internally consistent, and no reachable contract regression was established in the AI route decomposition.
|
| Filename | Overview |
|---|---|
| src/components/features/ExecutionWorkspace.tsx | Defers expensive display derivation while correctly rebuilding recipes from live state at every execution and export boundary. |
| src/lib/oliveRecipeBuilder.ts | Adds reference-equality memoization consistent with the store's immutable top-level state updates. |
| src/lib/pipelineValidation.ts | Consolidates cross-pass coercion and issue generation into shared declarative rules. |
| src/server/services/olive/gpu.ts | Bounds retained per-job logs using a watermark and records whether older entries were discarded. |
| src/server/routes/olive.ts | Exposes log truncation in status responses and emits a clear marker before truncated SSE replay. |
| src/server/routes/ai/index.ts | Composes the decomposed AI route modules in explicit provider and engine order with no concrete regression identified. |
| src/server/services/ai/localEngineState.ts | Centralizes local-engine single-flight, cooldown, cache, busy-tag, and subscriber state with a reset hook. |
| server.ts | Uses the new AI route entry point and conditionally loads Vite only on the development-serving path. |
| package.json | Removes an unused dependency and keeps Vite solely as a development dependency. |
Reviews (1): Last reviewed commit: "Merge branch 'main' into tech-debt-passe..." | Re-trigger Greptile
|
CodeFactor found multiple issues: Complex Method
|
|
CodeFactor found an issue: Literal carriage return. Run script through tr -d '\r' . It's currently on: |
|
CodeFactor found an issue: Literal carriage return. Run script through tr -d '\r' . It's currently on: |
|
CodeFactor found an issue: Literal carriage return. Run script through tr -d '\r' . It's currently on: |
|
CodeFactor found an issue: Literal carriage return. Run script through tr -d '\r' . It's currently on: |
|
CodeFactor found an issue: Literal carriage return. Run script through tr -d '\r' . It's currently on: |
ShellCheck rejects CRLF line endings; pin *.sh to LF via .gitattributes. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Too many files changed for review (231 files, 100 file limit). Bypass the limit by tagging |
Contributor
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tech-debt remediation passes 1–6 from
docs/Tech Debt & Issues.md, plus the accumulated work-in-progress that was in the tree.Pass 1 — Quick fixes
QueryClientscoped insideApp(no module-level singleton)OLIVE_SERVE_STATICenv switch for static serving (argv heuristic reduced to back-compat fallback)OliveRecipe.systems/PassConfig.config→Record<string, unknown>Pass 2 — Validation pipeline performance
buildOliveRecipebuildRecipeFromStatereusesvalidation.recipe(was rebuilding 4–19×/render)ExecutionWorkspacedefers pipeline derivation viauseDeferredValue; Execute/Queue rebuild fresh from live state at click timePass 3 — Hygiene
@mendable/firecrawl-jsdep +allowBuildsentryonnxruntime-web/@huggingface/transformersalready ship as separate lazy chunksPass 4 — Declarative rule tables
CROSS_PASS_RULES: single source of truth forcoercePassFields+getCrossPassIssues(drift now structurally impossible)inferHfTask/inferModelType→ explicit ordered lookup tablesPass 5 — Job pipeline robustness
logsTruncatedflag + SSE reconnect replay marker;/olive/statusexposes the flaggpu.test.ts+ truncated-replay stream testPass 6 — ai.ts split
routes/ai.ts→ 12 modules underroutes/ai/(provider, chat, lmStudio, ollama, installEngine, codex, devin, cloudflare, localEngines, modelCatalog, streamHelpers, index)services/ai/localEngineState.tswithresetLocalEngineRuntime()registerAiRoutesexportVerification
Full re-verification of the final tree is running and will be reported on this PR.
Notes
--pool=forks --no-file-parallelism(environmental worker-spawn flake on this disk; default pool silently drops 2 test files here).pnpm-lock.yamlsynced.