Repository navigation
Batch inbox job hydration per project - #259
Conversation
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 305f739
No actionable bugs found. The shared three-query hydration is a sensible simplification: it preserves project-scoped selection, job and child ordering, parsed results, and question/approval fields without an events×questions join. Reusing it for recent jobs avoids divergent hydration paths. The added regressions check field parity, empty children, project isolation, and statement counts through the full inbox load.
Verification limits: static inspection of the diff and relevant store/inbox code only; I did not run tests, benchmark D1, or validate a deployed Worker.
|
Merge summary: all job lists the inbox loads (decision jobs, agent-card jobs, recent jobs) now share one helper that reads them in a fixed three-statement D1 batch, no matter how many jobs there are. Selection, ordering, and UI behavior are unchanged. The finished quest Reviews: Codex code and security reviews of 305f739 completed with no findings. The maintainer-account automated review found no actionable bugs. CI: all required checks passed. The optional native Windows (windows-11-arm) job failed the gate test this PR doesn't touch (it expected four admissions and got three). That failure is already on base, and #233 tracks it. It doesn't block landing. No open decisions. Queuing 305f739 for (written by Claude Opus 5.5) |
Loading the inbox hydrated each decision job and each agent-card job with its own three-query
readJobbatch. Both selectors now hydrate all selected jobs in one three-statement D1 batch, preserving the selection, ordering, attempt state, parsed results, events, questions, answers and approvals. Separate child queries avoid multiplying events by questions. The recent-jobs reader shares the same hydration code.Validation: pinned Nix
just install,just check, andjust testpassed (538 CLI tests passed, 2 platform skips; 363 Worker tests passed). New real-D1 tests compare all hydrated fields with individual reads for 120 jobs, check project isolation and empty children, and count executed statements including batch members. Each selector uses three statements regardless of job count. The full inbox load stays constant; the same test against base reproduced growth from 18 statements with no jobs to 378 with 120 jobs. Both regressions pass with the fix.Choices: batch both decision and activity selectors because both run during an inbox load; reuse the helper for recent jobs to keep one list hydration path. No selection or UI behavior changes. Deleted the finished quest and its links, including its inline mention in the history-pass plan. No remaining decisions.
Current-head review: no actionable bugs; Codex added 👍. Required CI passed. The optional Windows ARM job failed the unchanged gate test (expected four admissions, got three); existing kixelated/quest#233 addresses that base failure.
(written by GPT-6)