Fix Codex capture and add high-fidelity event and usage tracing - #27
Merged
Conversation
Codex's notify is one program's argv, not a list of callbacks. The old installer appended thirdeye's command to any existing array, which both prevented thirdeye's handler from ever being invoked and passed a bogus extra argument to the incumbent program. install() now owns the whole notify value or nothing: - absent or [] -> notify = [our_cmd] - exactly [our_cmd] -> idempotent no-op - anything else -> raise a conflict (naming the incumbent and the two ways forward), leaving the file byte-identical; unless force=True, which takes over the slot. A force flag on CodexPlatform.__init__ keeps the Platform.install() signature compatible. uninstall() removes notify only when thirdeye owns slot 0 (the program position), so a foreign dispatcher is never clobbered even if our command trails as a mere argument (the corrupt state the old installer produced). Regex-based TOML editing is preserved to keep comments and unrelated sections intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The old codex_rollout.jsonl was 5 lines of hand-authored fiction (top-level payload token fields, a shape Codex never writes) that let a broken extractor pass a green suite. Replace it, and add a second older-schema fixture plus machine-readable and prose companions. - codex_rollout.jsonl: trimmed/scrubbed subset of a real 2026-07-30 rollout (gpt-5.6-sol, codex-cli 0.146.0-alpha.9.2). Keeps all 81 token_count frames verbatim so the repeat report (cumulative 4,784,765) survives: naive per-frame sum 6,832,295 vs true final 6,694,163. 75,030 bytes. - codex_rollout_v0626.jsonl: small 2026-06-26 rollout (gpt-5.5) proving the older token schema with no cache_write_input_tokens still parses. - codex_rollout.expected.json: measured census/token facts the codex-usage and codex-events tasks assert against. - CODEX_README.md: provenance, scrubbing rules, verified numbers. All prompts, code, outputs, and filesystem paths scrubbed; no PII (duncanmckinnon, /Users/, Desktop, Documents) remains. Session UUIDs kept consistent with filenames for rollout-resolution tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The v0626 section claimed codex-cli 0.146.0-alpha.9.2, a copy-paste of fixture 1's version. The shipped codex_rollout_v0626.jsonl reports session_meta.payload.cli_version = 0.141.0. Correct the README to match the actual shipped file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract rollout location and frame iteration into a shared platforms/codex/rollout.py, replacing the ambiguous, injection-prone glob in usage.py. resolve_rollout validates the session id against an anchored whitelist (injection guard), globs only the literal rollout-*.jsonl, rejects symlink escapes from the sessions root, and verifies the first session_meta frame names the expected session (accepting on filename alone when absent). iter_frames streams (offset, frame) tuples and tolerates a truncated final line; end_offset reports the position after the last complete line so bookmarks never land mid-record. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d whitelist Strengthens verification of resolve_rollout against defect #4 (wrong file among multiple candidates) and covers end_offset resume plus valid/overlong session-id boundaries. Test-only; implementation unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce src/thirdeye/platforms/codex/events.py with capture_events_codex, which tail-parses new rollout frames from a stored offset and appends events onto thirdeye's existing vocabulary (session_start, user_message, assistant_message, tool_call, tool_result, notification, error). - Maps both the function_call and custom_tool_call families to tool_call / tool_result so pairing works across Codex versions. - Preserves each frame's call_id in event data; records rollout_offset for post-hoc dedup after a crash-window replay. - Skips token_count frames (usage only) and any unrecognized frame type without raising; fail-soft via @safe_capture. - Documents the accepted append-then-advance replay window in the module docstring; no locking or event-dedup index. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read per-call usage from event_msg/token_count frames' info.last_token_usage instead of the nonexistent payload.input_tokens path (which extracted 0 rows on real rollouts). Mint call_id from the cumulative total_token_usage.total_tokens watermark so Codex's byte-identical repeat reports collapse under usage/read.py's last-wins dedup — naive per-frame summing overcounts by ~2.1%. - No cache arithmetic: Codex's input_tokens is already cache-inclusive. - Absent cache/reasoning keys stay None (absent vs zero); pre-2026-07-30 rollouts lack cache_write_input_tokens. - Replace _resolve_rollout / manual file loop with the hardened resolve_rollout / iter_frames / end_offset from the rollout module. - Add optional rollout_path / model params for a future hooks path. Rewrite tests against fixtures, asserting the reconciliation invariant: the deduped per-call deltas sum to the final cumulative total exactly while the naive per-frame sum differs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the full capture_events_codex scope: frame->event mapping, custom_tool_call family, token_count exclusion, unknown-frame skip, rollout_offset invariants, call_id pairing, offset tailing, the documented replay-duplication tradeoff, and fail-soft paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
notify now resolves the session's rollout file (cached path from usage.state.json, else resolve_rollout), then in one pass appends rollout-derived events, extracts token usage, and advances a single shared offset bookmark in one write_state — events, then usage, then bookmark. The agent_turn event from the notify payload is still recorded first (it uniquely carries input-messages / last-assistant-message), and survives even when the rollout is unresolvable (logged as open_source). Tests sandbox CODEX_SESSIONS_ROOT to tmp_path so notify never touches the developer's real ~/.codex/sessions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Codex capture was effectively broken: the
notifyinstaller produced a valueCodex could not invoke, usage extraction read a frame path that never exists,
and only
agent-turn-completewas ever recorded. This branch fixes the installand extraction defects, derives events from the full rollout frame set, hardens
rollout resolution, and wires the notify hook to capture both events and usage.
Changes
install.py: rewritenotifyinstall to Codex's single-argv semanticsinstead of appending to the existing argv array; add a
forceoverwrite path.usage.py: rewrite token extraction against the realpayload.info.last_token_usageshape ontoken_countframes, handling boththe v0626 and post-v0730 key schemas with no cache arithmetic.
rollout.py(new): hardened rollout resolution and offset-bookmarkediteration that verifies
session_metamatches the expected session id.events.py(new): derive events from the rollout frame types(
message,reasoning, tool calls includingcustom_tool_call,turn boundaries, etc.), skipping unrecognized frames fail-soft.
hooks.py(new): notify hook that reads theargv[1]payload and drivesevent plus usage capture from the resolved rollout.
rollout, an expected-output JSON, and
CODEX_README.mddocumenting them.single-argv semantics, and end-to-end capture; expand usage tests.
Test plan
uv run pytest tests/test_codex_install.py -v— install writes asingle-argv
notifyand theforcepath overwrites without corrupting anexisting entry.
uv run pytest tests/test_usage_codex.py -v— usage rows extract fromlast_token_usageacross both key schemas without overcounting repeats.uv run pytest tests/test_codex_rollout.py -v— rollout resolutiondisambiguates multiple files, handles resume, and enforces the session-id
whitelist.
uv run pytest tests/test_codex_events.py -v— events derive from allhandled frame types and unknown frames are skipped.
uv run pytest tests/test_codex_hooks.py -v— notify hook captures eventsand usage, and does not advance the offset on an events failure.
uv run pytest tests/test_e2e_codex.py -v— end-to-end capture runsagainst fixtures under
tmp_pathonly.uv run ruff format src tests && uv run ruff check src tests— clean.🤖 Generated by workbench pr_writer