Skip to content

[codex] Align Wework Codex transcript rendering - #1659

Merged
qdaxb merged 1 commit into
wecode-ai:mainfrom
icycrystal4:codex/wework-codex-ui-parity
Jul 1, 2026
Merged

qdaxb merged 1 commit into
wecode-ai:mainfrom
icycrystal4:codex/wework-codex-ui-parity

Conversation

@icycrystal4

@icycrystal4 icycrystal4 commented Jul 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Align Wework Codex transcript rendering with Codex App for tool activity rows, file change cards, plan cards, request-user-input confirmations, and hidden Codex UI directives.
  • Tighten Codex-like popovers/selectors and desktop chat layout details, including sidebar/titlebar behavior and floating composer clearance.
  • Add focused tests for transcript normalization, plan confirmation display, tool summaries, file changes, and layout interactions.

Conflict Resolution

  • Rebased the branch onto latest origin/main (afab8869c).
  • Resolved conflicts in wework/src/components/chat/requestUserInputMessages.ts by keeping the main-branch RequestUserInputBlock type guard structure while retaining the new Codex plan-confirmation labels and compatibility checks.
  • Resolved conflicts in wework/src/components/layout/DesktopWorkbenchMain.tsx by keeping the main-branch split floating-composer clearance constants and reapplying the Wework UI parity changes.

Validation

  • pnpm --dir wework exec prettier --check <touched files>
  • pnpm --dir wework exec eslint <touched files>
  • pnpm --dir wework exec tsc -b
  • pnpm --dir wework test -- --runInBand (109 files, 1054 tests)
  • git push pre-push hook passed with AI_VERIFIED=1 after verifying this UI-only Wework change does not require docs updates. The hook also passed frontend, Wework, backend syntax/format, knowledge syntax, executor cargo test/clippy, executor-manager syntax, and Alembic multi-head checks.

Summary by CodeRabbit

  • New Features

    • Desktop sidebar toggle interactions are more consistent, including reliable external toggle requests.
    • Tool activity summaries now better support multi-file changes, code-search/read summaries, and clearer labels.
    • Assistant plan cards and implementation-plan prompts now use updated wording and more accessible interactions.
  • Bug Fixes

    • Improved collapsed model submenu behavior and hover/mouse-exit handling.
    • File diff previews now use larger/tuned sizing and show full paths.
    • Codex UI directive markup is stripped from displayed assistant content.
  • Tests

    • Updated and added coverage for submenu, plan interactions, diffs, tool rows, and directive sanitization.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

This PR updates desktop sidebar toggling, desktop model submenu behavior, implementation-plan copy and handling, tool activity/file rendering, diff preview paths, and Codex UI directive stripping across chat and workbench flows.

Changes

Desktop Sidebar Toggle Request

Layer / File(s) Summary
Sidebar toggle request hook
wework/src/components/layout/useDesktopSidebarCollapsed.ts
Adds a cancelable window-event request API and matching listener for desktop sidebar toggles.
Sidebar toggle wiring
wework/src/App.tsx, wework/src/components/layout/DesktopWorkbenchLayout.tsx
The titlebar toggle delegates to the request API before falling back, and the desktop layout registers the toggle request listener.
Sidebar and titlebar tests
wework/src/components/topnav/ChromeTitlebar.tsx, wework/src/components/topnav/ChromeTitlebar.test.tsx, wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
Removes the root drag-region attribute and updates tests for drag-region, clearance, and sidebar expansion behavior.

Model Selector Desktop Submenu Refactor

Layer / File(s) Summary
Desktop submenu state and layout
wework/src/components/chat/composer/ModelSelector.tsx
Adds control refs, submenu activation, hidden control filtering, and control/family submenu layout branching.
Control and family menu rendering
wework/src/components/chat/composer/ModelSelector.tsx
Updates hover/focus behavior and submenu rendering for filtered controls and family groups.
Desktop model menu tests
wework/src/components/chat/ChatInput.test.tsx
Updates assertions for hidden collaboration controls and adds hover/close submenu coverage.

