Skip to content

[codex-analytics] add tool review event schema#18747

Draft
rhan-oai wants to merge 1 commit intopr17090from
pr18747
Draft

[codex-analytics] add tool review event schema#18747
rhan-oai wants to merge 1 commit intopr17090from
pr18747

Conversation

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 604b32c7a4

ℹ️ 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 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 address that feedback".

Comment on lines +585 to +586
self.analytics_events_client
.track_connection_notification(connection_id.0, notification);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Skip analytics tracking for high-volume delta notifications

send_server_notification_to_connection_and_wait records analytics for every notification, including streamed CommandExecOutputDelta chunks sent in command_exec::spawn_process_output. With a 256-item try_send queue, large command output can flood analytics facts and drop higher-value events, harming both performance and telemetry accuracy.

Useful? React with 👍 / 👎.

@rhan-oai rhan-oai marked this pull request as draft April 20, 2026 21:47
@rhan-oai rhan-oai force-pushed the pr18747 branch 3 times, most recently from 14ee419 to c536a3c Compare April 22, 2026 07:02
@rhan-oai rhan-oai force-pushed the pr18747 branch 13 times, most recently from 4b81969 to 9d74ec3 Compare April 29, 2026 17:51
@rhan-oai rhan-oai changed the base branch from main to pr17090 April 29, 2026 17:52
@rhan-oai rhan-oai force-pushed the pr17090 branch 2 times, most recently from 2dcdd29 to 05ee118 Compare April 29, 2026 18:27
@rhan-oai rhan-oai force-pushed the pr18747 branch 2 times, most recently from 71c9b60 to 565e39a Compare April 30, 2026 21:02
@rhan-oai rhan-oai force-pushed the pr17090 branch 2 times, most recently from 4f08aa0 to a1deac4 Compare April 30, 2026 21:12
@rhan-oai rhan-oai force-pushed the pr17090 branch 2 times, most recently from 1df18ff to eeee783 Compare April 30, 2026 23:32
rhan-oai added a commit that referenced this pull request May 1, 2026
## Why

Several analytics event families need the same per-thread attribution
state: the app-server client/runtime associated with a thread and, for
lifecycle-oriented events, the thread metadata captured during
initialization. Keeping connection ids and lifecycle metadata in
separate maps made each consumer rebuild the same thread context and
made subagent attribution harder to resolve consistently.

## What changed

- Replaces the separate thread connection and metadata maps with one
reducer-owned `threads` map.
- Routes guardian, compaction, turn-steer, and turn analytics through
shared thread-state lookups while preserving turn-origin attribution for
turn events and request-origin attribution for steer events.
- Lets newly observed spawned subagent threads inherit their parent
thread connection so later thread-scoped analytics can resolve through
the same state model.
- Adds regression coverage for standalone `SubAgentThreadStarted`
publication plus the `SubAgentSource::ThreadSpawn` parent fallback
through a thread-scoped consumer that depends on inherited connection
state.

## Verification

- `cargo test -p codex-analytics`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/20300).
* #18748
* #18747
* #17090
* #17089
* #20239
* #20515
* #20514
* __->__ #20300
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.

1 participant