Skip to content

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

Closed
rhan-oai wants to merge 5 commits intomainfrom
pr20482
Closed

[codex-analytics] add tool review event schema#20482
rhan-oai wants to merge 5 commits intomainfrom
pr20482

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

if let Some(analytics_events_client) = analytics_events_client.as_ref() {

P0 Badge Remove stale analytics_events_client reference

apply_bespoke_event_handling no longer takes an analytics_events_client parameter, but this block still reads analytics_events_client.as_ref(). That leaves an unresolved identifier and breaks compilation of codex-app-server before runtime tests can run. The same stale reference pattern appears again later in this function.

ℹ️ 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 +2017 to +2018
started_at_ms: None,
completed_at_ms: None,
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 Populate command approval completion timestamps

complete_command_execution_item emits ItemCompleted with started_at_ms/completed_at_ms set to None. The new analytics reducer drops command tool events when either timestamp is missing (option_i64_to_u64(...)?), so denied/timed-out guardian command executions never produce codex_command_execution_event even though their statuses are mapped.

Useful? React with 👍 / 👎.

Comment on lines +2556 to +2557
started_at_ms: None,
completed_at_ms: None,
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 Preserve file-change timing on denied approvals

Declined/failed file-change approvals are emitted as ItemCompleted with all timing fields None. In the analytics reducer, file-change events require non-null started_at_ms and completed_at_ms, so these rejected outcomes are silently dropped and no codex_file_change_event is recorded.

Useful? React with 👍 / 👎.

@rhan-oai
Copy link
Copy Markdown
Collaborator Author

Closing duplicate stack created during resubmission; canonical PR remains #18747.

@rhan-oai rhan-oai closed this Apr 30, 2026
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