Implementation-Plan Text Unification and Plan Card UX

Layer / File(s) Summary
Implementation-plan text constants
wework/src/components/chat/requestUserInputMessages.ts
Adds shared implementation-plan constants and a text-matching helper.
Request-user-input wording
wework/src/components/chat/RequestUserInputCard.tsx, wework/src/components/layout/requestUserInputOverlay.ts, related tests
Switches request-user-input flows to the shared helper/constants and updates the affected test expectations.
Plan card rendering
wework/src/components/chat/MessageList.tsx, wework/src/components/chat/MessageList.test.tsx
Normalizes Codex user-message content and changes assistant plan-card interaction semantics and tests.

Tool Block Activity and File Rendering

Layer / File(s) Summary
File path and patch helpers
wework/src/components/chat/blocks/toolBlockKinds.ts
Adds patch-apply recognition and multi-path file extraction.
Tool activity parsing
wework/src/components/chat/blocks/toolBlockActivity.ts, wework/src/components/chat/blocks/toolBlockActivity.test.ts
Exports activity metadata helpers, filters hidden/redundant blocks, and parses shell commands into read/search metadata.
ToolBlockItem multi-file labels
wework/src/components/chat/blocks/ToolBlockItem.tsx, wework/src/components/chat/blocks/ToolBlockItem.test.tsx
Uses multi-path extraction for file labels and expanded file details.
ToolBlocksDisplay activity rows
wework/src/components/chat/blocks/ToolBlocksDisplay.tsx, wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
Renders specialized read/search rows and updates activity-group icons and coverage.

Diff Preview Display Path

Layer / File(s) Summary
Preview path and layout
wework/src/components/chat/FileChangesCard.tsx, wework/src/components/chat/FileChangesCard.test.tsx
Adds display-path resolution for diff previews and updates preview sizing and assertions.

Codex UI Directive Stripping

Layer / File(s) Summary
Directive stripping utility
wework/src/lib/codex-directives.ts
Defines directive patterns and strips directive lines outside code fences.
Runtime message sanitization
wework/src/features/workbench/runtimePaneMessages.ts, wework/src/features/workbench/runtimePaneMessages.test.ts
Strips Codex directives from assistant runtime content and adds tests for stripping and preservation cases.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TitlebarSidebarToggle
  participant App
  participant DesktopWorkbenchLayout
  TitlebarSidebarToggle->>App: onClick
  App->>App: requestDesktopSidebarToggle()
  App->>DesktopWorkbenchLayout: dispatch cancelable window event
  DesktopWorkbenchLayout->>DesktopWorkbenchLayout: preventDefault + updateSidebarCollapsed(!effectiveSidebarCollapsed)
  DesktopWorkbenchLayout-->>App: defaultPrevented = true
  App->>App: skip local fallback toggle
Loading

Possibly related PRs

  • wecode-ai/Wegent#1307: Also changes the desktop sidebar-toggle flow and top-bar/sidebar UI behavior.
  • wecode-ai/Wegent#1403: Also touches chat progress-display rendering and tool activity presentation.
  • wecode-ai/Wegent#1640: Also refactors tool-activity/file-path handling in toolBlockActivity, toolBlockKinds, and ToolBlockItem.

Suggested reviewers: qdaxb

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: aligning Wework Codex transcript rendering and related UI behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@icycrystal4
icycrystal4 marked this pull request as ready for review July 1, 2026 07:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
wework/src/components/layout/useDesktopSidebarCollapsed.ts (1)

64-74: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Effect re-subscribes on every render for unstable callback identities.

useDesktopSidebarToggleRequest's effect depends on [onToggleSidebar]. Downstream, DesktopWorkbenchLayout passes an inline arrow function (see DesktopWorkbenchLayout.tsx Line 202-204), so a new function identity is created on every render, causing the window listener to be removed and re-added on every re-render of the layout. It still works correctly since the closure captures fresh state each time, but it's unnecessary churn on a hot component.

Consider storing the latest callback in a ref internally so the effect only needs to run once, removing the burden from callers to memoize:

