Skip to content

[codex-analytics] guardian review truncation#17695

Merged
rhan-oai merged 1 commit intomainfrom
pr17695
Apr 22, 2026
Merged

[codex-analytics] guardian review truncation#17695
rhan-oai merged 1 commit intomainfrom
pr17695

Conversation

@rhan-oai
Copy link
Copy Markdown
Collaborator

@rhan-oai rhan-oai commented Apr 13, 2026

Why

The Guardian review event needs to report whether the action shown to Guardian was truncated. That field should come from the same truncation path used to build the Guardian prompt, rather than being inferred after the fact.

What changed

Plumbs truncation metadata through Guardian action formatting, prompt construction, review session execution, and analytics emission. guardian_truncate_text now reports both the rendered text and whether it inserted the truncation marker, and reviewed_action_truncated is set from that prompt-building result.

This keeps the analytics field aligned with the model-visible reviewed action while preserving the existing Guardian prompt behavior.

Verification

  • Guardian truncation tests cover both truncated and non-truncated action payloads.
  • Guardian review tests assert the review session metadata and truncation field are propagated.

Stack created with Sapling. Best reviewed with ReviewStack.

@rhan-oai rhan-oai force-pushed the pr17695 branch 18 times, most recently from e8b6ed6 to fb3df42 Compare April 16, 2026 17:06
@rhan-oai rhan-oai changed the base branch from main to pr17693 April 16, 2026 17:20
@rhan-oai rhan-oai changed the base branch from pr17693 to main April 16, 2026 17:33
@rhan-oai rhan-oai changed the base branch from main to pr17693 April 16, 2026 18:47
@rhan-oai rhan-oai changed the base branch from pr17693 to main April 16, 2026 19:15
@rhan-oai rhan-oai force-pushed the pr17695 branch 3 times, most recently from a5037a9 to c6596ea Compare April 17, 2026 19:40
@rhan-oai rhan-oai force-pushed the pr17695 branch 7 times, most recently from 8b58144 to cb0ff55 Compare April 21, 2026 06:15
@rhan-oai rhan-oai changed the base branch from main to pr17693 April 21, 2026 06:42
@rhan-oai rhan-oai changed the base branch from pr17693 to main April 21, 2026 21:58
@rhan-oai rhan-oai changed the base branch from main to pr17693 April 21, 2026 23:37
@rhan-oai rhan-oai changed the base branch from pr17693 to main April 21, 2026 23:39
@rhan-oai rhan-oai force-pushed the pr17695 branch 2 times, most recently from 8f0316b to 737b94d Compare April 22, 2026 01:13
@rhan-oai
Copy link
Copy Markdown
Collaborator Author

@codex

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: 737b94d88d

ℹ️ 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 thread codex-rs/core/src/guardian/review.rs Outdated
review_id,
request,
retry_reason,
GuardianApprovalRequestSource::DelegatedSubagent,
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.

P2 Badge Propagate the correct approval source when spawning reviews

spawn_approval_request_review always passes GuardianApprovalRequestSource::DelegatedSubagent, but this helper is also used for main-turn reviews (e.g., session::request_permissions_for_cwd). Those main-turn reviews will now be misclassified in guardian-review analytics, corrupting the approval_request_source field and downstream metrics.

Useful? React with 👍 / 👎.

Comment on lines +699 to +701
return (
GuardianReviewSessionOutcome::PromptBuildFailed(err),
false,
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.

P2 Badge Classify submit failures as session errors

This error path maps all pre-wait failures to PromptBuildFailed, including review_session.codex.submit(...) failures from the guardian runtime/session. Those are later emitted as GuardianReviewFailureReason::PromptBuildError, which mislabels real session/runtime faults and skews failure analytics.

Useful? React with 👍 / 👎.

@rhan-oai rhan-oai force-pushed the pr17695 branch 2 times, most recently from 5d60863 to 6117400 Compare April 22, 2026 06:39
@rhan-oai rhan-oai changed the base branch from main to pr17694 April 22, 2026 07:26
@rhan-oai rhan-oai changed the base branch from pr17694 to pr17693 April 22, 2026 07:26
rhan-oai added a commit that referenced this pull request Apr 22, 2026
## Why

Guardian approvals now run as review sessions, but Codex analytics did
not have a terminal event for those reviews. That made it hard to
measure approval outcomes, failure modes, Guardian session reuse, model
metadata, token usage, and timing separately from the parent turn.

## What changed

Adds `codex_guardian_review` analytics emission for Guardian approval
reviews. The event is emitted from the Guardian review path with review
identity, target item id, approval request source, a PII-minimized
reviewed-action shape, terminal decision/status, failure reason,
Guardian assessment fields, Guardian session metadata, token usage, and
timing metadata.

The reviewed-action payload intentionally omits high-risk fields such as
shell commands, working directories, argv, file paths, network
targets/hosts, rationale, retry reason, and permission justifications.
It also classifies prompt-build failures separately from Guardian
session/runtime failures so fail-closed cases are distinguishable in
analytics.

## Verification

- Guardian review analytics tests cover terminal success,
timeout/cancel/fail-closed paths, session metadata, and token usage
plumbing.
- `cargo clippy -p codex-core --lib --tests -- -D warnings`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/17693).
* #17696
* #17695
* __->__ #17693
Base automatically changed from pr17693 to main April 22, 2026 08:02
@rhan-oai rhan-oai enabled auto-merge (squash) April 22, 2026 08:22
@rhan-oai rhan-oai merged commit 37aadea into main Apr 22, 2026
25 checks passed
@rhan-oai rhan-oai deleted the pr17695 branch April 22, 2026 08:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants