Harden parser + backend: structured status, turn attribution, Postgres CLI - #2
Merged
Merged
Conversation
…ostgres CLI Acts on a technical review. Verified each claim against the real Codex rollout data in ~/.codex/sessions before changing anything. Real fixes: - Postgres CLI was broken: sessions/inspect/stats hard-coded the SQLite table names (FROM sessions), but the PG backend uses codex_* tables and query() never rewrote them. Added a store.table() indirection so the CLI resolves the right physical name per backend. (Chose this over unprefixed PG views, which would reintroduce the exact name collision the codex_ prefix exists to avoid in a shared cc-logger warehouse.) - Tool-call status was shell-centric (scraped 'Process exited with code N'), leaving apply_patch 9/9 'unknown' on real data. Now resolved from the structured events Codex already emits: exec_command_end.exit_code and patch_apply_end.success. On real history this turns apply_patch into 8 success / 1 failure and cuts exec_command unknowns 27 -> 7. The scrape is now a fallback that never downgrades a structured result. - Added turn_id to tool_calls and messages (parser + both schemas + upserts), so every call/message is attributable to its turn (289/289 populated on real data). Enables turn-level questions: which prompt caused a failed patch, which subagent turn burned the most tokens. - launchd: the committed plist hard-coded /usr/bin/python3 and a stale WorkingDirectory. Replaced with a generated install-launchd command (real interpreter + repo path, plutil-valid) plus a placeholder template. Docs/claims tightened to match the code: - Softened 'captures everything'; status/message rows now state exactly what is and isn't resolved. Fixed a docstring that implied event_msg messages were parsed (they're duplicates of response_item/message, which is already complete - 164 assistant + 76 user on real data). - Added a Privacy & security section (this stores prompts/output/secrets). - Noted the CLI now works against Postgres; kept the honest 'no live-DB CI yet' caveat. Not changed (review points that were raw-GitHub-view rendering artifacts, verified against source): files are normally formatted (not one long line), pyproject parses as valid TOML, the plist was valid XML. Tests: added coverage for status-from-*_end events, per-turn attribution, and turn_id round-trip. Full suite green on 3.10-3.13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Acts on a round of technical review. I verified every claim against the real Codex rollout data in
~/.codex/sessionsbefore changing anything — a few review points turned out to be raw-GitHub-view rendering artifacts, and the fixes for the real ones are grounded in what the rollouts actually contain.Real bugs fixed
1. Postgres CLI was broken.
sessions/inspect/statshard-coded the SQLite table names (FROM sessions,FROM tool_calls), but the Postgres backend storescodex_*tables andquery()only translated placeholders — never the table names. So every read command failed against Postgres (ingest worked, which is why it wasn't obvious). Fixed with astore.table()indirection the CLI uses to resolve the physical name per backend.2. Tool-call status was shell-centric. Success/failure was scraped from the string
Process exited with code N, which non-shell tools don't emit — so on real dataapply_patchwas 9/9unknown. The rollouts already carry authoritative outcomes the parser ignored:exec_command_end.exit_codeandpatch_apply_end.success. Consuming those:apply_patchexec_commandThe string scrape is now a fallback that never downgrades a structured result.
3. No turn-level attribution. Added
turn_idtotool_callsandmessages(parser + both schemas + upserts). 289/289 tool calls on real data now carry their turn, enabling "which prompt caused the failed patch / which subagent turn burned the most tokens."4. launchd was non-portable. The committed plist hard-coded
/usr/bin/python3and a staleWorkingDirectory(kai-gtm-agents/codex-logger, which no longer exists). Replaced with a generatedinstall-launchdcommand — writes a plist wired to the real interpreter + repo path,plutil-valid — plus a placeholder template inlaunchd/.Docs tightened to match the code
event_msgagent_message/user_messagewere parsed — they're streamed duplicates ofresponse_item/message(which already captures all 164 assistant + 76 user messages on real data), so re-ingesting them would double-count.Verified as non-issues (not changed)
The reviewer read the GitHub raw view and several claims were rendering artifacts, confirmed against source: the Python files are normally formatted (not "one long physical line"),
pyproject.tomlparses as valid TOML, and the plist was valid XML.Tests
Added coverage for status-resolved-from-
*_end-events, per-turn attribution, andturn_idround-trip. Full suite green; also validatedpip install -e ., thecodex-loggerconsole script, andinstall-launchdoutput (plutil -lint: OK).🤖 Generated with Claude Code