♻️ Proposed fix using a ref to stabilize the listener
 export function useDesktopSidebarToggleRequest(onToggleSidebar: () => void) {
+  const onToggleSidebarRef = useRef(onToggleSidebar)
+  onToggleSidebarRef.current = onToggleSidebar
+
   useEffect(() => {
     const handleToggleRequest = (event: Event) => {
       event.preventDefault()
-      onToggleSidebar()
+      onToggleSidebarRef.current()
     }

     window.addEventListener(SIDEBAR_TOGGLE_REQUEST_EVENT, handleToggleRequest)
     return () => window.removeEventListener(SIDEBAR_TOGGLE_REQUEST_EVENT, handleToggleRequest)
-  }, [onToggleSidebar])
+  }, [])
 }
🤖 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/useDesktopSidebarCollapsed.ts` around lines 64 -
74, useDesktopSidebarToggleRequest is re-subscribing the window listener on
every render because the effect depends on onToggleSidebar and
DesktopWorkbenchLayout passes an inline callback. Update
useDesktopSidebarToggleRequest to keep the latest onToggleSidebar in a ref and
register the SIDEBAR_TOGGLE_REQUEST_EVENT listener only once, so
handleToggleRequest can always invoke the current callback without forcing
repeated add/remove cycles.
wework/src/components/layout/DesktopWorkbenchLayout.tsx (1)

202-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inline callback passed to useDesktopSidebarToggleRequest.

This passes a new arrow function on every render, unlike the rest of this file's handlers (collapseSidebar, updateSidebarCollapsed) which are consistently wrapped in useCallback. This causes the hook's internal effect to tear down and reattach the window listener on every re-render (see comment on useDesktopSidebarCollapsed.ts, where fixing the hook itself would be the better root-cause fix). If the hook isn't updated to use a ref internally, wrapping this callback in useCallback here would be a lower-effort mitigation.

🤖 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/DesktopWorkbenchLayout.tsx` around lines 202 -
205, The `useDesktopSidebarToggleRequest` handler in `DesktopWorkbenchLayout` is
recreated on every render, causing the hook to re-register its listener
repeatedly. Wrap the toggle callback in `useCallback` alongside the existing
`collapseSidebar` and `updateSidebarCollapsed` handlers, and keep its
dependencies aligned with `effectiveSidebarCollapsed` and
`updateSidebarCollapsed`. If the hook is later fixed to use an internal ref,
this can stay as the local mitigation for now.
wework/src/components/chat/FileChangesCard.tsx (1)

454-463: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Mixed path separators possible for non-POSIX workspace paths.

absoluteFilePath joins normalizedWorkspacePath with / unconditionally. If workspacePath uses backslashes (Windows-style), the resulting displayPath mixes \ and / separators (e.g. C:\Users\foo/src/file.ts). Purely cosmetic since this is display-only, but worth normalizing for consistency.

♻️ Optional normalization
 function absoluteFilePath(workspacePath: string, filePath: string): string {
   if (isAbsolutePath(filePath)) return filePath
   const normalizedWorkspacePath = workspacePath.replace(/[\\/]+$/, '')
   if (!normalizedWorkspacePath) return filePath
-  return `${normalizedWorkspacePath}/${filePath.replace(/^[\\/]+/, '')}`
+  const separator = /\\/.test(normalizedWorkspacePath) ? '\\' : '/'
+  return `${normalizedWorkspacePath}${separator}${filePath.replace(/^[\\/]+/, '')}`
 }
