Skip to content

feat(codex): add Codex session transcript adapter support#816

Open
rehan6025 wants to merge 5 commits into
repowise-dev:mainfrom
rehan6025:feat/codex-session-adapter
Open

feat(codex): add Codex session transcript adapter support#816
rehan6025 wants to merge 5 commits into
repowise-dev:mainfrom
rehan6025:feat/codex-session-adapter

Conversation

@rehan6025

@rehan6025 rehan6025 commented Jul 13, 2026

Copy link
Copy Markdown

Summary

  • Added Codex transcript support to the shared session adapter.
  • The adapter now parses Codex session metadata and tool payloads into the normal event format used by the session pipeline.
  • Added regression tests covering discovery and normalization for Codex transcripts.

Related Issues

Test Plan

  • Tests pass (pytest)
  • Lint passes (ruff check .)
  • Web build passes (npm run build) (if frontend changes)

Checklist

  • My code follows the project's code style
  • I have added tests for new functionality
  • All existing tests still pass
  • I have updated documentation if needed

@repowise-bot

repowise-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

✅ Health: 7.6 (unchanged)

📋 At a glance
6 new findings introduced.

⚠️ Change risk: moderate (riskier than 52% of this repo's commits · raw 9.0/10)
This change's risk is driven by:

  • more lines added than baseline
  • more scattered than baseline

📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-15 10:54 UTC
Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks for this @rehan6025, the adapter is wired in cleanly and the seam is right. One blocking issue and a few smaller things.

Blocking: discover() returns nothing in real usage, and the discover test fails.

The last commit switched to a non-recursive glob:

return sorted(path for path in root.glob("*.jsonl") if path.is_file())

But Codex writes rollouts into a date-nested tree (~/.codex/sessions/YYYY/MM/DD/*.jsonl), which the module docstring and #785 both call out. A non-recursive *.jsonl only matches files sitting directly under sessions/, so it finds nothing in the real layout and no transcripts ever reach the miners.

I ran the branch to confirm. test_discover_lists_jsonl_sorted creates nested files and currently fails:

tests/unit/sessions/test_codex_adapter.py::test_discover_lists_jsonl_sorted FAILED
E AssertionError: assert [] == ['a.jsonl', 'b.jsonl']
1 failed, 2 passed

So the "pytest passes" box isn't accurate on the current head. Switching to root.rglob("*.jsonl") fixes both the real-world discovery and the test. Worth double checking whatever motivated the non-recursive change, since it goes against the documented Codex layout.

Fixture isn't real-shaped. tests/unit/sessions/data/codex_session.jsonl uses an invented flat shape (top-level type: "assistant", top-level text and tool_calls[]). Real Codex rollouts, which _fill_response_item and _event_kind are actually built for, use session_meta / response_item / event_msg with a nested payload. The inline test does exercise the real shapes, so the code path is covered, but #785 specifically asks for a real-shaped fixture and to confirm the format first. Suggest replacing the fixture with an actual multi-line rollout.

No miner-level test. #785's "done when" includes the decisions and demand miners producing non-empty output on a fixture with the relevant tool calls. Right now the tests only check normalize() field mapping. Since tool-name normalization is the load-bearing part, one end-to-end test through a miner would guard against regressions the field checks miss.

Smaller stuff:

  • parse_timestamp is duplicated verbatim from claude_code.py. Import it instead of copying.
  • codex.py is missing a trailing newline.
  • discover() ignores repo_root and returns every session on the machine. That works because the miners re-filter by cwd, but a one-line comment noting that would help, since the Claude adapter narrows by repo in discover itself.

The tool-name aliases line up with what the demand miner keys on, so that part looks good. The recursive-glob fix is the one that has to land; the rest is follow-up.

@rehan6025

Copy link
Copy Markdown
Author

Thanks for the detailed review! I'll address the recursive discovery issue and work through the remaining feedback, then push an update.

@RaghavChamadiya

Copy link
Copy Markdown
Member

Thanks @rehan6025, this is a solid update. Discovery is fixed and using iter_glob instead of plain rglob is better than what I suggested, it's the repo's own pruned walker so you get the junk-dir skipping for free. parse_timestamp is imported now, the fixture is real-shaped, and all 90 tests in tests/unit/sessions/ pass.

Two things to sort before this lands.

The miner tests are green but hollow. The fixture is two sessions concatenated (a session_meta on line 1 and another on line 5), and every tool call in the second one is dropped on the floor: lines 8, 10 and 14 have no call_id or id, and _fill_response_item needs one before it appends a ToolUse. Run lines 5-15 on their own and you get zero decisions and zero demand. Everything both miner tests assert is coming from the single search call on line 3.

That also means the decision test is pinning a misattribution:

decision kind=explicit_choice files=['pkg/app.py']
quote: "We chose Flask because it is lightweight, keeps SQLite..."

The Flask quote lives in session 2 and is about outputs/sqlite-signup-demo/app.py. It only gets pkg/app.py because trailing_files is a running window still holding session 1's search hit. So assert decision.files == ["pkg/app.py"] is locking in cross-session bleed. Give the exec calls real call_ids and that assertion should change to the file the decision is actually about. A one-session fixture would sidestep the bleed entirely and is closer to what a real rollout looks like anyway.

session_id is picking up message ids. normalize() falls back to payload.get("id"). On session_meta that's correct, the payload id really is the session. On any other response_item it's the message or tool id:

event session_ids: ['0199f5aa-real-session-uuid', 'tool_09', 'msg_042']

MINED decision: kind=explicit_choice files=['pkg/app.py']
session_id='msg_042' <-- should be '0199f5aa-real-session-uuid'

This one has teeth: decisions.py stamps session_id=event.session_id onto every candidate (330, 355, 376), persists it to staging, and the injection-feedback loop keys correction_quotes(session_id) on it (661-669). Every Codex decision ends up under its own fake session, so that loop can never match. The Claude adapter reads one field with no id fallback, which is why it never hit this. Codex only carries the session id on session_meta, so it needs threading through the iteration rather than guessing per line.

While you're in there: ToolResult hardcodes is_error=False, and the dead-end miner keys on _tool_failure(result.payload, result.is_error), so that miner currently can't fire for Codex at all. Claude does is_error=bool(block.get("is_error")). Happy to take that as a follow-up if you'd rather keep this PR tight, just let me know which.

Minor, all mechanical: ruff check reports 3 errors on the branch (unused datetime import, and I001 on both files), ruff format wants both files, and codex.py is still missing its trailing newline. ruff check --fix && ruff format clears the lot.

@rehan6025

Copy link
Copy Markdown
Author

Codex represents many operations through the generic exec/bash tool, with the actual operation embedded in the command string. The demand miner currently recognizes only search_codebase/get_answer. Should the Codex adapter classify shell commands (e.g. rg) into those logical tool names, or should the miners be extended to understand Codex's generic bash tool?

@rehan6025

Copy link
Copy Markdown
Author

Hi sir @RaghavChamadiya , I just wanted to ask, I believe the adapter should normalize Codex's generic exec tool into canonical tool names (e.g. search_codebase) because the miners only consume canonical names. My remaining question is what heuristic you'd prefer for distinguishing search commands from other shell commands.

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.

[Feature] Codex session adapter: mine decisions and demand-weight docs from Codex transcripts

2 participants