Repository navigation
fix(wework): improve stream performance and file change stats - #1788
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (24)
📝 WalkthroughWalkthroughThis PR adds runtime-toggleable Codex stream debug logging across executor and frontend, throttled persistence for streaming runtime messages, context-compaction handling, unified diff detection, and frontend Streamdown rendering plus shared native listeners. ChangesExecutor runtime work
wework frontend
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
qdaxb
marked this pull request as ready for review
July 7, 2026 12:48
qdaxb
force-pushed
the
agent/wework-stream-performance-fixes
branch
from
July 7, 2026 12:53
8c5efa1 to
8694406
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
executor/src/runtime_work/transcript.rs (1)
1954-1982: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUnified-diff heuristic can misclassify created content that merely resembles a diff.
looks_like_unified_diffmatches if any line anywhere in the text starts with"@@ ","diff --git ","+++ ", or"--- ". If Codex sends full created-file content (not diff text) that happens to contain such lines — e.g., a markdown file documentinggit diffoutput or embedding an example patch —diff_statswill switch to prefix-based +/- counting on non-diff content instead of treating it as full added content, reintroducing a variant of the exact bug this change fixes for markdown list lines.Consider tightening the heuristic (e.g., requiring the
"@@ "hunk header specifically, or checking multiple diff markers together) rather than any single marker line.🤖 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 `@executor/src/runtime_work/transcript.rs` around lines 1954 - 1982, The unified-diff detection in looks_like_unified_diff is too loose because it treats any single marker line as a real diff, which can misclassify created file content that merely contains diff-like text. Tighten the heuristic in looks_like_unified_diff and its caller diff_stats so only genuine unified diffs use +/- counting, for example by requiring stronger evidence such as a hunk header plus other diff markers instead of any one line matching. Keep the fallback in diff_stats for created/deleted content so non-diff text is still counted as full added or removed content.wework/src/components/layout/useWorkbenchPaneSession.ts (1)
213-232: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
requestAnimationFramebatching stalls batchable updates while the window is hidden.
assistant_chunk/block_updatedactions are queued and only applied inside arequestAnimationFramecallback. When the Tauri window is minimized/backgrounded, rAF is paused, so these deltas accumulate inpendingMessageActionsRefand the visible transcript (andpaneStatusderived frommessages) won’t update until a non-batchable action flushes them or the window is foregrounded. This self-heals on restore, but if any always-batchable stream can complete without a trailing non-batchable dispatch, the final content stays deferred. Consider asetTimeoutfallback flush to bound the delay.🤖 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 `@wework/src/components/layout/useWorkbenchPaneSession.ts` around lines 213 - 232, The batching in dispatchMessages is deferred only through requestAnimationFrame, which can pause when the Tauri window is hidden and leave batchable RuntimePaneMessageAction updates stuck in pendingMessageActionsRef. Update the dispatchMessages logic in useWorkbenchPaneSession so assistant_chunk/block_updated actions are eventually flushed even without a frame callback, such as by adding a timeout-based fallback or equivalent non-rAF flush path, while preserving the existing applyMessageActions and flushPendingMessageActions behavior.executor/src/runtime_work/store.rs (1)
151-168: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
write_indexblocks all index reads during disk I/O and deep-clones every task.Reads (
get_task,list_tasks,find_by_thread_id) now acquire the sameself.indexmutex, andwrite_indexis invoked while that mutex is held — including the blockingfs::write+fs::rename. Sinceget_taskis on the per-delta hot path, a persist can stall the entire runtime pipeline for the duration of the file write. Additionally, the payload is rebuilt by fully cloningtasks/workspaces(eachRuntimeTaskLinkcarries full message JSON) just to stamp the version.Consider serializing to bytes while locked (or keeping
versionin the in-memory index) and performing thefswrite after releasing the lock, so I/O does not serialize concurrent reads.🤖 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 `@executor/src/runtime_work/store.rs` around lines 151 - 168, The current `write_index` flow in `RuntimeWorkStore` holds the `self.index` mutex while performing blocking disk I/O, which can stall hot-path reads like `get_task`, `list_tasks`, and `find_by_thread_id`. Refactor so the in-memory `RuntimeWorkIndex` is only used to build the serialized bytes while locked, then release the lock before calling `fs::write` and `fs::rename` on the temporary path. Also avoid deep-cloning `tasks` and `workspaces` just to stamp `INDEX_VERSION` by reusing the existing version field or serializing from a lightweight snapshot in `write_index`.wework/src/components/chat/blocks/ToolBlockItem.tsx (2)
774-781: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
ProcessMarkdownalways renders inmode="streaming", even for completed blocks.Unlike
AssistantMarkdown.tsx, which switchesmode/isAnimatingbased on anisStreamingprop, this component hardcodesmode="streaming". Per Streamdown's docs,"static"mode is intended for already-complete content and skips incomplete-markdown handling; since tool-block detail can render afterblock.status === 'done'(seeisRunninginToolBlockItem), threading that state through would keep behavior consistent withAssistantMarkdownand avoid unnecessary streaming-mode parsing on finished content.🤖 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 `@wework/src/components/chat/blocks/ToolBlockItem.tsx` around lines 774 - 781, ProcessMarkdown currently hardcodes Streamdown to streaming mode, so completed tool-block content is still parsed as live text. Update ProcessMarkdown in ToolBlockItem to accept the block’s running/streaming state (the same state used by ToolBlockItem’s isRunning logic) and switch Streamdown between mode="streaming" and mode="static" accordingly, following the pattern already used by AssistantMarkdown. Keep the behavior consistent for finished blocks by disabling streaming-specific parsing when the content is complete.
790-845: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
code/inlineCoderenderer logic andreactNodeToTexthelper vs.AssistantMarkdown.tsx.This block-detection heuristic, the
MarkdownCodeBlockwiring, andreactNodeToTextare nearly identical towework/src/components/chat/AssistantMarkdown.tsx(lines 88-110, 180-184). Consider extracting a shared helper module used by both Streamdown component-map definitions to avoid the two implementations drifting apart over time.🤖 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 `@wework/src/components/chat/blocks/ToolBlockItem.tsx` around lines 790 - 845, The `code`, `inlineCode`, and `reactNodeToText` logic in `ToolBlockItem` duplicates the same markdown rendering behavior used in `AssistantMarkdown`, so it should be factored into a shared helper to prevent drift. Extract the block-detection and `MarkdownCodeBlock` mapping into a reusable module or shared renderer function, then have both `ToolBlockItem` and `AssistantMarkdown` consume that shared implementation while keeping the existing `MarkdownCodeBlock`, `reactNodeToText`, and `Streamdown` component-map behavior intact.wework/src/api/local/localChatStream.test.ts (1)
46-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThese tests don't actually exercise the event-filtering logic.
Neither test enables stream debug (
setLocalChatStreamDebugEnabled(true)), sologLocalChatStreamEventshort-circuits on theisLocalChatStreamDebugEnabled()check beforeshouldLogLocalChatStreamEventis ever reached. As written, these tests only prove "no logging when debug is off" — not the actual intent ("delta/block-update events aren't logged even when debug is on").🧪 Proposed fix: enable debug before triggering the event
test('does not log every text delta event', async () => { let listener!: (event: LocalExecutorEvent) => void subscribe.mockImplementation(async handler => { listener = handler return vi.fn() }) const consoleDebug = vi.spyOn(console, 'debug').mockImplementation(() => undefined) + setLocalChatStreamDebugEnabled(true) const stream = createLocalChatStream({ subscribe, request })(apply the same change to the block-update test)
🤖 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 `@wework/src/api/local/localChatStream.test.ts` around lines 46 - 100, The two local chat stream tests are bypassing the filtering path because debug logging is never enabled, so they only verify the early return in logLocalChatStreamEvent. Update both tests in createLocalChatStream to enable stream debug with setLocalChatStreamDebugEnabled(true) before calling listener, then assert that response.output_text.delta and response.block.updated still do not emit the “[Wework] Local chat stream event” debug log. Keep the existing console.debug spy and restore it afterward.
🤖 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 `@executor/src/runtime_work/handler.rs`:
- Around line 4081-4107: The tests that toggle the shared process-wide debug
flag can race with other parallel tests because `codex_stream_debug_enabled` and
`set_codex_stream_debug_enabled` use a single global `AtomicBool`. Add a shared
serialization guard (for example on
`codex_stream_debug_rpc_toggles_runtime_flag` and the corresponding
`stream_debug_can_be_toggled_at_runtime` test) so these tests do not run
concurrently, and ensure the flag is reset within that isolated scope; if
needed, add the `serial_test` dev-dependency to support the guard.
In `@wework/src/api/local/localChatStream.ts`:
- Around line 217-228: The scope check in isLocalExecutorEventInScope currently
short-circuits when scope.taskId is missing, which prevents device-only
ChatStreamScope subscribers from being filtered correctly. Update the function
to evaluate scope.deviceId independently of taskId, and make sure events without
a matching payload deviceId are not treated as matches when a device scope is
set. Keep the existing taskId filtering behavior, but apply deviceId matching in
all cases where ChatStreamHandlers['scope'] includes a deviceId.
In `@wework/src/components/chat/AssistantMarkdown.tsx`:
- Around line 23-58: The `AssistantMarkdown` memoization is allowing a stale
`onOpenFile` handler to persist because `areAssistantMarkdownPropsEqual` does
not compare it, so `openFileRef.current` may never be refreshed after parent
rerenders. Update the memo comparator for `AssistantMarkdown` to include
`onOpenFile`, or refactor the ref synchronization so it is not skipped by
memoization, ensuring `openFile` always calls the latest handler from
`MessageList.tsx` and related file-link clicks use current closure state.
In `@wework/src/components/chat/composer/ProjectChatComposer.tsx`:
- Around line 75-132: The native drop listener management in
subscribeNativeDropHandler is racing between the timeout cleanup and the pending
onDragDropEvent promise. Update the ownership logic so only the active
subscription can clear nativeDropUnlisten/nativeDropUnlistenPromise or call
unlisten, and ignore stale promise resolutions after a resubscribe. Capture the
current promise/unlisten state locally inside subscribeNativeDropHandler and
compare before mutating shared globals. Ensure the cleanup path and the
.then/.catch handlers cannot both release the same listener or register a second
native listener while the first is still settling.
In `@wework/src/lib/developerCommandMenu.ts`:
- Around line 88-91: The async refresh callback is re-rendering the open
developer command menu and `renderDeveloperCommandList` is always auto-focusing
the first button, which steals keyboard focus from the current selection. Update
`renderDeveloperCommandList` so it only autofocuses on the initial menu render,
and make the `refreshCodexStreamDebugStatus` callback preserve the existing
focused item when it re-invokes the renderer. Apply the same change in both
places where this callback pattern is used, including the `MENU_ID` checks and
the `renderDeveloperCommandList(list)` calls.
In `@wework/src/lib/embedded-browser.ts`:
- Around line 171-217: The embedded browser listener management in
embedded-browser.ts can leak a stale listener because a late-resolving listen()
overwrites the currently tracked unlisten after the release timer has already
cleared state. Update the subscription flow around
embeddedBrowserOpenRequestUnlistenPromise, embeddedBrowserOpenRequestUnlisten,
and the release timer so only the most recent listen() resolution can claim
ownership, and ensure any superseded or stale promise result is ignored or
immediately unlistened. Keep the teardown logic in the returned unsubscribe path
and the setTimeout cleanup consistent so a second subscribe cannot leave the
first listener active.
---
Nitpick comments:
In `@executor/src/runtime_work/store.rs`:
- Around line 151-168: The current `write_index` flow in `RuntimeWorkStore`
holds the `self.index` mutex while performing blocking disk I/O, which can stall
hot-path reads like `get_task`, `list_tasks`, and `find_by_thread_id`. Refactor
so the in-memory `RuntimeWorkIndex` is only used to build the serialized bytes
while locked, then release the lock before calling `fs::write` and `fs::rename`
on the temporary path. Also avoid deep-cloning `tasks` and `workspaces` just to
stamp `INDEX_VERSION` by reusing the existing version field or serializing from
a lightweight snapshot in `write_index`.
In `@executor/src/runtime_work/transcript.rs`:
- Around line 1954-1982: The unified-diff detection in looks_like_unified_diff
is too loose because it treats any single marker line as a real diff, which can
misclassify created file content that merely contains diff-like text. Tighten
the heuristic in looks_like_unified_diff and its caller diff_stats so only
genuine unified diffs use +/- counting, for example by requiring stronger
evidence such as a hunk header plus other diff markers instead of any one line
matching. Keep the fallback in diff_stats for created/deleted content so
non-diff text is still counted as full added or removed content.
In `@wework/src/api/local/localChatStream.test.ts`:
- Around line 46-100: The two local chat stream tests are bypassing the
filtering path because debug logging is never enabled, so they only verify the
early return in logLocalChatStreamEvent. Update both tests in
createLocalChatStream to enable stream debug with
setLocalChatStreamDebugEnabled(true) before calling listener, then assert that
response.output_text.delta and response.block.updated still do not emit the
“[Wework] Local chat stream event” debug log. Keep the existing console.debug
spy and restore it afterward.
In `@wework/src/components/chat/blocks/ToolBlockItem.tsx`:
- Around line 774-781: ProcessMarkdown currently hardcodes Streamdown to
streaming mode, so completed tool-block content is still parsed as live text.
Update ProcessMarkdown in ToolBlockItem to accept the block’s running/streaming
state (the same state used by ToolBlockItem’s isRunning logic) and switch
Streamdown between mode="streaming" and mode="static" accordingly, following the
pattern already used by AssistantMarkdown. Keep the behavior consistent for
finished blocks by disabling streaming-specific parsing when the content is
complete.
- Around line 790-845: The `code`, `inlineCode`, and `reactNodeToText` logic in
`ToolBlockItem` duplicates the same markdown rendering behavior used in
`AssistantMarkdown`, so it should be factored into a shared helper to prevent
drift. Extract the block-detection and `MarkdownCodeBlock` mapping into a
reusable module or shared renderer function, then have both `ToolBlockItem` and
`AssistantMarkdown` consume that shared implementation while keeping the
existing `MarkdownCodeBlock`, `reactNodeToText`, and `Streamdown` component-map
behavior intact.
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 213-232: The batching in dispatchMessages is deferred only through
requestAnimationFrame, which can pause when the Tauri window is hidden and leave
batchable RuntimePaneMessageAction updates stuck in pendingMessageActionsRef.
Update the dispatchMessages logic in useWorkbenchPaneSession so
assistant_chunk/block_updated actions are eventually flushed even without a
frame callback, such as by adding a timeout-based fallback or equivalent non-rAF
flush path, while preserving the existing applyMessageActions and
flushPendingMessageActions behavior.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4532ab86-452a-436b-9236-7ac27c1c377f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
docs/en/developer-guide/wework-performance-diagnostics.mddocs/zh/developer-guide/wework-performance-diagnostics.mdexecutor/src/runtime_work/events.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/notification_mapping.rsexecutor/src/runtime_work/runtime_handle_messages.rsexecutor/src/runtime_work/store.rsexecutor/src/runtime_work/transcript.rswework/package.jsonwework/src/api/local/localChatStream.test.tswework/src/api/local/localChatStream.tswework/src/components/chat/AssistantMarkdown.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/chat/blocks/ToolBlockItem.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/layout/useWorkbenchPaneSession.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/runtimePaneMessages.tswework/src/lib/developerCommandMenu.test.tswework/src/lib/developerCommandMenu.tswework/src/lib/embedded-browser.tswework/src/stream/chatStream.tswework/tailwind.config.js
Comment on lines
+4081
to
+4107
| #[tokio::test] | ||
| async fn codex_stream_debug_rpc_toggles_runtime_flag() { | ||
| set_codex_stream_debug_enabled(false); | ||
| let handler = RuntimeWorkRpcHandler::new("device-1", "/bin/false"); | ||
|
|
||
| let initial = handler | ||
| .handle_runtime_rpc(json!({ | ||
| "method": "runtime.codex.stream_debug.get", | ||
| "payload": {} | ||
| })) | ||
| .await | ||
| .expect("debug state should return"); | ||
| assert_eq!(initial["enabled"], false); | ||
|
|
||
| let updated = handler | ||
| .handle_runtime_rpc(json!({ | ||
| "method": "runtime.codex.stream_debug.set", | ||
| "payload": {"enabled": true} | ||
| })) | ||
| .await | ||
| .expect("debug state should update"); | ||
| assert_eq!(updated["enabled"], true); | ||
| assert!(codex_stream_debug_enabled()); | ||
|
|
||
| set_codex_stream_debug_enabled(false); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== relevant files ==\n'
git ls-files | rg '(^|/)(handler\.rs|notification_mapping\.rs|Cargo\.toml)$'
printf '\n== search for shared flag and tests ==\n'
rg -n "codex_stream_debug_enabled|set_codex_stream_debug_enabled|stream_debug_can_be_toggled_at_runtime|codex_stream_debug_rpc_toggles_runtime_flag|serial_test|#\\[serial\\]" executor/src/runtime_work/handler.rs executor/src/notification_mapping.rs Cargo.toml
printf '\n== inspect surrounding code in handler.rs ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('executor/src/runtime_work/handler.rs')
lines = p.read_text().splitlines()
for start in [4045, 4070, 4080, 4108]:
end = min(len(lines), start + 40)
print(f'\n--- handler.rs {start}-{end} ---')
for i in range(start-1, end):
print(f'{i+1:5d}: {lines[i]}')
PY
printf '\n== inspect surrounding code in notification_mapping.rs ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('executor/src/notification_mapping.rs')
lines = p.read_text().splitlines()
for start in [1, 1]:
end = min(len(lines), 220)
print(f'\n--- notification_mapping.rs 1-{end} ---')
for i in range(0, end):
print(f'{i+1:5d}: {lines[i]}')
if i >= 220:
break
PY
printf '\n== Cargo.toml snippets ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('Cargo.toml')
if p.exists():
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 'serial_test' in line or 'tokio' in line or '[dev-dependencies]' in line:
s = max(1, i-4)
e = min(len(lines), i+6)
print(f'\n--- Cargo.toml around line {i} ---')
for j in range(s-1, e):
print(f'{j+1:5d}: {lines[j]}')
PY
printf '\n== test concurrency related markers in repo ==\n'
rg -n "tokio::test|serial_test|#[[:space:]]*serial" executor/src -g '!**/target/**'Repository: wecode-ai/Wegent
Length of output: 1310
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== inspect handler.rs test block ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('executor/src/runtime_work/handler.rs')
lines = p.read_text().splitlines()
for start, end in [(4070, 4110)]:
print(f'\n--- handler.rs {start}-{end} ---')
for i in range(start-1, min(end, len(lines))):
print(f'{i+1:5d}: {lines[i]}')
PY
printf '\n== inspect notification_mapping.rs ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('executor/src/runtime_work/notification_mapping.rs')
lines = p.read_text().splitlines()
for start in [1, 1]:
end = min(len(lines), 260)
print(f'\n--- notification_mapping.rs 1-{end} ---')
for i in range(0, end):
print(f'{i+1:5d}: {lines[i]}')
if i+1 >= end:
break
PY
printf '\n== search for the shared flag/tests in runtime_work ==\n'
rg -n "codex_stream_debug_enabled|set_codex_stream_debug_enabled|stream_debug_can_be_toggled_at_runtime|#\\[serial\\]|serial_test" executor/src/runtime_work
printf '\n== inspect executor/Cargo.toml relevant sections ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('executor/Cargo.toml')
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if '[dev-dependencies]' in line or 'serial_test' in line or 'tokio' in line:
s = max(1, i-4)
e = min(len(lines), i+8)
print(f'\n--- executor/Cargo.toml around line {i} ---')
for j in range(s-1, e):
print(f'{j+1:5d}: {lines[j]}')
PYRepository: wecode-ai/Wegent
Length of output: 25252
Serialize the tests that mutate the shared debug flag
codex_stream_debug_enabled/set_codex_stream_debug_enabled back a single process-wide AtomicBool, so this test can race with notification_mapping.rs::stream_debug_can_be_toggled_at_runtime under parallel cargo test. Add a shared #[serial] guard (and the serial_test dev-dependency) or otherwise isolate the flag reset.
🤖 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 `@executor/src/runtime_work/handler.rs` around lines 4081 - 4107, The tests
that toggle the shared process-wide debug flag can race with other parallel
tests because `codex_stream_debug_enabled` and `set_codex_stream_debug_enabled`
use a single global `AtomicBool`. Add a shared serialization guard (for example
on `codex_stream_debug_rpc_toggles_runtime_flag` and the corresponding
`stream_debug_can_be_toggled_at_runtime` test) so these tests do not run
concurrently, and ensure the flag is reset within that isolated scope; if
needed, add the `serial_test` dev-dependency to support the guard.
Comment on lines
+217
to
+228
| function isLocalExecutorEventInScope( | ||
| event: LocalExecutorEvent, | ||
| scope: ChatStreamHandlers['scope'] | ||
| ): boolean { | ||
| if (!scope?.taskId) return true | ||
| const payload = asRecord(event.payload) | ||
| const taskId = stringField(payload, 'taskId') | ||
| if (taskId && taskId !== scope.taskId) return false | ||
| const deviceId = stringField(payload, 'deviceId') | ||
| if (scope.deviceId && deviceId && deviceId !== scope.deviceId) return false | ||
| return true | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'interface RuntimeTaskAddress' wework/srcRepository: wecode-ai/Wegent
Length of output: 528
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== localChatStream scope handling ==\n'
sed -n '180,260p' wework/src/api/local/localChatStream.ts
printf '\n== ChatStreamHandlers scope type ==\n'
rg -n -C3 'scope\s*[:?]\s*ChatStreamHandlers|type ChatStreamHandlers|interface ChatStreamHandlers' wework/src/api wework/src -g '!**/dist/**'
printf '\n== runtimePaneMessages caller(s) ==\n'
rg -n -C4 'deviceId|taskId|scope:' wework/src -g '!*node_modules*' | sed -n '1,220p'
printf '\n== RuntimeTaskAddress definition ==\n'
sed -n '228,250p' wework/src/types/api.tsRepository: wecode-ai/Wegent
Length of output: 24196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ChatStream scope type ==\n'
sed -n '1,80p' wework/src/stream/chatStream.ts
printf '\n== localChatStream subscription setup ==\n'
sed -n '1,220p' wework/src/api/local/localChatStream.ts
printf '\n== isLocalExecutorEventInScope references ==\n'
rg -n 'isLocalExecutorEventInScope|scopeDeviceId|scopeTaskId|onLocalExecutorEvent' wework/src/api/local wework/src/components wework/src/streamRepository: wecode-ai/Wegent
Length of output: 11051
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== createLocalChatStream callers ==\n'
rg -n -C4 'createLocalChatStream\(|\.subscribe\(\{[^}]*scope' wework/src
printf '\n== scope object construction ==\n'
rg -n -C4 'scope:\s*\{[^}]*deviceId|scope:\s*\{[^}]*taskId|deviceId:\s*.*taskId:' wework/src
printf '\n== runtimePaneMessages / pane session scope wiring ==\n'
rg -n -C4 'scope\.deviceId|scope\.taskId|deviceId: .*taskId|taskId: .*deviceId' wework/src/components wework/src/api wework/src/streamRepository: wecode-ai/Wegent
Length of output: 23641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== LocalExecutorEvent definition ==\n'
rg -n -C4 'type LocalExecutorEvent|interface LocalExecutorEvent|export type LocalExecutorEvent' wework/src
printf '\n== event payload shapes mentioning deviceId ==\n'
rg -n -C3 'deviceId' wework/src/tauri wework/src/api wework/src/types | sed -n '1,220p'
printf '\n== localChatStream tests around scope ==\n'
sed -n '180,240p' wework/src/api/local/localChatStream.test.tsRepository: wecode-ai/Wegent
Length of output: 14775
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== localExecutor event emission and payload shape ==\n'
sed -n '1,240p' wework/src/tauri/localExecutor.ts
printf '\n== deviceId/taskId references in local executor source ==\n'
rg -n -C3 'deviceId|taskId|payload' wework/src/tauri/localExecutor.ts wework/src/tauri -g '!**/*.test.ts'Repository: wecode-ai/Wegent
Length of output: 8727
Filter device scope independently of taskId
ChatStreamScope already allows deviceId-only subscribers, but isLocalExecutorEventInScope() returns early whenever taskId is unset. That makes every event pass through for device-only scopes, and it also treats a missing deviceId in the payload as a match. Consider checking deviceId whenever scope.deviceId is set, regardless of taskId.
🤖 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 `@wework/src/api/local/localChatStream.ts` around lines 217 - 228, The scope
check in isLocalExecutorEventInScope currently short-circuits when scope.taskId
is missing, which prevents device-only ChatStreamScope subscribers from being
filtered correctly. Update the function to evaluate scope.deviceId independently
of taskId, and make sure events without a matching payload deviceId are not
treated as matches when a device scope is set. Keep the existing taskId
filtering behavior, but apply deviceId matching in all cases where
ChatStreamHandlers['scope'] includes a deviceId.
Comment on lines
+23
to
+58
| const WEWORK_MARKDOWN_FILE_LINK_HOST = 'wework.local' | ||
| const WEWORK_MARKDOWN_FILE_LINK_PATH = '/markdown-file' | ||
| const WEWORK_MARKDOWN_FILE_LINK_PREFIX = `https://${WEWORK_MARKDOWN_FILE_LINK_HOST}${WEWORK_MARKDOWN_FILE_LINK_PATH}?path=` | ||
| const MARKDOWN_LINK_PATTERN = /(!?)\[([^\]\n]+)\]\(([^)\n]+)\)/g | ||
|
|
||
| interface AssistantMarkdownProps { | ||
| content: string | ||
| isStreaming?: boolean | ||
| onOpenFile?: (path: string) => void | ||
| } | ||
|
|
||
| export function AssistantMarkdown({ content, onOpenFile }: AssistantMarkdownProps) { | ||
| const displayContent = normalizeAssistantMarkdownContent(content) | ||
| export const AssistantMarkdown = memo(function AssistantMarkdown({ | ||
| content, | ||
| isStreaming = false, | ||
| onOpenFile, | ||
| }: AssistantMarkdownProps) { | ||
| const displayContent = prepareAssistantMarkdownContent(content) | ||
| const openFileRef = useRef(onOpenFile) | ||
|
|
||
| useEffect(() => { | ||
| openFileRef.current = onOpenFile | ||
| }, [onOpenFile]) | ||
|
|
||
| const openFile = (path: string) => { | ||
| openFileRef.current?.(path) | ||
| } | ||
|
|
||
| return ( | ||
| <div className="assistant-markdown min-w-0 max-w-full break-words"> | ||
| <ReactMarkdown | ||
| remarkPlugins={[remarkGfm]} | ||
| <Streamdown | ||
| mode={isStreaming ? 'streaming' : 'static'} | ||
| isAnimating={isStreaming} | ||
| controls={false} | ||
| linkSafety={{ enabled: false }} | ||
| lineNumbers={false} | ||
| urlTransform={url => url} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- AssistantMarkdown.tsx outline ---'
ast-grep outline wework/src/components/chat/AssistantMarkdown.tsx --view expanded || true
echo
echo '--- Relevant slice from AssistantMarkdown.tsx ---'
sed -n '1,260p' wework/src/components/chat/AssistantMarkdown.tsx | cat -n
echo
echo '--- MessageList.tsx call site search ---'
rg -n "onOpenFile|openFileFromLink|AssistantMarkdown" wework/src/components/chat/MessageList.tsx wework/src/components/chat -n -A 3 -B 3Repository: wecode-ai/Wegent
Length of output: 50372
onOpenFile can go stale behind the memo comparator. areAssistantMarkdownPropsEqual ignores onOpenFile, so AssistantMarkdown can skip rerendering and never refresh openFileRef.current when the parent passes a new handler. Since MessageList.tsx recreates openFileFromLink on each render, file-link clicks can use stale closure state. Include onOpenFile in the comparison or move the ref-sync logic outside the memoized wrapper.
🤖 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 `@wework/src/components/chat/AssistantMarkdown.tsx` around lines 23 - 58, The
`AssistantMarkdown` memoization is allowing a stale `onOpenFile` handler to
persist because `areAssistantMarkdownPropsEqual` does not compare it, so
`openFileRef.current` may never be refreshed after parent rerenders. Update the
memo comparator for `AssistantMarkdown` to include `onOpenFile`, or refactor the
ref synchronization so it is not skipped by memoization, ensuring `openFile`
always calls the latest handler from `MessageList.tsx` and related file-link
clicks use current closure state.
Comment on lines
+88
to
+91
|
|
||
| void refreshCodexStreamDebugStatus(() => { | ||
| if (document.getElementById(MENU_ID)) renderDeveloperCommandList(list) | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Async refresh re-render steals keyboard focus.
renderDeveloperCommandList always autofocuses button[index 0]. Since it's re-invoked from the refreshCodexStreamDebugStatus callback after the menu is already open (Lines 88-91), any user who has already tabbed/arrowed to another command while the Codex RPC round-trip is pending gets their focus silently reset to the first item once the fetch resolves.
🎯 Proposed fix: only autofocus on initial render
-function renderDeveloperCommandList(list: HTMLElement) {
+function renderDeveloperCommandList(list: HTMLElement, options: { autoFocus?: boolean } = {}) {
list.innerHTML = ''
getDeveloperCommands().forEach((command, index) => {
const button = document.createElement('button')
...
list.appendChild(button)
- if (index === 0) {
+ if (options.autoFocus !== false && index === 0) {
window.setTimeout(() => button.focus(), 0)
}
})
} const list = document.createElement('div')
list.className = 'max-h-[60vh] overflow-y-auto p-2'
- renderDeveloperCommandList(list)
+ renderDeveloperCommandList(list, { autoFocus: true })
...
void refreshCodexStreamDebugStatus(() => {
- if (document.getElementById(MENU_ID)) renderDeveloperCommandList(list)
+ if (document.getElementById(MENU_ID)) renderDeveloperCommandList(list, { autoFocus: false })
})Also applies to: 133-136
🤖 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 `@wework/src/lib/developerCommandMenu.ts` around lines 88 - 91, The async
refresh callback is re-rendering the open developer command menu and
`renderDeveloperCommandList` is always auto-focusing the first button, which
steals keyboard focus from the current selection. Update
`renderDeveloperCommandList` so it only autofocuses on the initial menu render,
and make the `refreshCodexStreamDebugStatus` callback preserve the existing
focused item when it re-invokes the renderer. Apply the same change in both
places where this callback pattern is used, including the `MENU_ID` checks and
the `renderDeveloperCommandList(list)` calls.
This was referenced Jul 10, 2026
This was referenced Jul 18, 2026
Merged
4 tasks
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
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
Root Cause
Streaming updates could trigger excessive frontend work across markdown rendering, scroll scheduling, and repeated listener churn. File-change stats also treated any line beginning with
-as a deletion, even when Codex provided plain created-file content rather than a unified diff.Validation
pnpm --dir wework test -- ChatInput MessageList ToolBlockItem ToolBlocksDisplay embedded-browserpnpm --dir wework typecheckpnpm --dir wework lintcargo test created_plain_content_counts_lines_as_additionscargo test unified_diff_counts_prefixed_linescargo test maps_codex_patch_updates_to_streaming_file_changes_blockcargo clippy --all-features --lib --tests -- -D warningsSummary by CodeRabbit
New Features
Bug Fixes
Documentation