Conversation
|
Closing duplicate PR created during a relinked Sapling submit; continuing on #20239. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5bedde563
ℹ️ 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".
| parsed_cmd: exec_input.parsed_cmd.to_vec(), | ||
| source: exec_input.source, | ||
| interaction_input: exec_input.interaction_input.map(str::to_owned), | ||
| started_at_ms, | ||
| completed_at_ms: Some(completed_at_ms), |
There was a problem hiding this comment.
Reuse begin timestamp when emitting ExecCommandEnd
ExecCommandBegin records started_at_ms at emission time, but emit_exec_end recomputes start as completed_at_ms - duration. On rejection paths duration is forced to zero, so end events report started_at_ms == completed_at_ms even after long approval waits. This produces inconsistent begin/end timestamps and under-reports latency in history/analytics.
Useful? React with 👍 / 👎.
| success, | ||
| changes, | ||
| status, | ||
| started_at_ms, | ||
| completed_at_ms: Some(completed_at_ms), | ||
| duration_ms, |
There was a problem hiding this comment.
Preserve patch start time instead of deriving from duration
PatchApplyBegin emits a real started_at_ms, but emit_patch_end derives start from completed_at_ms - duration_ms. For declined/error paths duration is often zero, so completed events collapse start and end to the same instant, overwriting accurate start timing in projected thread items and skewing tool-duration analytics.
Useful? React with 👍 / 👎.
No description provided.