🤖 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/FileChangesCard.tsx` around lines 454 - 463, The
absoluteFilePath helper in FileChangesCard mixes separators by always joining
with “/”, which can produce inconsistent display paths when workspacePath is
Windows-style. Update absoluteFilePath (and any related path formatting in the
same component) to normalize separators consistently for display, while
preserving the existing absolute-path check in isAbsolutePath and the current
behavior for already-absolute inputs.
wework/src/components/chat/composer/ModelSelector.tsx (1)

398-431: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: expose submenu semantics to assistive tech.

renderControlMenuItem opens a submenu on hover/focus/click but the button doesn't advertise it. Consider aria-haspopup="menu" and aria-expanded={active} so screen-reader users get the same disclosure state the visual chevron implies. The existing family buttons share this gap, so it can be applied consistently there too.

♿ Suggested attributes
       <button
         ref={node => {
           if (node) {
             controlButtonRefs.current.set(control.id, node)
           } else {
             controlButtonRefs.current.delete(control.id)
           }
         }}
         key={control.id}
         type="button"
         data-testid={`model-control-menu-${control.id}`}
+        aria-haspopup="menu"
+        aria-expanded={active}
         onMouseEnter={event => activateControl(control.id, event.currentTarget)}
🤖 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/composer/ModelSelector.tsx` around lines 398 -
431, `renderControlMenuItem` in `ModelSelector.tsx` opens a submenu via
hover/focus/click but the trigger button does not expose that relationship to
assistive tech. Update the button in `renderControlMenuItem` to advertise
submenu behavior with `aria-haspopup="menu"` and reflect its open state with
`aria-expanded={active}`, and apply the same pattern consistently to the related
family buttons that use the same submenu activation flow.
wework/src/components/chat/MessageList.tsx (1)

588-608: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff

Nested interactive elements inside role="button" container.

PlanCardActions' native <button>s live inside a <section role="button">. This works today only because clicks are stopped via event.stopPropagation() and keyboard events are filtered via isNestedInteractiveElement, but nested interactive controls inside an element exposed as a button is a recognized anti-pattern (e.g., axe-core's "nested-interactive" check) — some assistive tech may not reliably expose/announce the inner buttons.

Also, activating on Space via keydown (line 589) diverges slightly from native <button> semantics, which fire on keyup to avoid accidental activation while scrolling.

Consider structuring the header actions outside the role="button" boundary (e.g., non-interactive wrapper with the primary "open" affordance separate from the secondary action buttons) for stronger AT compatibility.

Also applies to: 632-705, 719-726

🤖 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/MessageList.tsx` around lines 588 - 608, The
`assistant-plan-card` wrapper in `MessageList.tsx` is exposing a `section` as
`role="button"` while `PlanCardActions` renders native buttons inside it, which
creates a nested interactive control pattern. Refactor the
`openPlan`/`handleCardClick`/`handleCardKeyDown` interaction so the primary
expand target is separate from the action buttons, keeping `PlanCardActions`
outside any button-like container. Update the surrounding plan card markup and
related keyboard handling to avoid `role="button"` on a parent that contains
interactive children, and align Space activation behavior with native button
semantics where applicable.
wework/src/components/chat/blocks/ToolBlocksDisplay.tsx (1)

478-480: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Third copy of the identical basename helper.

An identical basename(path) implementation now exists in this file, in toolBlockActivity.ts, and in ToolBlockItem.tsx. Worth consolidating into a single exported helper (e.g. in toolBlockKinds.ts, alongside the other shared file-path utilities) and importing it from the three call sites.

♻️ Suggested consolidation
-function basename(path: string): string {
-  return path.split(/[\\/]/).filter(Boolean).pop() || path
-}
+import { basename } from './toolBlockKinds'

(with a corresponding export function basename(path: string): string { ... } added once to toolBlockKinds.ts, and the local copies in toolBlockActivity.ts / ToolBlockItem.tsx removed.)

🤖 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/ToolBlocksDisplay.tsx` around lines 478 -
480, The file-path basename logic is duplicated across ToolBlocksDisplay,
toolBlockActivity, and ToolBlockItem, so consolidate it into one shared exported
helper. Add basename(path: string) to toolBlockKinds.ts alongside the other
shared file-path utilities, then replace the local copies in ToolBlocksDisplay,
toolBlockActivity, and ToolBlockItem with imports from that shared helper.
wework/src/components/chat/blocks/toolBlockActivity.ts (1)

223-504: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Solid, well-tested parsing logic — consider extracting into its own module.

