Repository navigation
fix(wework): stop completed Codex block timer - #1393
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughBlock finalization now accepts an explicit ChangesBlock Finalization and Status Normalization
Subtask Timestamp Fallback and Terminal Finalization
WorkbenchProvider Type Layouts and Code Style
Type Additions, Configuration, and Test Updates
Sequence Diagram(s)sequenceDiagram
participant Reducer as reduceWorkbenchMessages
participant FPB as finalizeProcessingBlocks
participant StatusNorm as normalizeWorkbenchBlockStatus
participant Blocks as message.blocks
Reducer->>StatusNorm: normalizeWorkbenchBlockStatus(rawStatus)
Note over StatusNorm: trim, lowercase, normalize whitespace/dashes
StatusNorm->>StatusNorm: match canonical or alias<br/>(completed→done, failed→error)
StatusNorm-->>Reducer: WorkbenchToolBlockStatus
Reducer->>FPB: assistant_done → finalizeProcessingBlocks(blocks, 'done')
FPB->>Blocks: finalize streaming thinking → 'done'
FPB->>Blocks: coerce non-final → 'done'
FPB-->>Reducer: updated blocks
Reducer->>FPB: assistant_error → finalizeProcessingBlocks(blocks, 'error')
FPB->>Blocks: finalize streaming thinking → 'done'
FPB->>Blocks: coerce non-final → 'error'
FPB-->>Reducer: updated blocks
sequenceDiagram
participant Subtask as Subtask (completed/failed/cancelled)
participant Provider as subtaskToMessage
participant Parse as parseTimestampMs
participant Normalize as normalizeProcessingBlocks
participant Finalize as finalizeBlocksForTerminalSubtask
participant Message as WorkbenchMessage
Subtask->>Provider: completed_at/updated_at/created_at
Provider->>Parse: extract blockFallbackTimestamp
Provider->>Normalize: normalizeProcessingBlocks(blocks, fallback)
Normalize->>Normalize: resolve missing timestamps to fallback
Provider->>Finalize: finalizeBlocksForTerminalSubtask
Finalize->>Finalize: map failed→error, others→done
Finalize-->>Message: finalized message with coerced blocks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/chat-core/src/workbench-message-reducer.test.ts (1)
78-113: ⚡ Quick winAdd a companion regression test for
assistant_errorblock coercion.This test covers the new
assistant_donefinalization path well, but the pairedassistant_errorpath (wired in reducer changes) is still unasserted in this suite. Add one case asserting non-terminal blocks becomeerroronassistant_error.🤖 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 `@packages/chat-core/src/workbench-message-reducer.test.ts` around lines 78 - 113, Add a new test case after the existing test_finalizes_incoming_processing_blocks_on_done test to cover the `assistant_error` action path. This test should follow the same pattern: dispatch an `assistant_started` action to initialize state, then dispatch an `assistant_error` action with blocks in non-terminal states (streaming and pending), and assert that all blocks are transitioned to error status instead of done status. This ensures the error coercion path in the reducer is properly tested alongside the done finalization path.
🤖 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.
Nitpick comments:
In `@packages/chat-core/src/workbench-message-reducer.test.ts`:
- Around line 78-113: Add a new test case after the existing
test_finalizes_incoming_processing_blocks_on_done test to cover the
`assistant_error` action path. This test should follow the same pattern:
dispatch an `assistant_started` action to initialize state, then dispatch an
`assistant_error` action with blocks in non-terminal states (streaming and
pending), and assert that all blocks are transitioned to error status instead of
done status. This ensures the error coercion path in the reducer is properly
tested alongside the done finalization path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82ed28b7-5b6c-4b27-a071-4c0d9f027b0e
📒 Files selected for processing (2)
packages/chat-core/src/workbench-message-reducer.test.tspackages/chat-core/src/workbench-message-reducer.ts
6b829e6 to
f1f7f82
Compare
f1f7f82 to
693c8df
Compare
Summary
wework/.prettierrc.jsonso the wework pre-commit hook uses the project style instead of Prettier defaultsTests
Summary by CodeRabbit