fix(tracing): record session outcomes on root spans - #87
Hesham Salman (Iron-Ham) wants to merge 1 commit into
Conversation
Closed Claude, Pi, and Codex root spans never populated output, so root-based summaries and turn evaluators omitted every session outcome even when descendant turns held the final assistant response. Track the last completed turn output in each translator and merge it into the root span at session end (Claude SessionEnd and daemon finalize, Pi session_shutdown and finalize, Codex main-scope Stop). Pi turns now also carry their last completed assistant message; errored or aborted messages are not outcomes.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bac8fcedc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| if output.as_ref().is_some_and(|output| !output.is_null()) { | ||
| self.last_turn_output = output.clone(); |
There was a problem hiding this comment.
When the Stop hook supplies an empty last_assistant_message because the final assistant row is written afterward, this condition records "" as the session outcome. The existing claude_groups_streamed_rows_and_reads_late_final_output_at_session_end flow demonstrates that SessionEnd can subsequently read the real "done" output, but emit_main never refreshes last_turn_output, so the root closes with an empty output instead of the actual answer. Derive or update the tracked outcome when those late transcript rows are consumed.
Useful? React with 👍 / 👎.
| if error.is_none() { | ||
| self.turn_output = Some(output.clone()); |
There was a problem hiding this comment.
When a Pi turn emits a successful intermediate assistant message (for example, a tool call) and its later assistant message ends with stopReason: "error" or "aborted", this branch merely skips replacing turn_output; it leaves the intermediate message intact. close_turn then promotes that stale value to both the failed turn and root, making the failed session appear to have a completed outcome. Clear the current turn outcome on an error or abort, and base that decision on the stop reason even when errorMessage is absent.
Useful? React with 👍 / 👎.
|
Oof sorry yall. Agent opened this up overnight as a PR instead of an issue. Will convert to an issue or ping on Slack |
1 participant
[written by an AI agent working with Notion's internal tracing analytics]
Problem
Closed coding-agent root spans never populate
output. In Notion's internal-coding-agent-traces project, a 30-day scan found the predicateis_root AND metrics.end IS NOT NULL AND output IS NULLheld for 647/647 closed roots (566 Claude Code, 37 Pi, 44 Codex). Pi's completed turn/task spans also omitted output (89/89 in one window), while Codex child turns did populate output and were the positive counterexample.Descendant LLM spans contain the actual answers (e.g. a closed root with null output whose child turn says "Done. PR #346227 is green and addressed."), so root-based summaries, Topics, and Pi turn evaluators systematically omit every session outcome.
Change
Each translator now tracks the last completed turn output and merges it into the root span when the session closes:
stop_turnalready derives turn output from the Stop hook'slast_assistant_message; the value is kept and merged into the root atSessionEndand at daemonfinalize(teardown without a hook).message_endrecords the latest completed assistant message for the active turn;close_turnmerges it into the turn span andclose_rootmerges the last turn's output into the root. Messages withstop_reasonerror/aborted are not treated as outcomes.close_turnalready sets turn output fromlast_agent_message; the main scope keeps it andend_main_rootmerges it into the root.Merges with
output: Noneremain no-ops, so sessions without a completed turn are unchanged. Grok, OpenCode, and Antigravity roots have the same gap but different lifecycle shapes; left out to keep this PR reviewable — happy to follow up.Tests
claude_root_records_last_turn_output_on_session_end/..._on_finalize/claude_root_output_stays_unset_without_a_completed_turnpi_turn_and_root_carry_last_assistant_output/pi_errored_message_is_not_a_turn_or_root_outcomecodex_root_records_last_turn_output_on_stopcargo test --manifest-path bt-daemon/Cargo.toml --all-featuresis green.