This block adds a substantial shell-command tokenization/parsing layer (tokenizer, search/find/git argument parsing, path extraction) inside a file whose primary concern is tool-activity classification/grouping. Traced the logic against all provided test cases (read commands, rg/git grep/find, quoting, ||/&& merging) and it holds up correctly. Given its size and orthogonal concern, splitting it into a dedicated shellCommandParsing.ts module would improve cohesion and make toolBlockActivity.ts easier to navigate.

🤖 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/toolBlockActivity.ts` around lines 223 -
504, Extract the shell-command tokenization and parsing helpers from
toolBlockActivity into a dedicated shellCommandParsing module to improve
cohesion. Move the related functions and constants such as splitShellWords,
getPatternSearchArguments, getReadCommandFilePaths, getSearchCommandSummary,
getGitSearchCommandSummary, getFindSearchCommandSummary, getPathArguments, and
the command option sets into the new module. Update toolBlockActivity to import
and use those helpers so the file stays focused on tool-activity
classification/grouping while preserving the existing behavior and tests.
🤖 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 `@wework/src/features/workbench/runtimePaneMessages.test.ts`:
- Around line 89-136: Add an edge-case test for
runtimeMessagesToWorkbenchMessages covering assistant messages whose content is
undefined or non-string, since the current coverage in
runtimePaneMessages.test.ts only exercises string content. Update the test
around runtimeMessagesToWorkbenchMessages to assert it safely preserves or skips
such messages without crashing, so the unguarded content handling in
runtimePaneMessages.ts is protected by a regression test.

---

Nitpick comments:
In `@wework/src/components/chat/blocks/toolBlockActivity.ts`:
- Around line 223-504: Extract the shell-command tokenization and parsing
helpers from toolBlockActivity into a dedicated shellCommandParsing module to
improve cohesion. Move the related functions and constants such as
splitShellWords, getPatternSearchArguments, getReadCommandFilePaths,
getSearchCommandSummary, getGitSearchCommandSummary,
getFindSearchCommandSummary, getPathArguments, and the command option sets into
the new module. Update toolBlockActivity to import and use those helpers so the
file stays focused on tool-activity classification/grouping while preserving the
existing behavior and tests.

In `@wework/src/components/chat/blocks/ToolBlocksDisplay.tsx`:
- Around line 478-480: The file-path basename logic is duplicated across
ToolBlocksDisplay, toolBlockActivity, and ToolBlockItem, so consolidate it into
one shared exported helper. Add basename(path: string) to toolBlockKinds.ts
alongside the other shared file-path utilities, then replace the local copies in
ToolBlocksDisplay, toolBlockActivity, and ToolBlockItem with imports from that
shared helper.

In `@wework/src/components/chat/composer/ModelSelector.tsx`:
- Around line 398-431: `renderControlMenuItem` in `ModelSelector.tsx` opens a
submenu via hover/focus/click but the trigger button does not expose that
relationship to assistive tech. Update the button in `renderControlMenuItem` to
advertise submenu behavior with `aria-haspopup="menu"` and reflect its open
state with `aria-expanded={active}`, and apply the same pattern consistently to
the related family buttons that use the same submenu activation flow.

In `@wework/src/components/chat/FileChangesCard.tsx`:
- Around line 454-463: The absoluteFilePath helper in FileChangesCard mixes
separators by always joining with “/”, which can produce inconsistent display
paths when workspacePath is Windows-style. Update absoluteFilePath (and any
related path formatting in the same component) to normalize separators
consistently for display, while preserving the existing absolute-path check in
isAbsolutePath and the current behavior for already-absolute inputs.

In `@wework/src/components/chat/MessageList.tsx`:
- Around line 588-608: The `assistant-plan-card` wrapper in `MessageList.tsx` is
exposing a `section` as `role="button"` while `PlanCardActions` renders native
buttons inside it, which creates a nested interactive control pattern. Refactor
the `openPlan`/`handleCardClick`/`handleCardKeyDown` interaction so the primary
expand target is separate from the action buttons, keeping `PlanCardActions`
outside any button-like container. Update the surrounding plan card markup and
related keyboard handling to avoid `role="button"` on a parent that contains
interactive children, and align Space activation behavior with native button
semantics where applicable.

In `@wework/src/components/layout/DesktopWorkbenchLayout.tsx`:
- Around line 202-205: The `useDesktopSidebarToggleRequest` handler in
`DesktopWorkbenchLayout` is recreated on every render, causing the hook to
re-register its listener repeatedly. Wrap the toggle callback in `useCallback`
alongside the existing `collapseSidebar` and `updateSidebarCollapsed` handlers,
and keep its dependencies aligned with `effectiveSidebarCollapsed` and
`updateSidebarCollapsed`. If the hook is later fixed to use an internal ref,
this can stay as the local mitigation for now.

In `@wework/src/components/layout/useDesktopSidebarCollapsed.ts`:
- Around line 64-74: useDesktopSidebarToggleRequest is re-subscribing the window
listener on every render because the effect depends on onToggleSidebar and
DesktopWorkbenchLayout passes an inline callback. Update
useDesktopSidebarToggleRequest to keep the latest onToggleSidebar in a ref and
register the SIDEBAR_TOGGLE_REQUEST_EVENT listener only once, so
handleToggleRequest can always invoke the current callback without forcing
repeated add/remove cycles.
🪄 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: 132fa134-87db-4c3a-9910-0933c7726fea

📥 Commits

Reviewing files that changed from the base of the PR and between afab886 and 4f806e4.

📒 Files selected for processing (29)
  • wework/src/App.tsx
  • wework/src/components/chat/ChatInput.test.tsx
  • wework/src/components/chat/FileChangesCard.test.tsx
  • wework/src/components/chat/FileChangesCard.tsx
  • wework/src/components/chat/MessageList.test.tsx
  • wework/src/components/chat/MessageList.tsx
  • wework/src/components/chat/RequestUserInputCard.test.tsx
  • wework/src/components/chat/RequestUserInputCard.tsx
  • wework/src/components/chat/blocks/ToolBlockItem.test.tsx
  • wework/src/components/chat/blocks/ToolBlockItem.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
  • wework/src/components/chat/blocks/toolBlockActivity.test.ts
  • wework/src/components/chat/blocks/toolBlockActivity.ts
  • wework/src/components/chat/blocks/toolBlockKinds.ts
  • wework/src/components/chat/composer/ModelSelector.tsx
  • wework/src/components/chat/requestUserInputMessages.ts
  • wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
  • wework/src/components/layout/DesktopWorkbenchLayout.tsx
  • wework/src/components/layout/MobileWorkbenchLayout.test.tsx
  • wework/src/components/layout/requestUserInputOverlay.test.ts
  • wework/src/components/layout/requestUserInputOverlay.ts
  • wework/src/components/layout/useDesktopSidebarCollapsed.ts
  • wework/src/components/topnav/ChromeTitlebar.test.tsx
  • wework/src/components/topnav/ChromeTitlebar.tsx
  • wework/src/features/workbench/WorkbenchProvider.test.tsx
  • wework/src/features/workbench/runtimePaneMessages.test.ts
  • wework/src/features/workbench/runtimePaneMessages.ts
  • wework/src/lib/codex-directives.ts
💤 Files with no reviewable changes (1)
  • wework/src/components/topnav/ChromeTitlebar.tsx

Comment on lines +89 to +136
describe('runtimeMessagesToWorkbenchMessages', () => {
test('strips Codex UI directives from restored assistant transcript content', () => {
const messages = runtimeMessagesToWorkbenchMessages([
{
id: 'assistant-1',
role: 'assistant',
content: [
'完成了。',
'',
'```text',
'::git-stage{cwd="/workspace/project"}',
'```',
'',
'::git-commit{cwd="/workspace/project"}',
].join('\n'),
},
])

