Skip to content

fix(codex): preserve interleaved text and tool call block ordering on export - #56

Merged
NishantJoshi00 merged 4 commits into
skillsynchq:mainfrom
Adityakk9031:fix/codex-interleaved-block-ordering
Oct 23, 2026
Merged

NishantJoshi00 merged 4 commits into
skillsynchq:mainfrom
Adityakk9031:fix/codex-interleaved-block-ordering

Conversation

@Adityakk9031

Copy link
Copy Markdown
Contributor

When converting transcripts from Common to Codex via Codex::from_common, push_message_lines buffered all text and artifact blocks across the entire message while pushing Thinking, ToolUse, and ToolResult items immediately. At the end of the turn, the buffered text was flushed as a single response item.

In assistant turns where text precedes a tool call (for example [Block::Text("Checking directory..."), Block::ToolUse { ... }]), this inverted the order in the exported rollout JSONL: the tool call was emitted first, followed by the explanation text. In turns with text before and after a tool call, both text segments were lumped together after the tool call.

This change flushes any accumulated text content before emitting reasoning, function_call, or function_call_output records, preserving the chronological sequence across interleaved turns.

Testing

  • Added from_common_preserves_interleaved_block_ordering in tests/integration/codex.rs verifying that text before and after a tool call maintains exact line ordering in the emitted rollout.
  • Verified round-trip fixpoint test codec_fixpoint_through_common_loses_nothing continues to pass.
  • cargo test -p txcript --no-default-features --features opencode,hermes,search --test integration codex passed.
  • cargo clippy -p txcript --no-default-features --features opencode,hermes,search --all-targets -- -D warnings clean.

@NishantJoshi00 NishantJoshi00 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the Codex export ordering change against current main. Flushing pending message content before thinking, tool calls, and tool results preserves block order without changing other harnesses. The new ordering regression fails with the base serializer and passes with this fix.

Refreshed the branch against main and added the missing lineage: None to the test fixture. All 28 Codex integration tests pass. The full local pre-push gate also passes: formatting, both Clippy configurations, all workspace tests, and both WASM feature configurations on Rust 1.96.1. Required GitHub CI is running on this exact head.

@NishantJoshi00
NishantJoshi00 merged commit c60e686 into skillsynchq:main Oct 23, 2026
8 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