feat(codex): map plan_delta to canonical turn.proposed.delta#11
feat(codex): map plan_delta to canonical turn.proposed.delta#11ranvier2d2 merged 1 commit intomainfrom
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds mapping for Codex provider events with method Changes
Sequence DiagramsequenceDiagram
participant CodexProvider as Codex Provider
participant EventMapper as codexEventMapping
participant Runtime as Runtime Events
CodexProvider->>EventMapper: emit raw event (method: codex/event/plan_delta)
activate EventMapper
EventMapper->>EventMapper: extract delta from payload\n(msg.delta → msg.text → msg.content.text)
alt delta missing or empty
EventMapper-->>Runtime: return empty array / drop event
else delta present
EventMapper-->>Runtime: emit canonical event\n(type: turn.proposed.delta, payload.delta)
end
deactivate EventMapper
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
bot
added
size:S
vouch:trusted
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| * Unit tests for codexEventMapping — calls mapToRuntimeEvents directly | ||
| * without the full adapter/stream infrastructure. | ||
| */ | ||
| import { describe, it, expect } from "bun:test"; |
There was a problem hiding this comment.
🔴 New test file imports from bun:test instead of @effect/vitest, violating AGENTS.md and project conventions
The new test file codexEventMapping.test.ts imports describe, it, and expect from bun:test, while every other test file in apps/server/src/ uses @effect/vitest (see e.g. apps/server/src/provider/Layers/CodexAdapter.test.ts:15, apps/server/src/provider/Layers/ProviderHealth.test.ts:2, and all ~18 other test files in the server). AGENTS.md explicitly states: "NEVER run bun test. Always use bun run test (runs Vitest)." The server's test script is "test": "vitest run" (apps/server/package.json). When vitest run processes this file, tests registered via bun:test's describe/it won't integrate with Vitest's runner — they'll either fail to import or silently not execute, meaning these tests provide no verification.
Prompt for agents
In apps/server/src/provider/Layers/codexEventMapping.test.ts, replace the import on line 5 from bun:test with @effect/vitest, matching the convention used by every other test file in the server app. Change:
import { describe, it, expect } from "bun:test";
to:
import { describe, it, assert } from "@effect/vitest";
Then update all expect() calls to use Node's assert module (assert.equal, assert.deepStrictEqual, etc.) or vitest's assert, matching the patterns in CodexAdapter.test.ts and other sibling test files. For example, expect(events).toHaveLength(1) should become assert.equal(events.length, 1), and expect(events[0]!.type).toBe("turn.proposed.delta") should become assert.equal(events[0]?.type, "turn.proposed.delta").
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/provider/Layers/codexEventMapping.test.ts`:
- Around line 64-67: The test currently uses a conditional guard that lets the
test pass if the mapped event type is wrong; replace the guards with explicit
assertions on the event type before asserting payload contents. Specifically, in
codexEventMapping.test.ts ensure you assert
expect(events[0]!.type).toBe("turn.proposed.delta") before accessing
events[0]!.payload.delta (and likewise assert the expected type at the second
occurrence around lines 84-86) so the test fails if the mapper returns the wrong
event type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f163cb7-6d48-43de-9455-7eb8acbfadd1
📒 Files selected for processing (3)
apps/server/src/provider/Layers/CodexAdapter.test.tsapps/server/src/provider/Layers/codexEventMapping.test.tsapps/server/src/provider/Layers/codexEventMapping.ts
Codex emits ~200 plan_delta events per turn that were silently dropped, making the UI appear frozen while Codex streams its plan. Wire them through codexEventMapping to the canonical turn.proposed.delta event, matching the streaming fidelity of Claude/Cursor/OpenCode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ranvier2d2
force-pushed
the
feat/codex-plan-delta-mapping
branch
from
bbfa758 to
90a04d3
Compare
1 participant
Summary
codex/event/plan_delta→turn.proposed.deltaincodexEventMapping.ts— ~200 events per Codex turn were silently dropped, making the UI appear frozen while Codex streamed its planitem/plan/delta(delta → text → content.text fallback chain)mapToRuntimeEvents+ 1 integration test inCodexAdapter.test.tsProviderRuntimeIngestion.tsalready accumulatesturn.proposed.deltaviaappendBufferedProposedPlan()— no consumer-side changes neededTest plan
bun test codexEventMapping.test.ts— 5/5 pass (happy path, 2 fallbacks, 2 edge cases)tsc --noEmit)🤖 Generated with Claude Code
Summary by CodeRabbit