fix(storage): import Codex item_completed messages - #3520
Conversation
Generated-by: OpenAI Codex
|
Review at exact head The focused adapter change closes the Codex 0.149 P2 (follow-up, not introduced here) — Mechanism: Concrete trigger: in a real imported Codex transcript, one turn occupied sequences 0–53. Navigation rendered only the early prefix (approximately through sequence 18); the sequence-52 final assistant message—2,444 characters containing headings, a list, and a fenced code block—remained present in SQLite but absent from the DOM. Fix options:
Recommendation for this PR: the second option. Do not widen #3520; record the first option as a renderer follow-up. This is a product P2, not a regression introduced by this diff. P3 (follow-up, not introduced here) — Mechanism: the landmark index can contain multiple user-message entries with the same Concrete trigger: the real import contained one turn with three user prompts and another with two. Both prompts in the latter were visible in the transcript, while the rail logged duplicate keys for that shared turn ID. Fix options:
Recommendation: use Verification performed:
Coverage limit: the imported real-world samples had no non-empty reasoning summary, so real Electron rendering of the new Merge verdict: ready for the focused adapter fix; track the renderer P2/P3 outside this PR. 简体中文本次按 exact head 实机导入和页面检查发现两个既有的 Renderer 后续问题,但都不建议塞进 #3520 扩大 PR:
验证包括:针对性测试 9/9、构建/格式检查通过;7 个真实新版 rollout 的事件与转换计数一致;一份完整导入的 1,560 条消息与源记录逐条一致,工具调用/结果 674/674 配对,且没有重复 ID、空正文或时间倒序。代码块、链接、列表和工具卡片在完整加载的 turn 中均正常渲染。 真实样本没有非空 reasoning summary,因此新版 Reasoning 的真实 Electron 格式尚未覆盖,目前只有 sanitized fixture 的 adapter 测试证据。 结论:#3520 可以按聚焦修复合并;Renderer 的 P2/P3 应单独跟踪,不扩大本 PR。 |
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head 317b38e. Coverage: upstream OpenAI Codex TurnItem/UserMessageItem/AgentMessageItem/ReasoningItem shapes; missing and extra fields; content-part ordering; response-item mirror suppression; IDs, turns, timestamps, model metadata, title discovery, and terminal conversion. Exclusions: private user rollout contents and future unreleased Codex schemas. Local Core→Storage builds and the focused 9/9 adapter suite are green; hosted exact-head test is green. Current verdict: NO-GO with one inline P2. The branch is 77 commits behind main but a current-main merge-tree probe is clean.
Match the upstream Codex contract by concatenating completed user and agent text parts without injecting separators. Strengthen the regression fixture so split-word boundaries fail under the previous behavior. Generated-by: OpenAI Codex
jackwener
left a comment
There was a problem hiding this comment.
Independent re-review of exact head 0b4af5e4e3e48d40b41e04033dd23ce78046e35b。不继承 317b38e5 上的 NO-GO。
三问
- 要解决什么:Codex 0.149 把展示历史改成
item_completed,旧适配器只认user_message/agent_*,导入会丢对话。 - 怎么解决:从
event_msg.item_completed读 UserMessage / AgentMessage / Reasoning;response_item的 message/reasoning 镜像仍然不收,避免双份;tool call 仍走response_item。 - 奥卡姆:三条 item 类型 + 文本拼接 + catalog 首条用户文本,范围对。
旧 [P2](317b38e5 的 join \n)
这个 head 上 已经关掉。codexCompletedItemText 对 UserMessage / AgentMessage 是 join('')。新 fixture 把 image / canvas 拆在相邻 text part 里,旧的 join('\n') 会断言失败。Reasoning 的 summary_text 仍用 join('\n'),那是条目之间的分隔,不是把一个词切开。
CI
期望:test。未改 package.json / sandbox。
这个 head 的 CI 是 fork action_required,check-runs total_count=0。空不等于绿。workflow 放行之前门禁不能过。
代码侧 GO(仅 [P3])。门禁 NO-GO(action_required)。未 approve,未合。
| if (!activeTurnIsExplicit) { | ||
| activeTurnId = generatedCodexId(expectedSessionId, 'turn', record.line); | ||
| } | ||
| const text = codexCompletedItemText(item) || codexCompletedItemMediaText(item); |
There was a problem hiding this comment.
[P3] codexCompletedItemText(item) || codexCompletedItemMediaText(item):同一条 UserMessage 里只要有非空 text part,后面的 local_image / audio 就不会变成 [Image] / [Audio]。新 fixture 在两段文本中间夹了 local_image,只为了钉 join(''),导入结果是纯文本,图被丢掉。旧 user_message 路径也是「有 message 就不用 media placeholder」,所以不是新回归,只是 0.149 的混合 content 仍然没有图。不挡这个 PR。
Astro-Han
left a comment
There was a problem hiding this comment.
Gate check at exact head 0b4af5e4: hosted test is terminal completed/success (the only check this path set triggers) and mergeable=true.
I independently re-verified my earlier [P2] on the content-part concatenation contract at this head: codexCompletedItemText now ends in .join(''), so multi-part text is concatenated without an invented separator, matching upstream UserMessageItem::message(). That thread is resolved. The remaining open thread is a [P3] on the text/media fallback ordering.
Approving.
Summary
event_msg.item_completedrecords forUserMessage,Reasoning, andAgentMessage.user_messageandagent_*event shapes.event_msgas the presentation-message source so that correspondingresponse_itemmirrors are not imported twice.Fixes #3516
Refs #2499
Verification
codex-session-adaptersuite: 9 passed.NODE_NO_WARNINGS=1 npm --workspace @maka/storage run test:dist: 899 passed, 14 skipped.npm run lintnpm run format:checknpm run buildnpm run typechecknpm run check:asf-headersgit diff --checkWithout
NODE_NO_WARNINGS=1, one unrelated managed-dependency environment test treats Node.js's experimentalnode:sqlitewarning on child-process stderr as a failure. The full storage suite passes when that warning is disabled.Review focus
The adapter keeps the legacy and Codex 0.149 presentation formats side by side.
event_msgremains the authority for user, reasoning, and assistant presentation messages, whileresponse_itemremains responsible for tool calls and tool results. This preserves the existing duplicate-prevention boundary.AI use
Select exactly one:
Tool(s) and scope:
OpenAI Codex assisted with the rollout-format investigation, implementation, regression tests, local verification, and drafting this description. I reproduced the issue and reviewed the final diff, regression coverage, and test results before submission.
Checklist
Does this PR entail a change in behavior?