feat(coderd): key chat diff statuses by ref - #29081
DanielleMaywood wants to merge 1 commit into
Conversation
2e6e3f8 to
75833b2
Compare
75833b2 to
1a7076c
Compare
|
@codex review |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7076c4bd
ℹ️ 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".
| chat_id = @chat_id::uuid; | ||
| chat_id = @chat_id::uuid | ||
| ORDER BY | ||
| updated_at DESC, |
There was a problem hiding this comment.
Track the latest reported ref separately from refresh time
When a chat retains multiple refs, ordering by updated_at does not identify the most recently reported ref: every successful background UpsertChatDiffStatus sets that column to NOW(). Consequently, whichever historical ref the worker refreshes last becomes element zero, which getChat, list enrichment, and PublishDiffStatusChange expose as the chat's current diff status; this is especially visible when the current branch has no PR but an older branch does. Preserve a separate report/selection timestamp or otherwise keep refresh writes from changing this ordering.
Useful? React with 👍 / 👎.
e370f06 to
caa4caa
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caa4caa84c
ℹ️ 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".
| refreshed_at = EXCLUDED.refreshed_at, | ||
| stale_at = EXCLUDED.stale_at, | ||
| updated_at = NOW() | ||
| stale_at = EXCLUDED.stale_at |
There was a problem hiding this comment.
Deduplicate telemetry by refresh time
When multiple chat/ref rows share a PR URL and their cached states disagree, this refresh now updates refreshed_at while deliberately leaving updated_at as the report time. GetChatDiffStatusSummary still selects the duplicate with the greatest cds.updated_at, so a more recently reported but no-longer-refreshable row can indefinitely override a freshly refreshed state and misreport open/merged/closed telemetry. Deduplicate that query by refreshed_at (with deterministic fallbacks) rather than the report timestamp.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c6eef7d0cd: the dedup tie-break is now refreshed_at DESC NULLS LAST, updated_at DESC, id DESC, so the summary deduplicates by the most recently refreshed state, matching the query's comment. The option A change in this PR (refresh no longer bumps updated_at) is what made the old updated_at-based tie-break stale, so the fix belongs here.
Note one sub-case of the reported scenario does not mis-count: when a PR is deleted upstream, the worker clears pull_request_state to NULL, and the row drops out of the query's WHERE clause. Only the failed-refresh path (stale state left in place while reports keep bumping updated_at) mis-counts, which the new tie-break resolves.
chat_diff_statuses held one row per chat, so a chat could only track one pull request at a time and a branch switch carried the previous PR's URL onto the new branch's data. Re-key the table to (chat_id, git_remote_origin, git_branch) so every ref the agent reports keeps its own row. Existing rows satisfy the new key without a rewrite: both columns are NOT NULL with '' defaults and the old key capped rows at one per chat. The worker claims stale rows by the full key. Retained refs keep polling on their own schedule, so no pause state or resume protocol is needed. Telemetry reports one diff row per chat via DISTINCT ON. The diff GET's discovery write now keys its upsert to the stored ref. Under the old single-row key an empty ref key was safe; under the composite key it would create a stray row the worker ignores. Old coderd instances must finish before this migration runs: their ON CONFLICT (chat_id) writes have no arbiter under the new key.
caa4caa to
c6eef7d
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
A chat now keeps one row per git ref. Each ref that the agent reports gets its own stored pull request data. Retained refs keep refreshing on their own schedule, so no pause or resume state exists.
This is part 1 of 3. Part 2 adds the API fields. Part 3 adds the git panel views.
Problem
The
chat_diff_statusestable had one row per chat. Because of this, a chat could show data for only one pull request at a time. When a chat switched from one branch to another, the old branch's pull request data stayed with the new branch.Fix
(chat_id, git_remote_origin, git_branch). Each ref keeps its own row. A branch switch adds a new row. It does not change the old row.DISTINCT ON.Notes for review
000591swaps the primary key. The index build takes an ACCESS EXCLUSIVE lock for its duration.CREATE INDEX CONCURRENTLYis not possible because all migrations run in one transaction. This table is low volume, so the lock is acceptable.ON CONFLICT (chat_id)writes have no arbiter once the old key is gone.Generated by Coder Agents.