(via tonythethompson): Use submitted recipe from useOliveStream in completion effect instead of - #263
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider resetting
runRecipeJsonReftonullon new executions so the completion effect always captures the recipe from the most recent run and doesn’t accidentally reuse an older value. - If
setCapturedRunRecipeor downstream consumers expect a structured recipe object rather than a JSON string, it may be clearer foruseOliveStreamto exposerunRecipeJsonas a parsed object to avoid mixed types and repeated parsing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider resetting `runRecipeJsonRef` to `null` on new executions so the completion effect always captures the recipe from the most recent run and doesn’t accidentally reuse an older value.
- If `setCapturedRunRecipe` or downstream consumers expect a structured recipe object rather than a JSON string, it may be clearer for `useOliveStream` to expose `runRecipeJson` as a parsed object to avoid mixed types and repeated parsing.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Greptile SummaryThe PR now carries the exact submitted recipe through React state and uses it when a completed execution is captured for Playground.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the hook return contract is coherent and the conflict markers previously breaking compilation are absent from the current code.
|
| Filename | Overview |
|---|---|
| src/components/features/execute/ExecutionWorkspace.tsx | Uses the state-backed submitted recipe in the completion effect; the prior conflict markers and hook-property mismatch are resolved. |
| src/components/features/execute/useOliveStream.ts | Exposes and maintains submitted recipe state consistently with the hook interface and current consumer. |
| src/components/features/playground/PlaygroundPanel.tsx | Delegates JSON parsing and WebGPU provider detection to the shared recipe helper without changing invalid-input behavior. |
| src/lib/oliveRecipeHub.ts | Safely accepts JSON strings in provider detection while preserving existing structured-object handling. |
| src/lib/tests/oliveRecipeHub.deriveUiState.test.ts | Adds focused coverage for structured recipes, JSON strings, malformed JSON, and missing systems. |
Reviews (11): Last reviewed commit: "perf: memoize captured WebGPU recipe che..." | Re-trigger Greptile
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
| job.modelSource === "local" && state.localFiles.length === 0 | ||
| ? [{ name: job.modelIdentifier, size: 0 }] | ||
| : state.localFiles, | ||
| localFiles: state.localFiles, |
There was a problem hiding this comment.
🟠 High execute/BatchProcessingPanel.tsx:300
A queued local-source job with no uploaded workspace files is marked failed with “No model selected” and never runs. getPipelineValidation recognizes local models only when localFiles.length > 0, so removing the synthetic entry here makes job.modelIdentifier invisible to validation; restore that entry or validate the job identifier directly.
- localFiles: state.localFiles,
+ localFiles:
+ job.modelSource === "local" && state.localFiles.length === 0
+ ? [{ name: job.modelIdentifier, size: 0 }]
+ : state.localFiles,🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/components/features/execute/BatchProcessingPanel.tsx around line 300:
A queued local-source job with no uploaded workspace files is marked failed with “No model selected” and never runs. `getPipelineValidation` recognizes local models only when `localFiles.length > 0`, so removing the synthetic entry here makes `job.modelIdentifier` invisible to validation; restore that entry or validate the job identifier directly.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
8cd6091 to
3d9c571
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to make ExecutionWorkspace use the exact recipe JSON that was submitted for an Olive run (captured by useOliveStream) when handling run completion, instead of rebuilding the recipe from potentially-changed component state.
Changes:
- Extends
useOliveStream’s public return shape to expose the submitted run recipe JSON. - Updates
ExecutionWorkspace’s completionuseEffectto consume that submitted recipe. - Adjusts the completion effect dependencies to avoid ref-related hooks lint issues.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/features/execute/useOliveStream.ts | Adds runRecipeJson to the hook return contract, but currently returns a broken object shape (duplicate keys / missing field). |
| src/components/features/execute/ExecutionWorkspace.tsx | Switches completion handling toward using the submitted recipe, but currently contains unresolved merge conflict markers and inconsistent destructuring. |
Suppressed comments (1)
src/components/features/execute/ExecutionWorkspace.tsx:592
- Unresolved merge conflict markers in this
useEffectwill break JSX/TS parsing. Resolve the conflict and keep a single dependency array; usingrunRecipeJson(returned value) avoids any ref/deps confusion and matches the PR’s intent.
}, [executionStatus, setCapturedRunRecipe, runRecipeJsonRef]);
// Auto-save completed diagnoses to history
const prevDiagnosticRef = useRef(mcpDiagnostic);
useEffect(() => {
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
6f5cfaa to
06e276f
Compare
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
… of rebuilding from state
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…pExecutionProviderFromRecipe - Reset runRecipeJsonRef to null at the start of handleExecuteLive and inside beginNewRunEpoch so stale recipe JSON is never captured across runs. - Update mapExecutionProviderFromRecipe in oliveRecipeHub to handle stringified JSON input transparently. - Simplify isWebGpuRecipe in PlaygroundPanel to delegate string parsing directly to mapExecutionProviderFromRecipe. - Add unit tests for mapExecutionProviderFromRecipe with object, string, and invalid inputs. Co-authored-by: Cursor <cursoragent@cursor.com>
…interface contract - Add runRecipeJson: string | null to UseOliveStreamReturn interface and return object in useOliveStream. - Manage runRecipeJson state in useOliveStream alongside runRecipeJsonRef to avoid render-time ref access while satisfying the hook contract. - Update ExecutionWorkspace to consume runRecipeJson from useOliveStream in the completion effect. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
b272419 to
72597ad
Compare
Summary
This PR fixes the completion effect in
ExecutionWorkspace.tsxto use the actual submitted recipe fromuseOliveStreaminstead of rebuilding it from component state.Changes
useOliveStreamhook: AddedrunRecipeJsonto the return object, exposingrunRecipeJsonRef.currentwhich contains the recipe that was actually submitted for execution.ExecutionWorkspace.tsx: Updated the completion effect to consumerunRecipeJsonfrom the hook rather than reconstructing the recipe from state. Updated the dependency array to userunRecipeJsoninstead ofstate.Motivation
Using the submitted recipe directly ensures consistency between what was executed and what is used in the completion effect. Rebuilding from state could potentially introduce inconsistencies if the state changed between submission and completion.
Note
Macroscope: Fix It For Me
Activity
Currently: Merged by tonythethompson
Previously
Note
Use
runRecipeJsonstate fromuseOliveStreamto capture submitted recipe on completionuseOliveStreamnow exposesrunRecipeJsonas a state value (alongside the existing ref), resetting it on new run epochs and setting it when a run is initiated.ExecutionWorkspacereplaces its dependency onrunRecipeJsonRefwith the newrunRecipeJsonstate in the completion effect that writes to the Playground store viasetCapturedRunRecipe.mapExecutionProviderFromRecipein oliveRecipeHub.ts is extended to accept a JSON string directly, returningundefinedon parse failure;isWebGpuRecipeinPlaygroundPanelis simplified to use this updated signature.Macroscope summarized 72597ad.