expect(messages[0].content).toBe(
['完成了。', '', '```text', '::git-stage{cwd="/workspace/project"}', '```'].join('\n')
)
})

test('keeps user-authored Codex directive text unchanged', () => {
const messages = runtimeMessagesToWorkbenchMessages([
{
id: 'user-1',
role: 'user',
content: '解释一下 ::git-stage{cwd="/workspace/project"} 是什么',
},
])

expect(messages[0].content).toBe('解释一下 ::git-stage{cwd="/workspace/project"} 是什么')
})

test('keeps assistant prose that mentions a Codex directive inline', () => {
const messages = runtimeMessagesToWorkbenchMessages([
{
id: 'assistant-1',
role: 'assistant',
content: '这类 ::git-stage{cwd="/workspace/project"} 指令会刷新 Git UI。',
},
])

expect(messages[0].content).toBe(
'这类 ::git-stage{cwd="/workspace/project"} 指令会刷新 Git UI。'
)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Missing edge-case test for non-string/undefined assistant content.

Given the unguarded call at runtimePaneMessages.ts Line 286 flagged above, a test with an assistant message that has content: undefined (or non-string) would have caught the potential runtime crash. As per coding guidelines, tests should cover edge cases.

As per coding guidelines, "follow the project's testing principles: AAA, mock external services, test edge cases, and keep tests isolated."

🤖 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/features/workbench/runtimePaneMessages.test.ts` around lines 89 -
136, Add an edge-case test for runtimeMessagesToWorkbenchMessages covering
assistant messages whose content is undefined or non-string, since the current
coverage in runtimePaneMessages.test.ts only exercises string content. Update
the test around runtimeMessagesToWorkbenchMessages to assert it safely preserves
or skips such messages without crashing, so the unguarded content handling in
runtimePaneMessages.ts is protected by a regression test.

Source: Coding guidelines

@icycrystal4
icycrystal4 force-pushed the codex/wework-codex-ui-parity branch from 4f806e4 to e9e23c7 Compare July 1, 2026 07:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
wework/src/lib/codex-directives.ts (1)

21-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add unit tests for stripCodexUiDirectives.

This utility has non-trivial branching (code-fence toggling, directive matching, blank-line collapsing) but no accompanying test file is included in this cohort. Recommend covering: directive stripping outside fences, preservation inside fences, mixed directive/non-directive content, and the blank-line collapsing behavior.

As per coding guidelines, **/*.{py,ts,tsx,js,jsx} files should "follow the project's testing principles: AAA, mock external services, test edge cases, and keep tests isolated."

🤖 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/codex-directives.ts` around lines 21 - 51, Add a focused unit
test suite for stripCodexUiDirectives to cover its branching behavior: stripping
CODEX UI directive lines outside code fences, preserving the same directive text
inside fences, handling mixed directive and non-directive content, and verifying
the newline-collapsing/trim behavior when multiple blank lines are introduced.
Place the tests alongside the codex-directives module and use the
stripCodexUiDirectives function name in the assertions so the cases stay tied to
the implementation.

Source: Coding guidelines

🤖 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 `@wework/src/lib/codex-directives.ts`:
- Around line 38-41: The directive-stripping logic in codex-directives.ts
removes matched lines entirely, which can collapse neighboring content that was
only separated by a directive. Update the line-processing in the directive
handling path around CODEX_UI_DIRECTIVE_LINE_PATTERN so matched directive lines
are replaced with an empty line placeholder instead of being skipped, preserving
separation between adjacent content lines/paragraphs. Keep the behavior
localized to the existing stripping flow that sets changed and continues.

---

Nitpick comments:
In `@wework/src/lib/codex-directives.ts`:
- Around line 21-51: Add a focused unit test suite for stripCodexUiDirectives to
cover its branching behavior: stripping CODEX UI directive lines outside code
fences, preserving the same directive text inside fences, handling mixed
directive and non-directive content, and verifying the newline-collapsing/trim
behavior when multiple blank lines are introduced. Place the tests alongside the
codex-directives module and use the stripCodexUiDirectives function name in the
assertions so the cases stay tied to the implementation.
🪄 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: 6158b50c-9a20-45a3-ac10-dd5aabb9be18

📥 Commits

Reviewing files that changed from the base of the PR and between 4f806e4 and e9e23c7.

📒 Files selected for processing (29)
  • wework/src/App.tsx
  • wework/src/components/chat/ChatInput.test.tsx
  • wework/src/components/chat/FileChangesCard.test.tsx
  • wework/src/components/chat/FileChangesCard.tsx
  • wework/src/components/chat/MessageList.test.tsx
  • wework/src/components/chat/MessageList.tsx
  • wework/src/components/chat/RequestUserInputCard.test.tsx
  • wework/src/components/chat/RequestUserInputCard.tsx
  • wework/src/components/chat/blocks/ToolBlockItem.test.tsx
  • wework/src/components/chat/blocks/ToolBlockItem.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
  • wework/src/components/chat/blocks/toolBlockActivity.test.ts
  • wework/src/components/chat/blocks/toolBlockActivity.ts
  • wework/src/components/chat/blocks/toolBlockKinds.ts
  • wework/src/components/chat/composer/ModelSelector.tsx
  • wework/src/components/chat/requestUserInputMessages.ts
  • wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
  • wework/src/components/layout/DesktopWorkbenchLayout.tsx
  • wework/src/components/layout/MobileWorkbenchLayout.test.tsx
  • wework/src/components/layout/requestUserInputOverlay.test.ts
  • wework/src/components/layout/requestUserInputOverlay.ts
  • wework/src/components/layout/useDesktopSidebarCollapsed.ts
  • wework/src/components/topnav/ChromeTitlebar.test.tsx
  • wework/src/components/topnav/ChromeTitlebar.tsx
  • wework/src/features/workbench/WorkbenchProvider.test.tsx
  • wework/src/features/workbench/runtimePaneMessages.test.ts
  • wework/src/features/workbench/runtimePaneMessages.ts
  • wework/src/lib/codex-directives.ts
💤 Files with no reviewable changes (1)
  • wework/src/components/topnav/ChromeTitlebar.tsx
✅ Files skipped from review due to trivial changes (2)
  • wework/src/components/layout/requestUserInputOverlay.test.ts
  • wework/src/features/workbench/WorkbenchProvider.test.tsx
🚧 Files skipped from review as they are similar to previous changes (25)
  • wework/src/components/layout/DesktopWorkbenchLayout.tsx
  • wework/src/App.tsx
  • wework/src/components/layout/useDesktopSidebarCollapsed.ts
  • wework/src/components/chat/blocks/toolBlockActivity.test.ts
  • wework/src/components/layout/requestUserInputOverlay.ts
  • wework/src/components/topnav/ChromeTitlebar.test.tsx
  • wework/src/features/workbench/runtimePaneMessages.test.ts
  • wework/src/components/chat/RequestUserInputCard.tsx
  • wework/src/components/chat/blocks/ToolBlockItem.test.tsx
  • wework/src/components/chat/RequestUserInputCard.test.tsx
  • wework/src/components/layout/MobileWorkbenchLayout.test.tsx
  • wework/src/features/workbench/runtimePaneMessages.ts
  • wework/src/components/chat/requestUserInputMessages.ts
  • wework/src/components/chat/blocks/ToolBlockItem.tsx
  • wework/src/components/chat/MessageList.test.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.test.tsx
  • wework/src/components/chat/FileChangesCard.tsx
  • wework/src/components/chat/MessageList.tsx
  • wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
  • wework/src/components/layout/DesktopWorkbenchLayout.test.tsx
  • wework/src/components/chat/blocks/toolBlockKinds.ts
  • wework/src/components/chat/ChatInput.test.tsx
  • wework/src/components/chat/blocks/toolBlockActivity.ts
  • wework/src/components/chat/composer/ModelSelector.tsx
  • wework/src/components/chat/FileChangesCard.test.tsx

Comment thread wework/src/lib/codex-directives.ts
@icycrystal4
icycrystal4 force-pushed the codex/wework-codex-ui-parity branch from e9e23c7 to 58be11f Compare July 1, 2026 07:50
@qdaxb
qdaxb merged commit f1fe174 into wecode-ai:main Jul 1, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants