Repository navigation
Conversation
![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: a96641c3e8
ℹ️ 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".
| if (transition === "thread_deleted") { | ||
| deps.db | ||
| .delete(telemetryThreads) | ||
| .where(eq(telemetryThreads.threadId, thread.id)) | ||
| .run(); |
There was a problem hiding this comment.
When a running thread is soft-deleted, its runtime can settle later, but this immediately removes its persisted analytics identity and request linkage. A subsequent lifecycle settlement therefore either recreates a different thread_analytics_id or cannot emit the final outcome, severing the turn from its earlier turn_started and message events. Defer this cleanup until the runtime has settled or the thread is physically removed.
Useful? React with 👍 / 👎.
| if (event.type === "run.started") { | ||
| const requestId = state?.pendingRequestId ?? null; | ||
| db.update(telemetryThreads) | ||
| .set({ activeRequestId: requestId, pendingRequestId: null }) | ||
| .where(eq(telemetryThreads.threadId, threadId)) |
There was a problem hiding this comment.
When reconcileDaemonReportedThreads revives an errored thread because the daemon still reports its existing turn as active, this unconditional run.started handling emits another turn_started and replaces the active request with the usually empty pending request. The same host turn can consequently be counted once as failed and again as a new unavailable-origin turn when it completes; distinguish reconciliation/revival starts from actual dispatch starts.
Useful? React with 👍 / 👎.
| .where(isNull(threads.parentThreadId)) | ||
| .get()?.count ?? 0; | ||
| return rootThreads <= 1; |
There was a problem hiding this comment.
On upgrade, an established installation with exactly one root thread—regardless of how many turns it has already completed—passes this check and labels its next outcome first_turn: true. Deleted or consolidated thread histories can produce the same false positive, corrupting first-turn conversion metrics; this needs a historical marker rather than the current number of root rows.
Useful? React with 👍 / 👎.
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. |
2ea2674 to
46c5a16
Compare
|
This is an automated message. We have closed this pull request because it has been open for more than two weeks. |
Human comments
What was wrong
Work events cannot reliably connect an accepted message to its turn, especially after queueing or retry.
What changed
How you verified
a96641c3e8.failed/auth.trigger: schedule, using the same turn events as other work.207404a8b7; the later Capture anonymous plugin usage through shared boundaries #5439 fix changes only plugin RPC request metadata.Prior implementation verification and review history
41e9b51b.654570f7; its provider privacy finding is fixed and validated without repeating the review.other.40972c58emitted both accepted message IDs onturn_startedandturn_finished. The original QA request used an incomplete expected queue prefix; correcting that fixture request completed the check.Recorded P2 follow-ups
start_failed.first_turnuses current root-thread count rather than historical first completion.Original implementation thread: Retention measurement
BB-Thread: Growth analytics
BB-Thread-ID: thr_a4mhd93zji