Repository navigation
feat(kix): keep the board and inbox accurate on busy projects - #182
Conversation
Quest-Claim: 3dd62499cca25163
A draft PR its starter still holds (no `## Decisions needed`) shows as Claimed on the board and in progress in the inbox; a handed-back draft or ready PR is turned in. The hand-back rule moves to quest/core so the CLI and the Worker agree. GitHub's UNKNOWN merge state keeps a recorded PR's last known state at the same head, and defers its change time at a new head until a definite state arrives. readState reads only open entities and unread pointers through a partial index, the activity log pages, and a closed entity's row is pruned once it has been closed past RETENTION (30 days, pending the user's choice). The seed adds a held draft, a handed-back draft, and an UNKNOWN PR, and its test opens only the items it checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Implementation pushed in dd2a391. (written by Claude Opus 5.5) |
The user chose 30 days, so RETENTION is final and the remaining inbox-accuracy quest is deleted with its links. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
![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: a3ca546dcf
ℹ️ 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".
| SELECT ?,json_extract(value,'$.kind'),json_extract(value,'$.item.databaseId'),json_extract(value,'$.item.number'),?,?,json_extract(value,'$.item'),?,json_extract(value,'$.stale') | ||
| FROM json_each(?) WHERE ${current} | ||
| ON CONFLICT(repository,kind,id) DO UPDATE SET number=excluded.number,changedAt=CASE WHEN github_project_activity.data IS NOT excluded.data THEN excluded.changedAt ELSE github_project_activity.changedAt END,data=excluded.data,seenIn=excluded.seenIn,missing=0,stale=CASE WHEN excluded.stale>0 AND github_project_activity.stale=excluded.stale THEN 0 ELSE github_project_activity.stale END`, | ||
| ON CONFLICT(repository,kind,id) DO UPDATE SET number=excluded.number,changedAt=CASE WHEN ${unknown} AND NOT (${keeps}) THEN github_project_activity.changedAt WHEN ${prior} IS NOT ${data} THEN excluded.changedAt ELSE github_project_activity.changedAt END,data=${data},seenIn=excluded.seenIn,missing=0,stale=CASE WHEN excluded.stale>0 AND github_project_activity.stale=excluded.stale THEN 0 ELSE github_project_activity.stale END`, |
There was a problem hiding this comment.
When an existing PR webhook triggers this reconciliation, webhook.ts has already overwritten the row's changedAt with the delivery time. Consequently, both UNKNOWN branches here retain that new timestamp rather than the timestamp from the last definite observation, so a synchronize delivery that is reread while mergeStateStatus is UNKNOWN still marks the PR unread immediately—the behavior this change is intended to prevent. The tests only exercise direct refreshes after manually resetting changedAt, so they do not cover the production webhook path; retain the pre-webhook version separately or avoid advancing the user-visible timestamp until reconciliation determines a definite change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I disagree that this is a regression here. webhook.ts (unchanged by this PR) sets changedAt on every named delivery by design: the delivery itself, such as a synchronize push, is the news, and that holds whatever the reread later finds, UNKNOWN or not. The UNKNOWN handling targets rereads that have no event of their own, such as history passes after the base branch moves, where GitHub flips mergeability to UNKNOWN and may never send a follow-up event. Those are the spurious unread marks the quest set out to remove.
Making webhook deliveries count as news only once the reread finds a definite change would redefine unread for every webhook event, not just UNKNOWN. I'm leaving that as a possible follow-up rather than widening this PR.
(written by Claude Opus 5.5)
|
Merge summary for head
Enabling auto-merge. (written by Claude Opus 5.5) |
Implements and finishes
quest/m0/kix/inbox-accuracy.md("[M] Keep the inbox accurate on busy projects"). The user settled the retention period at 30 days, so the quest is deleted along with its link inquest/m0/kix/README.md.Behavior
bodyandclosedAt. A PR's body is kept only ashandedBack, meaning it has a## Decisions neededsection. The regex moved intoquest/core(handedBack()), so the CLI's claim checks and the Worker read it the same way.mergeStateStatus: UNKNOWNon a recorded PR:UNKNOWNbut leaveschangedAtunchanged. The item becomes unread once a definite state arrives.readState, which backs/board,/data, the inbox, and jobs, now reads only open entities and unread webhook pointers, through a new partial index. That read grows with open work, not with history. The activity log on/statepages 50 rows at a time with keyset paging and an "Older activity" link. Migration0015_activity_reads.sqlalso adds a partial index forstaleEntities. I checked the query plans with SQLite: the open-rows read uses the partial index whether or not table statistics exist. I dropped a separate log-order index because, without statistics, the planner preferred it over the partial index.RETENTIONdeletes its row instead of upserting it. The exception is a row a webhook named again since the read, which keeps it until it is read by number. History rereads every entity, so each pass prunes every expired row.RETENTIONis 30 days; see Decisions.cloudflare/dev/seed.tsadds a held draft (feat(skills): rename quest-complete to quest-finish #17, onquest/m1/themes), a handed-back draft (feat(skills): add quest-audit #18, on a newquest/m1/search), and a PR (fix(skills): planning PRs leave issues open and summarize the decision #19) that is clean when first read andUNKNOWNwhen the webhook read rereads it.cloudflare/test/seed.test.tschecks each one on the inbox and the board.inbox.test.tscovers the other threads. Idle runs dropped from about 850 ms (with the larger seed) to about 360–570 ms.cloudflare/GITHUB.mdanddesign/theme.mddescribe the new states and retention.quest/m0/kix/execution.mdnotes that the held-draft half of "owned PRs" in the inbox is done and thequest:*label half remains.Retention measurement
Activity rows mirror GitHub entities one to one: one row per issue or PR that the history pass reads. So I measured the counts on GitHub with the search API on 2026-10-21. No pilot deployment exists yet to measure D1 directly.
On moq-dev/moq, the steady-state rows per period are about 0.9k at 7 days, 2.5k at 30, and 3.8k at 90. Without pruning there are 6.3k rows today, growing by about 2.4k a month.
Decisions
The user accepted each recommendation:
RETENTIONis final, with that reason beside it. The alternatives were 7 days (about 0.9k rows, but last month's closed work leaves the log) and 90 days (about 3.8k rows). To check on a live pilot, runSELECT count(*), sum(json_extract(data,'$.state')='OPEN') FROM github_project_activity WHERE repository=<id>against D1 after a full history pass.closedAtandhandedBackto stored data changes every open item's observation on the next read, so open items show as new once, as in kixelated/quest#163. Accepted.Validation
just check: passed.just test: passed after merging main (382 CLI tests, 2 skipped; 312 Worker tests; the hook, installer, and lifecycle suites).quest check --base origin/mainon the head branch: passed.projects.test.ts: held and handed-back drafts (board status, body not stored);UNKNOWNat the same head, at a new head, and on a newly seen PR; pruning on a history read and on a read by number after a webhook; activity-log paging.inbox.test.ts: a held draft is in progress, then becomes a decision once handed back.seed.test.ts: the three seeded states, on both the inbox and the board.BLOCKEDinstead ofUNKNOWNfails the seed test's Merge-button assertion. That confirms the background read does reread fix(skills): planning PRs leave issues open and summarize the decision #19.Suggested follow-ups
ready_for_reviewand non-draft opens, so adding## Decisions neededto a draft makes it a decision without a push. Recommendation: fold this into Execution, which owns PR decision items./quest-planquest to page history byUPDATED_ATand stop at the last pass's high-water mark, and to read bodies only for open drafts.(written by Claude Opus 5.5)
🤖 Generated with Claude Code