From 6d20de1d15c540566c6a7c7ded23e1ea2dcd3006 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Mon, 5 Oct 2026 22:06:40 +0200 Subject: [PATCH 1/3] feat(devex): stop fetching and storing CI checks Decided 2026-10-05: drop CI entirely. Checks were the costliest part of every PR fetch, usually stale by the time anyone looked, and since 2026-09-29 nothing ranks or speaks on them. What was left (a quiet CI event, the pane's neutral Checks fact, the "Until CI is green" snooze, the CLI rollup) did not pay for the fetch. Replaces the checks summary pilot, which is not shipped. - Fetch: no statusCheckRollup and no head-commit connection in the PR query. On 12 PostHog/posthog PRs (100 checks each) the batch response went 621 -> 471 KB and 5.3-8.1 s -> 3.7-4.3 s; rate-limit cost unchanged at 7 points, 1,212 fewer nodes. - Model: Pr.checks, summarizeChecks, the CI event, the 'ci' event kind, the Checks fact, the CLI rollup and the ci_green snooze are gone. Actor-less events are named "GitHub" now (was "CI"). - Old CI events: migration 030 deletes them and their event log rows by id, rather than waiting for re-derivation that never comes for merged and closed PRs. Log readers join pr_event, rows only leave, so nothing turns unseen and no cursor moves (checked on both copies). 0.12 s on normal, 0.5 s on heavy, once. 030 is also what makes 0.20.0 refuse the database after the strip. - Old ci_green snoozes read as until_time at their start: they end like an expired snooze. Unparseable conditions do the same instead of throwing. - Stored JSON: reads drop the old checks key at parse, so it never reaches memory or a local rewrite. Storage job checks_strip removes it on disk, one snapshot per unit with json_remove inside the slice, no revision bump. Fail closed: done only when no snapshot written since the walk started (newer revision) holds checks. Heavy copy: 93 MB of JSON freed in 138 slices (max 54 ms), every snapshot equal to the original minus checks. Hot-set heap 283 -> 264 MB. - Every PR read (getMany, keepParsed, listAll) runs in one read transaction (GPT-6.1 point 4), kept from the pilot as it is generic. NO_CI_RULE stays in the writing prompts: glances and dossiers stored before can still mention CI status. --- CHANGELOG.md | 4 + DESIGN.md | 135 +++++++++++------ NEXT.md | 44 ++++-- apps/cli/src/format.ts | 2 +- apps/desktop/CLAUDE.md | 16 +- .../src/components/DetailPane.test.tsx | 17 +-- .../src/renderer/src/components/PrFacts.tsx | 15 +- .../renderer/src/components/SnoozeMenu.tsx | 1 - .../src/renderer/src/components/icons.tsx | 1 - apps/desktop/src/renderer/src/lib/events.ts | 2 - apps/desktop/src/renderer/src/lib/pr.test.ts | 15 +- apps/desktop/src/renderer/src/lib/pr.ts | 13 +- apps/server/src/app.ts | 1 - apps/server/src/fake/fake-engine.test.ts | 4 +- apps/server/src/fake/fake-quiet.ts | 6 +- apps/server/src/fake/sample-builders.ts | 24 +-- apps/server/src/fake/sample-data.ts | 88 ++++++----- apps/server/src/routes.test.ts | 8 +- packages/agent/src/hashes.test.ts | 10 +- packages/agent/src/prompts-memory.test.ts | 6 +- packages/agent/src/prompts.test.ts | 10 +- packages/agent/src/prompts/dossier-update.ts | 4 +- packages/agent/src/prompts/memory-recheck.ts | 4 +- packages/agent/src/prompts/ping-decision.ts | 4 +- packages/agent/src/prompts/shared.ts | 7 +- packages/agent/src/test-fixtures.ts | 1 - packages/core/src/activity.test.ts | 23 ++- packages/core/src/activity.ts | 8 +- packages/core/src/checks.test.ts | 64 -------- packages/core/src/checks.ts | 53 ------- packages/core/src/delta.test.ts | 9 -- packages/core/src/event-roles.test.ts | 3 - packages/core/src/event-roles.ts | 2 +- packages/core/src/events.test.ts | 17 --- packages/core/src/events.ts | 28 ---- packages/core/src/fixtures.ts | 1 - packages/core/src/index.ts | 1 - packages/core/src/last-touch.test.ts | 4 +- packages/core/src/loudness.test.ts | 13 +- packages/core/src/loudness.ts | 2 +- packages/core/src/pings.test.ts | 7 +- packages/core/src/pr-pane.test.ts | 14 -- packages/core/src/pr-pane.ts | 7 +- packages/core/src/pr-status.test.ts | 8 +- packages/core/src/quiet-reads.test.ts | 73 +++++----- packages/core/src/quiet-reads.ts | 16 +- packages/core/src/saw-before-acting.test.ts | 10 +- packages/core/src/snooze.test.ts | 14 +- packages/core/src/snooze.ts | 12 +- packages/core/src/telemetry-events.test.ts | 1 + packages/core/src/telemetry-events.ts | 7 +- packages/core/src/testing/board-spec.ts | 9 +- packages/core/src/testing/build-board.ts | 12 +- packages/core/src/testing/event-corpus.ts | 15 +- packages/core/src/testing/labels.ts | 6 - packages/core/src/testing/spec-events.ts | 6 +- packages/core/src/testing/spec-rules.ts | 6 +- packages/core/src/tiles.test.ts | 1 - packages/core/src/types.ts | 17 --- packages/core/src/whats-new.test.ts | 8 +- packages/core/src/whose-turn.test.ts | 15 +- packages/engine/src/actions.test.ts | 11 ++ packages/engine/src/bot-noise.test.ts | 6 +- packages/engine/src/engine-telemetry.test.ts | 4 +- packages/engine/src/memory-sync.test.ts | 20 --- .../src/storage-jobs/checks-strip.test.ts | 135 +++++++++++++++++ .../engine/src/storage-jobs/checks-strip.ts | 48 ++++++ packages/engine/src/storage-jobs/jobs.ts | 3 +- packages/github/src/client.test.ts | 14 +- packages/github/src/fixtures/pr-batch.json | 12 -- packages/github/src/normalize.test.ts | 1 - packages/github/src/normalize.ts | 40 ----- packages/github/src/queries.ts | 10 -- packages/github/src/raw.ts | 19 --- packages/store/src/drop-ci.test.ts | 137 ++++++++++++++++++ packages/store/src/migrate.ts | 3 +- packages/store/src/migrations/030_drop_ci.ts | 22 +++ packages/store/src/repos.test.ts | 16 +- packages/store/src/repos/prs.ts | 72 ++++++++- packages/store/src/repos/snoozes.ts | 28 +++- 80 files changed, 786 insertions(+), 749 deletions(-) delete mode 100644 packages/core/src/checks.test.ts delete mode 100644 packages/core/src/checks.ts create mode 100644 packages/engine/src/storage-jobs/checks-strip.test.ts create mode 100644 packages/engine/src/storage-jobs/checks-strip.ts create mode 100644 packages/store/src/drop-ci.test.ts create mode 100644 packages/store/src/migrations/030_drop_ci.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c48fb0c..c48c1f09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Notable changes per release. Versions follow semver. PostPile is alpha software: ## 0.21.0 (unreleased) +### Removed + +- PostPile no longer shows CI status: the PR pane's Checks fact, the CI lines in a PR's activity and the "Until CI is green" snooze are gone. A snooze set that way ends after the update, like one whose time is up. CI results go stale fast and bringing a PR to green is its author's job, so they never drove anything in PostPile; now it doesn't fetch them either. That makes each PR fetch from GitHub smaller and faster (on PRs with many checks about a quarter less data), and the checks already stored are removed once in the background a little after the update. + ### Fixed - A tile that already says "Not yours" no longer offers "Not mine" in its ⋯ menu. Mark read clears it. A stack or set only counts when its verdict says Not yours, so one PR the agent calls Not yours next to one that needs a look keeps the option. diff --git a/DESIGN.md b/DESIGN.md index 00151900..6a40604a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -204,7 +204,7 @@ agent-grouped among pinged and found PRs; the agent never pulls PRs in. | loudness | effect | examples | |---|---|---| | loud | pings, coral "new since you looked", lights up the topic (the tile is unread while its thread is unread on GitHub, loud or not: "GitHub unread is PostPile unread") | mention, review requested, question to the user, a push after the user approved when the agent raises it, the author's push or comment after the user requested changes ("addressed your changes") | -| quiet | dot, no ping | bots, CI (always, see "CI is not a signal"), deploys, merge queue (except trunk taking your own PR out of it: loud, see "Merge queue"), pushes after the user approved (by default), merged without the user's review (never loud; surfaced by the done rule instead, see "Merged without your review") | +| quiet | dot, no ping | bots, deploys, merge queue (except trunk taking your own PR out of it: loud, see "Merge queue"), pushes after the user approved (by default), merged without the user's review (never loud; surfaced by the done rule instead, see "Merged without your review") | | muted | hidden as noise, one click to unmute | bot rebase on a draft | | seen | already read | any of the above after reading, or before the user's own last action on the PR (see "You already dealt with it") | @@ -270,7 +270,7 @@ against the user's own instructions), `does`, `risk`, `othersSaid`. Cached by a hash of its inputs; regenerated only when the PR moves or instructions change. **User actions**: approve (single press, immediate, no undo), mark read, snooze -(until someone replies | new push | CI green, the user's own pick | a time), "ask " (agent +(until someone replies | new push | a time; "until CI is green" until 0.21.0), "ask " (agent drafts a PR comment, user edits and sends), feedback on a tile ("not mine", "not related" for sets, "wrong topic"), chat on a tile. Lasting points from chat come back for the user to place: "Keep for this topic" (tailoring), "Keep @@ -726,25 +726,74 @@ the reasons and does not flip back: merged without your review"). Idea from 2026-09-28: "when people come from vacation, I had 500". +## CI is not tracked + +Decided 2026-10-05 (0.21.0): PostPile fetches no CI at all. The checks were +the costliest part of every PR fetch, usually stale by the time anyone +looked, and CI had been taken off everything that ranks or speaks since +2026-09-29 ("CI is not a signal", below). What was left (a quiet CI event in +the activity list, the pane's neutral Checks fact, the "Until CI is green" +snooze and the CLI's rollup) did not pay for the fetch. + +- **The fetch.** The PR query asks for no `statusCheckRollup` and no head + commit (`commits(last: 1)`, which was there only for the checks). + Measured on 12 merged PostHog/posthog PRs (100 checks each): the + response went from 621 KB to 471 KB and from 5.3–8.1 s to 3.7–4.3 s; the + GraphQL rate limit cost stayed 7 points per batch, 1,212 fewer nodes + (24,492 → 23,280). +- **The model.** `Pr` has no `checks`; `summarizeChecks`, the CI event, the + `ci` event kind, the pane's Checks fact and the CLI's `CI ` are + gone. The GitHub "Checks" tab stays one click away under "Open on GitHub". +- **Old CI events.** Migration 030 deletes the stored `ci` events and every + event log row of one, by id (`:ci::`), also log + rows whose event a newer head commit had already dropped. Every log reader + joins `pr_event`, so nothing a reader showed changes but the CI lines + themselves; rows only leave, so nothing turns unseen and no cursor moves + (on the copies: unseen non-CI events and the highest seq the same before + and after). Chosen over letting re-derivation drop them: merged and closed + PRs are never fetched again, so their CI events would stay forever and + the `ci` kind with them. A PR whose newest event was a CI result can fall + out of the hot set's "active in the last 7 days" a little earlier. It took + 0.12 s on the normal copy and 0.5 s on the heavy one, once. +- **Old "Until CI is green" snoozes.** `SnoozeRepo` reads a condition this + build no longer offers (or cannot parse) as `until_time` at the snooze's + start: it ends like an expired snooze on the next load. The menu and the + server's schema offer only someone replies, a new push and times. +- **Old checks in the stored json.** Reads drop the `checks` key when they + parse a snapshot (`PrRepo` `parsePr`), so they never reach memory or a + rewrite. Storage job 2, `checks_strip`, removes them on disk: one unit is + one snapshot in key order, `json_remove` in place inside the slice's + transaction, no revision (no read changes). Its check: the walk covered + every snapshot stored when it started, and the ones stored behind its + cursor since carry a newer revision; it is done only when none of those + holds `checks` (meta `storage_job:checks_strip:since`), else it walks + again. Migration 030 is also what keeps 0.20.0 off the stripped json. + Measured: heavy 93 MB of json freed in 138 slices (p95 37 ms, max 54 ms), + 4.3 s of work, 11 s wall; normal 6.7 MB in 10 slices. Every snapshot + equals the original but for its checks. Hot-set heap on heavy 283 MB (the + 0.20.0 read) → 264 MB (this build, before the strip) → 263 MB after. +- **What stays.** `NO_CI_RULE` in every writing prompt: glances and + dossiers stored before can still talk about CI status. The MCP + instructions keep saying PostPile does not track CI. CI as a subject of + the work (topic names, the "CI" area, workflow files) is code and stays. + ## CI is not a signal -Decided 2026-09-29. Julian: "I don't think we should focus on or even take in +Decided 2026-09-29, and since 2026-10-05 the CI side of it is gone entirely +("CI is not tracked", above): no checks are fetched, so no prompt, rule or +view can get them. What still holds from here is how agents treat CI in older +stored text. Julian: "I don't think we should focus on or even take in any CI at any point because that's too fuzzy. It could flake, it could fail at any time, and it's always the responsibility of the author to bring the PR to green. Except for maybe some details in the detail pane, we shouldn't highlight it or put it into text or into any risk." -**The rule.** CI status (the check rollup, check results, `ci` events) never -drives anything the app says or ranks: - -- *No prompt gets it.* `prDetails` has no `CI:` line, the rendered dossier's - timeline has no "CI failing" (`prStateWords`), and `ci` events are left out - of the ping decision, memory recheck and dossier update prompts - (`withoutCi`). The topic delta drops them (`selectTopicDelta`), so a - CI-only change starts no dossier update and bumps no dossier version; the - digest cursor still moves past them, so they are not read again. - Checks were never in the glance hash (`prGlanceSnapshot`) and stay out, so - a re-run never makes a glance stale. +**The rule.** CI status never drives anything the app says or ranks: + +- *No prompt gets it.* `prDetails` has no `CI:` line and the rendered + dossier's timeline has no "CI failing" (`prStateWords`). Until 0.21.0 the + `ci` events were left out of the prompts and the topic delta; now there are + none. - *The writing agents are told.* Every prompt that writes something the user reads carries `NO_CI_RULE`: glance, dossier update, ping decision, memory recheck, chat, topic assignment, sets, consolidation. No CI or check @@ -753,20 +802,12 @@ drives anything the app says or ranks: claim answers drop (or fix without the CI part), never holds. The draft comment prompt is the exception: it writes the user's own ask. The MCP server's instructions say PostPile does not track CI. -- *Not a move.* Whose turn has no `fix_ci`; failing CI on the user's own PR - is not their move by itself. -- *Never loud.* `ci` events are machine activity: quiet by the rules, never a - ping (bot-only activity), never an unread reason. The events agent never - sees them (it only judges loud events and pushes after approval), so - nothing overrides that. +- *Not a move.* Whose turn has no `fix_ci`. **What stays, and why.** CI as a subject of the work is code, not status: topic names ("Move CI to Depot"), the "CI" area, changed workflow files in the prompt, and the events agent raising a push that makes a substantial change -in CI, build or devex areas the user approved. The detail pane keeps the -"Checks" fact (`PrFacts`) as a neutral detail, no more prominent than now, -and the activity list keeps CI results in the folded bot/CI line. The snooze -option "Until CI is green" (`ci_green`) stays: the user picks it. +in CI, build or devex areas the user approved. **History.** Design 3a (2026-09-29 morning) took CI off rows, tiles, the detail state line and the RISK box, leaving it only in the facts. CI still fed @@ -777,7 +818,9 @@ Existing glances and dossiers that mention CI are not regenerated on purpose (no `GLANCE_PROMPT_VERSION` / `DOSSIER_PROMPT_VERSION` bump): a glance goes stale on the PR's next push, review or human comment, a dossier on the topic's next real activity, and a bump would rewrite every one of them in -one go for a line that fades on its own. +one go for a line that fades on its own. Until 0.21.0 the pane kept a +neutral Checks fact, the activity list a quiet CI line, and the snooze menu +"Until CI is green". ## Merge queue @@ -1159,7 +1202,7 @@ stale facts and claims, and topic feedback. It: - adds `joinedHistory`: log entries of joined members at or below the cursor (`joinedMembers` decides who joined) -- drops noise (see "Event roles": muted, CI, bot status refreshes), keeps +- drops noise (see "Event roles": muted, bot status refreshes), keeps ride-along bots (the prompt compacts them to counts) - caps at `DELTA_LIMITS.maxEvents` (120) with at most 15 per PR, newest kept; the rest are only counted in `omittedEvents` @@ -1198,7 +1241,7 @@ core `event-roles.ts`), first match wins: | Event | Role | | --- | --- | | effective loudness loud (incl. the app's Look closer, an event the agent raised) | trigger | -| muted (by rule, agent or user), CI result | noise | +| muted (by rule, agent or user) | noise | | a push (commits pushed, after approval, force push), from a person or a bot | ride_along (since 2026-10-05; a loud push, answering the viewer's changes request, is a trigger by the first row) | | anything a person did, the viewer included | trigger | | a state change, whoever did it: merged, merged without review, closed, reopened, ready for review, back to draft, review requested or removed, a bot's approval; Look closer even when turned down | trigger | @@ -1302,8 +1345,7 @@ assignment and consolidation, where many topics share one prompt. **Dossier update prompt** input, in order: the rendered previous dossier (or "none yet"), the user's context block, member PR state lines, intros of joined PRs, the new events (short ids `e1..eN`, bots compacted to counts -per PR; CI results are dropped from the delta, so a CI-only change starts no -update), left PRs, known facts (short ids `F1..Fn`), stale facts to +per PR), left PRs, known facts (short ids `F1..Fn`), stale facts to recheck with their stale reason, new feedback. The answer (JSON, `dossierUpdateOutput`) is the whole new dossier plus `flags`, `facts`, `closeFacts`, `confirmedFactIds`. Refs in the answer are the short ids; the @@ -2718,9 +2760,8 @@ chats. `PrPaneView` (`prPaneView` in `pr-pane.ts`), not the stored `Pr`: header fields, the description whole, the files with their counts (key files read them), reviews as author, state and time (no text), the last commit's time -and a checks summary (`summarizeChecks` in `checks.ts`: rollup, passed, -failed, pending, newest finish, FAILURE names; the pane shows only the -counts). No comments, threads, commits, timeline or check contexts: the +and nothing about checks (CI is not tracked since 0.21.0). No comments, +threads, commits or timeline: the activity list, its bodies and its reply and react targets come built on `PrDetail.activity`, made from the stored PR before the view. The biggest open PR on a real copy went from 1.18 MB to 389 KB per open (its `pr` part @@ -2853,7 +2894,8 @@ A one-time rewrite of stored data that needs JS (parse, cut, rebuild) runs as a storage job, never in a numbered migration: a migration runs at startup in one transaction on Electron's main thread, and on a heavy install that is seconds of a frozen app and a WAL the size of the rewrite. -The bot body trim is job 1; the PR snapshot normalization (NEXT.md) adds +The bot body trim is job 1, the strip of the old checks (`checks_strip`, +"CI is not tracked") job 2; the PR snapshot normalization (NEXT.md) adds its backfills and strips as later jobs. Code in `packages/engine/src/storage-jobs/`: `runner.ts` (`StorageJobRunner`), `jobs.ts` (the ordered list), one file per job. Checked with Codex @@ -3028,10 +3070,8 @@ amber). State colors stay on done tiles; only titles and counts go grey. **CI sh detail pane's facts** ("Checks"): not on rows, tiles, the detail state line or the RISK box, and not in whose turn or any agent text (see "CI is not a signal"). -The Checks fact itself is neutral (2026-09-29): a grey bar (passing a notch -darker than the rest) and "12 checks · 2 not passing" (failed and running -together; "all passing" at none) in the normal muted text, no pass or fail -colour. The Size fact next to it draws deletions (count and bar) in their +The Checks fact is gone since 0.21.0 ("CI is not tracked"); until then it +was neutral, a grey bar and "12 checks · 2 not passing". The Size fact draws deletions (count and bar) in their own diff red (`--diff-red`), never coral: coral stays for "new". **PR rows** (`PrRow`): state icon, the coral dot for an unread PR @@ -3462,7 +3502,7 @@ as `PrSummary.whatsNew` and `PrDetail.whatsNew`; words from the renderer's only its text changes, and only when the viewer already touched the PR before the new loud events. Rules: -- New events are the unseen loud ones, the same loud news that pings. Quiet bot, CI and other events never change the text or the count. +- New events are the unseen loud ones, the same loud news that pings. Quiet bot and other events never change the text or the count. - A touch is one of the viewer's own events (review, approval, changes request, comment, a push to their own PR, a merge or close they did; core `lastTouch` in `last-touch.ts`, shared with "You already dealt with it"), @@ -3715,8 +3755,8 @@ and detail start equally wide (2026-09-30, was a 420-480px tile clamp). At LOOKED · since your changes request yesterday" (anchor from `whatsNew`, relative day from `whenLabel`; plain header on a first look). Up to 3 loud lines (`activity.fresh`, same rows as the activity list), then "N - more"; the quiet bot and CI events since the touch fold into one line - (`activity.freshNoise`, `noiseSummary`: "10 bot comments, CI") that + more"; the quiet bot events since the touch fold into one line + (`activity.freshNoise`, `noiseSummary`: "10 bot comments, a deploy") that expands. The activity list below no longer repeats any of it. - **Look at first** (2026-09-29, `KeyFiles`): the glance's `keyFiles`, up to 3 changed files a reviewer should open first with the agent's why (max 12 @@ -5299,8 +5339,8 @@ too). 1. *Read before, bots since.* GitHub has the thread unread, it has a `last_read_at`, and every stored event by someone else after it is - automation (`event.isBot`, or no actor at all: CI results carry an empty - actor and are flagged as bots already). The viewer's own events (a + automation (`event.isBot`, or no actor at all, named "GitHub"; CI results + had an empty actor until 0.21.0). The viewer's own events (a review from the CLI does not move the read time) are not someone else's activity and are left out (2026-09-29; before they blocked the rule). No known event by someone else after the read counts as "don't know": left @@ -6424,8 +6464,8 @@ topic names are never event props. (origin `tile`, `detail`, `debug`, `cleanup`, `agent_tile` or `agent_topic`; count is the tile count for the agent ones), `team_request_removed` (no props: no PR, no team slug), `snoozed` - (the condition name for an event-based snooze — someone replies, a push, - CI green — or a time bucket for `until_time`), `opened_on_github`, + (the condition name for an event-based snooze — someone replies or a + push; `ci_green` until 0.21.0 — or a time bucket for `until_time`), `opened_on_github`, `ask_sent` (the Ask popover's send), `reply_sent` (target `thread` or `comment`, 2026-10-05), `reaction_sent` (a thumbs up, 2026-10-05), `chat_message_sent` (tile and topic chat), `mac_ping_shown` / @@ -6471,7 +6511,8 @@ topic names are never event props. `work_shed` (skipped_prs: PRs with news that syncs and polls left alone in the last hour because they are outside the hot slice; at most hourly, since 0.18.0, see "Big inboxes: what PostPile loads and works on"), - `storage_job_done` (name, one of the known jobs (`bot_body_trim`); + `storage_job_done` (name, one of the known jobs (`bot_body_trim`, + `checks_strip`); units, work_ms, longest_slice_ms, wall_ms: a background storage job finished and its check passed, this run's share of it, see "Storage jobs"; since 0.20.0), @@ -7095,7 +7136,7 @@ generated boards instead of a handful of fixed ones. `@postpile/core/testing` holds the board recipe (`boardSpecArb`: one topic, 1-3 tiles, 1-4 PRs each as single, stack or set, pinged, found or pulled in; authors viewer, teammate, other or bot; a short history of review requests, comments and mentions, -reviews, pushes, readiness, merge or close, CI; thread read state, handled, +reviews, pushes, readiness, merge or close; thread read state, handled, in-app approval, per-PR and whole-tile snoozes, glance verdicts, Look closer, agent overrides, truncated or stale snapshots, pending writes), `buildBoard` (snapshots from the steps, events from `deriveEvents` made seen the way the @@ -7108,7 +7149,7 @@ core). Where rules differ on purpose the invariant names the exception cases the first runs found were decided 2026-09-30 (above) and their exceptions removed, each with a scenario next to its property. `properties/coverage.test.ts` fails when a branch-relevant label (PR state x author, request target, review state -including dismissed, CI, thread and seen state, snooze kind and phase, +including dismissed, thread and seen state, snooze kind and phase, truncated, and the shapes past bugs needed) shows on under 1% of boards. Each invariant checks 2000 boards by default (the property files take about 4s, `pnpm test` about 7s); `POSTPILE_PROPERTY_RUNS=10000 pnpm test` checks diff --git a/NEXT.md b/NEXT.md index 3b8c0256..9b418232 100644 --- a/NEXT.md +++ b/NEXT.md @@ -6,6 +6,17 @@ now". ## Done +- No CI (2026-10-05, for 0.21.0; DESIGN.md "CI is not tracked"; step 3 of + normalizing the PR snapshot, Later): the PR query asks for no checks, + `Pr` has none, the CI event, the pane's Checks fact, the CLI rollup and + the "Until CI is green" snooze are gone (a stored one ends like an + expired snooze). Migration 030 deletes the CI events and their log rows; + the storage job `checks_strip` removes the old checks from the stored + JSON; reads drop them meanwhile; every PR read runs in one read + transaction. Measured: on 12 PostHog PRs the batch response 621 → 471 + KB and 5.3–8.1 → 3.7–4.3 s; heavy copy 93 MB of JSON freed in 138 slices + (max 54 ms), migration 0.5 s; hot-set heap 283 → 264 MB. Not tried by + hand: the app on a real database. - No "Not mine" on a Not yours tile (2026-10-05, for 0.21.0; DESIGN.md Product model › "Action details"): core's `TileOffers.notMine` leaves it out of the tile's ⋯ menu while the verdict pill says Not yours, read from @@ -1320,15 +1331,14 @@ the app meanwhile. (`pr`, migration 028), the rest of each PR is one JSON blob in `pr_snapshot.json`: comments are half of it (95% of their text from bots, cut since 0.19.0), thread comments and review bodies are second - copies of comments, and check contexts are 10% that no rule reads. So a + copies of comments, and check contexts were 10% (dropped in step 3). So a hot board parses whole PRs to read a few fields. Design checked with Codex GPT-6.1 (2026-10-05); its review points win where they differ from the first draft. Estimate from prototyped tables, hot set of 1,500 PRs on the heavy copy: 284 MB of heap today, about 140 MB with comment rows, about 60 MB with the board diet. The plan, one PR each, in this order: 1. Newer-schema guard: `openDatabase` refuses a database from a newer - PostPile (DESIGN.md "Safety while building"). Ships before anything - destructive. + PostPile (DESIGN.md "Safety while building"). Done, 0.20.0. 2. One storage job runner (`packages/engine/src/storage-jobs/`), with the bot body trim ported as its first job under the trim's existing meta keys. Fails closed (never `done` unless the job's check passes), @@ -1336,14 +1346,17 @@ the app meanwhile. reschedule on SQLITE_BUSY, ~30 ms slices 50 ms apart, pauses while sync, poll, consolidation or catch-up run and while the Mac sleeps, cursor and done flag in the unit's transaction, telemetry - `storage_job_done`. - 3. Checks summary pilot (migration 030): `pr.rows_version` and `check_*` - header columns (rollup, passed / failed / pending / total, newest - finish, FAILURE names), dual-write, a backfill job, the read switch. - Next, after the runner. + `storage_job_done`. Done, 0.20.0. + 3. Drop CI checks (decided 2026-10-05, replacing the checks summary + pilot): no checks fetched or stored, CI events deleted (migration + 030), the old checks stripped from the stored JSON by the storage job + `checks_strip`, every PR read in one read transaction (DESIGN.md "CI is + not tracked"). Built for 0.21.0. `rows_version` waits for step 4. 4. Comments, reviews and threads as rows (`pr_comment`, `pr_thread`, - `pr_review`, header `mentioned_teams`), shipped in one release together - with the strip of the switched fields from the stored JSON. + `pr_review`, header `mentioned_teams`, `rows_version`), shipped in one + release together with the strip of the switched fields from the stored + JSON. The first phase that runs the dual-write, backfill and read + switch protocol. 5. Board diet: board reads leave out bot bodies no rule reads (`isBodyReadByRules`), `FullPr` for the readers that need every body (event derivation, write actions, lessons, "Why?" excerpts). @@ -1412,6 +1425,17 @@ the app meanwhile. ## Decided +- **Drop CI checks: costly to fetch, usually stale, deprioritized** + (2026-10-05, DESIGN.md "CI is not tracked"). PostPile fetches no + checks, keeps no CI event, shows no Checks fact and offers no "Until CI + is green" snooze. Replaces the checks summary pilot (a summary on the PR + header with a backfill; built, not shipped). Fetching the checks was the + costliest part of a PR fetch (on PostHog PRs with 100 checks: a quarter of + the response, and the query ran in about 60% of the time without them), + and CI had been off everything that ranks or speaks since 2026-09-29. The + stored CI events go in migration 030 rather than with the next + re-derivation, which never comes for merged and closed PRs. + - **No "Not mine" where the tile already says Not yours** (2026-10-05, owner report): the menu offered to teach the agent what its verdict already said. Mark read is the way to clear such a tile. A stack or set counts as diff --git a/apps/cli/src/format.ts b/apps/cli/src/format.ts index 81be801e..2fae5034 100644 --- a/apps/cli/src/format.ts +++ b/apps/cli/src/format.ts @@ -96,7 +96,7 @@ export function formatPr(detail: PrDetail, events: EventView[]): string { const { pr } = detail; const lines = [ `${pr.key} ${pr.title}`, - `${pr.state.toLowerCase()} by ${pr.author}, +${pr.additions} -${pr.deletions}, CI ${pr.checks.rollup.toLowerCase()}`, + `${pr.state.toLowerCase()} by ${pr.author}, +${pr.additions} -${pr.deletions}`, pr.url, `topic ${detail.topicId ?? 'none'}, tiles ${detail.tileIds.join(', ') || 'none'}`, ]; diff --git a/apps/desktop/CLAUDE.md b/apps/desktop/CLAUDE.md index 8c566909..6825ffbb 100644 --- a/apps/desktop/CLAUDE.md +++ b/apps/desktop/CLAUDE.md @@ -107,9 +107,9 @@ with a `title` that says why. Hiding it makes the gap invisible to the next agen type (`views.ts`), fill it in the engine's `read-models.ts` and in `FakeEngine`, then read it here. - `PrDetail.pr` is core's slim `PrPaneView` (`pr-pane.ts`), never the - stored `Pr`: no comments, threads, commits, timeline or check contexts. - A new pane field goes into `prPaneView`, which both engines call. The - Checks fact reads `pr.checks` (a `ChecksSummary`), "pushed" reads + stored `Pr`: no comments, threads, commits or timeline, and no checks at + all (PostPile does not fetch CI since 0.21.0). A new pane field goes into + `prPaneView`, which both engines call. "Pushed" reads `pr.lastCommitAt`; comment text, Reply and Thumbs up come from `PrDetail.activity`. Its rows are core's `ActivityEvent` (lines extend it with body, `eventCount` and the reply): no raw events, and no @@ -340,7 +340,8 @@ with a `title` that says why. Hiding it makes the gap invisible to the next agen actions). `text-faint` (2.3-2.6:1) is decoration only: separators, chevrons, ages next to a louder line, done tiles. - **Diff red**: `--diff-red` for deletions in the Size fact. Coral - (`unread`) is never a diff or CI colour; the Checks fact is grey. + (`unread`) is never a diff colour. There is no CI anywhere (DESIGN.md "CI + is not tracked"). - **One colour per meaning** (2026-10-01, DESIGN.md "Colour per meaning"): `closer` only for the agent's Look closer; `status-bad` the one red for bad (closed, changes requested, risk, errors); `safe` the one @@ -500,7 +501,7 @@ detail and fix commands); `lib/tools.ts` only picks where it shows, and stack (`stackQueueWord`, "Merge queue: with 3/3"): the top branch holds their commits, so they merge with it. Review state: `StateWordLabel` with a `StateWord` from `reviewWord` / `rowStateWord` (`lib/pr.ts`). Never a CI - icon or word outside `PrFacts`. + icon or word: PostPile has no CI data. - Icons carry words: a lone icon gets a `title` (and `aria-label` when it is the only content of a control). @@ -543,9 +544,8 @@ tints (`lib/why.ts`, `lib/events.ts`, `reviewWord` / `rowStateWord` in `lib/pr.t state keeps its color on done tiles; title and counts go grey. When only agents approved (`PrStatus.agentApprovers`) the word reads "Approved by agent", names in the tooltip; the detail uses `PrDetail.agentApprovers` - with `approvedText` in `lib/pr.ts`. **No CI on rows, tiles, the detail - state line, the RISK box or the your-move chip**: checks only show in - `PrFacts` (DESIGN.md "CI is not a signal"; `PrStatus` has no checks). + with `approvedText` in `lib/pr.ts`. **No CI anywhere**: PostPile fetches no + checks (DESIGN.md "CI is not tracked"; `PrStatus` and `PrPaneView` have none). - PR rows: a single-PR tile's row has no title (`PrRow` `showTitle` false; the heading is the title). The author's avatar is who opened it (`PrSummary.author`, a bot for agent PRs); when someone else is assigned, diff --git a/apps/desktop/src/renderer/src/components/DetailPane.test.tsx b/apps/desktop/src/renderer/src/components/DetailPane.test.tsx index 46075c20..67a6f1a6 100644 --- a/apps/desktop/src/renderer/src/components/DetailPane.test.tsx +++ b/apps/desktop/src/renderer/src/components/DetailPane.test.tsx @@ -162,25 +162,16 @@ describe('DetailPane', () => { commits: [makeCommit({ committedAt: at(30) })], reviews: [makeReview({ id: 'r1', author: 'lyra', state: 'APPROVED', body: 'Ship it.' })], comments: [makeComment({ id: 'c1', author: 'bob', body: 'Why one key for all jobs?', url: 'https://github.com/acme/app/pull/11#issuecomment-1' })], - checks: { - rollup: 'FAILURE', - contexts: [ - { name: 'lint', conclusion: 'SUCCESS', completedAt: at(40) }, - { name: 'test', conclusion: 'FAILURE', completedAt: at(45) }, - { name: 'e2e', conclusion: null, completedAt: null }, - ], - }, }); const comment = eventView(makeEvent({ id: 'acme/app#11:comment:c1', prKey: stored.key, actor: 'bob', sourceId: 'c1', summary: 'bob commented' })); - const ci = eventView( - makeEvent({ id: 'acme/app#11:ci:head:FAILURE', prKey: stored.key, kind: 'ci', actor: '', isBot: true, summary: 'CI failed: test', ruleReason: 'bot activity', seenAt: at(50), at: at(45) }), + const deploy = eventView( + makeEvent({ id: 'acme/app#11:deploy:d1', prKey: stored.key, kind: 'deploy', actor: 'vercel[bot]', isBot: true, summary: 'Preview deployed', ruleReason: 'bot activity', seenAt: at(50), at: at(45) }), ); - const detail = detailOf(stored, activityList([comment, ci], viewer, null, stored)); + const detail = detailOf(stored, activityList([comment, deploy], viewer, null, stored)); expect(detail.pr).not.toHaveProperty('comments'); renderCached(stored.key, detail); - expect(screen.getByText('3 checks · 2 not passing')).toBeTruthy(); expect(screen.getByText(/^pushed /)).toBeTruthy(); expect(screen.getByText('One key for every job.')).toBeTruthy(); expect(screen.getByText('lyra').parentElement?.textContent).toContain('approved'); @@ -189,6 +180,6 @@ describe('DetailPane', () => { expect(screen.getByRole('button', { name: /^Reply$/ })).toBeTruthy(); // The folded bot/CI rows draw from the slim items too: summary, and the reason in the hover title. fireEvent.click(screen.getByRole('button', { name: 'Show 1 bot/CI event' })); - expect(screen.getByText('CI failed: test').closest('[title]')?.getAttribute('title')).toBe('seen: bot activity'); + expect(screen.getByText('Preview deployed').closest('[title]')?.getAttribute('title')).toBe('seen: bot activity'); }); }); diff --git a/apps/desktop/src/renderer/src/components/PrFacts.tsx b/apps/desktop/src/renderer/src/components/PrFacts.tsx index ee3c1754..9d647310 100644 --- a/apps/desktop/src/renderer/src/components/PrFacts.tsx +++ b/apps/desktop/src/renderer/src/components/PrFacts.tsx @@ -1,6 +1,6 @@ import type { ReactNode } from 'react'; import type { PrPaneView } from '@postpile/core'; -import { checksNote, mergeStatus } from '../lib/pr.ts'; +import { mergeStatus } from '../lib/pr.ts'; import { ageLabel } from '../lib/time.ts'; import { useNow } from '../lib/use-now.ts'; import { SectionLabel } from './SectionLabel.tsx'; @@ -29,12 +29,11 @@ function Fact(props: { label: string; children: ReactNode }) { ); } -/** Size, checks, age and what stands between the PR and a merge. */ +/** Size, age and what stands between the PR and a merge. No checks: PostPile does not fetch CI (0.21.0). */ /** `agentApprovers` (`PrDetail.agentApprovers`) lets "To merge" say "approved by reviewbot (agent)". */ export function PrFacts(props: { pr: PrPaneView; agentApprovers: string[] }) { const now = useNow(); const { pr } = props; - const { checks } = pr; const pushedAt = pr.lastCommitAt; const age = pr.mergedAt ? `merged ${ageLabel(pr.mergedAt, now)}` : `opened ${ageLabel(pr.createdAt, now)}`; return ( @@ -53,16 +52,6 @@ export function PrFacts(props: { pr: PrPaneView; agentApprovers: string[] }) { ]} /> - {/* Neutral on purpose: CI is not a signal in PostPile, so no pass or fail colour (2026-09-29). */} - - {checks.total === 0 ? 'none' : checksNote(checks)} - - {age} diff --git a/apps/desktop/src/renderer/src/components/SnoozeMenu.tsx b/apps/desktop/src/renderer/src/components/SnoozeMenu.tsx index 707a792d..61af1582 100644 --- a/apps/desktop/src/renderer/src/components/SnoozeMenu.tsx +++ b/apps/desktop/src/renderer/src/components/SnoozeMenu.tsx @@ -13,7 +13,6 @@ function tomorrowAtNine(): string { const OPTIONS: { label: string; condition: () => SnoozeCondition }[] = [ { label: 'Until someone replies', condition: () => ({ kind: 'someone_replies' }) }, { label: 'Until a new push', condition: () => ({ kind: 'new_push' }) }, - { label: 'Until CI is green', condition: () => ({ kind: 'ci_green' }) }, { label: 'For 1 hour', condition: () => ({ kind: 'until_time', until: new Date(Date.now() + 3_600_000).toISOString() }) }, { label: 'Until tomorrow 9:00', condition: () => ({ kind: 'until_time', until: tomorrowAtNine() }) }, ]; diff --git a/apps/desktop/src/renderer/src/components/icons.tsx b/apps/desktop/src/renderer/src/components/icons.tsx index 2d6bb859..33a62da5 100644 --- a/apps/desktop/src/renderer/src/components/icons.tsx +++ b/apps/desktop/src/renderer/src/components/icons.tsx @@ -321,7 +321,6 @@ const GLYPH_PATHS: Record = { closed: `${ring(4, 3.5, 1.6)} ${ring(4, 12.5, 1.6)} ${ring(12, 12.5, 1.6)} M4 5.1v5.8 M12 7.5v3.4 M10.3 2.3l3.4 3.4 M13.7 2.3l-3.4 3.4`, ready: `${ring(4, 3.5, 1.6)} ${ring(4, 12.5, 1.6)} ${ring(12, 12.5, 1.6)} M4 5.1v5.8 M12 10.9V6a2 2 0 0 0-2-2H7.5 M9 2.5L7.5 4 9 5.5`, draft: `${ring(4, 3.5, 1.6)} ${ring(4, 12.5, 1.6)} ${ring(12, 12.5, 1.6)} M4 5.1v5.8 M12 8v.01 M12 5v.01`, - ci: `${ring(8, 8, 6.2)} M5.8 5.8l4.4 4.4 M10.2 5.8l-4.4 4.4`, deploy: 'M8 11.5V2.5 M4.5 6L8 2.5 11.5 6 M3 14h10', queue: 'M2.5 4h7 M2.5 8h7 M2.5 12h7 M12 6.2l2 1.8-2 1.8', bot: 'M3.5 6h9v7h-9z M8 3.5V6 M6 9.3v.01 M10 9.3v.01 M1.5 9v2 M14.5 9v2', diff --git a/apps/desktop/src/renderer/src/lib/events.ts b/apps/desktop/src/renderer/src/lib/events.ts index d509d1d0..70460b9c 100644 --- a/apps/desktop/src/renderer/src/lib/events.ts +++ b/apps/desktop/src/renderer/src/lib/events.ts @@ -19,7 +19,6 @@ export type EventGlyph = | 'closed' | 'ready' | 'draft' - | 'ci' | 'deploy' | 'queue' | 'bot'; @@ -44,7 +43,6 @@ const GLYPHS: Record = { reopened: 'ready', ready_for_review: 'ready', converted_to_draft: 'draft', - ci: 'ci', deploy: 'deploy', merge_queue: 'queue', bot_comment: 'bot', diff --git a/apps/desktop/src/renderer/src/lib/pr.test.ts b/apps/desktop/src/renderer/src/lib/pr.test.ts index 7d0f0aa3..0d071999 100644 --- a/apps/desktop/src/renderer/src/lib/pr.test.ts +++ b/apps/desktop/src/renderer/src/lib/pr.test.ts @@ -1,17 +1,12 @@ import { describe, expect, it } from 'vitest'; -import { prPaneView, type ChecksSummary, type PrStatus, type Review } from '@postpile/core'; +import { prPaneView, type PrStatus, type Review } from '@postpile/core'; import { at, makePr } from '@postpile/core/fixtures'; -import { approvedText, checksNote, ICON_WORDS, mergeQueueWord, mergeStatus, reviewRows, reviewWord, rowStateWord, stackQueueWord } from './pr.ts'; +import { approvedText, ICON_WORDS, mergeQueueWord, mergeStatus, reviewRows, reviewWord, rowStateWord, stackQueueWord } from './pr.ts'; function review(author: string, state: Review['state'], minutes: number): Review { return { id: `${author}-${minutes}`, author, state, body: '', submittedAt: at(minutes), commitOid: null }; } -/** The pane's checks summary with these counts. */ -function checks(passed: number, failed: number, pending: number): ChecksSummary { - return { rollup: 'PENDING', total: passed + failed + pending, passed, failed, pending, finishedAt: null, failedNames: [] }; -} - describe('pr helpers', () => { it('lists pending requests first, then newest reviews, teams last', () => { const pr = prPaneView( @@ -34,12 +29,6 @@ describe('pr helpers', () => { expect(reviewRows(pr)).toEqual([{ login: 'lyra', status: 'changes_requested', at: at(20) }]); }); - it('words checks neutrally, failed and running together as not passing', () => { - expect(checksNote(checks(2, 1, 1))).toBe('4 checks · 2 not passing'); - expect(checksNote(checks(3, 0, 0))).toBe('3 checks · all passing'); - expect(checksNote(checks(1, 0, 0))).toBe('1 check · all passing'); - }); - it('describes the merge status', () => { expect(mergeStatus(prPaneView(makePr({ state: 'MERGED', mergedBy: 'rowan' })), [])).toBe('merged by rowan'); expect(mergeStatus(prPaneView(makePr()), [])).toBe('needs review'); diff --git a/apps/desktop/src/renderer/src/lib/pr.ts b/apps/desktop/src/renderer/src/lib/pr.ts index 191e4b99..558ca08b 100644 --- a/apps/desktop/src/renderer/src/lib/pr.ts +++ b/apps/desktop/src/renderer/src/lib/pr.ts @@ -1,18 +1,7 @@ -import type { ChecksSummary, MergeQueueStep, PaneReview, PrIcon, PrPaneView, PrStatus, TileStack } from '@postpile/core'; +import type { MergeQueueStep, PaneReview, PrIcon, PrPaneView, PrStatus, TileStack } from '@postpile/core'; import { prNumber } from './tiles.ts'; import { sinceLabel } from './time.ts'; -/** - * The Checks fact's note, "12 checks · 2 not passing" ("all passing" at - * none). Neutral words: CI is not a signal here (2026-09-29), so failed and - * still running both count as not passing, without a colour. - */ -export function checksNote(checks: ChecksSummary): string { - const total = `${checks.total} ${checks.total === 1 ? 'check' : 'checks'}`; - const notPassing = checks.failed + checks.pending; - return notPassing === 0 ? `${total} · all passing` : `${total} · ${notPassing} not passing`; -} - export type ReviewStatus = 'requested' | 'approved' | 'changes_requested' | 'commented' | 'dismissed'; export interface ReviewRow { diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 492f0a3e..89932a8b 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -23,7 +23,6 @@ export const TOKEN_HEADER = 'x-postpile-token'; const snoozeCondition = z.discriminatedUnion('kind', [ z.object({ kind: z.literal('someone_replies') }), z.object({ kind: z.literal('new_push') }), - z.object({ kind: z.literal('ci_green') }), // Snoozes compare ISO strings, so any offset is normalised to UTC "Z" form here. z.object({ kind: z.literal('until_time'), diff --git a/apps/server/src/fake/fake-engine.test.ts b/apps/server/src/fake/fake-engine.test.ts index 487e659f..ba2d2863 100644 --- a/apps/server/src/fake/fake-engine.test.ts +++ b/apps/server/src/fake/fake-engine.test.ts @@ -533,7 +533,7 @@ describe('FakeEngine addressed your changes', () => { }); describe('FakeEngine what is new on a revisit', () => { - it('summarises the pushes since your changes request on #1960, bots and CI left out', async () => { + it('summarises the pushes since your changes request on #1960, bots left out', async () => { const engine = new FakeEngine(); const devEnv = (await engine.getTopic('topic-dev-env'))?.tiles ?? []; const view = devEnv.find((item) => item.tile.id === 'pr:acme/app#1960'); @@ -541,7 +541,7 @@ describe('FakeEngine what is new on a revisit', () => { const detail = await engine.getPr('acme/app#1960'); expect(detail?.whatsNew?.anchor.kind).toBe('changes_request'); expect(detail?.activity.fresh.map((line) => line.summary)).toEqual(['pim pushed 3 commits']); - expect(detail?.activity.freshNoiseLabel).toBe('2 bot comments, CI'); + expect(detail?.activity.freshNoiseLabel).toBe('2 bot comments'); expect(detail?.activity.earlier.map((line) => line.summary)).toEqual(['you requested changes']); expect(detail?.activity.noise).toEqual([]); }); diff --git a/apps/server/src/fake/fake-quiet.ts b/apps/server/src/fake/fake-quiet.ts index c667e540..61a90846 100644 --- a/apps/server/src/fake/fake-quiet.ts +++ b/apps/server/src/fake/fake-quiet.ts @@ -12,13 +12,13 @@ function hoursBefore(now: Date, hours: number): string { * since the viewer last looked was judged as not needing them. Invented. */ const QUIET_SAMPLES: { number: number; detail: string; hoursAgo: number }[] = [ - { number: 1904, detail: quietReadDetail(['trunk-io[bot]', 'CI']), hoursAgo: 2 }, + { number: 1904, detail: quietReadDetail(['trunk-io[bot]', 'vercel[bot]']), hoursAgo: 2 }, { number: 1911, detail: quietReasonDetail('approved'), hoursAgo: 3.5 }, - { number: 1934, detail: judgedReadDetail(['lyra', 'CI']), hoursAgo: 4 }, + { number: 1934, detail: judgedReadDetail(['lyra', 'vercel[bot]']), hoursAgo: 4 }, // After mergify queued it (1h ago): the tile is done again. { number: 1899, detail: quietReadDetail(['renovate[bot]', 'mergify[bot]']), hoursAgo: 0.5 }, { number: 1960, detail: quietReasonDetail('changes_requested'), hoursAgo: 29 }, - { number: 1921, detail: quietReadDetail(['renovate[bot]', 'CI']), hoursAgo: 50 }, + { number: 1921, detail: quietReadDetail(['renovate[bot]', 'vercel[bot]']), hoursAgo: 50 }, { number: 1963, detail: quietReadDetail(['chatgpt-codex-connector[bot]', 'coderabbitai[bot]']), hoursAgo: 75 }, // Older than the view's 7 days: in the log, not in "Handled quietly". { number: 1855, detail: quietReadDetail(['vercel[bot]']), hoursAgo: 9 * 24 }, diff --git a/apps/server/src/fake/sample-builders.ts b/apps/server/src/fake/sample-builders.ts index e44627de..404c611e 100644 --- a/apps/server/src/fake/sample-builders.ts +++ b/apps/server/src/fake/sample-builders.ts @@ -1,7 +1,7 @@ // Small builders that keep sample-data.ts readable. Everything here fills in // the fields a fake does not care about with plain defaults. import { prKey } from '@postpile/core'; -import type { CheckContext, CheckRollup, Comment, EventKind, Glance, KeyFile, Loudness, Pr, PrEvent, PrKey, Provenance, PrState, Review, ReviewDecision, ReviewState, ReviewThread, Tile, TileKind, TileMember, Topic, TopicKind, UserRole, Verdict } from '@postpile/core'; +import type { Comment, EventKind, Glance, KeyFile, Loudness, Pr, PrEvent, PrKey, Provenance, PrState, Review, ReviewDecision, ReviewState, ReviewThread, Tile, TileKind, TileMember, Topic, TopicKind, UserRole, Verdict } from '@postpile/core'; export const SAMPLE_REPO = 'acme/app'; export const SAMPLE_VIEWER = 'you'; @@ -67,7 +67,6 @@ export interface SamplePrInput { assignees?: string[]; state: PrState; size: [additions: number, deletions: number, files: number]; - checks: CheckRollup; baseRef?: string; headRef?: string; openedHoursAgo: number; @@ -102,25 +101,6 @@ function sampleReviewDecision(reviews: [string, ReviewState, string?, string?, n return reviews.some(([, state]) => state === 'APPROVED') ? 'APPROVED' : 'REVIEW_REQUIRED'; } -/** - * A few check runs that match the rollup, so the pane's Checks fact has - * something to count: five on a finished run, one failing on FAILURE, one - * still running on PENDING, none for NONE. - */ -function sampleCheckContexts(rollup: CheckRollup, finishedAt: string): CheckContext[] { - if (rollup === 'NONE') { - return []; - } - const passed = (name: string): CheckContext => ({ name, conclusion: 'SUCCESS', completedAt: finishedAt }); - const last: CheckContext = - rollup === 'FAILURE' - ? { name: 'test (backend)', conclusion: 'FAILURE', completedAt: finishedAt } - : rollup === 'PENDING' - ? { name: 'test (backend)', conclusion: null, completedAt: null } - : passed('test (backend)'); - return [passed('lint'), passed('typecheck'), { name: 'storybook', conclusion: 'SKIPPED', completedAt: finishedAt }, passed('test (frontend)'), last]; -} - /** Review bodies with text, as comments, like the GitHub reader adds them to `pr.comments`. */ function reviewBodyComments(reviews: Review[], url: string): Comment[] { return reviews @@ -207,8 +187,6 @@ export function samplePr(clock: SampleClock, input: SamplePrInput): Pr { timeline: input.queued ? [{ id: `queue-${input.number}`, kind: 'added_to_merge_queue' as const, actor: 'mergify[bot]', at: clock.hoursAgo(1), subject: null }] : [], - // Checks finish half an hour after the PR opened; no rule in sample mode reads the time. - checks: { rollup: input.checks, contexts: sampleCheckContexts(input.checks, clock.hoursAgo(Math.max(0, input.openedHoursAgo - 0.5))) }, headOid, createdAt: clock.hoursAgo(input.openedHoursAgo), updatedAt: clock.hoursAgo(input.mergedHoursAgo ?? 0), diff --git a/apps/server/src/fake/sample-data.ts b/apps/server/src/fake/sample-data.ts index 8555ee42..4345a620 100644 --- a/apps/server/src/fake/sample-data.ts +++ b/apps/server/src/fake/sample-data.ts @@ -253,7 +253,7 @@ function buildPrs(clock: SampleClock): Pr[] { return [ samplePr(clock, { number: 1902, title: 'Use Depot cache backend for Turbo', author: 'rowan', state: 'OPEN', - size: [186, 42, 7], checks: 'FAILURE', openedHoursAgo: 5, + size: [186, 42, 7], openedHoursAgo: 5, baseRef: 'rowan/depot-2', headRef: 'rowan/depot-3', body: ` @@ -320,27 +320,27 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, // lyra's two-layer stack, inside the Turbo cache set: the stack mark shows in a set too. samplePr(clock, { number: 1904, title: 'Hash Turbo inputs by lockfile only', author: 'lyra', state: 'OPEN', - size: [22, 9, 2], checks: 'SUCCESS', openedHoursAgo: 8, + size: [22, 9, 2], openedHoursAgo: 8, baseRef: 'master', headRef: 'lyra/turbo-keys-1', reviewerUsers: [SAMPLE_VIEWER], }), samplePr(clock, { number: 1907, title: 'Drop the per-job Turbo cache salt', author: 'lyra', state: 'OPEN', - size: [6, 14, 2], checks: 'SUCCESS', openedHoursAgo: 7, + size: [6, 14, 2], openedHoursAgo: 7, baseRef: 'lyra/turbo-keys-1', headRef: 'lyra/turbo-keys-2', reviewerUsers: [SAMPLE_VIEWER], comments: [{ id: 'issuecomment-5', author: 'lyra', body: '@you ok to drop the salt now that keys come from the lockfile?', hoursAgo: 0.8 }], }), samplePr(clock, { number: 1921, title: 'Bump turbo to 2.5', author: 'renovate[bot]', state: 'OPEN', - size: [4, 4, 2], checks: 'SUCCESS', openedHoursAgo: 3, reviewerTeams: ['acme/team-platform'], + size: [4, 4, 2], openedHoursAgo: 3, reviewerTeams: ['acme/team-platform'], }), samplePr(clock, { number: 1855, title: 'Skip Turbo remote cache for Storybook', author: 'jude', state: 'MERGED', - size: [3, 1, 1], checks: 'SUCCESS', openedHoursAgo: 30, mergedHoursAgo: 14, reviews: [['lyra', 'APPROVED']], + size: [3, 1, 1], openedHoursAgo: 30, mergedHoursAgo: 14, reviews: [['lyra', 'APPROVED']], comments: [{ id: 'issuecomment-3', author: 'jude', body: 'Are the snapshots stale because of the cache or because of the Vite upgrade?', hoursAgo: 16 }], }), samplePr(clock, { number: 1911, title: 'Run e2e on Depot runners', author: 'rowan', state: 'OPEN', - size: [48, 48, 5], checks: 'SUCCESS', openedHoursAgo: 6, + size: [48, 48, 5], openedHoursAgo: 6, baseRef: 'rowan/depot-3', headRef: 'rowan/depot-4', reviewerUsers: ['nell'], reviews: [[SAMPLE_VIEWER, 'APPROVED', 'Labels match #1880.', 'sha1911-a']], commits: [ @@ -352,47 +352,47 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, samplePr(clock, { // Closed on top of the stack: it still shows there, greyed. number: 1930, title: 'Drop GitHub runners for release builds', author: 'rowan', state: 'CLOSED', - size: [2, 30, 2], checks: 'SUCCESS', openedHoursAgo: 5, + size: [2, 30, 2], openedHoursAgo: 5, baseRef: 'rowan/depot-4', headRef: 'rowan/depot-5', }), samplePr(clock, { number: 1862, title: 'Backend jobs on Depot', author: 'rowan', state: 'MERGED', - size: [60, 60, 6], checks: 'SUCCESS', openedHoursAgo: 96, mergedHoursAgo: 72, + size: [60, 60, 6], openedHoursAgo: 96, mergedHoursAgo: 72, baseRef: 'rowan/depot-1', headRef: 'rowan/depot-2', reviews: [[SAMPLE_VIEWER, 'APPROVED'], ['lyra', 'APPROVED']], }), samplePr(clock, { number: 1851, title: 'Add Depot project config', author: 'rowan', state: 'MERGED', - size: [12, 0, 1], checks: 'SUCCESS', openedHoursAgo: 170, mergedHoursAgo: 144, + size: [12, 0, 1], openedHoursAgo: 170, mergedHoursAgo: 144, baseRef: 'master', headRef: 'rowan/depot-1', reviews: [[SAMPLE_VIEWER, 'APPROVED']], comments: [{ id: 'issuecomment-1', author: 'nell', body: 'Can we keep GitHub runners for release builds until Depot has an SLA?', hoursAgo: 150 }], }), samplePr(clock, { number: 1915, title: 'DEPOT_TOKEN as repo secret', author: 'rowan', state: 'MERGED', - size: [9, 3, 3], checks: 'SUCCESS', openedHoursAgo: 30, mergedHoursAgo: 24, reviews: [['lyra', 'APPROVED']], + size: [9, 3, 3], openedHoursAgo: 30, mergedHoursAgo: 24, reviews: [['lyra', 'APPROVED']], comments: [{ id: 'issuecomment-4', author: 'rowan', body: 'Keeping DEPOT_TOKEN a repo secret for now. The org secret move comes with the release workflow.', hoursAgo: 25 }], }), samplePr(clock, { number: 1899, title: 'Rename workflow files to ci-*.yml', author: 'rowan', state: 'OPEN', - size: [0, 0, 9], checks: 'SUCCESS', openedHoursAgo: 48, queued: true, + size: [0, 0, 9], openedHoursAgo: 48, queued: true, reviews: [[SAMPLE_VIEWER, 'APPROVED'], ['lyra', 'APPROVED'], ['nell', 'APPROVED']], }), samplePr(clock, { number: 1840, title: 'Remove the nightly cache warmer job', author: 'nell', state: 'MERGED', - size: [0, 64, 2], checks: 'SUCCESS', openedHoursAgo: 150, mergedHoursAgo: 120, reviews: [[SAMPLE_VIEWER, 'APPROVED']], + size: [0, 64, 2], openedHoursAgo: 150, mergedHoursAgo: 120, reviews: [[SAMPLE_VIEWER, 'APPROVED']], }), samplePr(clock, { number: 1790, title: 'Raise Django test timeout to 45 min', author: 'nell', state: 'MERGED', - size: [1, 1, 1], checks: 'SUCCESS', openedHoursAgo: 60, mergedHoursAgo: 48, reviews: [['rowan', 'APPROVED']], + size: [1, 1, 1], openedHoursAgo: 60, mergedHoursAgo: 48, reviews: [['rowan', 'APPROVED']], }), samplePr(clock, { number: 1822, title: 'Split backend tests by timing data', author: 'remy', state: 'OPEN', - size: [240, 80, 11], checks: 'SUCCESS', openedHoursAgo: 72, + size: [240, 80, 11], openedHoursAgo: 72, reviews: [['lyra', 'APPROVED'], ['sol', 'APPROVED']], reviewerTeams: ['acme/team-platform'], }), samplePr(clock, { number: 1801, title: 'Move billing models to modules/', author: SAMPLE_VIEWER, state: 'OPEN', - size: [410, 380, 24], checks: 'SUCCESS', openedHoursAgo: 48, + size: [410, 380, 24], openedHoursAgo: 48, body: 'Moves the six billing models into `modules/billing/`. State-only: `db_table` stays, so no table is renamed.\n\nFollow-up: drop the old re-exports in #1808.', files: [ ['modules/billing/models.py', 320, 0], @@ -425,115 +425,115 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, }), samplePr(clock, { number: 1932, title: 'RFC: self-hosted runners for ingestion CI', author: 'ines', state: 'OPEN', - size: [140, 12, 3], checks: 'SUCCESS', openedHoursAgo: 20, reviewerTeams: ['acme/team-platform'], + size: [140, 12, 3], openedHoursAgo: 20, reviewerTeams: ['acme/team-platform'], body: 'Ingestion jobs need more memory than Depot offers. This RFC adds a runner pool and one workflow change.', }), samplePr(clock, { number: 1940, title: 'Release desktop 2.3', author: 'mae', state: 'OPEN', - size: [30, 10, 4], checks: 'PENDING', openedHoursAgo: 30, reviews: [['koa', 'APPROVED']], + size: [30, 10, 4], openedHoursAgo: 30, reviews: [['koa', 'APPROVED']], }), // Added for the sidebar's sections: your own PRs, a team mention (Team // mentioned), a bot bump (Other topics, FYI) and a merged PR with news on // it (unread, but calm). samplePr(clock, { number: 1945, title: 'Cap CI shard retries at 2', author: SAMPLE_VIEWER, state: 'OPEN', draft: true, - size: [14, 6, 2], checks: 'SUCCESS', openedHoursAgo: 8, reviewerUsers: ['lyra'], + size: [14, 6, 2], openedHoursAgo: 8, reviewerUsers: ['lyra'], reviews: [['remy', 'COMMENTED', 'Would 3 hide fewer real flakes?']], }), samplePr(clock, { number: 1808, title: 'Drop the old billing re-exports', author: SAMPLE_VIEWER, state: 'OPEN', // Approved by an agent only: the pill reads "approved by agent", whose turn stays "Merge". - size: [3, 40, 2], checks: 'SUCCESS', openedHoursAgo: 20, reviews: [['reviewbot[bot]', 'APPROVED']], + size: [3, 40, 2], openedHoursAgo: 20, reviews: [['reviewbot[bot]', 'APPROVED']], }), samplePr(clock, { number: 1934, title: 'Ingestion runner pool as a Terraform module', author: 'ines', state: 'OPEN', - size: [220, 0, 6], checks: 'SUCCESS', openedHoursAgo: 10, + size: [220, 0, 6], openedHoursAgo: 10, comments: [{ id: 'issuecomment-5', author: 'ines', body: '@acme/team-platform do the runner labels clash with yours?', hoursAgo: 1.5 }], }), // A routing team's request (client-approvers, added by an assigner bot) and a routing team's mention. samplePr(clock, { number: 1966, title: 'Retry uploads with jittered backoff', author: 'koa', state: 'OPEN', - size: [64, 18, 3], checks: 'SUCCESS', openedHoursAgo: 6, reviewerTeams: ['acme/client-approvers'], + size: [64, 18, 3], openedHoursAgo: 6, reviewerTeams: ['acme/client-approvers'], body: 'Uploads retried at a fixed 1s interval and piled up after an outage. This adds jittered backoff capped at 30s.', }), samplePr(clock, { number: 1967, title: 'Document the new retry settings', author: 'koa', state: 'OPEN', - size: [40, 4, 2], checks: 'SUCCESS', openedHoursAgo: 5, + size: [40, 4, 2], openedHoursAgo: 5, comments: [{ id: 'issuecomment-7', author: 'koa', body: '@acme/client-approvers heads-up: the defaults change in the next release', hoursAgo: 4 }], }), samplePr(clock, { number: 1925, title: 'Bump ruff to 0.7', author: 'renovate[bot]', state: 'OPEN', - size: [2, 2, 1], checks: 'SUCCESS', openedHoursAgo: 12, + size: [2, 2, 1], openedHoursAgo: 12, }), samplePr(clock, { number: 1857, title: 'Upgrade to Vite 7', author: 'lyra', state: 'MERGED', - size: [120, 90, 14], checks: 'SUCCESS', openedHoursAgo: 50, mergedHoursAgo: 3, reviews: [['jude', 'APPROVED']], + size: [120, 90, 14], openedHoursAgo: 50, mergedHoursAgo: 3, reviews: [['jude', 'APPROVED']], comments: [{ id: 'issuecomment-6', author: 'jude', body: '@you are the stale snapshots gone after this?', hoursAgo: 2 }], }), samplePr(clock, { number: 1870, title: 'Make devbox start default to minimal stack', author: 'sol', state: 'OPEN', - size: [70, 12, 4], checks: 'SUCCESS', openedHoursAgo: 72, reviewerTeams: ['acme/team-platform'], + size: [70, 12, 4], openedHoursAgo: 72, reviewerTeams: ['acme/team-platform'], }), // Found outside the inbox: the viewer's own open PR, and a review asked of them they already read on GitHub. // Approved, submitted to the Trunk merge queue, and taken out again: its checks never finished. samplePr(clock, { number: 1950, title: 'Cache pnpm store in the devbox CI image', author: SAMPLE_VIEWER, state: 'OPEN', - size: [34, 8, 2], checks: 'PENDING', openedHoursAgo: 30, reviews: [['lyra', 'APPROVED', '', undefined, 26]], + size: [34, 8, 2], openedHoursAgo: 30, reviews: [['lyra', 'APPROVED', '', undefined, 26]], comments: [{ id: 'issuecomment-1950-trunk', author: 'trunk-io[bot]', body: TRUNK_REMOVED, hoursAgo: 29, editedHoursAgo: 0.6 }], }), samplePr(clock, { number: 1975, title: 'Pin Depot runner images by digest', author: SAMPLE_VIEWER, state: 'OPEN', - size: [18, 18, 6], checks: 'SUCCESS', openedHoursAgo: 20, reviews: [['rowan', 'APPROVED', '', undefined, 3]], + size: [18, 18, 6], openedHoursAgo: 20, reviews: [['rowan', 'APPROVED', '', undefined, 3]], body: 'Runner images float on `:latest` today, so a Depot image update can change CI under us. This pins every image by digest.', comments: [{ id: 'issuecomment-1975-trunk', author: 'trunk-io[bot]', body: TRUNK_TESTING, hoursAgo: 2.5, editedHoursAgo: 0.4 }], }), // The rest of the merge queue in fake mode: submitted (checks still running), waiting, and merged through it. samplePr(clock, { number: 1977, title: 'Pin the macOS runner image too', author: SAMPLE_VIEWER, state: 'OPEN', - size: [6, 6, 2], checks: 'PENDING', openedHoursAgo: 4, reviews: [['rowan', 'APPROVED', '', undefined, 1.5]], + size: [6, 6, 2], openedHoursAgo: 4, reviews: [['rowan', 'APPROVED', '', undefined, 1.5]], comments: [{ id: 'issuecomment-1977-trunk', author: 'trunk-io[bot]', body: TRUNK_SUBMITTED, hoursAgo: 1 }], }), samplePr(clock, { number: 1978, title: 'Drop the floating runner image tag', author: 'rowan', state: 'OPEN', - size: [2, 9, 3], checks: 'SUCCESS', openedHoursAgo: 6, reviews: [[SAMPLE_VIEWER, 'APPROVED', '', undefined, 2]], + size: [2, 9, 3], openedHoursAgo: 6, reviews: [[SAMPLE_VIEWER, 'APPROVED', '', undefined, 2]], comments: [{ id: 'issuecomment-1978-trunk', author: 'trunk-io[bot]', body: TRUNK_WAITING, hoursAgo: 1.8, editedHoursAgo: 0.2 }], }), // The ownership sections' samples. samplePr(clock, { number: 1980, title: 'Run quarantined tests in their own job', author: 'sol', state: 'OPEN', - size: [64, 12, 3], checks: 'SUCCESS', openedHoursAgo: 10, + size: [64, 12, 3], openedHoursAgo: 10, }), samplePr(clock, { number: 1981, title: 'Report retries of quarantined tests', author: SAMPLE_VIEWER, state: 'OPEN', - size: [48, 4, 2], checks: 'PENDING', openedHoursAgo: 4, reviewerUsers: ['sol'], + size: [48, 4, 2], openedHoursAgo: 4, reviewerUsers: ['sol'], }), samplePr(clock, { number: 1982, title: 'Allow the Depot cache host', author: 'nell', state: 'OPEN', - size: [3, 0, 1], checks: 'SUCCESS', openedHoursAgo: 9, + size: [3, 0, 1], openedHoursAgo: 9, }), samplePr(clock, { number: 1984, title: 'Move recordings older than 30 days to cold storage', author: 'pia', state: 'OPEN', - size: [210, 40, 9], checks: 'SUCCESS', openedHoursAgo: 28, + size: [210, 40, 9], openedHoursAgo: 28, }), samplePr(clock, { number: 1985, title: 'Cap the replay player buffer', author: 'gus', state: 'OPEN', - size: [40, 18, 2], checks: 'SUCCESS', openedHoursAgo: 50, + size: [40, 18, 2], openedHoursAgo: 50, }), samplePr(clock, { number: 1986, title: 'Run usage exports on the shared runners', author: SAMPLE_VIEWER, state: 'OPEN', - size: [12, 6, 2], checks: 'SUCCESS', openedHoursAgo: 7, reviewerUsers: ['omar'], + size: [12, 6, 2], openedHoursAgo: 7, reviewerUsers: ['omar'], }), samplePr(clock, { number: 1987, title: 'Add alert threshold presets', author: 'gus', state: 'OPEN', - size: [90, 5, 4], checks: 'SUCCESS', openedHoursAgo: 40, + size: [90, 5, 4], openedHoursAgo: 40, }), samplePr(clock, { number: 1988, title: 'Rebuild the docs search index nightly', author: 'tove', state: 'OPEN', - size: [22, 3, 2], checks: 'SUCCESS', openedHoursAgo: 3, + size: [22, 3, 2], openedHoursAgo: 3, }), samplePr(clock, { number: 1974, title: 'Pin the Linux runner image', author: SAMPLE_VIEWER, state: 'MERGED', - size: [8, 8, 2], checks: 'SUCCESS', openedHoursAgo: 30, mergedHoursAgo: 5, reviews: [['rowan', 'APPROVED', '', undefined, 7]], + size: [8, 8, 2], openedHoursAgo: 30, mergedHoursAgo: 5, reviews: [['rowan', 'APPROVED', '', undefined, 7]], comments: [{ id: 'issuecomment-1974-trunk', author: 'trunk-io[bot]', body: TRUNK_MERGED, hoursAgo: 6, editedHoursAgo: 5 }], }), // Addressed your changes, seen on a revisit: you asked for changes @@ -542,7 +542,7 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, // "3 commits since your changes request". samplePr(clock, { number: 1960, title: 'Split the toolbar into its own bundle', author: 'pim', state: 'OPEN', - size: [260, 90, 9], checks: 'SUCCESS', openedHoursAgo: 50, + size: [260, 90, 9], openedHoursAgo: 50, reviews: [[SAMPLE_VIEWER, 'CHANGES_REQUESTED', 'The chunk names change on every build, which busts the CDN cache.', 'sha1960-a', 30]], threads: [ { @@ -563,7 +563,7 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, // requested after the addressed #1960, quiet. samplePr(clock, { number: 1963, title: 'Inline small SVG icons into the bundle', author: 'tove', state: 'OPEN', - size: [80, 30, 5], checks: 'SUCCESS', openedHoursAgo: 40, + size: [80, 30, 5], openedHoursAgo: 40, reviews: [[SAMPLE_VIEWER, 'CHANGES_REQUESTED', 'Inlining drops the long cache on the icon sprite.', 'sha1963', 20]], threads: [ { @@ -576,7 +576,7 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, }), samplePr(clock, { number: 1955, title: 'Pin the Playwright browser version', author: 'nell', state: 'OPEN', - size: [12, 4, 2], checks: 'SUCCESS', openedHoursAgo: 26, reviewerUsers: [SAMPLE_VIEWER], + size: [12, 4, 2], openedHoursAgo: 26, reviewerUsers: [SAMPLE_VIEWER], }), // Agent PRs: a coding agent's GitHub App opens them on someone's behalf // and assigns that person, who owns the PR. #1970 is the viewer's own @@ -584,12 +584,12 @@ See the [Depot cache docs](https://example.com/docs/cache) for the backend.`, // so the platform team request on it is the viewer's (For you). samplePr(clock, { number: 1970, title: 'Check that billing migrations stay state-only', author: SAMPLE_AGENT, assignees: [SAMPLE_VIEWER], state: 'OPEN', - size: [28, 6, 2], checks: 'SUCCESS', openedHoursAgo: 5, headRef: 'acme-agent/billing-migration-check', reviewerUsers: ['lyra'], + size: [28, 6, 2], openedHoursAgo: 5, headRef: 'acme-agent/billing-migration-check', reviewerUsers: ['lyra'], body: 'Opened by the coding agent for @you. Fails CI when a billing migration renames or drops a table.', }), samplePr(clock, { number: 1972, title: 'Drop unused env vars from devbox start', author: SAMPLE_AGENT, assignees: ['rowan', 'sol', 'nell'], state: 'OPEN', - size: [4, 19, 3], checks: 'SUCCESS', openedHoursAgo: 4, headRef: 'acme-agent/devbox-env-cleanup', reviewerTeams: ['acme/team-platform'], + size: [4, 19, 3], openedHoursAgo: 4, headRef: 'acme-agent/devbox-env-cleanup', reviewerTeams: ['acme/team-platform'], body: 'Opened by the coding agent for @rowan. Removes env vars no devbox service reads.', }), ]; @@ -627,7 +627,6 @@ function buildEvents(clock: SampleClock): PrEvent[] { rule: 'quiet', raisedBecause: 'Changes the CI runner image you approved, not a plain follow-up.', }, - { kind: 'ci', actor: 'ci-bot', text: 'all checks passed', hoursAgo: 0.1, rule: 'quiet', isBot: true }, ]), ...sampleEvents(clock, 1862, [ { kind: 'merged', actor: 'rowan', text: 'merged it', hoursAgo: 72, rule: 'quiet', seen: true }, @@ -726,7 +725,6 @@ function buildEvents(clock: SampleClock): PrEvent[] { { kind: 'commits_pushed', actor: 'pim', text: 'pushed: Stable chunk names for the toolbar', hoursAgo: 2.2, rule: 'loud' }, { kind: 'bot_comment', actor: 'reviewbot[bot]', text: 'commented: "No issues found in 9 files"', hoursAgo: 2.1, rule: 'quiet', isBot: true }, { kind: 'bot_comment', actor: 'sizebot[bot]', text: 'commented: "toolbar.js -18 kB"', hoursAgo: 2, rule: 'quiet', isBot: true }, - { kind: 'ci', actor: 'ci-bot', text: 'all checks passed', hoursAgo: 1.9, rule: 'quiet', isBot: true }, ]), ...sampleEvents(clock, 1963, [ { kind: 'review_changes_requested', actor: SAMPLE_VIEWER, text: 'requested changes', hoursAgo: 20, rule: 'quiet', seen: true }, diff --git a/apps/server/src/routes.test.ts b/apps/server/src/routes.test.ts index 5d8b6b1d..c53e166f 100644 --- a/apps/server/src/routes.test.ts +++ b/apps/server/src/routes.test.ts @@ -267,9 +267,9 @@ describe('server routes over the fake engine', () => { const app = appWithFake(); const quiet = (await (await app.request('/api/handled-quietly')).json()) as QuietReadView[]; expect(quiet.map((item) => item.prKey)).toEqual(['acme/app#1899', 'acme/app#1904', 'acme/app#1911', 'acme/app#1934', 'acme/app#1960', 'acme/app#1921', 'acme/app#1963']); - expect(quiet[1]).toMatchObject({ repo: 'acme/app', number: 1904, title: 'Hash Turbo inputs by lockfile only', reason: 'bots', bots: ['trunk-io[bot]', 'CI'] }); + expect(quiet[1]).toMatchObject({ repo: 'acme/app', number: 1904, title: 'Hash Turbo inputs by lockfile only', reason: 'bots', bots: ['trunk-io[bot]', 'vercel[bot]'] }); expect(quiet[2]).toMatchObject({ number: 1911, reason: 'approved', bots: [] }); - expect(quiet[3]).toMatchObject({ number: 1934, reason: 'judged', bots: ['lyra', 'CI'] }); + expect(quiet[3]).toMatchObject({ number: 1934, reason: 'judged', bots: ['lyra', 'vercel[bot]'] }); const rows = (await (await app.request('/api/debug/notifications')).json()) as NotificationDebugRow[]; expect(rows.find((row) => row.prKey === 'acme/app#1904')?.lastAction).toMatchObject({ origin: 'quiet', outcome: 'github' }); @@ -355,7 +355,7 @@ describe('server routes over the fake engine', () => { expect(detail.activity.fresh[0]).not.toHaveProperty('events'); }); - it('sends the slim PR view: no comments, threads, commits, timeline or check contexts', async () => { + it('sends the slim PR view: no comments, threads, commits, timeline or checks', async () => { const detail = (await (await appWithFake().request('/api/prs/acme/app/1902')).json()) as PrDetail; expect(Object.keys(detail.pr).sort()).toEqual([ 'additions', @@ -364,7 +364,6 @@ describe('server routes over the fake engine', () => { 'baseRef', 'body', 'changedFiles', - 'checks', 'createdAt', 'deletions', 'files', @@ -386,7 +385,6 @@ describe('server routes over the fake engine', () => { 'url', ]); expect(Object.keys(detail.pr.reviews[0] ?? {}).sort()).toEqual(['author', 'state', 'submittedAt']); - expect(detail.pr.checks).toMatchObject({ rollup: 'FAILURE', total: 5, passed: 4, failed: 1, pending: 0, failedNames: ['test (backend)'] }); expect(detail.pr.files.map((file) => file.path)).toContain('turbo.json'); // Replies and reactions still find their comment: the activity line carries it. const lines = [...detail.activity.fresh, ...detail.activity.earlier]; diff --git a/packages/agent/src/hashes.test.ts b/packages/agent/src/hashes.test.ts index be9493b2..5760ec31 100644 --- a/packages/agent/src/hashes.test.ts +++ b/packages/agent/src/hashes.test.ts @@ -34,16 +34,8 @@ describe('glanceItemInputHash', () => { expect(glanceHash()).toBe(base); }); - it('ignores bot comments, PR update timestamps and CI re-runs', () => { + it('ignores bot comments and PR update timestamps', () => { expect(glanceHash(makePr({ updatedAt: '2026-09-09T00:00:00Z', comments: [makeComment({ author: 'dependabot[bot]' })] }))).toBe(base); - expect(glanceHash(makePr({ checks: { rollup: 'FAILURE', contexts: [] } }))).toBe(base); - }); - - it('ignores every checks change: CI is not a signal', () => { - const failed = { name: 'backend-tests', conclusion: 'FAILURE', completedAt: '2026-09-02T09:30:00Z' }; - expect(glanceHash(makePr({ checks: { rollup: 'FAILURE', contexts: [failed] } }))).toBe(base); - expect(glanceHash(makePr({ checks: { rollup: 'PENDING', contexts: [{ ...failed, conclusion: null, completedAt: null }] } }))).toBe(base); - expect(glanceHash(makePr({ checks: { rollup: 'NONE', contexts: [] } }))).toBe(base); }); it('ignores bot review comments but changes on an agent approval', () => { diff --git a/packages/agent/src/prompts-memory.test.ts b/packages/agent/src/prompts-memory.test.ts index 1176d104..b062415a 100644 --- a/packages/agent/src/prompts-memory.test.ts +++ b/packages/agent/src/prompts-memory.test.ts @@ -23,7 +23,7 @@ import { viewer, } from './test-fixtures.ts'; -const pr1 = makePr({ checks: { rollup: 'FAILURE', contexts: [] } }); +const pr1 = makePr(); const pr2 = makePr({ ref: { repo: 'acme/app', number: 2 }, title: 'Docker builds on Depot', author: 'bob', body: 'Moves docker builds.' }); function dossierInput(overrides: Partial = {}): DossierUpdateInput { @@ -33,8 +33,6 @@ function dossierInput(overrides: Partial = {}): DossierUpdat delta: makeDelta({ events: [ makeEvent({ id: 'ev-human', summary: 'bob asked: are release builds staying?' }), - makeEvent({ id: 'ev-bot', actor: 'github-actions', isBot: true, kind: 'ci', summary: 'CI failed' }), - makeEvent({ id: 'ev-bot2', actor: 'github-actions', isBot: true, kind: 'ci', summary: 'CI failed again' }), makeEvent({ id: 'ev-bot3', actor: 'reviewbot[bot]', isBot: true, kind: 'bot_comment', summary: 'reviewbot left a summary' }), ], joinedPrKeys: [pr2.key], @@ -104,8 +102,6 @@ describe('dossierUpdatePrompt', () => { }); it('carries no CI status and says not to bring it up, while CI as a subject of the work stays', () => { - expect(prompt).not.toContain('CI failed'); - expect(prompt).not.toContain('(ci)'); expect(prompt).not.toContain('CI failing'); expect(prompt).not.toMatch(/CI: (failure|success|pending|none)/); expect(prompt).toContain(NO_CI_RULE); diff --git a/packages/agent/src/prompts.test.ts b/packages/agent/src/prompts.test.ts index b7f9f878..e72e8b94 100644 --- a/packages/agent/src/prompts.test.ts +++ b/packages/agent/src/prompts.test.ts @@ -31,10 +31,8 @@ describe('contextBlock', () => { }); describe('no prompt carries CI status (DESIGN.md "CI is not a signal")', () => { - const failing = makePr({ - checks: { rollup: 'FAILURE', contexts: [{ name: 'backend-tests', conclusion: 'FAILURE', completedAt: '2026-09-02T09:30:00Z' }] }, - }); - const ciEvent = makeEvent({ id: 'acme/app#1:ci:x', kind: 'ci', actor: '', isBot: true, summary: 'CI failed: backend-tests', sourceId: 'abc:FAILURE' }); + // PostPile fetches no checks since 0.21.0; stored notes from before can still talk about CI. + const failing = makePr(); const mention = makeEvent({ kind: 'mention', summary: 'bob: can you look at the cache key?', ruleLoudness: 'loud', ruleReason: 'mentions you' }); const topic = makeTopic(); const prompts: Record = { @@ -71,7 +69,7 @@ describe('no prompt carries CI status (DESIGN.md "CI is not a signal")', () => { model: 'm', createdAt: '2026-09-02T09:00:00Z', }, - events: [ciEvent, mention], + events: [mention], rule: { loudness: 'loud', reason: 'mentions you', whoseTurn: { kind: 'you', move: 'reply', who: null, what: 'Reply to bob', prKey: failing.key }, why: '@', conversation: false }, template: { title: 'bob mentioned you', body: 'can you look at the cache key?' }, }, @@ -86,7 +84,7 @@ describe('no prompt carries CI status (DESIGN.md "CI is not a signal")', () => { dossier: null, sources: [], prs: [failing], - events: [ciEvent, mention], + events: [mention], viewer, context: fullContext, }), diff --git a/packages/agent/src/prompts/dossier-update.ts b/packages/agent/src/prompts/dossier-update.ts index 1a694f5e..4e0b7251 100644 --- a/packages/agent/src/prompts/dossier-update.ts +++ b/packages/agent/src/prompts/dossier-update.ts @@ -3,7 +3,7 @@ import type { Fact, PrEvent } from '@postpile/core'; import type { DossierRefs, UserSource } from '../dossier-refs.ts'; import type { DossierUpdateInput } from '../service.ts'; import { renderDossier } from './dossier.ts'; -import { clip, contextBlock, entityText, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prLine, viewerLine, withoutCi, WORK_GLOSSARY, workContextBlock } from './shared.ts'; +import { clip, contextBlock, entityText, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prLine, viewerLine, WORK_GLOSSARY, workContextBlock } from './shared.ts'; /** * A standing topic has no finish line (core TopicKind): its dossier follows @@ -26,7 +26,7 @@ function eventLine(event: PrEvent, shortId: string): string { /** Bots are most of the volume and none of the story: one count per PR. CI results are left out (NO_CI_RULE). */ function botCounts(events: PrEvent[]): string[] { const byPr = new Map }>(); - for (const event of withoutCi(events).filter((e) => e.isBot)) { + for (const event of events.filter((e) => e.isBot)) { const entry = byPr.get(event.prKey) ?? { count: 0, kinds: new Set() }; entry.count += 1; entry.kinds.add(event.kind); diff --git a/packages/agent/src/prompts/memory-recheck.ts b/packages/agent/src/prompts/memory-recheck.ts index 5784e517..0a4e7fe6 100644 --- a/packages/agent/src/prompts/memory-recheck.ts +++ b/packages/agent/src/prompts/memory-recheck.ts @@ -1,7 +1,7 @@ import type { MemorySource, PrEvent } from '@postpile/core'; import type { MemoryRecheckInput } from '../service.ts'; import { renderDossier } from './dossier.ts'; -import { clip, contextBlock, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prDetails, shortDetail, viewerLine, withoutCi } from './shared.ts'; +import { clip, contextBlock, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prDetails, shortDetail, viewerLine } from './shared.ts'; function sourceLine(source: MemorySource): string { const who = source.who ? `@${source.who} ` : ''; @@ -28,7 +28,7 @@ export function memoryRecheckPrompt(input: MemoryRecheckInput): string { const github = input.sources.filter((source) => source.who !== null).map(sourceLine); const own = input.sources.filter((source) => source.who === null).map(sourceLine); const prs = input.prs.map((pr) => prDetails(pr, input.viewer, shortDetail)).join('\n\n'); - const events = withoutCi(input.events); + const events = input.events; const dossier = input.dossier ? githubData(renderDossier(input.dossier, new Map(input.prs.map((pr) => [pr.key, pr])))) : '(no dossier)'; return `You keep a developer's memory of their code review work. ${viewerLine(input.viewer)} They asked you to recheck one line you remember ${topic}. Check it against GitHub as it is now. diff --git a/packages/agent/src/prompts/ping-decision.ts b/packages/agent/src/prompts/ping-decision.ts index a05b97c3..8d2f117f 100644 --- a/packages/agent/src/prompts/ping-decision.ts +++ b/packages/agent/src/prompts/ping-decision.ts @@ -1,6 +1,6 @@ import type { PrEvent } from '@postpile/core'; import type { PingDecisionInput, PingDecisionItem } from '../service.ts'; -import { clip, contextBlock, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prLine, viewerLine, withoutCi, workContextBlock } from './shared.ts'; +import { clip, contextBlock, GITHUB_DATA_RULE, githubData, jsonOnly, NO_CI_RULE, prLine, viewerLine, workContextBlock } from './shared.ts'; function eventLine(event: PrEvent): string { const bot = event.isBot ? ' (bot)' : ''; @@ -39,7 +39,7 @@ function itemSection(item: PingDecisionItem): string { lines.push('Live conversation: a person answers what the user said on this PR in the last two hours. It always pings; write the text.'); } lines.push( - `New activity, newest first:\n${githubData(`${prLine(item.pr)}\n${withoutCi(item.events).map(eventLine).join('\n')}`)}`, + `New activity, newest first:\n${githubData(`${prLine(item.pr)}\n${item.events.map(eventLine).join('\n')}`)}`, // The template quotes the comment, so it is GitHub text too. `Default notification:\n${githubData(`title: ${item.template.title}\nbody: ${item.template.body.replaceAll('\n', ' / ')}`)}`, ); diff --git a/packages/agent/src/prompts/shared.ts b/packages/agent/src/prompts/shared.ts index 03d7908d..bc39ad77 100644 --- a/packages/agent/src/prompts/shared.ts +++ b/packages/agent/src/prompts/shared.ts @@ -1,5 +1,5 @@ import { homeTeamsOf, isBot, isMachineComment, isPrOwner, prOwners, sameLogin, standingApprovals } from '@postpile/core'; -import type { Comment, EntityRef, Feedback, FeedbackKind, Pr, PrEvent, Provenance, Viewer } from '@postpile/core'; +import type { Comment, EntityRef, Feedback, FeedbackKind, Pr, Provenance, Viewer } from '@postpile/core'; import type { PromptContext } from '../service.ts'; /** Trims a body to keep prompts bounded without losing the point. */ @@ -34,11 +34,6 @@ facts. It flakes, and bringing a PR to green is the author's job. Older notes, s earlier reads above may still mention CI status: it is stale, ignore it. Changes to CI files and CI as the subject of the work are code, not status: those are fine to talk about.`; -/** Events without CI results, which never reach a prompt (NO_CI_RULE). */ -export function withoutCi(events: PrEvent[]): PrEvent[] { - return events.filter((event) => event.kind !== 'ci'); -} - /** * What area, topic, tile and set mean, said the same way to every agent that * sorts, groups or tidies PRs, so they cut work at the same grain (decided diff --git a/packages/agent/src/test-fixtures.ts b/packages/agent/src/test-fixtures.ts index fc875de5..f7d6c033 100644 --- a/packages/agent/src/test-fixtures.ts +++ b/packages/agent/src/test-fixtures.ts @@ -32,7 +32,6 @@ export function makePr(overrides: Partial = {}): Pr { comments: [], threads: [], timeline: [], - checks: { rollup: 'SUCCESS', contexts: [] }, headOid: 'abc', createdAt: '2026-09-01T10:00:00Z', updatedAt: '2026-09-02T10:00:00Z', diff --git a/packages/core/src/activity.test.ts b/packages/core/src/activity.test.ts index cb2b65df..b865e4a7 100644 --- a/packages/core/src/activity.test.ts +++ b/packages/core/src/activity.test.ts @@ -18,15 +18,15 @@ function ev(overrides: Partial, display: EventDisplayState = 'seen'): E } describe('activityList', () => { - it('keeps human talk and lifecycle, folds bots and CI into noise', () => { + it('keeps human talk and lifecycle, folds bots into noise', () => { const comment = ev({ kind: 'comment', actor: 'lyra', summary: 'lyra commented', at: at(1) }); const review = ev({ kind: 'review_approved', actor: 'ada', summary: 'ada approved', at: at(2) }); - const ci = ev({ kind: 'ci', actor: '', isBot: true, summary: 'CI passed', at: at(3) }); + const deploy = ev({ kind: 'deploy', actor: 'vercel', isBot: true, summary: 'vercel deployed', at: at(3) }); const bot = ev({ kind: 'bot_comment', actor: 'greptile-apps[bot]', isBot: true, summary: 'greptile commented', at: at(4) }); const merged = ev({ kind: 'merged', actor: 'ada', summary: 'ada merged', at: at(5) }); - const list = activityList([comment, review, ci, bot, merged], who); + const list = activityList([comment, review, deploy, bot, merged], who); expect(list.earlier.map((line) => line.summary)).toEqual(['ada merged', 'ada approved', 'lyra commented']); - expect(list.noise.map((item) => item.summary)).toEqual(['greptile commented', 'CI passed']); + expect(list.noise.map((item) => item.summary)).toEqual(['greptile commented', 'vercel deployed']); expect(list.fresh).toEqual([]); }); @@ -75,25 +75,25 @@ describe('activityList', () => { }); it('labels machine-only noise as bot/CI events', () => { - const ci = ev({ kind: 'ci', actor: '', isBot: true, summary: 'CI passed' }); + const queue = ev({ kind: 'merge_queue', actor: '', isBot: true, summary: 'queued' }); const deploy = ev({ kind: 'deploy', actor: 'vercel', isBot: true, summary: 'vercel deploy' }); - expect(noiseLabel([ci, deploy])).toBe('2 bot/CI events'); + expect(noiseLabel([queue, deploy])).toBe('2 bot/CI events'); }); }); describe('activityList fresh noise', () => { const bot = (minutes: number, display: EventDisplayState = 'quiet') => ev({ kind: 'bot_comment', actor: 'greptile[bot]', isBot: true, summary: 'greptile commented', at: at(minutes) }, display); - const ci = (minutes: number) => ev({ kind: 'ci', actor: '', isBot: true, summary: 'CI passed', at: at(minutes) }, 'quiet'); + const deploy = (minutes: number) => ev({ kind: 'deploy', actor: 'vercel', isBot: true, summary: 'vercel deployed', at: at(minutes) }, 'quiet'); it('moves unseen noise after the last touch into freshNoise while something loud is new', () => { const before = bot(1); const seen = bot(6, 'seen'); - const after = [bot(7), bot(8), ci(9)]; + const after = [bot(7), bot(8), deploy(9)]; const push = ev({ kind: 'commits_pushed', actor: 'pim', summary: 'pim pushed', at: at(10) }, 'loud'); const list = activityList([before, seen, ...after, push], who, at(5)); expect(list.freshNoise.map((item) => item.id)).toEqual(after.map((view) => view.event.id).toReversed()); - expect(list.freshNoiseLabel).toBe('2 bot comments, CI'); + expect(list.freshNoiseLabel).toBe('2 bot comments, a deploy'); expect(list.noise.map((item) => item.id)).toEqual([seen.event.id, before.event.id]); }); @@ -105,7 +105,7 @@ describe('activityList fresh noise', () => { }); it('keeps all noise in the list while nothing loud is new', () => { - const list = activityList([bot(1), ci(2)], who, at(0)); + const list = activityList([bot(1), deploy(2)], who, at(0)); expect(list.freshNoise).toEqual([]); expect(list.freshNoiseLabel).toBe(''); expect(list.noise).toHaveLength(2); @@ -210,8 +210,7 @@ describe('threadChangedAt', () => { describe('noiseSummary', () => { it('says what the noise is', () => { const comments = Array.from({ length: 10 }, (_, n) => ev({ kind: 'bot_comment', actor: 'greptile[bot]', isBot: true, at: at(n) })); - const ci = ev({ kind: 'ci', actor: '', isBot: true }); - expect(noiseSummary([...comments, ci, ci])).toBe('10 bot comments, CI'); + expect(noiseSummary(comments)).toBe('10 bot comments'); }); it('names bot pushes, deploys, the merge queue and the rest', () => { diff --git a/packages/core/src/activity.ts b/packages/core/src/activity.ts index c30d5b99..2525c1d5 100644 --- a/packages/core/src/activity.ts +++ b/packages/core/src/activity.ts @@ -283,7 +283,7 @@ function byTime(a: EventView, b: EventView): number { /** "4 bot/CI events", or "4 bot/CI and other events" when review requests between others are in it. */ export function noiseLabel(noise: EventView[]): string { - const machineKinds: EventKind[] = ['ci', 'deploy', 'merge_queue', 'bot_comment']; + const machineKinds: EventKind[] = ['deploy', 'merge_queue', 'bot_comment']; const machine = noise.every((view) => view.event.isBot || machineKinds.includes(view.event.kind)); const events = noise.length === 1 ? 'event' : 'events'; return machine ? `${noise.length} bot/CI ${events}` : `${noise.length} bot/CI and other ${events}`; @@ -295,21 +295,19 @@ function countWord(count: number, word: string, plural = `${word}s`): string { /** * The noise by what it is, for the "New since you looked" box: "10 bot - * comments, CI", "2 bot pushes, a deploy, merge queue, 1 other". + * comments", "2 bot pushes, a deploy, merge queue, 1 other". */ export function noiseSummary(noise: EventView[]): string { const count = (test: (view: EventView) => boolean) => noise.filter(test).length; const isKind = (kinds: EventKind[]) => (view: EventView) => kinds.includes(view.event.kind); const comments = count((view) => view.event.kind === 'bot_comment' || (view.event.isBot && HUMAN_TALK.includes(view.event.kind))); const pushes = count((view) => view.event.isBot && PUSH_KINDS.includes(view.event.kind)); - const ci = count(isKind(['ci'])); const deploys = count(isKind(['deploy'])); const queue = count(isKind(['merge_queue'])); - const other = noise.length - comments - pushes - ci - deploys - queue; + const other = noise.length - comments - pushes - deploys - queue; const parts = [ comments > 0 ? countWord(comments, 'bot comment') : '', pushes > 0 ? countWord(pushes, 'bot push', 'bot pushes') : '', - ci > 0 ? 'CI' : '', deploys > 0 ? (deploys === 1 ? 'a deploy' : `${deploys} deploys`) : '', queue > 0 ? 'merge queue' : '', other > 0 ? `${other} other` : '', diff --git a/packages/core/src/checks.test.ts b/packages/core/src/checks.test.ts deleted file mode 100644 index f49e2256..00000000 --- a/packages/core/src/checks.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { summarizeChecks } from './checks.ts'; -import { deriveEvents } from './events.ts'; -import { at, makePr, viewer } from './fixtures.ts'; -import type { CheckContext, Checks } from './types.ts'; - -function context(name: string, conclusion: string | null, minutes: number | null): CheckContext { - return { name, conclusion, completedAt: minutes === null ? null : at(minutes) }; -} - -const mixed: Checks = { - rollup: 'FAILURE', - contexts: [ - context('lint', 'SUCCESS', 3), - context('docs', 'SKIPPED', 1), - context('size', 'NEUTRAL', 2), - context('test', 'FAILURE', 7), - context('deploy', 'CANCELLED', 4), - context('e2e', null, null), - context('types', 'FAILURE', 5), - ], -}; - -describe('summarizeChecks', () => { - it('counts success, neutral and skipped as passed, other conclusions as failed, no conclusion as pending', () => { - expect(summarizeChecks(mixed)).toMatchObject({ rollup: 'FAILURE', total: 7, passed: 3, failed: 3, pending: 1 }); - }); - - it('keeps the newest finish time and the FAILURE names in context order', () => { - const summary = summarizeChecks(mixed); - expect(summary.finishedAt).toBe(at(7)); - // A cancelled run counts as failed but is not named, like in the CI event. - expect(summary.failedNames).toEqual(['test', 'types']); - }); - - it('says nothing finished while every check still runs, and handles no checks', () => { - expect(summarizeChecks({ rollup: 'PENDING', contexts: [context('e2e', null, null)] })).toEqual({ - rollup: 'PENDING', - total: 1, - passed: 0, - failed: 0, - pending: 1, - finishedAt: null, - failedNames: [], - }); - expect(summarizeChecks({ rollup: 'NONE', contexts: [] })).toEqual({ - rollup: 'NONE', - total: 0, - passed: 0, - failed: 0, - pending: 0, - finishedAt: null, - failedNames: [], - }); - }); - - it('holds what the CI event reads: its time and the names in its summary', () => { - const pr = makePr({ headOid: 'abc', checks: mixed }); - const ci = deriveEvents(pr, viewer, null).find((event) => event.kind === 'ci'); - const summary = summarizeChecks(mixed); - expect(ci?.at).toBe(summary.finishedAt); - expect(ci?.summary).toBe(`CI failed: ${summary.failedNames.join(', ')}`); - }); -}); diff --git a/packages/core/src/checks.ts b/packages/core/src/checks.ts deleted file mode 100644 index 89476ba1..00000000 --- a/packages/core/src/checks.ts +++ /dev/null @@ -1,53 +0,0 @@ -// A PR's checks in a few numbers (DESIGN.md "CI is not a signal"). Nothing -// in PostPile reads a single check context: the PR pane counts them, the CI -// event reads the rollup, the newest finish time and the failing names. -// Rules only, no IO. -import type { CheckRollup, Checks, IsoTime } from './types.ts'; - -/** Conclusions that count as passed: success, plus neutral and skipped, which never block a merge. */ -const PASSED_CONCLUSIONS = new Set(['SUCCESS', 'NEUTRAL', 'SKIPPED']); - -export interface ChecksSummary { - rollup: CheckRollup; - /** Contexts on the head commit as fetched (the query takes the first 100), so not always every check. */ - total: number; - /** SUCCESS, NEUTRAL, SKIPPED. */ - passed: number; - /** Any other conclusion: FAILURE, CANCELLED, TIMED_OUT, ACTION_REQUIRED, … */ - failed: number; - /** No conclusion yet: still running or queued. */ - pending: number; - /** The newest completedAt among them; null when none finished. */ - finishedAt: IsoTime | null; - /** Names of the contexts whose conclusion is FAILURE, in context order: what the CI event names. */ - failedNames: string[]; -} - -/** - * The summary of one PR's checks. Counts follow the PR pane's Checks fact - * (failed and still running both read as "not passing" there); - * `finishedAt` and `failedNames` are what the CI event reads (`ciEvent`). - */ -export function summarizeChecks(checks: Checks): ChecksSummary { - let passed = 0; - let failed = 0; - let pending = 0; - let finishedAt: IsoTime | null = null; - const failedNames: string[] = []; - for (const context of checks.contexts) { - if (context.conclusion === null) { - pending += 1; - } else if (PASSED_CONCLUSIONS.has(context.conclusion)) { - passed += 1; - } else { - failed += 1; - } - if (context.conclusion === 'FAILURE') { - failedNames.push(context.name); - } - if (context.completedAt !== null && (finishedAt === null || context.completedAt > finishedAt)) { - finishedAt = context.completedAt; - } - } - return { rollup: checks.rollup, total: checks.contexts.length, passed, failed, pending, finishedAt, failedNames }; -} diff --git a/packages/core/src/delta.test.ts b/packages/core/src/delta.test.ts index 2c4bd7b6..2a366b54 100644 --- a/packages/core/src/delta.test.ts +++ b/packages/core/src/delta.test.ts @@ -85,15 +85,6 @@ describe('selectTopicDelta', () => { expect(delta.toSeq).toBe(14); }); - it('drops CI results, so a CI-only change starts no dossier update, and still moves the cursor', () => { - const ci = { kind: 'ci', actor: '', isBot: true, ruleLoudness: 'quiet', summary: 'CI failed: test' } as const; - const onlyCi = selectTopicDelta(input({ logged: [logged(11, pr1, ci), logged(12, pr2, ci)] })); - expect(isEmptyDelta(onlyCi)).toBe(true); - expect(onlyCi.toSeq).toBe(12); - const mixed = selectTopicDelta(input({ logged: [logged(11, pr1, ci), logged(12, pr1)] })); - expect(mixed.events.map((event) => event.sourceId)).toEqual(['12']); - }); - it('lets review bot comments ride along: alone they start nothing and keep the cursor before them', () => { const rabbit = { kind: 'bot_comment', actor: 'coderabbitai[bot]', isBot: true, ruleLoudness: 'quiet' } as const; const alone = selectTopicDelta(input({ logged: [logged(11, pr1, rabbit)] })); diff --git a/packages/core/src/event-roles.test.ts b/packages/core/src/event-roles.test.ts index 9eaa9f50..fb7cd1d8 100644 --- a/packages/core/src/event-roles.test.ts +++ b/packages/core/src/event-roles.test.ts @@ -104,8 +104,6 @@ const ROWS: Row[] = [ { scenario: 'agentForViewer', entry: 'agentPushes', kind: 'commits_pushed', automation: true, loudness: 'muted', role: 'noise', alone: false, withTrigger: false, newer: false, quietRead: 'mark' }, { scenario: 'teamRouted', entry: 'agentPushes', kind: 'commits_pushed', automation: true, loudness: 'quiet', role: 'ride_along', alone: false, withTrigger: true, newer: false, quietRead: 'mark' }, { scenario: 'agentForViewer', entry: 'agentMarksReady', kind: 'ready_for_review', automation: true, loudness: 'quiet', role: 'trigger', alone: true, withTrigger: true, newer: true, quietRead: 'mark' }, - { scenario: 'ownOpen', entry: 'ciFails', kind: 'ci', automation: true, loudness: 'quiet', role: 'noise', alone: false, withTrigger: false, newer: false, quietRead: 'mark' }, - { scenario: 'dependabot', entry: 'ciFails', kind: 'ci', automation: true, loudness: 'quiet', role: 'noise', alone: false, withTrigger: false, newer: false, quietRead: 'mark' }, // The viewer: always a trigger (as today), never someone else's activity for a quiet read. { scenario: 'ownOpen', entry: 'viewerComments', kind: 'comment', automation: false, loudness: 'quiet', role: 'trigger', alone: true, withTrigger: true, newer: true, quietRead: 'human_activity' }, @@ -233,7 +231,6 @@ describe('the event corpus through the pipeline', () => { reopened: true, ready_for_review: true, converted_to_draft: true, - ci: true, deploy: true, merge_queue: true, bot_comment: true, diff --git a/packages/core/src/event-roles.ts b/packages/core/src/event-roles.ts index b783c427..c3ca3fe1 100644 --- a/packages/core/src/event-roles.ts +++ b/packages/core/src/event-roles.ts @@ -61,7 +61,7 @@ export function memoryRole(event: PrEvent): MemoryRole { if (loudness === 'loud') { return 'trigger'; } - if (loudness === 'muted' || event.kind === 'ci') { + if (loudness === 'muted') { return 'noise'; } if (PUSH_KINDS.includes(event.kind)) { diff --git a/packages/core/src/events.test.ts b/packages/core/src/events.test.ts index 07de170e..3c6b7626 100644 --- a/packages/core/src/events.test.ts +++ b/packages/core/src/events.test.ts @@ -247,23 +247,6 @@ describe('deriveEvents: reviews, commits, timeline, CI', () => { expect(only(deriveEvents(reviewed, viewer, null), 'merged')).toHaveLength(1); }); - it('emits one CI line per head commit and result', () => { - const pr = makePr({ - headOid: 'abc', - checks: { - rollup: 'FAILURE', - contexts: [ - { name: 'lint', conclusion: 'SUCCESS', completedAt: at(3) }, - { name: 'test', conclusion: 'FAILURE', completedAt: at(5) }, - ], - }, - }); - const [ci] = deriveEvents(pr, viewer, null); - expect(ci).toMatchObject({ kind: 'ci', sourceId: 'abc:FAILURE', at: at(5), summary: 'CI failed: test' }); - expect(ci?.ruleLoudness).toBe('quiet'); - expect(deriveEvents(makePr({ checks: { rollup: 'PENDING', contexts: [] } }), viewer, null)).toHaveLength(0); - }); - it('returns events oldest first', () => { const pr = makePr({ comments: [makeComment({ id: 'late', createdAt: at(50) })], diff --git a/packages/core/src/events.ts b/packages/core/src/events.ts index b96fb656..827a8ce0 100644 --- a/packages/core/src/events.ts +++ b/packages/core/src/events.ts @@ -389,30 +389,6 @@ function timelineEvents(pr: Pr, viewer: Viewer): RawEvent[] { })); } -/** One line for the latest finished CI result on the head commit. */ -function ciEvent(pr: Pr): RawEvent | null { - const { rollup, contexts } = pr.checks; - if (rollup !== 'SUCCESS' && rollup !== 'FAILURE') { - return null; - } - const finished = contexts.map((c) => c.completedAt).filter((at): at is string => at !== null); - if (finished.length === 0) { - return null; - } - const failed = contexts.filter((c) => c.conclusion === 'FAILURE').map((c) => c.name); - const summary = failed.length > 0 ? `CI failed: ${failed.join(', ')}` : 'CI passed'; - return { - kind: 'ci', - actor: '', - isBot: true, - at: finished.sort().at(-1) as string, - summary, - url: null, - sourceId: `${pr.headOid}:${rollup}`, - subject: null, - }; -} - function collectRawEvents(pr: Pr, viewer: Viewer, userState: UserPrState | null): RawEvent[] { const raw: RawEvent[] = []; for (const comment of pr.comments) { @@ -428,10 +404,6 @@ function collectRawEvents(pr: Pr, viewer: Viewer, userState: UserPrState | null) raw.push(...reviewEvents(pr)); raw.push(...commitEvents(pr, viewer, userState)); raw.push(...timelineEvents(pr, viewer)); - const ci = ciEvent(pr); - if (ci) { - raw.push(ci); - } return raw; } diff --git a/packages/core/src/fixtures.ts b/packages/core/src/fixtures.ts index e30f5fa7..68ed6937 100644 --- a/packages/core/src/fixtures.ts +++ b/packages/core/src/fixtures.ts @@ -63,7 +63,6 @@ export function makePr(overrides: Partial & { number?: number; repo?: string comments: [], threads: [], timeline: [], - checks: { rollup: 'NONE', contexts: [] }, headOid: 'head', createdAt: at(0), updatedAt: at(0), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a93db3f9..e3b996ce 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -27,7 +27,6 @@ export * from './for-whom.ts'; export * from './approvals.ts'; export * from './merge-queue.ts'; export * from './pr-status.ts'; -export * from './checks.ts'; export * from './pr-pane.ts'; export * from './tile-people.ts'; export * from './changes-answered.ts'; diff --git a/packages/core/src/last-touch.test.ts b/packages/core/src/last-touch.test.ts index 3f926b6a..4734d37c 100644 --- a/packages/core/src/last-touch.test.ts +++ b/packages/core/src/last-touch.test.ts @@ -52,9 +52,9 @@ describe('touchKindOf', () => { expect(touchKindOf(makeEvent({ kind: 'force_pushed', actor: 'rowan' }), ownPr, viewer)).toBeNull(); }); - it('is null for other people, CI and own events that are not a touch', () => { + it('is null for other people, bots and own events that are not a touch', () => { expect(touchKindOf(makeEvent({ kind: 'review_approved', actor: 'rowan' }), alicePr, viewer)).toBeNull(); - expect(touchKindOf(makeEvent({ kind: 'ci', actor: '', isBot: true }), alicePr, viewer)).toBeNull(); + expect(touchKindOf(makeEvent({ kind: 'deploy', actor: 'vercel', isBot: true }), alicePr, viewer)).toBeNull(); expect(touchKindOf(own('review_requested', 0), alicePr, viewer)).toBeNull(); }); }); diff --git a/packages/core/src/loudness.test.ts b/packages/core/src/loudness.test.ts index c92b0719..e392faff 100644 --- a/packages/core/src/loudness.test.ts +++ b/packages/core/src/loudness.test.ts @@ -32,20 +32,13 @@ describe('ruleLoudness', () => { expect(decision).toEqual({ loudness: 'quiet', reason: 'you already replied' }); }); - it('keeps bot mentions, CI, deploys and the merge queue quiet', () => { + it('keeps bot mentions, deploys and the merge queue quiet', () => { expect(ruleLoudness(input({ kind: 'mention', actor: 'github-actions', isBot: true })).loudness).toBe('quiet'); - for (const kind of ['ci', 'deploy', 'merge_queue', 'bot_comment'] as const) { + for (const kind of ['deploy', 'merge_queue', 'bot_comment'] as const) { expect(ruleLoudness(input({ kind, actor: 'someone' })).loudness).toBe('quiet'); } }); - it('never makes a CI result loud, not even a failure on the viewer own PR (CI is not a signal)', () => { - const own = makePr({ author: viewer.login, checks: { rollup: 'FAILURE', contexts: [] } }); - for (const pr of [own, makePr({ checks: { rollup: 'FAILURE', contexts: [] } }), makePr({ isDraft: true })]) { - expect(ruleLoudness(input({ kind: 'ci', actor: '', isBot: true, pr })).loudness).toBe('quiet'); - } - }); - it('mutes a bot rebase on a draft but not on a ready PR', () => { const draft = makePr({ isDraft: true }); expect(ruleLoudness(input({ kind: 'force_pushed', actor: 'trunk-io', isBot: true, pr: draft }))).toEqual({ @@ -161,7 +154,7 @@ describe('loudness table', () => { question_to_user: true, comment: true, review_approved: true, review_changes_requested: true, review_commented: true, commits_pushed: true, commits_after_approval: true, force_pushed: true, merged: true, merged_without_review: true, closed: true, reopened: true, ready_for_review: true, converted_to_draft: true, - ci: true, deploy: true, merge_queue: true, bot_comment: true, comment_edited: true, look_closer: true, + deploy: true, merge_queue: true, bot_comment: true, comment_edited: true, look_closer: true, }; const kinds = Object.keys(kindNames) as LoudnessInput['kind'][]; const prs = [makePr(), makePr({ author: viewer.login }), makePr({ isDraft: true }), makePr({ state: 'MERGED' })]; diff --git a/packages/core/src/loudness.ts b/packages/core/src/loudness.ts index 36fe7f90..c06d1970 100644 --- a/packages/core/src/loudness.ts +++ b/packages/core/src/loudness.ts @@ -37,7 +37,7 @@ export interface LoudnessDecision { } // Machine activity that never needs a person: shown with a dot at most. -const machineKinds: EventKind[] = ['ci', 'deploy', 'merge_queue', 'bot_comment']; +const machineKinds: EventKind[] = ['deploy', 'merge_queue', 'bot_comment']; const reviewKinds: EventKind[] = ['review_approved', 'review_changes_requested', 'review_commented']; diff --git a/packages/core/src/pings.test.ts b/packages/core/src/pings.test.ts index f05f28cb..32ad10fb 100644 --- a/packages/core/src/pings.test.ts +++ b/packages/core/src/pings.test.ts @@ -85,11 +85,6 @@ describe('pingRule', () => { expect(pingRule([raisedComment], pr, viewer, false)).toMatchObject({ class: 'not_addressed', reason: 'coverage dropped' }); }); - it('never pings for CI, not even a failure on the viewer own PR', () => { - const ci = makeEvent({ id: 'ci', kind: 'ci', actor: '', isBot: true, summary: 'CI failed: test', ruleLoudness: 'quiet', ruleReason: 'bot activity' }); - expect(pingRule([ci], ownPr, viewer, false)).toMatchObject({ class: 'bot', loudness: 'quiet' }); - }); - it('falls back to quiet, then muted', () => { expect(pingRule([makeEvent({ ruleLoudness: 'quiet' })], pr, viewer, false).class).toBe('quiet'); expect(pingRule([makeEvent({ ruleLoudness: 'muted' })], pr, viewer, false).class).toBe('muted'); @@ -174,7 +169,7 @@ describe('a review request counts by whom it asks, not who clicked it', () => { describe('ping table', () => { it('has a row for every mix of loudness, kind, PR state and quiet repo', () => { - const kinds: EventKind[] = ['mention', 'review_requested', 'comment', 'ci', 'commits_after_approval']; + const kinds: EventKind[] = ['mention', 'review_requested', 'comment', 'deploy', 'commits_after_approval']; const loudnesses: Loudness[] = ['loud', 'quiet', 'muted']; const prs = [pr, ownPr, makePr({ isDraft: true }), makePr({ state: 'MERGED' })]; for (const target of prs) { diff --git a/packages/core/src/pr-pane.test.ts b/packages/core/src/pr-pane.test.ts index a9a5c97d..ed35cedb 100644 --- a/packages/core/src/pr-pane.test.ts +++ b/packages/core/src/pr-pane.test.ts @@ -9,7 +9,6 @@ const PANE_FIELDS = [ 'baseRef', 'body', 'changedFiles', - 'checks', 'createdAt', 'deletions', 'files', @@ -96,19 +95,6 @@ describe('prPaneView', () => { expect(prPaneView(makePr()).lastCommitAt).toBeNull(); }); - it('sums up the checks', () => { - const pr = makePr({ - checks: { - rollup: 'FAILURE', - contexts: [ - { name: 'lint', conclusion: 'SUCCESS', completedAt: at(3) }, - { name: 'test', conclusion: 'FAILURE', completedAt: at(4) }, - ], - }, - }); - expect(prPaneView(pr).checks).toEqual({ rollup: 'FAILURE', total: 2, passed: 1, failed: 1, pending: 0, finishedAt: at(4), failedNames: ['test'] }); - }); - it('stays small however much the PR has talked', () => { const report = 'CI report '.repeat(300); const comments = Array.from({ length: 200 }, (_, index) => makeComment({ id: `c${index}`, author: 'ci-bot[bot]', body: report })); diff --git a/packages/core/src/pr-pane.ts b/packages/core/src/pr-pane.ts index 2c344dae..b4bb909a 100644 --- a/packages/core/src/pr-pane.ts +++ b/packages/core/src/pr-pane.ts @@ -1,10 +1,9 @@ // What the PR pane reads of a PR (`PrDetail.pr`). The stored `Pr` holds every -// comment, thread, commit, timeline item and check context; the pane shows -// none of them directly. Its activity list, reply and react targets come +// comment, thread, commit and timeline item; the pane shows none of them +// directly. Its activity list, reply and react targets come // from core's `activityList` on `PrDetail.activity`, built from the stored // PR before the slim view is made. MCP `pr_context` and the CLI read the // same view. Rules only, no IO. -import { summarizeChecks, type ChecksSummary } from './checks.ts'; import type { IsoTime, Pr, PrFile, PrKey, PrRef, PrState, Review, ReviewDecision } from './types.ts'; /** A review as the pane reads it: who, which state, when. Review text shows in the activity list. */ @@ -41,7 +40,6 @@ export interface PrPaneView { reviews: PaneReview[]; /** The last stored commit's time ("pushed 2h"); null for a PR without commits in the snapshot. */ lastCommitAt: IsoTime | null; - checks: ChecksSummary; createdAt: IsoTime; updatedAt: IsoTime; mergedAt: IsoTime | null; @@ -73,7 +71,6 @@ export function prPaneView(pr: Pr): PrPaneView { reviewerTeams: pr.reviewerTeams, reviews: pr.reviews.map((review) => ({ author: review.author, state: review.state, submittedAt: review.submittedAt })), lastCommitAt: lastCommit ? lastCommit.committedAt : null, - checks: summarizeChecks(pr.checks), createdAt: pr.createdAt, updatedAt: pr.updatedAt, mergedAt: pr.mergedAt, diff --git a/packages/core/src/pr-status.test.ts b/packages/core/src/pr-status.test.ts index f4c63a5d..b890d8e8 100644 --- a/packages/core/src/pr-status.test.ts +++ b/packages/core/src/pr-status.test.ts @@ -4,12 +4,12 @@ import { isQueued, openThreadCount, prStatus } from './pr-status.ts'; describe('prStatus', () => { it('has lifecycle and review for an open PR', () => { - const pr = makePr({ reviewDecision: 'APPROVED', checks: { rollup: 'SUCCESS', contexts: [] } }); + const pr = makePr({ reviewDecision: 'APPROVED' }); expect(prStatus(pr)).toEqual({ lifecycle: 'open', review: 'approved', agentApprovers: [], mergeQueue: null, icon: 'open' }); }); - it('maps changes and never carries checks: CI is not a signal', () => { - expect(prStatus(makePr({ reviewDecision: 'CHANGES_REQUESTED', checks: { rollup: 'FAILURE', contexts: [] } }))).toEqual({ + it('maps changes', () => { + expect(prStatus(makePr({ reviewDecision: 'CHANGES_REQUESTED' }))).toEqual({ lifecycle: 'open', review: 'changes', agentApprovers: [], @@ -20,7 +20,7 @@ describe('prStatus', () => { it('leaves out parts that do not apply', () => { expect(prStatus(makePr({ reviewDecision: 'NONE' }))).toEqual({ lifecycle: 'open', review: null, agentApprovers: [], mergeQueue: null, icon: 'open' }); - expect(prStatus(makePr({ isDraft: true, checks: { rollup: 'SUCCESS', contexts: [] } }))).toEqual({ + expect(prStatus(makePr({ isDraft: true }))).toEqual({ lifecycle: 'draft', review: null, agentApprovers: [], diff --git a/packages/core/src/quiet-reads.test.ts b/packages/core/src/quiet-reads.test.ts index 90ec2987..00890c68 100644 --- a/packages/core/src/quiet-reads.test.ts +++ b/packages/core/src/quiet-reads.test.ts @@ -34,8 +34,8 @@ function botComment(minute: number, actor = 'trunk-io[bot]'): PrEvent { return makeEvent({ id: `bot-${minute}`, prKey: pr.key, kind: 'bot_comment', actor, isBot: true, at: at(minute), summary: `${actor} commented` }); } -function ciResult(minute: number): PrEvent { - return makeEvent({ id: `ci-${minute}`, prKey: pr.key, kind: 'ci', actor: '', isBot: true, at: at(minute), summary: 'CI passed' }); +function deployResult(minute: number): PrEvent { + return makeEvent({ id: `deploy-${minute}`, prKey: pr.key, kind: 'deploy', actor: 'vercel[bot]', isBot: true, at: at(minute), summary: 'vercel[bot] deployed' }); } function botReview(minute: number, actor = 'coderabbitai[bot]'): PrEvent { @@ -50,7 +50,7 @@ function input(overrides: Partial = {}): QuietReadInput { return { thread: makeThreadFor(pr, { lastReadAt: at(20), updatedAt: at(31), unread: true, reason: 'subscribed' }), pr, - events: [humanComment(5), botComment(30), ciResult(31)], + events: [humanComment(5), botComment(30), deployResult(31)], userState: null, viewer, notYours: false, @@ -60,9 +60,9 @@ function input(overrides: Partial = {}): QuietReadInput { } describe('botOnlySinceRead', () => { - it('returns the events after the read when every one is a bot, CI results without an actor included', () => { - const events = [humanComment(5), botComment(30), ciResult(31)]; - expect(botOnlySinceRead(makePr(), events, at(20), viewer)?.map((event) => event.id)).toEqual(['bot-30', 'ci-31']); + it('returns the events after the read when every one is a bot', () => { + const events = [humanComment(5), botComment(30), deployResult(31)]; + expect(botOnlySinceRead(makePr(), events, at(20), viewer)?.map((event) => event.id)).toEqual(['bot-30', 'deploy-31']); }); it('is null once a person took part after the read', () => { @@ -97,13 +97,13 @@ describe('botOnlySinceRead', () => { describe('botNames', () => { it('lists each bot once, CI for actor-less events', () => { - expect(botNames([botComment(30), ciResult(31), botComment(32)])).toEqual(['trunk-io[bot]', 'CI']); + expect(botNames([botComment(30), deployResult(31), botComment(32)])).toEqual(['trunk-io[bot]', 'vercel[bot]']); }); }); describe('quietReadCheck', () => { it('marks a read thread that turned unread only because of bots, naming them', () => { - expect(quietReadCheck(input())).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'CI'] }); + expect(quietReadCheck(input())).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'vercel[bot]'] }); }); it('leaves threads alone that GitHub has read or the user never read', () => { @@ -137,27 +137,27 @@ describe('quietReadCheck', () => { it('marks the user own open PR after a bot review or inline comment too (2026-10-01)', () => { const own = { ...pr, author: viewer.login }; - expect(quietReadCheck(input({ pr: own, events: [humanComment(5), botReview(30), ciResult(31)] })).kind).toBe('mark'); + expect(quietReadCheck(input({ pr: own, events: [humanComment(5), botReview(30), deployResult(31)] })).kind).toBe('mark'); const inline = makeComment({ id: 'rc9', kind: 'review_comment', threadId: 't1', path: 'a.ts', author: 'coderabbitai[bot]', createdAt: at(30) }); const inlineEvent = makeEvent({ id: 'inline', prKey: pr.key, kind: 'bot_comment', actor: 'coderabbitai[bot]', isBot: true, at: at(30), sourceId: 'rc9' }); - expect(quietReadCheck(input({ pr: { ...own, comments: [inline] }, events: [humanComment(5), inlineEvent, ciResult(31)] })).kind).toBe('mark'); + expect(quietReadCheck(input({ pr: { ...own, comments: [inline] }, events: [humanComment(5), inlineEvent, deployResult(31)] })).kind).toBe('mark'); }); it('marks the user own open PR when the bots only commented, ran CI or deployed (2026-09-30)', () => { const own = { ...pr, author: viewer.login }; - expect(quietReadCheck(input({ pr: own }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'CI'] }); + expect(quietReadCheck(input({ pr: own }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'vercel[bot]'] }); }); it('marks the user own merged or closed PR when only bots came after the read', () => { const merged = { ...pr, author: viewer.login, state: 'MERGED' as const, mergedAt: at(30) }; - expect(quietReadCheck(input({ pr: merged }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'CI'] }); + expect(quietReadCheck(input({ pr: merged }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'vercel[bot]'] }); const closed = { ...pr, author: viewer.login, state: 'CLOSED' as const }; expect(quietReadCheck(input({ pr: closed })).kind).toBe('mark'); }); it('does not take the viewer own review after the read for a person', () => { const ownReview = makeEvent({ id: 'own-review', prKey: pr.key, kind: 'review_approved', actor: viewer.login, at: at(25), seenAt: at(25) }); - expect(quietReadCheck(input({ events: [humanComment(5), ownReview, botComment(30), ciResult(31)] }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'CI'] }); + expect(quietReadCheck(input({ events: [humanComment(5), ownReview, botComment(30), deployResult(31)] }))).toEqual({ kind: 'mark', bots: ['trunk-io[bot]', 'vercel[bot]'] }); }); it('never marks a PR with an unseen merge without the user review, even when a bot merged it', () => { @@ -168,7 +168,7 @@ describe('quietReadCheck', () => { it('marks although the tile is unread (its thread is), but leaves it while the PR has unseen loud news', () => { const raised = { ...botComment(30), override: { loudness: 'loud' as const, reason: 'the finding needs a look', by: 'agent' as const } }; - expect(quietReadCheck(input({ events: [humanComment(5), raised, ciResult(31)] }))).toEqual({ kind: 'skip', why: 'unseen_loud' }); + expect(quietReadCheck(input({ events: [humanComment(5), raised, deployResult(31)] }))).toEqual({ kind: 'skip', why: 'unseen_loud' }); }); it('leaves it while the user move is new since the read, not for a move that stood before it', () => { @@ -176,7 +176,7 @@ describe('quietReadCheck', () => { // A bot marked the draft ready after the read: the review asked before is a move only now. const readied = makePr({ number: 7, author: 'alice', reviewerUsers: [viewer.login], timeline: [request, makeTimelineItem({ id: 'rd', kind: 'ready_for_review', actor: 'readybot[bot]', subject: null, at: at(30) })] }); const readyEvent = makeEvent({ id: 'ready', prKey: pr.key, kind: 'ready_for_review', actor: 'readybot[bot]', isBot: true, at: at(30), sourceId: 'rd' }); - expect(quietReadCheck(input({ pr: readied, events: [readyEvent, ciResult(31)] }))).toEqual({ kind: 'skip', why: 'your_move' }); + expect(quietReadCheck(input({ pr: readied, events: [readyEvent, deployResult(31)] }))).toEqual({ kind: 'skip', why: 'your_move' }); // Asked before the read and still owed: the move stood when the user read it. const asked = makePr({ number: 7, author: 'alice', reviewerUsers: [viewer.login], timeline: [request] }); expect(quietReadCheck(input({ pr: asked })).kind).toBe('mark'); @@ -244,13 +244,13 @@ describe('touchedReadCheck', () => { it('includes the user own PR, a bot review after the touch included', () => { const ownPr = { ...pr, author: viewer.login }; expect(touchedReadCheck(touched({ pr: ownPr, events: [humanComment(25), own('comment', 30)] }))).toEqual({ kind: 'mark', reason: 'replied' }); - expect(touchedReadCheck(touched({ pr: ownPr, events: [humanComment(25), own('comment', 30), ciResult(35)] }))).toEqual({ kind: 'mark', reason: 'replied' }); + expect(touchedReadCheck(touched({ pr: ownPr, events: [humanComment(25), own('comment', 30), deployResult(35)] }))).toEqual({ kind: 'mark', reason: 'replied' }); expect(touchedReadCheck(touched({ pr: ownPr, events: [humanComment(25), own('comment', 30), botReview(35)] })).kind).toBe('mark'); }); it('lets bots after the touch pass on the user own PR once it is merged', () => { const merged = { ...pr, author: viewer.login, state: 'MERGED' as const, mergedAt: at(36) }; - const events = [humanComment(25), own('comment', 30), ciResult(35), botComment(36, 'trunk-io[bot]')]; + const events = [humanComment(25), own('comment', 30), deployResult(35), botComment(36, 'trunk-io[bot]')]; expect(touchedReadCheck(touched({ pr: merged, events }))).toEqual({ kind: 'mark', reason: 'replied' }); }); @@ -291,14 +291,14 @@ describe('judgedReadCheck', () => { // Read at 20; lyra commented at 30 and the agent judged it quiet; CI at 31. function judged(overrides: Partial = {}): QuietReadInput { - return input({ events: [humanComment(5), teammate(30, { override: judgedQuiet }), ciResult(31)], ...overrides }); + return input({ events: [humanComment(5), teammate(30, { override: judgedQuiet }), deployResult(31)], ...overrides }); } it('marks when everything since the read is automation or a person the agent judged as not needing you', () => { - expect(judgedReadCheck(judged())).toEqual({ kind: 'mark', actors: ['lyra', 'CI'] }); - expect(judgedReadDetail(['lyra', 'CI'])).toBe('nothing that needs you since you last looked: lyra, CI'); - expect(quietReasonFromDetail(judgedReadDetail(['lyra', 'CI']))).toBe('judged'); - expect(actorsFromQuietDetail(judgedReadDetail(['lyra', 'CI']))).toEqual(['lyra', 'CI']); + expect(judgedReadCheck(judged())).toEqual({ kind: 'mark', actors: ['lyra', 'vercel[bot]'] }); + expect(judgedReadDetail(['lyra', 'vercel[bot]'])).toBe('nothing that needs you since you last looked: lyra, vercel[bot]'); + expect(quietReasonFromDetail(judgedReadDetail(['lyra', 'vercel[bot]']))).toBe('judged'); + expect(actorsFromQuietDetail(judgedReadDetail(['lyra', 'vercel[bot]']))).toEqual(['lyra', 'vercel[bot]']); }); it('counts from the newer of the read and the viewer review or comment', () => { @@ -310,7 +310,7 @@ describe('judgedReadCheck', () => { }); it('waits for the agent: a person not judged yet, or judged as needing you, keeps it unread', () => { - expect(judgedReadCheck(judged({ events: [teammate(30), ciResult(31)] }))).toEqual({ kind: 'skip', why: 'not_judged' }); + expect(judgedReadCheck(judged({ events: [teammate(30), deployResult(31)] }))).toEqual({ kind: 'skip', why: 'not_judged' }); const raised = teammate(30, { override: { loudness: 'loud', reason: 'asks for a decision', by: 'agent' } }); expect(judgedReadCheck(judged({ events: [raised] }))).toEqual({ kind: 'skip', why: 'unseen_loud' }); }); @@ -328,10 +328,10 @@ describe('judgedReadCheck', () => { }); it('leaves bots-only threads to the other rules, a bot review on your own open PR included', () => { - expect(judgedReadCheck(judged({ events: [ciResult(31)] }))).toEqual({ kind: 'skip', why: 'no_people' }); + expect(judgedReadCheck(judged({ events: [deployResult(31)] }))).toEqual({ kind: 'skip', why: 'no_people' }); const own = { ...pr, author: viewer.login }; expect(judgedReadCheck(judged({ pr: own, events: [teammate(30, { override: judgedQuiet }), botReview(31)] }))).toEqual({ kind: 'mark', actors: ['lyra', 'coderabbitai[bot]'] }); - expect(judgedReadCheck(judged({ pr: own }))).toEqual({ kind: 'mark', actors: ['lyra', 'CI'] }); + expect(judgedReadCheck(judged({ pr: own }))).toEqual({ kind: 'mark', actors: ['lyra', 'vercel[bot]'] }); }); it('keeps the safety checks: snapshot, a new move', () => { @@ -348,9 +348,9 @@ describe('judgedReadCheck', () => { describe('quiet read detail', () => { it('round-trips the bot names through the action log detail', () => { - const detail = quietReadDetail(['trunk-io[bot]', 'CI']); - expect(detail).toBe('only bot activity since your last read: trunk-io[bot], CI'); - expect(botsFromQuietDetail(detail)).toEqual(['trunk-io[bot]', 'CI']); + const detail = quietReadDetail(['trunk-io[bot]', 'vercel[bot]']); + expect(detail).toBe('only bot activity since your last read: trunk-io[bot], vercel[bot]'); + expect(botsFromQuietDetail(detail)).toEqual(['trunk-io[bot]', 'vercel[bot]']); expect(botsFromQuietDetail('already read on GitHub')).toEqual([]); }); @@ -360,7 +360,7 @@ describe('quiet read detail', () => { expect(quietReasonDetail('opened')).toBe('opened in PostPile'); expect(quietReasonFromDetail(quietReasonDetail('changes_requested'))).toBe('changes_requested'); expect(quietReasonFromDetail(quietReasonDetail('opened'))).toBe('opened'); - expect(quietReasonFromDetail(quietReadDetail(['CI']))).toBe('bots'); + expect(quietReasonFromDetail(quietReadDetail(['vercel[bot]']))).toBe('bots'); }); }); @@ -442,9 +442,9 @@ describe('clickedReadCheck', () => { it('marks a bot review on the viewer own open PR: the click was explicit, unlike the quiet reads', () => { const botReview = makeEvent({ id: 'bot-review', prKey: ownPr.key, kind: 'review_commented', actor: 'codex[bot]', isBot: true, at: at(38) }); - const check = clickedReadCheck(clicked({ events: [ownPush(35), botReview, ciResult(39)] })); - expect(check).toEqual({ kind: 'mark', bots: ['codex[bot]', 'CI'] }); - expect(clickedReadDetail(check)).toBe('marked after refresh: only your own activity and automation (codex[bot], CI)'); + const check = clickedReadCheck(clicked({ events: [ownPush(35), botReview, deployResult(39)] })); + expect(check).toEqual({ kind: 'mark', bots: ['codex[bot]', 'vercel[bot]'] }); + expect(clickedReadDetail(check)).toBe('marked after refresh: only your own activity and automation (codex[bot], vercel[bot])'); }); it('keeps it unread when a person did something after the click, naming the newest', () => { @@ -492,7 +492,6 @@ describe('scenario: own approved PR, only old moves and bot nudges since the rea makeTimelineItem({ id: 'rm', kind: 'review_request_removed', actor: 'rowan', subject: 'acme/team-infra', at: day(22) }), ], comments: [makeComment({ id: 'nudge', author: 'stale-nudge[bot]', body: 'This PR has been open for 14 days', createdAt: day(30, 10) })], - checks: { rollup: 'SUCCESS', contexts: [{ name: 'ci', conclusion: 'SUCCESS', completedAt: day(30, 11) }] }, updatedAt: day(30, 11), ...extra, }); @@ -508,21 +507,21 @@ describe('scenario: own approved PR, only old moves and bot nudges since the rea return { thread, pr: realPr, events, userState: null, viewer, notYours: false, prFetchedAt: day(30, 12) }; } - it('clears it: a new merge move asks nothing, and a stale nudge and CI are no finding', () => { + it('clears it: a new merge move asks nothing, and a stale nudge is no finding', () => { const input = caseInput(agentPr()); expect(prWhoseTurn({ pr: input.pr, events: input.events, userState: null, viewer })).toMatchObject({ kind: 'you', move: 'merge' }); expect(isNewYourMove(input, lastReadAt)).toBe(false); - expect(judgedReadCheck(input)).toEqual({ kind: 'mark', actors: ['rowan', 'stale-nudge[bot]', 'CI'] }); + expect(judgedReadCheck(input)).toEqual({ kind: 'mark', actors: ['rowan', 'stale-nudge[bot]'] }); }); - it('clears it when the merge move stood at the read: a stale nudge and CI are no finding', () => { + it('clears it when the merge move stood at the read: a stale nudge is no finding', () => { // The team request was removed before the read, so the PR was already waiting on the viewer to merge. const base = agentPr(); const earlyRemoval = base.timeline.map((item) => (item.id === 'rm' ? { ...item, at: day(17) } : item)); const input = caseInput(agentPr({ timeline: earlyRemoval })); expect(isNewYourMove(input, lastReadAt)).toBe(false); // Only bots since the read now: the bots-only rule clears it. - expect(quietReadCheck(input)).toEqual({ kind: 'mark', bots: ['stale-nudge[bot]', 'CI'] }); + expect(quietReadCheck(input)).toEqual({ kind: 'mark', bots: ['stale-nudge[bot]'] }); }); it('clears it after a bot reopens the approved PR: the new move is only a merge', () => { diff --git a/packages/core/src/quiet-reads.ts b/packages/core/src/quiet-reads.ts index 4af1ad95..a227c26c 100644 --- a/packages/core/src/quiet-reads.ts +++ b/packages/core/src/quiet-reads.ts @@ -1,5 +1,5 @@ // "Handled quietly": a PR thread the user had read that turned unread again -// only because of bots (CI, merge queue, review bots, deploys). PostPile marks +// only because of bots (merge queue, review bots, deploys). PostPile marks // it read on GitHub by itself when nothing is asked of the user. A second // reason: the user acted on the PR after every unread event ("You already // dealt with it"); a third: everything since they last looked is automation @@ -37,8 +37,8 @@ export const HANDLED_QUIETLY_DAYS = 7; /** Threads one run marks read at most; the rest wait for the next run. */ export const QUIET_READS_PER_RUN = 50; -/** Name shown for bot activity without an actor (a CI result). */ -export const CI_ACTOR = 'CI'; +/** Name shown for activity without an actor (GitHub itself; CI results had none until 0.21.0). */ +export const NO_ACTOR_NAME = 'GitHub'; /** * Automation by the shared rule (`isAutomation`): a bot-made review request @@ -63,9 +63,9 @@ export function botOnlySinceRead(pr: Pr, events: PrEvent[], lastReadAt: IsoTime, return since; } -/** The bots behind the events, in order of first appearance, "CI" for actor-less ones. */ +/** The bots behind the events, in order of first appearance, "GitHub" for actor-less ones. */ export function botNames(events: PrEvent[]): string[] { - return [...new Set(events.map((event) => (event.actor === '' ? CI_ACTOR : event.actor)))]; + return [...new Set(events.map((event) => (event.actor === '' ? NO_ACTOR_NAME : event.actor)))]; } /** @@ -335,7 +335,7 @@ export type JudgedSkip = | 'unseen_merge' | 'your_move'; -/** `actors`: everyone since the user last looked, in order of first appearance, "CI" for actor-less events. */ +/** `actors`: everyone since the user last looked, in order of first appearance, "GitHub" for actor-less events. */ export type JudgedReadCheck = { kind: 'mark'; actors: string[] } | { kind: 'skip'; why: JudgedSkip }; /** @@ -415,7 +415,7 @@ export type RequestGoneSkip = | 'unseen_merge' | 'your_move'; -/** `actors`: everyone since the request, in order of first appearance, "CI" for actor-less events. */ +/** `actors`: everyone since the request, in order of first appearance, "GitHub" for actor-less events. */ export type RequestGoneReadCheck = { kind: 'mark'; actors: string[] } | { kind: 'skip'; why: RequestGoneSkip }; /** When the newest review request of the viewer or one of their teams was made; null when there is none. */ @@ -590,7 +590,7 @@ function newsWords(news: PrEvent[]): string { return 'activity'; } const more = news.length > 1 ? ` and ${news.length - 1} more` : ''; - return `${newsNoun(newest.kind)} from ${newest.actor === '' ? CI_ACTOR : newest.actor}${more}`; + return `${newsNoun(newest.kind)} from ${newest.actor === '' ? NO_ACTOR_NAME : newest.actor}${more}`; } /** Why a clicked mark-read stays unread, for the sync report and the pending-send result: "new review from alice". */ diff --git a/packages/core/src/saw-before-acting.test.ts b/packages/core/src/saw-before-acting.test.ts index 69ea81c5..baf26a78 100644 --- a/packages/core/src/saw-before-acting.test.ts +++ b/packages/core/src/saw-before-acting.test.ts @@ -102,12 +102,10 @@ describe('scenario: marking a PR ready right after a comment', () => { expect(isPrDone(readied, null, viewer, synced(readied, lastReadAt), false, lastReadAt)).toBe(true); // Before this rule it needed a Mark read in PostPile; without the read it still does. expect(isPrDone(readied, null, viewer, synced(readied, time(1)), false, time(1))).toBe(false); - // The same with a comment as the touch and CI after it: the acted-after read clears the thread. + // The same with a comment as the touch and a bot after it: the acted-after read clears the thread. const cliComment = makeComment({ id: 'mine', author: viewer.login, body: 'ready for review', createdAt: time(31) }); - const withCi = ownPr([lyra, cliComment], { - checks: { rollup: 'SUCCESS', contexts: [{ name: 'ci', conclusion: 'SUCCESS', completedAt: time(50) }] }, - updatedAt: time(50), - }); - expect(touched(withCi, synced(withCi, lastReadAt), lastReadAt)).toEqual({ kind: 'mark', reason: 'replied' }); + const preview = makeComment({ id: 'preview', author: 'github-actions[bot]', body: 'Preview ready', createdAt: time(50) }); + const withBot = ownPr([lyra, cliComment, preview], { updatedAt: time(50) }); + expect(touched(withBot, synced(withBot, lastReadAt), lastReadAt)).toEqual({ kind: 'mark', reason: 'replied' }); }); }); diff --git a/packages/core/src/snooze.test.ts b/packages/core/src/snooze.test.ts index 35f1bdb6..f37c9aec 100644 --- a/packages/core/src/snooze.test.ts +++ b/packages/core/src/snooze.test.ts @@ -36,18 +36,11 @@ describe('isSnoozeOver', () => { expect(isSnoozeOver(push, context({ events: [makeEvent({ kind: 'force_pushed', at: at(11) })] }))).toBe(true); }); - it('ci_green ends when the PR is green', () => { - const green = snooze({ kind: 'ci_green' }); - expect(isSnoozeOver(green, context({ pr: makePr({ checks: { rollup: 'FAILURE', contexts: [] } }) }))).toBe(false); - expect(isSnoozeOver(green, context({ pr: makePr({ checks: { rollup: 'SUCCESS', contexts: [] } }) }))).toBe(true); - }); - it('every snooze ends when the PR is merged or closed', () => { - const failingMerged = makePr({ state: 'MERGED', checks: { rollup: 'FAILURE', contexts: [] } }); - const closed = makePr({ state: 'CLOSED', checks: { rollup: 'FAILURE', contexts: [] } }); + const merged = makePr({ state: 'MERGED' }); + const closed = makePr({ state: 'CLOSED' }); const later = snooze({ kind: 'until_time', until: at(999) }); - for (const pr of [failingMerged, closed]) { - expect(isSnoozeOver(snooze({ kind: 'ci_green' }), context({ pr }))).toBe(true); + for (const pr of [merged, closed]) { expect(isSnoozeOver(snooze({ kind: 'new_push' }), context({ pr }))).toBe(true); expect(isSnoozeOver(snooze({ kind: 'someone_replies' }), context({ pr }))).toBe(true); expect(isSnoozeOver(later, context({ pr }))).toBe(true); @@ -95,7 +88,6 @@ describe('snoozeTelemetryBucket', () => { it('names the condition directly for an event-based snooze', () => { expect(snoozeTelemetryBucket({ kind: 'someone_replies' }, nowMs)).toBe('someone_replies'); expect(snoozeTelemetryBucket({ kind: 'new_push' }, nowMs)).toBe('new_push'); - expect(snoozeTelemetryBucket({ kind: 'ci_green' }, nowMs)).toBe('ci_green'); }); it('buckets a time-based snooze by how far out it is', () => { diff --git a/packages/core/src/snooze.ts b/packages/core/src/snooze.ts index e72207f0..255183ef 100644 --- a/packages/core/src/snooze.ts +++ b/packages/core/src/snooze.ts @@ -52,13 +52,9 @@ function newPush(snooze: Snooze, context: SnoozeContext): boolean { return context.events.some((event) => event.at > snooze.since && PUSH_KINDS.includes(event.kind)); } -function ciGreen(context: SnoozeContext): boolean { - return context.pr.checks.rollup === 'SUCCESS'; -} - /** - * True once the snooze condition is met: a human reply, a push, green CI, or - * the time passed. Every snooze also ends when the PR is merged or closed + * True once the snooze condition is met: a human reply, a push, or the time + * passed. Every snooze also ends when the PR is merged or closed * (decided 2026-09-30): nothing left to wait for, and a snooze that holds a * finished PR keeps its topic from retiring. */ @@ -71,8 +67,6 @@ export function isSnoozeOver(snooze: Snooze, context: SnoozeContext): boolean { return someoneReplied(snooze, context); case 'new_push': return newPush(snooze, context); - case 'ci_green': - return ciGreen(context); case 'until_time': return context.now >= snooze.condition.until; } @@ -135,7 +129,7 @@ export function snoozeWrites(tile: Tile, change: SnoozeChange): SnoozeWrites { } } -export type SnoozeTelemetryBucket = 'hours' | 'a_day' | 'days' | 'a_week' | 'someone_replies' | 'new_push' | 'ci_green'; +export type SnoozeTelemetryBucket = 'hours' | 'a_day' | 'days' | 'a_week' | 'someone_replies' | 'new_push'; /** The `snoozed` telemetry event's prop: a time bucket for `until_time`, the condition name otherwise. */ export function snoozeTelemetryBucket(condition: SnoozeCondition, nowMs: number): SnoozeTelemetryBucket { diff --git a/packages/core/src/telemetry-events.test.ts b/packages/core/src/telemetry-events.test.ts index 518e3fdc..d90d677e 100644 --- a/packages/core/src/telemetry-events.test.ts +++ b/packages/core/src/telemetry-events.test.ts @@ -81,6 +81,7 @@ describe('TELEMETRY_EVENTS', () => { it('takes a finished storage job by its known name, with counts and whole milliseconds only', () => { const done = { name: 'bot_body_trim', units: 11326, work_ms: 8500, longest_slice_ms: 41, wall_ms: 24000 }; expect(TELEMETRY_EVENTS.storage_job_done.safeParse(done).success).toBe(true); + expect(TELEMETRY_EVENTS.storage_job_done.safeParse({ ...done, name: 'checks_strip' }).success).toBe(true); expect(TELEMETRY_EVENTS.storage_job_done.safeParse({ ...done, name: 'acme/app#1' }).success).toBe(false); expect(TELEMETRY_EVENTS.storage_job_done.safeParse({ ...done, work_ms: 8.5 }).success).toBe(false); expect(TELEMETRY_EVENTS.storage_job_done.safeParse({ ...done, wrote: 3 }).success).toBe(false); diff --git a/packages/core/src/telemetry-events.ts b/packages/core/src/telemetry-events.ts index 3bf0f023..41d76970 100644 --- a/packages/core/src/telemetry-events.ts +++ b/packages/core/src/telemetry-events.ts @@ -36,10 +36,11 @@ const verdict = z.enum(['looks_safe', 'look_closer', 'not_yours']).nullable(); const approveFrom = z.enum(['detail', 'tile', 'agent_tile', 'agent_topic']); const markReadOrigin = z.enum(['tile', 'detail', 'debug', 'cleanup', 'agent_tile', 'agent_topic']); // A snooze is either a time (bucketed) or a condition (someone replies, a -// push, CI going green - see packages/core/src/snooze.ts SnoozeCondition): +// push - see packages/core/src/snooze.ts SnoozeCondition; CI going green +// until 0.21.0, no longer sent): // the same prop name the spec uses ("duration bucket"), widened to the // condition-based snoozes the product actually has. -const snoozeDurationBucket = z.enum(['hours', 'a_day', 'days', 'a_week', 'someone_replies', 'new_push', 'ci_green']); +const snoozeDurationBucket = z.enum(['hours', 'a_day', 'days', 'a_week', 'someone_replies', 'new_push']); const queryLengthBucket = z.enum(['short', 'medium', 'long']); const queueFilter = z.enum(['mine', 'team', 'reply', 'review', 'none']); // The sidebar section the opened topic sits in, core's `TopicSection` as is. @@ -69,7 +70,7 @@ const quotaResource = z.enum(['core', 'graphql']); const quotaLevel = z.enum(['low', 'critical']); const percent = z.number().int().min(0).max(100); // packages/engine/src/storage-jobs: every background storage job by name. Append only. -const storageJobName = z.enum(['bot_body_trim']); +const storageJobName = z.enum(['bot_body_trim', 'checks_strip']); // ----------------------------------------------------------------------- // 6. MCP server (postpile-mcp, a separate process that reads the database and asks the app for the rest) diff --git a/packages/core/src/testing/board-spec.ts b/packages/core/src/testing/board-spec.ts index b12cad7d..5bf55db5 100644 --- a/packages/core/src/testing/board-spec.ts +++ b/packages/core/src/testing/board-spec.ts @@ -80,8 +80,6 @@ export type StepSpec = export type EndSpec = { kind: 'open' } | { kind: 'merged'; by: Person } | { kind: 'closed'; by: Person }; -export type CiSpec = 'none' | 'pending' | 'success' | 'failure'; - /** * How the app knows the PR: a notification thread (read after `readAfter` * steps, null for never), found by the full sync, or pulled in as a stack or @@ -89,7 +87,7 @@ export type CiSpec = 'none' | 'pending' | 'success' | 'failure'; */ export type TrackingSpec = { kind: 'thread'; reason: NotificationReason; readAfter: number | null } | { kind: 'found' } | { kind: 'pulled_in' }; -export type SnoozeConditionKind = 'someone_replies' | 'new_push' | 'ci_green' | 'until_time'; +export type SnoozeConditionKind = 'someone_replies' | 'new_push' | 'until_time'; /** A snooze started after `after` steps; an until_time snooze has passed or not. */ export interface SnoozeSpec { @@ -115,7 +113,6 @@ export interface PrSpec { draft: boolean; steps: StepSpec[]; end: EndSpec; - ci: CiSpec; tracking: TrackingSpec; /** The unresolved review threads are resolved. */ threadsResolved: boolean; @@ -154,7 +151,6 @@ export const QUIET_PR: PrSpec = { draft: false, steps: [], end: { kind: 'open' }, - ci: 'none', tracking: { kind: 'thread', reason: 'review_requested', readAfter: null }, threadsResolved: false, approvedAfter: null, @@ -338,7 +334,7 @@ const trackingArb: fc.Arbitrary = fc.oneof( ); const snoozeArb: fc.Arbitrary = fc.record({ - condition: fc.constantFrom('until_time', 'someone_replies', 'new_push', 'ci_green'), + condition: fc.constantFrom('until_time', 'someone_replies', 'new_push'), after: stepIndex, untilPassed: fc.boolean(), }); @@ -350,7 +346,6 @@ export const prSpecArb: fc.Arbitrary = fc.record({ draft: sometimes(1, 4), steps: fc.array(stepArb, { maxLength: 8 }), end: endArb, - ci: fc.constantFrom('none', 'success', 'failure', 'pending'), tracking: trackingArb, threadsResolved: fc.boolean(), approvedAfter: maybe(stepIndex, 20), diff --git a/packages/core/src/testing/build-board.ts b/packages/core/src/testing/build-board.ts index 8d010522..faf1f8cd 100644 --- a/packages/core/src/testing/build-board.ts +++ b/packages/core/src/testing/build-board.ts @@ -439,7 +439,7 @@ interface CompiledPr { pr: Pr; /** The head commit after each number of steps. */ headAfter: string[]; - /** Last activity on GitHub (steps, merge or close, CI). */ + /** Last activity on GitHub (steps, merge or close). */ lastMinute: number; } @@ -448,7 +448,6 @@ function compilePr(spec: PrSpec, place: PrPlace): CompiledPr { const history = new PrHistory(place.number, author, spec.draft); spec.steps.forEach((step, index) => history.apply(step, index, at(stepMinute(place.prIndex, index)))); const endMinute = stepMinute(place.prIndex, spec.steps.length); - const ciMinute = afterMinute(place.prIndex, spec.steps.length) - 3; let state: Pr['state'] = 'OPEN'; let mergedAt: string | null = null; let mergedBy: string | null = null; @@ -467,10 +466,7 @@ function compilePr(spec: PrSpec, place: PrPlace): CompiledPr { history.timeline.push({ id: `tl${place.number}-end`, kind: 'closed', actor: LOGINS[spec.end.by], at: at(endMinute), subject: null }); } const lastStepMinute = spec.steps.length === 0 ? 0 : stepMinute(place.prIndex, spec.steps.length - 1); - const ciRan = spec.ci !== 'none'; - const lastMinute = Math.max(lastStepMinute, state === 'OPEN' ? 0 : endMinute, ciRan ? ciMinute : 0); - const rollup = { none: 'NONE', pending: 'PENDING', success: 'SUCCESS', failure: 'FAILURE' } as const; - const finished = spec.ci === 'success' || spec.ci === 'failure'; + const lastMinute = Math.max(lastStepMinute, state === 'OPEN' ? 0 : endMinute); const pr = makePr({ number: place.number, repo: PROPERTY_REPO, @@ -488,10 +484,6 @@ function compilePr(spec: PrSpec, place: PrPlace): CompiledPr { comments: history.comments, threads: history.threads(spec.threadsResolved), timeline: history.timeline, - checks: { - rollup: rollup[spec.ci], - contexts: ciRan ? [{ name: 'ci', conclusion: finished ? spec.ci.toUpperCase() : null, completedAt: finished ? at(ciMinute) : null }] : [], - }, headOid: history.headOid, createdAt: at(0), updatedAt: at(lastMinute), diff --git a/packages/core/src/testing/event-corpus.ts b/packages/core/src/testing/event-corpus.ts index 0af49075..d11715a3 100644 --- a/packages/core/src/testing/event-corpus.ts +++ b/packages/core/src/testing/event-corpus.ts @@ -7,7 +7,7 @@ // every body are invented. import { at, makeComment, makeCommit, makePr, makeReview, makeThreadFor, makeTimelineItem } from '../fixtures.ts'; import { deriveEvents } from '../events.ts'; -import type { Checks, Comment, Commit, NotificationThread, Pr, PrEvent, Review, TimelineItem, Viewer } from '../types.ts'; +import type { Comment, Commit, NotificationThread, Pr, PrEvent, Review, TimelineItem, Viewer } from '../types.ts'; /** * The viewer of every scenario: home team acme/team-platform, and @@ -29,8 +29,8 @@ export const CORPUS_AT = at(30); /** Well after everything, so no time-based rule (a grace period) decides a quiet read. */ export const CORPUS_NOW = at(24 * 60); -/** Something GitHub shows on the PR: a comment or review body, a review, a timeline item, a commit or the checks. */ -export type CorpusArtifact = { comment: Comment } | { review: Review } | { timeline: TimelineItem } | { commit: Commit } | { checks: Checks }; +/** Something GitHub shows on the PR: a comment or review body, a review, a timeline item or a commit. */ +export type CorpusArtifact = { comment: Comment } | { review: Review } | { timeline: TimelineItem } | { commit: Commit }; export interface CorpusEntry { /** What happened, the way a person would say it. */ @@ -352,10 +352,6 @@ export const CORPUS = { adds: [timeline('tl-agent-ready', 'ready_for_review', 'posthog[bot]')], pr: { isDraft: false }, }, - ciFails: { - says: 'CI fails on the head commit', - adds: [{ checks: { rollup: 'FAILURE', contexts: [{ name: 'backend-tests', conclusion: 'FAILURE', completedAt: CORPUS_AT }] } }], - }, // The viewer. viewerComments: { @@ -572,10 +568,7 @@ function applyArtifact(pr: Pr, artifact: CorpusArtifact): Pr { if ('timeline' in artifact) { return { ...pr, timeline: upsertById(pr.timeline, artifact.timeline) }; } - if ('commit' in artifact) { - return { ...pr, commits: [...pr.commits, artifact.commit], headOid: artifact.commit.oid }; - } - return { ...pr, checks: artifact.checks }; + return { ...pr, commits: [...pr.commits, artifact.commit], headOid: artifact.commit.oid }; } /** The PR with the entry's `before` artifacts: what was there when the viewer last read it. */ diff --git a/packages/core/src/testing/labels.ts b/packages/core/src/testing/labels.ts index b654829b..7250e977 100644 --- a/packages/core/src/testing/labels.ts +++ b/packages/core/src/testing/labels.ts @@ -106,7 +106,6 @@ function prLabels(board: PropertyBoard, key: PrKey, pr: Pr): string[] { if (viewerHeadReview(pr, board.viewer)) { labels.push('viewer reviewed head'); } - labels.push(`ci:${pr.checks.rollup}`); const thread = board.threads.get(key); if (thread) { labels.push(thread.unread ? 'thread:unread' : 'thread:read', thread.lastReadAt === null ? 'thread:never read' : 'thread:read once'); @@ -484,10 +483,6 @@ export const REQUIRED_LABELS: readonly string[] = [ 'viewer-review:CHANGES_REQUESTED:head', 'viewer-review:CHANGES_REQUESTED:older', 'viewer-review:COMMENTED:head', - 'ci:NONE', - 'ci:PENDING', - 'ci:SUCCESS', - 'ci:FAILURE', 'thread:unread', 'thread:read', 'thread:never read', @@ -500,7 +495,6 @@ export const REQUIRED_LABELS: readonly string[] = [ 'approved in app', 'snooze:someone_replies', 'snooze:new_push', - 'snooze:ci_green', 'snooze:until_time', 'snooze-phase:active', 'snooze-phase:broken', diff --git a/packages/core/src/testing/spec-events.ts b/packages/core/src/testing/spec-events.ts index b73bc588..6e89e2d6 100644 --- a/packages/core/src/testing/spec-events.ts +++ b/packages/core/src/testing/spec-events.ts @@ -57,7 +57,7 @@ export const SPEC_REVIEW_KINDS: readonly EventKind[] = ['review_approved', 'revi const ANSWER_KINDS: readonly EventKind[] = [...SPEC_PUSH_KINDS, 'comment', 'review_commented', 'reply_to_user', 'question_to_user', 'mention']; /** Machine kinds: quiet whoever made them. */ -const MACHINE_KINDS: readonly EventKind[] = ['ci', 'deploy', 'merge_queue', 'bot_comment']; +const MACHINE_KINDS: readonly EventKind[] = ['deploy', 'merge_queue', 'bot_comment']; /** The viewer wrote in this thread before `comment`. */ function viewerSpokeEarlierInThread(pr: Pr, comment: Comment, viewer: Viewer): boolean { @@ -197,10 +197,6 @@ function rawEvents(pr: Pr, viewer: Viewer, userState: UserPrState | null): RawEx } add(kind, item.id, item.actor, item.at, isAutomationLogin(item.actor), item.subject); } - const finished = pr.checks.contexts.map((context) => context.completedAt).filter((at): at is IsoTime => at !== null); - if ((pr.checks.rollup === 'SUCCESS' || pr.checks.rollup === 'FAILURE') && finished.length > 0) { - add('ci', `${pr.headOid}:${pr.checks.rollup}`, '', finished.sort().at(-1)!, true); - } return events; } diff --git a/packages/core/src/testing/spec-rules.ts b/packages/core/src/testing/spec-rules.ts index b6ce9e53..1cd40008 100644 --- a/packages/core/src/testing/spec-rules.ts +++ b/packages/core/src/testing/spec-rules.ts @@ -537,8 +537,6 @@ export function expectedSnoozePhase(input: { pr: Pr; events: PrEvent[]; viewer: return after.some((event) => REPLY_KINDS.includes(event.kind) && !isAutomationEvent(pr, viewer, event) && !isViewerLogin(viewer, event.actor)) ? 'over' : 'active'; case 'new_push': return after.some((event) => SPEC_PUSH_KINDS.includes(event.kind)) ? 'over' : 'active'; - case 'ci_green': - return pr.checks.rollup === 'SUCCESS' ? 'over' : 'active'; case 'until_time': return input.now >= condition.until ? 'over' : 'active'; } @@ -799,9 +797,9 @@ function isUnseenLoudEvent(event: PrEvent): boolean { return event.seenAt === null && effectiveLoudnessOf(event) === 'loud'; } -/** Who acted, in order of first appearance, "CI" for actor-less events. */ +/** Who acted, in order of first appearance, "GitHub" for actor-less events. */ function actorNames(events: PrEvent[]): string[] { - return [...new Set(events.map((event) => (event.actor === '' ? 'CI' : event.actor)))]; + return [...new Set(events.map((event) => (event.actor === '' ? 'GitHub' : event.actor)))]; } /** diff --git a/packages/core/src/tiles.test.ts b/packages/core/src/tiles.test.ts index 4b436412..9d8615eb 100644 --- a/packages/core/src/tiles.test.ts +++ b/packages/core/src/tiles.test.ts @@ -212,7 +212,6 @@ describe('deriveTileState', () => { const events = [ makeEvent({ id: 'approval', kind: 'review_approved', at: at(10), actor: 'lyra', isBot: false }), makeEvent({ id: 'merge', kind: 'merged_without_review', at: at(20), actor: 'trunk-io[bot]', isBot: true, summary: 'trunk-io[bot] merged without your review' }), - makeEvent({ id: 'ci', kind: 'ci', at: at(25), actor: '', isBot: true }), makeEvent({ id: 'deploy', kind: 'deploy', at: at(30), actor: 'deployment-status-posthog[bot]', isBot: true, summary: 'deploy' }), ]; const state = deriveTileState(stateInput(tile, [pr], events, [handled], [unreadThread])); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 120705c3..84e6785c 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -86,20 +86,6 @@ export interface ReviewThread { comments: Comment[]; } -export type CheckRollup = 'SUCCESS' | 'FAILURE' | 'PENDING' | 'NONE'; - -export interface CheckContext { - name: string; - /** SUCCESS, FAILURE, NEUTRAL, SKIPPED, CANCELLED, TIMED_OUT, ACTION_REQUIRED, or null while running. */ - conclusion: string | null; - completedAt: IsoTime | null; -} - -export interface Checks { - rollup: CheckRollup; - contexts: CheckContext[]; -} - export type TimelineItemKind = | 'review_requested' | 'review_request_removed' @@ -167,7 +153,6 @@ export interface Pr { comments: Comment[]; threads: ReviewThread[]; timeline: TimelineItem[]; - checks: Checks; headOid: string; createdAt: IsoTime; updatedAt: IsoTime; @@ -314,7 +299,6 @@ export type EventKind = | 'reopened' | 'ready_for_review' | 'converted_to_draft' - | 'ci' | 'deploy' | 'merge_queue' | 'bot_comment' @@ -712,7 +696,6 @@ export interface UserPrState { export type SnoozeCondition = | { kind: 'someone_replies' } | { kind: 'new_push' } - | { kind: 'ci_green' } | { kind: 'until_time'; until: IsoTime }; /** diff --git a/packages/core/src/whats-new.test.ts b/packages/core/src/whats-new.test.ts index 8a2da304..269fcb5d 100644 --- a/packages/core/src/whats-new.test.ts +++ b/packages/core/src/whats-new.test.ts @@ -107,14 +107,14 @@ describe('whatsNew', () => { }); it('prefers the viewer\'s own action over a later mark-read', () => { - const events = [own('review_changes_requested', 0), ev({ kind: 'ci', actor: '', isBot: true, at: at(5), seenAt: at(10) }), loud('commits_pushed', 'pim', 20)]; + const events = [own('review_changes_requested', 0), ev({ kind: 'deploy', actor: 'vercel[bot]', isBot: true, at: at(5), seenAt: at(10) }), loud('commits_pushed', 'pim', 20)]; expect(whatsNew(pr, events, viewer)?.anchor.kind).toBe('changes_request'); }); - it('never counts quiet bot and CI events', () => { + it('never counts quiet bot events', () => { const bot = ev({ kind: 'bot_comment', actor: 'greptile[bot]', isBot: true, at: at(8), ruleLoudness: 'quiet' }); - const ci = ev({ kind: 'ci', actor: '', isBot: true, at: at(9), ruleLoudness: 'quiet' }); - const events = [own('review_changes_requested', 0), loud('commits_pushed', 'pim', 5), bot, ci]; + const deploy = ev({ kind: 'deploy', actor: 'vercel[bot]', isBot: true, at: at(9), ruleLoudness: 'quiet' }); + const events = [own('review_changes_requested', 0), loud('commits_pushed', 'pim', 5), bot, deploy]; const result = whatsNew(pr, events, viewer); expect(result?.lead.count).toBe(1); expect(result?.extraCount).toBe(0); diff --git a/packages/core/src/whose-turn.test.ts b/packages/core/src/whose-turn.test.ts index 0b5025dc..80f4c00b 100644 --- a/packages/core/src/whose-turn.test.ts +++ b/packages/core/src/whose-turn.test.ts @@ -3,7 +3,7 @@ import { at, makeComment, makeCommit, makeEvent, makePr, makeReview, makeThread, import type { Pr, PrEvent, Tile, UserPrState, Viewer } from './types.ts'; import { mergeQueueFailureAt, mergeQueueState } from './merge-queue.ts'; import { prStatus } from './pr-status.ts'; -import { isMergeApprovedMove, NO_TURN, whoseTurn, YOUR_MOVE_ORDER, type WhoseTurn } from './whose-turn.ts'; +import { isMergeApprovedMove, whoseTurn, YOUR_MOVE_ORDER, type WhoseTurn } from './whose-turn.ts'; const me = viewer.login; @@ -265,14 +265,6 @@ describe('whoseTurn: on your own PR', () => { expect(single({ ...pushed, reviewerUsers: ['bob', 'carol'] })).toEqual({ kind: 'them', who: 'bob', what: 'to re-review', prKey: own.key }); }); - it('never makes failing CI on your own PR a move of yours: CI is not a signal', () => { - const failing = { name: 'backend-tests', conclusion: 'FAILURE', completedAt: at(20) }; - const red = { ...own, checks: { rollup: 'FAILURE' as const, contexts: [failing] } }; - expect(single(red)).toEqual(NO_TURN); - expect(single({ ...red, reviewerUsers: ['sol'] })).toMatchObject({ kind: 'them', who: 'sol', lead: 'Waiting on' }); - expect(single({ ...red, reviewDecision: 'APPROVED' })).toMatchObject({ kind: 'you', move: 'merge' }); - }); - it('says it waits on the first requested reviewer', () => { expect(single({ ...own, reviewerUsers: ['sol', 'lyra'] })).toEqual({ kind: 'them', who: 'sol', what: 'and 1 more', prKey: own.key, lead: 'Waiting on' }); expect(single({ ...own, reviewerTeams: ['acme/team-platform'] })).toEqual({ kind: 'them', who: 'acme/team-platform', what: '', prKey: own.key, lead: 'Waiting on' }); @@ -311,7 +303,6 @@ describe('whoseTurn: on your own PR', () => { it('asks you to merge once it is approved', () => { expect(single({ ...own, reviewDecision: 'APPROVED' }).what).toBe('Merge, it is approved'); expect(isMergeApprovedMove(single({ ...own, reviewDecision: 'APPROVED' }))).toBe(true); - expect(isMergeApprovedMove(single({ ...own, checks: { ...own.checks, rollup: 'FAILURE' }, reviewDecision: 'APPROVED' }))).toBe(true); expect(single({ ...own, reviewDecision: 'APPROVED', isDraft: true }).kind).toBe('none'); }); @@ -392,7 +383,7 @@ describe('whoseTurn: multi-PR tiles', () => { it('picks the most urgent pinged member and names the PR', () => { const approved = makePr({ number: 1, author: 'rowan', reviews: [makeReview({ author: me })] }); const asked = makePr({ number: 2, author: 'rowan', reviewerUsers: [me] }); - const pulled = makePr({ number: 3, author: me, checks: { rollup: 'FAILURE', contexts: [] } }); + const pulled = makePr({ number: 3, author: me }); const tile: Tile = { id: 'stack:x', topicId: 'topic-1', @@ -441,7 +432,7 @@ describe('whoseTurn: drafts', () => { it('asks you to address comments on your own draft', () => { const own = makePr({ author: me, isDraft: true, threads: [makeThread('t1', [makeComment({ author: 'mira' })]), makeThread('t2', [makeComment({ author: 'mira' })])] }); expect(single(own)).toMatchObject({ kind: 'you', what: 'Address 2 comments on your draft' }); - expect(single({ ...own, threads: [], checks: { rollup: 'FAILURE' as const, contexts: [] } }).kind).toBe('none'); + expect(single({ ...own, threads: [] }).kind).toBe('none'); }); it('turns back to a review once the draft is ready', () => { diff --git a/packages/engine/src/actions.test.ts b/packages/engine/src/actions.test.ts index d7f81acf..b01697ab 100644 --- a/packages/engine/src/actions.test.ts +++ b/packages/engine/src/actions.test.ts @@ -253,6 +253,17 @@ describe('snooze', () => { await h.engine.unsnooze(tileId); expect(await tileState(h)).toBe('done'); }); + + it('wakes a stored "Until CI is green" snooze from before 0.21.0 like an expired one', async () => { + const h = await synced(); + await h.engine.markRead(tileId); + await h.engine.snooze(tileId, { kind: 'until_time', until: '2099-01-01T00:00:00.000Z' }); + expect(await tileState(h)).toBe('snoozed'); + + h.store.db.prepare(`UPDATE pr_snooze SET condition_json = '{"kind":"ci_green"}'`).run(); + + expect(await tileState(h)).toBe('done'); + }); }); describe('feedback', () => { diff --git a/packages/engine/src/bot-noise.test.ts b/packages/engine/src/bot-noise.test.ts index 674ecb74..a45eeb3c 100644 --- a/packages/engine/src/bot-noise.test.ts +++ b/packages/engine/src/bot-noise.test.ts @@ -59,8 +59,8 @@ describe('bot noise and topic memory', () => { expect(h.agent.dossierInputs).toHaveLength(1); expect(await eventsBehind()).toBe(0); - // CI fails and the preview deploy refreshes its comment: nothing. - await land(CORPUS.ciFails, CORPUS.deployComment, editedAt(CORPUS.deployEdit, '2026-09-02T16:30:00.000Z')); + // The preview deploy refreshes its comment: nothing. + await land(CORPUS.deployComment, editedAt(CORPUS.deployEdit, '2026-09-02T16:30:00.000Z')); expect(h.agent.dossierInputs).toHaveLength(1); expect(await eventsBehind()).toBe(0); @@ -91,7 +91,7 @@ describe('bot noise and topic memory', () => { await h.engine.sync({ agentJobs: ['dossiers'] }); // Logged by a sync that runs no agent jobs: the dossier stays where it was. - pr = [CORPUS.trunkSubmitted, CORPUS.coderabbitReview, CORPUS.ciFails, CORPUS.teammateAsksViewer].reduce(corpusPrAfter, pr); + pr = [CORPUS.trunkSubmitted, CORPUS.coderabbitReview, CORPUS.teammateAsksViewer].reduce(corpusPrAfter, pr); h.reader.addPr(pr, makeThreadFor(pr, { updatedAt: nextHour() })); h.reader.etag = 'etag-2'; await h.engine.sync({ agentJobs: [] }); diff --git a/packages/engine/src/engine-telemetry.test.ts b/packages/engine/src/engine-telemetry.test.ts index 5dc48826..d2e709d6 100644 --- a/packages/engine/src/engine-telemetry.test.ts +++ b/packages/engine/src/engine-telemetry.test.ts @@ -145,8 +145,8 @@ describe('engine telemetry', () => { it('fires snoozed with the condition name for an event-based snooze', async () => { const h = await synced(); - await h.engine.snooze(tileId, { kind: 'ci_green' }); - expect(h.telemetry.events).toContainEqual({ event: 'snoozed', props: { duration_bucket: 'ci_green' } }); + await h.engine.snooze(tileId, { kind: 'new_push' }); + expect(h.telemetry.events).toContainEqual({ event: 'snoozed', props: { duration_bucket: 'new_push' } }); }); it('fires chat_message_sent on a topic chat', async () => { diff --git a/packages/engine/src/memory-sync.test.ts b/packages/engine/src/memory-sync.test.ts index 32f8d4b7..a08d26fd 100644 --- a/packages/engine/src/memory-sync.test.ts +++ b/packages/engine/src/memory-sync.test.ts @@ -139,26 +139,6 @@ describe('dossier updates', () => { expect(h.agent.dossierInputs).toHaveLength(2); }); - it('makes no call for CI results alone, and stores the digest cursor past them', async () => { - const { h, setNow } = movableHarness(); - const pr = reviewRequestedPr(1); - topicWithPrs(h, 'depot', [pr]); - await h.engine.sync({ agentJobs: ['dossiers'] }); - const before = h.store.cursors.get('digest', 'depot')?.seq ?? 0; - - setNow(LATER); - const failing = { name: 'backend-tests', conclusion: 'FAILURE', completedAt: at(30) }; - pushSnapshot(h, { ...pr, checks: { rollup: 'FAILURE', contexts: [failing] } }, 'etag-2'); - const report = await h.engine.sync({ agentJobs: ['dossiers'] }); - - expect(report.agentCalls).toBe(0); - expect(h.agent.dossierInputs).toHaveLength(1); - const cursor = h.store.cursors.get('digest', 'depot'); - expect(cursor?.seq).toBeGreaterThan(before); - expect(cursor?.dossierVersion).toBe(1); - expect(h.store.eventLog.listSince([pr.key], cursor?.seq ?? 0)).toEqual([]); - }); - it('refreshes a dossier once when the tailoring changes, even without new events', async () => { const h = makeHarness(); topicWithPrs(h, 'depot', [reviewRequestedPr(1)]); diff --git a/packages/engine/src/storage-jobs/checks-strip.test.ts b/packages/engine/src/storage-jobs/checks-strip.test.ts new file mode 100644 index 00000000..eac83d88 --- /dev/null +++ b/packages/engine/src/storage-jobs/checks-strip.test.ts @@ -0,0 +1,135 @@ +// Storage job 2 (DESIGN.md "CI is not tracked"): removes the checks older +// builds stored in the snapshot json, one snapshot per unit, without moving +// a revision or changing a read, and is done only when the data says so. +import { at, FakeTimers, makePr } from '@postpile/core/fixtures'; +import { Store } from '@postpile/store'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { makeHarness, NOW } from '../testing/fakes.ts'; +import { CHECKS_STRIP_SINCE_KEY, ChecksStripJob } from './checks-strip.ts'; +import { storageJobs } from './jobs.ts'; +import { PAUSE_MS, START_DELAY_MS, StorageJobRunner, type StorageJobReport } from './runner.ts'; + +const OLD_CHECKS = { rollup: 'SUCCESS', contexts: [{ name: 'lint', conclusion: 'SUCCESS', completedAt: '2026-09-01T09:05:00.000Z' }] }; + +/** The PR as a build before 0.21.0 stored it: its checks in the json. */ +function storedWithChecks(store: Store, number: number): string { + const pr = makePr({ number }); + store.prs.upsert(pr, at(1)); + store.db.prepare("UPDATE pr_snapshot SET json = json_set(json, '$.checks', json(?)) WHERE key = ?").run(JSON.stringify(OLD_CHECKS), pr.key); + return pr.key; +} + +function withChecks(store: Store): string[] { + return (store.db.prepare("SELECT key FROM pr_snapshot WHERE json_type(json, '$.checks') IS NOT NULL ORDER BY key").all() as { key: string }[]).map((row) => row.key); +} + +function revisions(store: Store): unknown[] { + return store.db.prepare('SELECT key, snapshot_revision FROM pr ORDER BY key').all(); +} + +describe('the checks_strip job', () => { + let store: Store; + let reports: StorageJobReport[]; + let lines: string[]; + + beforeEach(() => { + store = Store.open(':memory:'); + reports = []; + lines = []; + }); + + afterEach(() => { + store.close(); + }); + + function runner(): StorageJobRunner { + return new StorageJobRunner({ + store, + jobs: [new ChecksStripJob()], + now: () => NOW, + timers: new FakeTimers(), + busy: () => false, + log: (line) => lines.push(line), + onDone: (report) => reports.push(report), + sliceBudgetMs: 0, + }); + } + + it('removes the checks from every stored snapshot, without a new revision or a different read', () => { + storedWithChecks(store, 1); + storedWithChecks(store, 2); + store.prs.upsert(makePr({ number: 3 }), at(1)); + const before = store.prs.listAll(); + const revisionsBefore = revisions(store); + const jobs = runner(); + + for (let index = 0; index < 10 && jobs.slice() !== 'idle'; index += 1) { + // One snapshot per slice. + } + + expect(withChecks(store)).toEqual([]); + expect(store.prs.listAll()).toEqual(before); + expect(revisions(store)).toEqual(revisionsBefore); + expect(store.meta.get(new ChecksStripJob().doneKey)).toBe(NOW.toISOString()); + expect(store.meta.get(CHECKS_STRIP_SINCE_KEY)).toBeNull(); + // Three snapshots walked, two of them rewritten. + expect(reports).toMatchObject([{ name: 'checks_strip', units: 3, wrote: 2 }]); + }); + + it('walks again when a snapshot with checks was written behind its cursor, and is done only once none is left', () => { + storedWithChecks(store, 1); + storedWithChecks(store, 3); + const jobs = runner(); + expect(jobs.slice()).toBe('worked'); + expect(jobs.slice()).toBe('worked'); + // Something wrote old-style json behind the cursor meanwhile (this build never does): a newer revision. + storedWithChecks(store, 2); + + for (let index = 0; index < 20 && jobs.slice() !== 'idle'; index += 1) { + // On to the end, the check, the second walk. + } + + expect(lines[0]).toMatch(/checks_strip: its check failed at the end, walking it once more/); + expect(withChecks(store)).toEqual([]); + expect(store.meta.get(new ChecksStripJob().doneKey)).not.toBeNull(); + }); + + it('is not held up by snapshots this build wrote while it walked', () => { + storedWithChecks(store, 1); + storedWithChecks(store, 3); + const jobs = runner(); + expect(jobs.slice()).toBe('worked'); + expect(jobs.slice()).toBe('worked'); + // A fetch behind the cursor: this build writes no checks. + store.prs.upsert(makePr({ number: 2, title: 'fetched again' }), at(5)); + + for (let index = 0; index < 10 && jobs.slice() !== 'idle'; index += 1) { + // On to the end. + } + + expect(lines).toEqual([expect.stringMatching(/^storage job checks_strip done:/)]); + expect(withChecks(store)).toEqual([]); + }); + + it('runs after the bot body trim', () => { + expect(storageJobs().map((job) => job.name)).toEqual(['bot_body_trim', 'checks_strip']); + }); +}); + +describe('Engine.startStorageJobs with the checks strip', () => { + it('runs the trim, then the strip, on the engine timers and reports each', async () => { + const h = makeHarness(); + storedWithChecks(h.store, 1); + + h.engine.startStorageJobs(); + h.timers.advance(START_DELAY_MS); + for (let index = 0; index < 100 && h.store.meta.get(new ChecksStripJob().doneKey) === null; index += 1) { + h.timers.advance(PAUSE_MS); + } + + expect(withChecks(h.store)).toEqual([]); + const done = h.telemetry.events.filter((event) => event.event === 'storage_job_done').map((event) => (event.props as { name: string }).name); + expect(done).toEqual(['bot_body_trim', 'checks_strip']); + await h.engine.close(); + }); +}); diff --git a/packages/engine/src/storage-jobs/checks-strip.ts b/packages/engine/src/storage-jobs/checks-strip.ts new file mode 100644 index 00000000..99da07f2 --- /dev/null +++ b/packages/engine/src/storage-jobs/checks-strip.ts @@ -0,0 +1,48 @@ +// Storage job 2: removes `checks` from the stored snapshot json (2026-10-05, +// DESIGN.md "CI is not tracked"). PostPile fetches no CI since 0.21.0, and +// reads drop the old checks already (PrRepo `parsePr`), so this only gives +// the disk space back: on a heavy install about a tenth of the json. +// +// One unit is one stored snapshot, in key order: SQLite removes the field in +// place (`json_remove`), inside the slice's transaction, so nothing written +// meanwhile is overwritten and no JS parses the blob. No revision moves: no +// read changes. +import type { Store } from '@postpile/store'; +import type { StorageJob, StorageJobUnit } from './runner.ts'; + +/** Meta key: the store-wide revision counter when the current walk started, for the check at its end. */ +export const CHECKS_STRIP_SINCE_KEY = 'storage_job:checks_strip:since'; + +export class ChecksStripJob implements StorageJob { + readonly name = 'checks_strip'; + readonly cursorKey = 'storage_job:checks_strip:after'; + readonly doneKey = 'storage_job:checks_strip:done'; + + step(store: Store, after: string): StorageJobUnit | null { + if (after === '') { + // A walk starts: what is written from here on is checked again at its end. + store.meta.set(CHECKS_STRIP_SINCE_KEY, String(store.prs.latestRevision())); + } + const key = store.prs.nextSnapshotKey(after); + if (key === null) { + return null; + } + return { key, wrote: store.prs.stripChecks(key) }; + } + + /** + * The walk went through every snapshot stored when it started. The ones + * stored behind its cursor since then (a fetch, a local rewrite) carry a + * newer revision: done only when none of them holds `checks`, counted + * from the data. This build never writes them, so a failure means + * something else did, and the walk starts over. + */ + complete(store: Store): 'done' | 'again' { + const since = Number(store.meta.get(CHECKS_STRIP_SINCE_KEY) ?? 0); + if (store.prs.countChecksWrittenSince(since) > 0) { + return 'again'; + } + store.meta.delete(CHECKS_STRIP_SINCE_KEY); + return 'done'; + } +} diff --git a/packages/engine/src/storage-jobs/jobs.ts b/packages/engine/src/storage-jobs/jobs.ts index b6cc7119..56100535 100644 --- a/packages/engine/src/storage-jobs/jobs.ts +++ b/packages/engine/src/storage-jobs/jobs.ts @@ -1,4 +1,5 @@ import { BotBodyTrimJob } from './bot-body-trim.ts'; +import { ChecksStripJob } from './checks-strip.ts'; import type { StorageJob } from './runner.ts'; /** @@ -7,5 +8,5 @@ import type { StorageJob } from './runner.ts'; * in turn. Append only: never reorder them, rename one or reuse a meta key. */ export function storageJobs(): StorageJob[] { - return [new BotBodyTrimJob()]; + return [new BotBodyTrimJob(), new ChecksStripJob()]; } diff --git a/packages/github/src/client.test.ts b/packages/github/src/client.test.ts index 8cad2183..16861d4a 100644 --- a/packages/github/src/client.test.ts +++ b/packages/github/src/client.test.ts @@ -110,7 +110,7 @@ describe('fetchPrs', () => { expect(pr.threads[0]?.comments[0]?.body).toBe('Why this project id?'); }); - it('maps timeline items and checks', async () => { + it('maps timeline items, and asks for no checks', async () => { const fake = new FakeFetch([{ body: loadFixture('pr-batch.json') }]); const prs = await new GitHubClient(fakeTokens, fake.fn).fetchPrs(refs); const pr = prs.get('acme/app#42')!; @@ -120,15 +120,8 @@ describe('fetchPrs', () => { { id: 'E2', kind: 'head_ref_force_pushed', actor: 'alice', at: '2026-09-20T09:00:00.000Z', subject: null }, { id: 'E3', kind: 'added_to_merge_queue', actor: 'trunk-io[bot]', at: '2026-09-20T10:00:00.000Z', subject: null }, ]); - expect(pr.checks).toEqual({ - rollup: 'PENDING', - contexts: [ - { name: 'test', conclusion: 'SUCCESS', completedAt: '2026-09-20T09:10:00.000Z' }, - { name: 'lint', conclusion: null, completedAt: null }, - { name: 'deploy/preview', conclusion: 'FAILURE', completedAt: '2026-09-20T09:05:00.000Z' }, - { name: 'ci/legacy', conclusion: null, completedAt: null }, - ], - }); + // No CI: the query asks for no checks, and the PR holds none. + expect(pr).not.toHaveProperty('checks'); const merged = prs.get('acme/api#8')!; expect(merged).toMatchObject({ @@ -138,7 +131,6 @@ describe('fetchPrs', () => { mergedAt: '2026-09-17T10:00:00.000Z', reviewDecision: 'NONE', files: [], - checks: { rollup: 'NONE', contexts: [] }, }); }); diff --git a/packages/github/src/fixtures/pr-batch.json b/packages/github/src/fixtures/pr-batch.json index 63954a52..b3d74c46 100644 --- a/packages/github/src/fixtures/pr-batch.json +++ b/packages/github/src/fixtures/pr-batch.json @@ -71,17 +71,6 @@ { "commit": { "oid": "c2", "messageHeadline": "Fix cache", "committedDate": "2026-09-20T09:00:00Z", "author": { "name": "Alice Laptop", "user": null }, "committer": { "name": "GitHub", "user": { "login": "web-flow" } } } } ] }, - "headCommit": { "nodes": [ - { "commit": { "statusCheckRollup": { - "state": "PENDING", - "contexts": { "nodes": [ - { "__typename": "CheckRun", "name": "test", "conclusion": "SUCCESS", "completedAt": "2026-09-20T09:10:00Z" }, - { "__typename": "CheckRun", "name": "lint", "conclusion": null, "completedAt": null }, - { "__typename": "StatusContext", "context": "deploy/preview", "state": "ERROR", "createdAt": "2026-09-20T09:05:00Z" }, - { "__typename": "StatusContext", "context": "ci/legacy", "state": "PENDING", "createdAt": "2026-09-20T09:01:00Z" } - ] } - } } } - ] }, "timelineItems": { "nodes": [ { "__typename": "ReviewRequestedEvent", "id": "E1", "createdAt": "2026-09-18T09:05:00Z", "actor": { "__typename": "User", "login": "alice" }, @@ -123,7 +112,6 @@ "comments": { "nodes": [] }, "reviewThreads": { "nodes": [] }, "commits": { "nodes": [] }, - "headCommit": { "nodes": [{ "commit": { "statusCheckRollup": null } }] }, "timelineItems": { "nodes": [ { "__typename": "MergedEvent", "id": "M1", "createdAt": "2026-09-17T10:00:00Z", "actor": { "__typename": "User", "login": "bob" } } ] } diff --git a/packages/github/src/normalize.test.ts b/packages/github/src/normalize.test.ts index 2a994c93..36b0cab6 100644 --- a/packages/github/src/normalize.test.ts +++ b/packages/github/src/normalize.test.ts @@ -138,7 +138,6 @@ describe('toPr: truncation', () => { raw.reviews.nodes = []; raw.commits.nodes = []; raw.timelineItems.nodes = []; - raw.headCommit.nodes = []; raw.comments.nodes = Array.from({ length: 60 }, (_, index) => ({ id: `BOT${index}`, url: `https://github.com/acme/app/pull/42#issuecomment-${100 + index}`, diff --git a/packages/github/src/normalize.ts b/packages/github/src/normalize.ts index 8a1c70fa..ab048154 100644 --- a/packages/github/src/normalize.ts +++ b/packages/github/src/normalize.ts @@ -2,9 +2,6 @@ import { prKey, trimBotBody, type CapHit, - type CheckContext, - type CheckRollup, - type Checks, type Comment, type Commit, type Pr, @@ -23,7 +20,6 @@ import type { RawActor, RawBaseRefChanges, RawBranchPr, - RawCheckContext, RawComment, RawCommit, RawConnection, @@ -33,7 +29,6 @@ import type { RawRequestedReviewer, RawReview, RawReviewThread, - RawStatusCheckRollup, RawTimelineItem, } from './raw.ts'; @@ -289,40 +284,6 @@ function toTimelineItem(raw: RawTimelineItem): TimelineItem | null { }; } -function toCheckRollup(state: string | undefined): CheckRollup { - switch (state) { - case 'SUCCESS': - return 'SUCCESS'; - case 'FAILURE': - case 'ERROR': - return 'FAILURE'; - case 'PENDING': - case 'EXPECTED': - return 'PENDING'; - default: - return 'NONE'; - } -} - -/** Old-style commit statuses have a state instead of a conclusion; map them onto check-run terms. */ -function toCheckContext(raw: RawCheckContext): CheckContext { - if (raw.__typename === 'CheckRun') { - return { name: raw.name ?? '', conclusion: raw.conclusion ?? null, completedAt: isoTimeOrNull(raw.completedAt ?? null) }; - } - const rollup = toCheckRollup(raw.state); - if (rollup === 'PENDING' || rollup === 'NONE') { - return { name: raw.context ?? '', conclusion: null, completedAt: null }; - } - return { name: raw.context ?? '', conclusion: rollup, completedAt: isoTimeOrNull(raw.createdAt ?? null) }; -} - -function toChecks(raw: RawStatusCheckRollup | null | undefined): Checks { - if (!raw) { - return { rollup: 'NONE', contexts: [] }; - } - return { rollup: toCheckRollup(raw.state), contexts: raw.contexts.nodes.map(toCheckContext) }; -} - function pendingReviewers(raw: RawPullRequest): { users: string[]; teams: string[] } { const users: string[] = []; const teams: string[] = []; @@ -450,7 +411,6 @@ export function toPr(ref: PrRef, raw: RawPullRequest): Pr { comments: allComments(raw, threads), threads, timeline, - checks: toChecks(raw.headCommit.nodes[0]?.commit.statusCheckRollup), headOid: raw.headRefOid, createdAt: isoTime(raw.createdAt), updatedAt: isoTime(raw.updatedAt), diff --git a/packages/github/src/queries.ts b/packages/github/src/queries.ts index 378a91d5..485c3316 100644 --- a/packages/github/src/queries.ts +++ b/packages/github/src/queries.ts @@ -108,16 +108,6 @@ fragment prData on PullRequest { comments(last: ${QUERY_CAPS.comments}) { totalCount ${OLDER_PAGE_INFO} nodes { ${COMMENT_NODE} } } reviewThreads(last: ${QUERY_CAPS.reviewThreads}) { totalCount ${OLDER_PAGE_INFO} nodes { ${THREAD_NODE} } } commits(last: ${QUERY_CAPS.commits}) { totalCount ${OLDER_PAGE_INFO} nodes { ${COMMIT_NODE} } } - headCommit: commits(last: 1) { - nodes { commit { statusCheckRollup { - state - contexts(first: 100) { nodes { - __typename - ... on CheckRun { name conclusion completedAt } - ... on StatusContext { context state createdAt } - } } - } } } - } timelineItems(last: ${QUERY_CAPS.timeline}, itemTypes: [${TIMELINE_TYPES.join(', ')}]) { totalCount ${OLDER_PAGE_INFO} diff --git a/packages/github/src/raw.ts b/packages/github/src/raw.ts index 203bf85a..42eb8562 100644 --- a/packages/github/src/raw.ts +++ b/packages/github/src/raw.ts @@ -117,23 +117,6 @@ export interface RawTimelineItem { requestedReviewer?: RawRequestedReviewer | null; } -export interface RawCheckContext { - __typename: 'CheckRun' | 'StatusContext'; - /** CheckRun */ - name?: string; - conclusion?: string | null; - completedAt?: string | null; - /** StatusContext */ - context?: string; - state?: string; - createdAt?: string; -} - -export interface RawStatusCheckRollup { - state: string; - contexts: { nodes: RawCheckContext[] }; -} - /** One aliased pullRequest node from the batched GraphQL query. */ export interface RawPullRequest { number: number; @@ -166,8 +149,6 @@ export interface RawPullRequest { comments: RawConnection; reviewThreads: RawConnection; commits: RawConnection; - /** commits(last: 1) again, only for the head commit's check rollup. */ - headCommit: { nodes: { commit: { statusCheckRollup: RawStatusCheckRollup | null } }[] }; timelineItems: RawConnection; } diff --git a/packages/store/src/drop-ci.test.ts b/packages/store/src/drop-ci.test.ts new file mode 100644 index 00000000..48906e3e --- /dev/null +++ b/packages/store/src/drop-ci.test.ts @@ -0,0 +1,137 @@ +// What dropping CI needs from the store (DESIGN.md "CI is not tracked"): +// migration 030 removes the CI events and their log rows and nothing else, +// reads never hand out the checks an older build stored, and the strip +// helpers remove them from the json without moving a revision. +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; +import { at, makePr } from '@postpile/core/fixtures'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { runMigrations, Store } from './index.ts'; + +const OLD_CHECKS = { rollup: 'FAILURE', contexts: [{ name: 'test', conclusion: 'FAILURE', completedAt: '2026-09-01T09:05:00.000Z' }] }; + +let dir: string; +let store: Store; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'postpile-drop-ci-')); + store = Store.open(join(dir, 'db.sqlite')); +}); + +afterEach(() => { + store.close(); + rmSync(dir, { recursive: true, force: true }); +}); + +/** The PR as a build before 0.21.0 stored it: its checks in the json. */ +function storedWithChecks(number: number): string { + const pr = makePr({ number }); + store.prs.upsert(pr, at(1)); + store.db.prepare("UPDATE pr_snapshot SET json = json_set(json, '$.checks', json(?)) WHERE key = ?").run(JSON.stringify(OLD_CHECKS), pr.key); + return pr.key; +} + +function hasChecks(key: string): boolean { + return (store.db.prepare("SELECT json_type(json, '$.checks') AS type FROM pr_snapshot WHERE key = ?").get(key) as { type: string | null }).type !== null; +} + +function revisionOf(key: string): number { + return (store.db.prepare('SELECT snapshot_revision FROM pr WHERE key = ?').get(key) as { snapshot_revision: number }).snapshot_revision; +} + +describe('migration 030', () => { + it('deletes the CI events and every log row of a CI event, and keeps the rest with their seqs', () => { + const db = new DatabaseSync(':memory:'); + runMigrations(db, 29); + const insertEvent = db.prepare( + `INSERT INTO pr_event (id, pr_key, kind, actor, is_bot, at, summary, source_id, rule_loudness, rule_reason, seen_at) + VALUES (?, 'acme/app#1', ?, '', 1, '2026-09-01T09:00:00.000Z', '', ?, 'quiet', '', ?)`, + ); + insertEvent.run('acme/app#1:ci:abc:FAILURE', 'ci', 'abc:FAILURE', null); + insertEvent.run('acme/app#1:comment:c1', 'comment', 'c1', null); + insertEvent.run('acme/app#1:bot_comment:c2', 'bot_comment', 'c2', '2026-09-01T10:00:00.000Z'); + // Not a CI event, though "ci" follows a colon in its id. + insertEvent.run('acme/app#1:comment:ci:x', 'comment', 'ci:x', null); + const log = db.prepare("INSERT INTO event_log (event_id, pr_key, logged_at) VALUES (?, 'acme/app#1', '2026-09-01T09:00:00.000Z')"); + log.run('acme/app#1:comment:c1'); + // A CI event of an older head commit: its pr_event row went with the next fetch, its log row stayed. + log.run('acme/app#1:ci:old:SUCCESS'); + log.run('acme/app#1:ci:abc:FAILURE'); + log.run('acme/app#1:bot_comment:c2'); + log.run('acme/app#1:comment:ci:x'); + + runMigrations(db); + + expect(db.prepare('SELECT id, seen_at FROM pr_event ORDER BY id').all()).toEqual([ + { id: 'acme/app#1:bot_comment:c2', seen_at: '2026-09-01T10:00:00.000Z' }, + { id: 'acme/app#1:comment:c1', seen_at: null }, + { id: 'acme/app#1:comment:ci:x', seen_at: null }, + ]); + expect(db.prepare('SELECT seq, event_id FROM event_log ORDER BY seq').all()).toEqual([ + { seq: 1, event_id: 'acme/app#1:comment:c1' }, + { seq: 4, event_id: 'acme/app#1:bot_comment:c2' }, + { seq: 5, event_id: 'acme/app#1:comment:ci:x' }, + ]); + db.close(); + }); +}); + +describe('PrRepo and checks stored before 0.21.0', () => { + it('never hands them out, and a rewrite of what it read does not store them again', () => { + const key = storedWithChecks(1); + + expect(store.prs.get(key)).not.toHaveProperty('checks'); + expect(store.prs.getMany([key]).get(key)).not.toHaveProperty('checks'); + expect(store.prs.keepParsed([key]).get(key)).not.toHaveProperty('checks'); + expect(store.prs.listAll()[0]).not.toHaveProperty('checks'); + expect(store.prs.nextAfter('')?.pr).not.toHaveProperty('checks'); + expect(store.prs.get(key)).toEqual(makePr({ number: 1 })); + + store.prs.upsert(store.prs.get(key)!, at(2)); + expect(hasChecks(key)).toBe(false); + }); + + it('strips them in SQL without a new revision, and says when there was nothing to strip', () => { + const key = storedWithChecks(1); + const revision = revisionOf(key); + + expect(store.prs.stripChecks(key)).toBe(true); + expect(hasChecks(key)).toBe(false); + expect(revisionOf(key)).toBe(revision); + expect(store.prs.get(key)).toEqual(makePr({ number: 1 })); + expect(store.prs.stripChecks(key)).toBe(false); + }); + + it('counts the snapshots with checks written after a revision, and walks every snapshot key', () => { + storedWithChecks(1); + const since = store.prs.latestRevision(); + const later = storedWithChecks(2); + + expect(store.prs.countChecksWrittenSince(since)).toBe(1); + expect(store.prs.countChecksWrittenSince(store.prs.latestRevision())).toBe(0); + store.prs.stripChecks(later); + expect(store.prs.countChecksWrittenSince(since)).toBe(0); + expect(store.prs.nextSnapshotKey('')).toBe('acme/app#1'); + expect(store.prs.nextSnapshotKey('acme/app#1')).toBe('acme/app#2'); + expect(store.prs.nextSnapshotKey('acme/app#2')).toBeNull(); + }); + + it('reads in one read transaction, and leaves none open', () => { + storedWithChecks(1); + const reader = Store.openReadOnly(join(dir, 'db.sqlite')); + try { + expect(reader.prs.getMany(['acme/app#1']).size).toBe(1); + expect(reader.prs.keepParsed(['acme/app#1']).size).toBe(1); + expect(reader.db.isTransaction).toBe(false); + // Inside a caller's transaction a read joins it. + store.transaction(() => { + expect(store.prs.getMany(['acme/app#1']).size).toBe(1); + expect(store.db.isTransaction).toBe(true); + }); + } finally { + reader.close(); + } + }); +}); diff --git a/packages/store/src/migrate.ts b/packages/store/src/migrate.ts index 743efb65..3e38e5ba 100644 --- a/packages/store/src/migrate.ts +++ b/packages/store/src/migrate.ts @@ -28,6 +28,7 @@ import * as pendingCatchUp from './migrations/026_pending_catch_up.ts'; import * as macPing from './migrations/027_mac_ping.ts'; import * as prHeader from './migrations/028_pr_header.ts'; import * as snapshotRevision from './migrations/029_snapshot_revision.ts'; +import * as dropCi from './migrations/030_drop_ci.ts'; interface Migration { version: number; @@ -37,7 +38,7 @@ interface Migration { } // Append new migrations here, in order. Never edit one that has shipped. -const migrations: Migration[] = [init, engineMemory, factRecheck, instructionsVersions, topicAreas, pullIns, pingDecisions, actionLog, workContext, dropBroughtBack, pendingWrite, prEventOrderIndex, pendingWriteKind, foundPr, dropTopicDeferred, glanceKeyFiles, topicProposalSource, cleanTopicNames, prSnooze, topicRetiredAt, dropStartFresh, prSetChange, topicKind, topicDriverPick, lesson, pendingCatchUp, macPing, prHeader, snapshotRevision]; +const migrations: Migration[] = [init, engineMemory, factRecheck, instructionsVersions, topicAreas, pullIns, pingDecisions, actionLog, workContext, dropBroughtBack, pendingWrite, prEventOrderIndex, pendingWriteKind, foundPr, dropTopicDeferred, glanceKeyFiles, topicProposalSource, cleanTopicNames, prSnooze, topicRetiredAt, dropStartFresh, prSetChange, topicKind, topicDriverPick, lesson, pendingCatchUp, macPing, prHeader, snapshotRevision, dropCi]; /** The schema version this build writes and expects. */ export const LATEST_VERSION = migrations[migrations.length - 1]!.version; diff --git a/packages/store/src/migrations/030_drop_ci.ts b/packages/store/src/migrations/030_drop_ci.ts new file mode 100644 index 00000000..555e2977 --- /dev/null +++ b/packages/store/src/migrations/030_drop_ci.ts @@ -0,0 +1,22 @@ +// PostPile no longer tracks CI (2026-10-05, DESIGN.md "CI is not tracked"). +// Checks were the costliest part of a PR fetch, usually stale by the time +// anyone looked, and nothing but a quiet event and a pane fact read them. +// +// The CI events go: no build derives them any more, so a stored one would +// only sit there, folded into the activity list's bot line. Their event log +// rows go with them, matched by id (`:ci::`), also +// the ones whose event a newer head commit already dropped. Every reader of +// the log joins pr_event, so this removes nothing a reader still shows, and +// it can make no row unseen: rows only leave. +// +// The snapshot json's `checks` is removed in the background by the storage +// job checks_strip, and reads drop it until then. This migration is also +// what keeps 0.20.0 away from the stripped json: it refuses a database with +// a schema newer than its own. + +export const version = 30; + +export const sql = ` +DELETE FROM event_log WHERE substr(event_id, 1, length(pr_key) + 4) = pr_key || ':ci:'; +DELETE FROM pr_event WHERE kind = 'ci'; +`; diff --git a/packages/store/src/repos.test.ts b/packages/store/src/repos.test.ts index c987cf16..6f660f35 100644 --- a/packages/store/src/repos.test.ts +++ b/packages/store/src/repos.test.ts @@ -93,7 +93,7 @@ describe('NotificationRepo', () => { describe('PrRepo', () => { it('round-trips the full snapshot', () => { - const pr = makePr({ number: 2, labels: ['devex'], checks: { rollup: 'SUCCESS', contexts: [] } }); + const pr = makePr({ number: 2, labels: ['devex'] }); store.prs.upsert(pr, at(1)); store.prs.upsert(makePr({ number: 1 }), at(1)); store.prs.upsert(makePr({ number: 9, repo: 'acme/other' }), at(1)); @@ -558,6 +558,20 @@ describe('SnoozeRepo', () => { store.snoozes.remove('a/b#1'); expect(store.snoozes.list()).toEqual([]); }); + + it('reads a condition this build no longer offers, or cannot read, as a time that passed at the start', () => { + const insert = store.db.prepare('INSERT INTO pr_snooze (pr_key, condition_json, since) VALUES (?, ?, ?)'); + insert.run('a/b#1', '{"kind":"ci_green"}', at(5)); + insert.run('a/b#2', '{"kind":"until_time"}', at(6)); + insert.run('a/b#3', 'not json', at(7)); + insert.run('a/b#4', 'null', at(8)); + expect(store.snoozes.list().map((snooze) => [snooze.prKey, snooze.condition])).toEqual([ + ['a/b#1', { kind: 'until_time', until: at(5) }], + ['a/b#2', { kind: 'until_time', until: at(6) }], + ['a/b#3', { kind: 'until_time', until: at(7) }], + ['a/b#4', { kind: 'until_time', until: at(8) }], + ]); + }); }); describe('FeedbackRepo', () => { diff --git a/packages/store/src/repos/prs.ts b/packages/store/src/repos/prs.ts index cabd7699..c34fffd7 100644 --- a/packages/store/src/repos/prs.ts +++ b/packages/store/src/repos/prs.ts @@ -42,6 +42,22 @@ interface HeaderRow { last_event_at: string | null; } +/** + * A stored snapshot, parsed, without the `checks` builds before 0.21.0 + * wrote (DESIGN.md "CI is not tracked"). Until the storage job checks_strip + * has reached a PR its json still holds them; dropping them here keeps them + * out of memory and out of anything written back (a local rewrite stores + * what it read). + */ +function parsePr(text: string): Pr { + const parsed = JSON.parse(text) as Pr & { checks?: unknown }; + if (!('checks' in parsed)) { + return parsed; + } + const { checks: _checks, ...pr } = parsed; + return pr; +} + /** A JSON list column; most are empty, which needs no parse. */ function listOf(text: string): string[] { return text === '[]' ? [] : (JSON.parse(text) as string[]); @@ -75,6 +91,10 @@ function toHeader(row: HeaderRow): PrHeader { * existence authority: a PR is stored if and only if it has one) and the * snapshot json in `pr_snapshot` (the blob being phased out). Both are * written together; reads of the json ignore a snapshot without a header. + * + * Every read of PRs runs in one read transaction, so the header revisions + * and the snapshots it takes come from the same commit, also on a read-only + * connection next to the app's writes (checked with Codex GPT-6.1). */ export class PrRepo { /** @@ -118,7 +138,7 @@ export class PrRepo { this.parsed.delete(row.key); continue; } - const pr = JSON.parse(text) as Pr; + const pr = parsePr(text); fresh.set(row.key, pr); if (keep(row.key)) { this.parsed.set(row.key, { revision: row.snapshot_revision, pr }); @@ -258,12 +278,12 @@ export class PrRepo { /** The stored snapshot; null without a header (a snapshot alone is not a stored PR) or without a snapshot. */ get(key: PrKey): Pr | null { const row = one<{ json: string }>(this.db, 'SELECT s.json FROM pr p JOIN pr_snapshot s ON s.key = p.key WHERE p.key = ?', key); - return row ? (JSON.parse(row.json) as Pr) : null; + return row ? parsePr(row.json) : null; } /** Stored PRs by key. A hot PR comes from the cache; any other is parsed for this call only and not kept. */ getMany(keys: PrKey[]): Map { - return keys.length === 0 ? new Map() : this.parse(this.revisionRows(keys), () => false); + return keys.length === 0 ? new Map() : inTransaction(this.db, () => this.parse(this.revisionRows(keys), () => false)); } /** @@ -278,7 +298,7 @@ export class PrRepo { this.parsed.delete(key); } } - return this.parse(this.revisionRows(keys), (key) => wanted.has(key)); + return inTransaction(this.db, () => this.parse(this.revisionRows(keys), (key) => wanted.has(key))); } /** @@ -287,8 +307,10 @@ export class PrRepo { * `getMany` for the PRs it needs. */ listAll(): Pr[] { - const rows = all(this.db, 'SELECT key, snapshot_revision FROM pr ORDER BY repo, number'); - return [...this.parse(rows, () => false).values()]; + return inTransaction(this.db, () => { + const rows = all(this.db, 'SELECT key, snapshot_revision FROM pr ORDER BY repo, number'); + return [...this.parse(rows, () => false).values()]; + }); } /** Every stored key. */ @@ -360,6 +382,42 @@ export class PrRepo { 'SELECT p.key, p.fetched_at, s.json FROM pr p JOIN pr_snapshot s ON s.key = p.key WHERE p.key > ? ORDER BY p.key LIMIT 1', afterKey, ); - return row === null ? null : { key: row.key, pr: JSON.parse(row.json) as Pr, fetchedAt: row.fetched_at }; + return row === null ? null : { key: row.key, pr: parsePr(row.json), fetchedAt: row.fetched_at }; + } + + /** The last snapshot revision handed out (the store-wide counter), 0 before the first. */ + latestRevision(): number { + return Number(one<{ value: string }>(this.db, 'SELECT value FROM meta WHERE key = ?', SNAPSHOT_REVISION_KEY)?.value ?? 0); + } + + /** For the storage job checks_strip: the next stored snapshot's key after `afterKey` ('' for the first); null after the last. */ + nextSnapshotKey(afterKey: PrKey): PrKey | null { + return one<{ key: string }>(this.db, 'SELECT key FROM pr_snapshot WHERE key > ? ORDER BY key LIMIT 1', afterKey)?.key ?? null; + } + + /** + * Removes `checks` from one stored snapshot's json, in SQL, when it holds + * them; true when it did. No new revision: reads drop them anyway + * (`parsePr`), so no read changes. + */ + stripChecks(key: PrKey): boolean { + return ( + run(this.db, "UPDATE pr_snapshot SET json = json_remove(json, '$.checks') WHERE key = ? AND json_type(json, '$.checks') IS NOT NULL", key) > 0 + ); + } + + /** + * Snapshots written after revision `since` (by a fetch or a local + * rewrite) whose json holds `checks`: the strip's check from the data for + * the PRs stored behind its cursor while it walked. + */ + countChecksWrittenSince(since: number): number { + return ( + one<{ n: number }>( + this.db, + "SELECT count(*) AS n FROM pr_snapshot WHERE key IN (SELECT key FROM pr WHERE snapshot_revision > ?) AND json_type(json, '$.checks') IS NOT NULL", + since, + )?.n ?? 0 + ); } } diff --git a/packages/store/src/repos/snoozes.ts b/packages/store/src/repos/snoozes.ts index f1ee5270..2840b6dd 100644 --- a/packages/store/src/repos/snoozes.ts +++ b/packages/store/src/repos/snoozes.ts @@ -8,8 +8,34 @@ interface SnoozeRow { since: string; } +/** + * The stored condition. One this build no longer offers ("Until CI is + * green", gone in 0.21.0) or cannot read becomes a time that already + * passed (the snooze's start), so the snooze ends like an expired one. + */ +function conditionOf(row: SnoozeRow): SnoozeCondition { + const expired: SnoozeCondition = { kind: 'until_time', until: row.since }; + let parsed: unknown; + try { + parsed = JSON.parse(row.condition_json); + } catch { + return expired; + } + if (typeof parsed !== 'object' || parsed === null) { + return expired; + } + const stored = parsed as { kind?: unknown; until?: unknown }; + if (stored.kind === 'someone_replies' || stored.kind === 'new_push') { + return { kind: stored.kind }; + } + if (stored.kind === 'until_time' && typeof stored.until === 'string') { + return { kind: 'until_time', until: stored.until }; + } + return expired; +} + function toSnooze(row: SnoozeRow): Snooze { - return { prKey: row.pr_key, condition: JSON.parse(row.condition_json) as SnoozeCondition, since: row.since }; + return { prKey: row.pr_key, condition: conditionOf(row), since: row.since }; } /** One snooze per PR (see `snoozeWrites` in core for how a tile's snooze is written). */ From 14a8d1d8633fcbe56dece37b36a30940d3f62c71 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Mon, 5 Oct 2026 22:38:36 +0200 Subject: [PATCH 2/3] fix(devex): keep the event log high-water mark and verify the strip by a full walk GPT-6.1 review on #128: - Migration 030 can delete the newest event_log rows, and maxSeq read MAX(seq), so it could drop below a cursor. CursorRepo.advance only moves forward, so marking a topic seen or finishing consolidation would then silently keep the old dossier version and time. maxSeq now reads the AUTOINCREMENT counter in sqlite_sequence (never lowered by deletes). - checks_strip trusted revisions to find snapshots written behind its cursor, but unguarded 0.18/0.19 builds keep the revision when they write checks back, and the final check could parse a large backlog in one slice. Done now means a whole walk, one snapshot per unit within the slice budget, found nothing to strip; a walk that stripped anything starts over to verify. A downgrade after completion can leave bytes behind, which reads drop anyway. - The activity fold says "N bot events": there is no CI in it any more. --- DESIGN.md | 28 +++++--- .../src/components/DetailPane.test.tsx | 4 +- apps/server/src/routes.test.ts | 2 +- packages/core/src/activity.test.ts | 6 +- packages/core/src/activity.ts | 10 +-- .../src/storage-jobs/checks-strip.test.ts | 69 ++++++++++++------- .../engine/src/storage-jobs/checks-strip.ts | 50 ++++++++------ packages/store/src/drop-ci.test.ts | 57 ++++++++++++--- packages/store/src/memory-repos.test.ts | 3 +- packages/store/src/repos/event-log.ts | 14 +++- packages/store/src/repos/prs.ts | 20 ------ 11 files changed, 162 insertions(+), 101 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 6a40604a..a7be7c10 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -749,8 +749,11 @@ snooze and the CLI's rollup) did not pay for the fetch. rows whose event a newer head commit had already dropped. Every log reader joins `pr_event`, so nothing a reader showed changes but the CI lines themselves; rows only leave, so nothing turns unseen and no cursor moves - (on the copies: unseen non-CI events and the highest seq the same before - and after). Chosen over letting re-derivation drop them: merged and closed + (on the copies: unseen non-CI events the same before and after). The + log's high-water mark (`EventLogRepo.maxSeq`) reads SQLite's + AUTOINCREMENT counter, not `MAX(seq)`: deleting the newest rows must not + lower it, or a cursor already past it could never be marked seen again + (`CursorRepo.advance` only moves forward; found by GPT-6.1). Chosen over letting re-derivation drop them: merged and closed PRs are never fetched again, so their CI events would stay forever and the `ci` kind with them. A PR whose newest event was a CI result can fall out of the hot set's "active in the last 7 days" a little earlier. It took @@ -763,13 +766,16 @@ snooze and the CLI's rollup) did not pay for the fetch. parse a snapshot (`PrRepo` `parsePr`), so they never reach memory or a rewrite. Storage job 2, `checks_strip`, removes them on disk: one unit is one snapshot in key order, `json_remove` in place inside the slice's - transaction, no revision (no read changes). Its check: the walk covered - every snapshot stored when it started, and the ones stored behind its - cursor since carry a newer revision; it is done only when none of those - holds `checks` (meta `storage_job:checks_strip:since`), else it walks - again. Migration 030 is also what keeps 0.20.0 off the stripped json. - Measured: heavy 93 MB of json freed in 138 slices (p95 37 ms, max 54 ms), - 4.3 s of work, 11 s wall; normal 6.7 MB in 10 slices. Every snapshot + transaction, no revision (no read changes). Done means a whole walk, + unit by unit within the slice budget, found nothing to strip: a walk that + stripped something (meta `storage_job:checks_strip:stripped`) starts over + to verify. Revisions cannot prove it, since an unguarded older build + (0.19.0 and before) keeps them when it writes checks back. Such a + downgrade after the job finished leaves bytes behind, harmlessly: reads + drop them, and the next fetch of that PR writes them out. Migration 030 is also what keeps 0.20.0 off the stripped json. + Measured (one walk, before the verifying walk was added): heavy 93 MB of + json freed in 138 slices (p95 37 ms, max 54 ms), 4.3 s of work, 11 s + wall; normal 6.7 MB in 10 slices. Every snapshot equals the original but for its checks. Hot-set heap on heavy 283 MB (the 0.20.0 read) → 264 MB (this build, before the strip) → 263 MB after. - **What stays.** `NO_CI_RULE` in every writing prompt: glances and @@ -3780,9 +3786,9 @@ and detail start equally wide (2026-09-30, was a 420-480px tile clamp). At unseen noise after the viewer's last touch (`activityList(events, viewer, since)`) go to the box under the title (2026-09-29); the list, titled "Earlier activity" then, keeps only the rest. About 12 lines - (`ACTIVITY_LINE_CAP`) before "Show all N". Bots, CI, deploys, merge queue, + (`ACTIVITY_LINE_CAP`) before "Show all N". Bots, deploys, merge queue, agent-muted events and review requests between others fold into one "N - bot/CI events" line that expands (Unmute lives there). + bot events" line that expands (Unmute lives there). Comments and reviews from people show in full (2026-09-29): the event `summary` is one clipped line (100 chars, first line) for tiles, MCP and the agent, so `activityList(events, viewer, since, pr)` also puts the diff --git a/apps/desktop/src/renderer/src/components/DetailPane.test.tsx b/apps/desktop/src/renderer/src/components/DetailPane.test.tsx index 67a6f1a6..dd199722 100644 --- a/apps/desktop/src/renderer/src/components/DetailPane.test.tsx +++ b/apps/desktop/src/renderer/src/components/DetailPane.test.tsx @@ -178,8 +178,8 @@ describe('DetailPane', () => { // The reply target comes with the activity line, built from the stored PR on the server. expect(screen.getByText('Why one key for all jobs?')).toBeTruthy(); expect(screen.getByRole('button', { name: /^Reply$/ })).toBeTruthy(); - // The folded bot/CI rows draw from the slim items too: summary, and the reason in the hover title. - fireEvent.click(screen.getByRole('button', { name: 'Show 1 bot/CI event' })); + // The folded bot rows draw from the slim items too: summary, and the reason in the hover title. + fireEvent.click(screen.getByRole('button', { name: 'Show 1 bot event' })); expect(screen.getByText('Preview deployed').closest('[title]')?.getAttribute('title')).toBe('seen: bot activity'); }); }); diff --git a/apps/server/src/routes.test.ts b/apps/server/src/routes.test.ts index c53e166f..6059c2e4 100644 --- a/apps/server/src/routes.test.ts +++ b/apps/server/src/routes.test.ts @@ -54,7 +54,7 @@ async function post(app: TestApp, path: string, body: unknown = {}): Promise< return { status: res.status, json: (await res.json()) as T }; } -/** Every row the PR pane can show, lines and folded bot/CI rows alike. */ +/** Every row the PR pane can show, lines and folded bot rows alike. */ function activityItems(detail: PrDetail): ActivityEvent[] { const { fresh, earlier, noise, freshNoise } = detail.activity; return [...fresh, ...earlier, ...noise, ...freshNoise]; diff --git a/packages/core/src/activity.test.ts b/packages/core/src/activity.test.ts index b865e4a7..b364f29b 100644 --- a/packages/core/src/activity.test.ts +++ b/packages/core/src/activity.test.ts @@ -37,7 +37,7 @@ describe('activityList', () => { const list = activityList([forMe, forTeam, forSam], who); expect(list.earlier.map((line) => line.id)).toEqual([forTeam.event.id, forMe.event.id]); expect(list.noise.map((item) => item.id)).toEqual([forSam.event.id]); - expect(list.noiseLabel).toBe('1 bot/CI and other event'); + expect(list.noiseLabel).toBe('1 bot and other event'); }); it('collapses a burst of pushes by one person into one line', () => { @@ -74,10 +74,10 @@ describe('activityList', () => { expect(list.noise).toHaveLength(2); }); - it('labels machine-only noise as bot/CI events', () => { + it('labels machine-only noise as bot events', () => { const queue = ev({ kind: 'merge_queue', actor: '', isBot: true, summary: 'queued' }); const deploy = ev({ kind: 'deploy', actor: 'vercel', isBot: true, summary: 'vercel deploy' }); - expect(noiseLabel([queue, deploy])).toBe('2 bot/CI events'); + expect(noiseLabel([queue, deploy])).toBe('2 bot events'); }); }); diff --git a/packages/core/src/activity.ts b/packages/core/src/activity.ts index 2525c1d5..5019d198 100644 --- a/packages/core/src/activity.ts +++ b/packages/core/src/activity.ts @@ -1,5 +1,5 @@ // The detail pane's activity list: the meaningful events of a PR, with push -// bursts collapsed and bot / CI noise folded into one line. Rules only; the +// bursts collapsed and bot noise folded into one line. Rules only; the // engine and FakeEngine ship the result on `PrDetail.activity`, with each // event cut down to what a row draws (`ActivityEvent`). import { reviewRequestSubject } from './events.ts'; @@ -37,7 +37,7 @@ export interface LineReply { /** * One event as a row of the pane draws it: glyph by kind, the actor in bold, * the summary, its age, the unread dot and why it is loud or quiet in the - * hover title. The folded bot / CI rows are these; a line adds to it. + * hover title. The folded bot rows are these; a line adds to it. */ export interface ActivityEvent { /** The event's id: the row's key, and what Unmute sends for an agent-muted one. */ @@ -85,7 +85,7 @@ export interface ActivityList { earlier: ActivityLine[]; /** Bot and CI events, agent-muted ones and review requests between others, newest first. Without `freshNoise`. */ noise: ActivityEvent[]; - /** The folded noise line: "4 bot/CI events". */ + /** The folded noise line: "4 bot events". */ noiseLabel: string; /** * The unseen part of the noise since the viewer's last touch, newest first, @@ -281,12 +281,12 @@ function byTime(a: EventView, b: EventView): number { return a.event.at < b.event.at ? -1 : a.event.at > b.event.at ? 1 : 0; } -/** "4 bot/CI events", or "4 bot/CI and other events" when review requests between others are in it. */ +/** "4 bot events", or "4 bot and other events" when review requests between others are in it. */ export function noiseLabel(noise: EventView[]): string { const machineKinds: EventKind[] = ['deploy', 'merge_queue', 'bot_comment']; const machine = noise.every((view) => view.event.isBot || machineKinds.includes(view.event.kind)); const events = noise.length === 1 ? 'event' : 'events'; - return machine ? `${noise.length} bot/CI ${events}` : `${noise.length} bot/CI and other ${events}`; + return machine ? `${noise.length} bot ${events}` : `${noise.length} bot and other ${events}`; } function countWord(count: number, word: string, plural = `${word}s`): string { diff --git a/packages/engine/src/storage-jobs/checks-strip.test.ts b/packages/engine/src/storage-jobs/checks-strip.test.ts index eac83d88..d9b7fe41 100644 --- a/packages/engine/src/storage-jobs/checks-strip.test.ts +++ b/packages/engine/src/storage-jobs/checks-strip.test.ts @@ -1,11 +1,12 @@ // Storage job 2 (DESIGN.md "CI is not tracked"): removes the checks older // builds stored in the snapshot json, one snapshot per unit, without moving -// a revision or changing a read, and is done only when the data says so. +// a revision or changing a read, and is done only after a whole walk found +// none left. import { at, FakeTimers, makePr } from '@postpile/core/fixtures'; import { Store } from '@postpile/store'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { makeHarness, NOW } from '../testing/fakes.ts'; -import { CHECKS_STRIP_SINCE_KEY, ChecksStripJob } from './checks-strip.ts'; +import { CHECKS_STRIP_STRIPPED_KEY, ChecksStripJob } from './checks-strip.ts'; import { storageJobs } from './jobs.ts'; import { PAUSE_MS, START_DELAY_MS, StorageJobRunner, type StorageJobReport } from './runner.ts'; @@ -15,10 +16,15 @@ const OLD_CHECKS = { rollup: 'SUCCESS', contexts: [{ name: 'lint', conclusion: ' function storedWithChecks(store: Store, number: number): string { const pr = makePr({ number }); store.prs.upsert(pr, at(1)); - store.db.prepare("UPDATE pr_snapshot SET json = json_set(json, '$.checks', json(?)) WHERE key = ?").run(JSON.stringify(OLD_CHECKS), pr.key); + addChecks(store, pr.key); return pr.key; } +/** Checks put back into a stored json the way an older build writes them, its revision kept. */ +function addChecks(store: Store, key: string): void { + store.db.prepare("UPDATE pr_snapshot SET json = json_set(json, '$.checks', json(?)) WHERE key = ?").run(JSON.stringify(OLD_CHECKS), key); +} + function withChecks(store: Store): string[] { return (store.db.prepare("SELECT key FROM pr_snapshot WHERE json_type(json, '$.checks') IS NOT NULL ORDER BY key").all() as { key: string }[]).map((row) => row.key); } @@ -55,60 +61,71 @@ describe('the checks_strip job', () => { }); } - it('removes the checks from every stored snapshot, without a new revision or a different read', () => { + function runToEnd(jobs: StorageJobRunner): void { + for (let index = 0; index < 30 && jobs.slice() !== 'idle'; index += 1) { + // One snapshot per slice. + } + } + + it('removes the checks from every stored snapshot, without a new revision or a different read, then walks once more to verify', () => { storedWithChecks(store, 1); storedWithChecks(store, 2); store.prs.upsert(makePr({ number: 3 }), at(1)); const before = store.prs.listAll(); const revisionsBefore = revisions(store); - const jobs = runner(); - for (let index = 0; index < 10 && jobs.slice() !== 'idle'; index += 1) { - // One snapshot per slice. - } + runToEnd(runner()); expect(withChecks(store)).toEqual([]); expect(store.prs.listAll()).toEqual(before); expect(revisions(store)).toEqual(revisionsBefore); expect(store.meta.get(new ChecksStripJob().doneKey)).toBe(NOW.toISOString()); - expect(store.meta.get(CHECKS_STRIP_SINCE_KEY)).toBeNull(); - // Three snapshots walked, two of them rewritten. - expect(reports).toMatchObject([{ name: 'checks_strip', units: 3, wrote: 2 }]); + expect(store.meta.get(CHECKS_STRIP_STRIPPED_KEY)).toBeNull(); + // Two walks over three snapshots: the first rewrote two, the second found nothing. + expect(reports).toMatchObject([{ name: 'checks_strip', units: 6, wrote: 2 }]); + expect(lines).toEqual([expect.stringMatching(/^storage job checks_strip done:/)]); }); - it('walks again when a snapshot with checks was written behind its cursor, and is done only once none is left', () => { + it('is done after one walk when nothing held checks', () => { + store.prs.upsert(makePr({ number: 1 }), at(1)); + store.prs.upsert(makePr({ number: 2 }), at(1)); + + runToEnd(runner()); + + expect(reports).toMatchObject([{ units: 2, wrote: 0 }]); + }); + + it('catches checks an older build wrote back behind the cursor with its revision kept', () => { storedWithChecks(store, 1); storedWithChecks(store, 3); const jobs = runner(); expect(jobs.slice()).toBe('worked'); expect(jobs.slice()).toBe('worked'); - // Something wrote old-style json behind the cursor meanwhile (this build never does): a newer revision. - storedWithChecks(store, 2); + // An unguarded 0.19.0 rewrote #1 meanwhile: checks back, revision as it was. + const revision = revisions(store); + addChecks(store, 'acme/app#1'); + expect(revisions(store)).toEqual(revision); - for (let index = 0; index < 20 && jobs.slice() !== 'idle'; index += 1) { - // On to the end, the check, the second walk. - } + runToEnd(jobs); - expect(lines[0]).toMatch(/checks_strip: its check failed at the end, walking it once more/); expect(withChecks(store)).toEqual([]); expect(store.meta.get(new ChecksStripJob().doneKey)).not.toBeNull(); }); - it('is not held up by snapshots this build wrote while it walked', () => { + it('keeps walking while snapshots come back with checks, and is not done before a clean walk', () => { storedWithChecks(store, 1); - storedWithChecks(store, 3); const jobs = runner(); expect(jobs.slice()).toBe('worked'); + // The first walk ends here and the verifying walk starts at #1; an older build writes it back each time. + addChecks(store, 'acme/app#1'); expect(jobs.slice()).toBe('worked'); - // A fetch behind the cursor: this build writes no checks. - store.prs.upsert(makePr({ number: 2, title: 'fetched again' }), at(5)); + expect(store.meta.get(new ChecksStripJob().doneKey)).toBeNull(); + expect(store.meta.get(CHECKS_STRIP_STRIPPED_KEY)).toBe('1'); - for (let index = 0; index < 10 && jobs.slice() !== 'idle'; index += 1) { - // On to the end. - } + runToEnd(jobs); - expect(lines).toEqual([expect.stringMatching(/^storage job checks_strip done:/)]); expect(withChecks(store)).toEqual([]); + expect(store.meta.get(new ChecksStripJob().doneKey)).not.toBeNull(); }); it('runs after the bot body trim', () => { diff --git a/packages/engine/src/storage-jobs/checks-strip.ts b/packages/engine/src/storage-jobs/checks-strip.ts index 99da07f2..eaa44451 100644 --- a/packages/engine/src/storage-jobs/checks-strip.ts +++ b/packages/engine/src/storage-jobs/checks-strip.ts @@ -3,46 +3,52 @@ // reads drop the old checks already (PrRepo `parsePr`), so this only gives // the disk space back: on a heavy install about a tenth of the json. // -// One unit is one stored snapshot, in key order: SQLite removes the field in -// place (`json_remove`), inside the slice's transaction, so nothing written -// meanwhile is overwritten and no JS parses the blob. No revision moves: no -// read changes. +// One unit is one stored snapshot, in key order: SQLite looks for the field +// and removes it in place (`json_remove`), inside the slice's transaction, so +// nothing written meanwhile is overwritten and no JS parses the blob. No +// revision moves: no read changes. import type { Store } from '@postpile/store'; import type { StorageJob, StorageJobUnit } from './runner.ts'; -/** Meta key: the store-wide revision counter when the current walk started, for the check at its end. */ -export const CHECKS_STRIP_SINCE_KEY = 'storage_job:checks_strip:since'; +/** Meta flag: the current walk stripped at least one snapshot, so another walk follows to verify. */ +export const CHECKS_STRIP_STRIPPED_KEY = 'storage_job:checks_strip:stripped'; export class ChecksStripJob implements StorageJob { readonly name = 'checks_strip'; readonly cursorKey = 'storage_job:checks_strip:after'; readonly doneKey = 'storage_job:checks_strip:done'; + /** + * The next snapshot after the cursor, its checks removed if it has any. + * At the end of a walk that stripped something, the walk starts over: the + * job ends only after a whole walk, unit by unit within the slice budget, + * found nothing left. That also catches checks an older, unguarded build + * (0.19.0 and before) wrote back behind the cursor; the revisions such a + * build keeps say nothing about it. + */ step(store: Store, after: string): StorageJobUnit | null { - if (after === '') { - // A walk starts: what is written from here on is checked again at its end. - store.meta.set(CHECKS_STRIP_SINCE_KEY, String(store.prs.latestRevision())); + let key = store.prs.nextSnapshotKey(after); + if (key === null && store.meta.get(CHECKS_STRIP_STRIPPED_KEY) !== null) { + store.meta.delete(CHECKS_STRIP_STRIPPED_KEY); + key = store.prs.nextSnapshotKey(''); } - const key = store.prs.nextSnapshotKey(after); if (key === null) { return null; } - return { key, wrote: store.prs.stripChecks(key) }; + const wrote = store.prs.stripChecks(key); + if (wrote) { + store.meta.set(CHECKS_STRIP_STRIPPED_KEY, '1'); + } + return { key, wrote }; } /** - * The walk went through every snapshot stored when it started. The ones - * stored behind its cursor since then (a fetch, a local rewrite) carry a - * newer revision: done only when none of them holds `checks`, counted - * from the data. This build never writes them, so a failure means - * something else did, and the walk starts over. + * Done when the last walk stripped nothing: every snapshot was read and + * none held checks. A downgrade to a build before 0.21.0 after that can + * write some back; reads drop them, so they only cost disk until the + * snapshot is fetched again. */ complete(store: Store): 'done' | 'again' { - const since = Number(store.meta.get(CHECKS_STRIP_SINCE_KEY) ?? 0); - if (store.prs.countChecksWrittenSince(since) > 0) { - return 'again'; - } - store.meta.delete(CHECKS_STRIP_SINCE_KEY); - return 'done'; + return store.meta.get(CHECKS_STRIP_STRIPPED_KEY) === null ? 'done' : 'again'; } } diff --git a/packages/store/src/drop-ci.test.ts b/packages/store/src/drop-ci.test.ts index 48906e3e..46c9bb2a 100644 --- a/packages/store/src/drop-ci.test.ts +++ b/packages/store/src/drop-ci.test.ts @@ -78,6 +78,53 @@ describe('migration 030', () => { }); }); +describe('the event log high-water mark after migration 030', () => { + function logAt29(ids: string[]): DatabaseSync { + const db = new DatabaseSync(':memory:'); + runMigrations(db, 29); + const insertEvent = db.prepare( + `INSERT INTO pr_event (id, pr_key, kind, actor, is_bot, at, summary, source_id, rule_loudness, rule_reason) + VALUES (?, 'acme/app#1', ?, '', 1, '2026-09-01T09:00:00.000Z', '', ?, 'quiet', '')`, + ); + const log = db.prepare("INSERT INTO event_log (event_id, pr_key, logged_at) VALUES (?, 'acme/app#1', '2026-09-01T09:00:00.000Z')"); + for (const id of ids) { + const [, kind, sourceId] = id.split(':'); + insertEvent.run(id, kind!, sourceId!); + log.run(id); + } + return db; + } + + it('stays where it was when the newest log rows were CI events, so a cursor there can still advance', () => { + const db = logAt29(['acme/app#1:comment:c1', 'acme/app#1:ci:a', 'acme/app#1:ci:b']); + + runMigrations(db); + + const store = new Store(db); + expect(store.db.prepare('SELECT max(seq) AS seq FROM event_log').get()).toEqual({ seq: 1 }); + expect(store.eventLog.maxSeq()).toBe(3); + // The seen cursor was at 3 before the migration; marking seen again must still move its version and time. + store.cursors.advance({ kind: 'seen', scope: 'topic-1', seq: 3, dossierVersion: 1, updatedAt: '2026-09-01T10:00:00.000Z' }); + store.cursors.advance({ kind: 'seen', scope: 'topic-1', seq: store.eventLog.maxSeq(), dossierVersion: 2, updatedAt: '2026-09-01T11:00:00.000Z' }); + expect(store.cursors.get('seen', 'topic-1')).toMatchObject({ seq: 3, dossierVersion: 2, updatedAt: '2026-09-01T11:00:00.000Z' }); + db.close(); + }); + + it('stays where it was when the whole log was CI events', () => { + const db = logAt29(['acme/app#1:ci:a', 'acme/app#1:ci:b']); + + runMigrations(db); + + expect(db.prepare('SELECT count(*) AS n FROM event_log').get()).toEqual({ n: 0 }); + expect(new Store(db).eventLog.maxSeq()).toBe(2); + db.close(); + }); + + it('is 0 on a log that never had a row', () => { + expect(store.eventLog.maxSeq()).toBe(0); + }); +}); + describe('PrRepo and checks stored before 0.21.0', () => { it('never hands them out, and a rewrite of what it read does not store them again', () => { const key = storedWithChecks(1); @@ -104,15 +151,9 @@ describe('PrRepo and checks stored before 0.21.0', () => { expect(store.prs.stripChecks(key)).toBe(false); }); - it('counts the snapshots with checks written after a revision, and walks every snapshot key', () => { + it('walks every snapshot key', () => { storedWithChecks(1); - const since = store.prs.latestRevision(); - const later = storedWithChecks(2); - - expect(store.prs.countChecksWrittenSince(since)).toBe(1); - expect(store.prs.countChecksWrittenSince(store.prs.latestRevision())).toBe(0); - store.prs.stripChecks(later); - expect(store.prs.countChecksWrittenSince(since)).toBe(0); + storedWithChecks(2); expect(store.prs.nextSnapshotKey('')).toBe('acme/app#1'); expect(store.prs.nextSnapshotKey('acme/app#1')).toBe('acme/app#2'); expect(store.prs.nextSnapshotKey('acme/app#2')).toBeNull(); diff --git a/packages/store/src/memory-repos.test.ts b/packages/store/src/memory-repos.test.ts index 5e4528db..861246fb 100644 --- a/packages/store/src/memory-repos.test.ts +++ b/packages/store/src/memory-repos.test.ts @@ -48,7 +48,8 @@ describe('EventLogRepo', () => { store.eventLog.append([{ id: `${pr1}:comment:a`, prKey: pr1 }, { id: `${pr1}:comment:b`, prKey: pr1 }], at(60)); store.eventLog.append([{ id: `${pr2}:comment:c`, prKey: pr2 }, { id: `${pr1}:comment:a`, prKey: pr1 }], at(61)); - expect(store.eventLog.maxSeq()).toBe(3); + // The high-water mark: the ignored second sighting of a used up seq 4 in SQLite's AUTOINCREMENT counter; the next row gets 5. + expect(store.eventLog.maxSeq()).toBe(4); const all = store.eventLog.listSince([pr1, pr2], 0); expect(all.map((entry) => [entry.seq, entry.event.sourceId])).toEqual([ [1, 'a'], diff --git a/packages/store/src/repos/event-log.ts b/packages/store/src/repos/event-log.ts index 69f3d950..6c67bed9 100644 --- a/packages/store/src/repos/event-log.ts +++ b/packages/store/src/repos/event-log.ts @@ -31,9 +31,19 @@ export class EventLogRepo { }); } - /** Highest seq so far, 0 on an empty log. */ + /** + * The log's high-water mark: every row so far has a seq at or below it, + * every later one above it; 0 on a log that never had a row. Read from + * SQLite's AUTOINCREMENT counter (`sqlite_sequence`), not `MAX(seq)`: rows + * can leave (migration 030 deleted the CI events' rows, maybe the newest), + * and a cursor already past the remaining maximum must never be asked to + * move back (`CursorRepo.advance` refuses that). + */ maxSeq(): number { - const row = one<{ seq: number | null }>(this.db, 'SELECT MAX(seq) AS seq FROM event_log'); + const row = one<{ seq: number | null }>( + this.db, + "SELECT max(coalesce((SELECT seq FROM sqlite_sequence WHERE name = 'event_log'), 0), coalesce((SELECT MAX(seq) FROM event_log), 0)) AS seq", + ); return row?.seq ?? 0; } diff --git a/packages/store/src/repos/prs.ts b/packages/store/src/repos/prs.ts index c34fffd7..8a14e7f8 100644 --- a/packages/store/src/repos/prs.ts +++ b/packages/store/src/repos/prs.ts @@ -385,11 +385,6 @@ export class PrRepo { return row === null ? null : { key: row.key, pr: parsePr(row.json), fetchedAt: row.fetched_at }; } - /** The last snapshot revision handed out (the store-wide counter), 0 before the first. */ - latestRevision(): number { - return Number(one<{ value: string }>(this.db, 'SELECT value FROM meta WHERE key = ?', SNAPSHOT_REVISION_KEY)?.value ?? 0); - } - /** For the storage job checks_strip: the next stored snapshot's key after `afterKey` ('' for the first); null after the last. */ nextSnapshotKey(afterKey: PrKey): PrKey | null { return one<{ key: string }>(this.db, 'SELECT key FROM pr_snapshot WHERE key > ? ORDER BY key LIMIT 1', afterKey)?.key ?? null; @@ -405,19 +400,4 @@ export class PrRepo { run(this.db, "UPDATE pr_snapshot SET json = json_remove(json, '$.checks') WHERE key = ? AND json_type(json, '$.checks') IS NOT NULL", key) > 0 ); } - - /** - * Snapshots written after revision `since` (by a fetch or a local - * rewrite) whose json holds `checks`: the strip's check from the data for - * the PRs stored behind its cursor while it walked. - */ - countChecksWrittenSince(since: number): number { - return ( - one<{ n: number }>( - this.db, - "SELECT count(*) AS n FROM pr_snapshot WHERE key IN (SELECT key FROM pr WHERE snapshot_revision > ?) AND json_type(json, '$.checks') IS NOT NULL", - since, - )?.n ?? 0 - ); - } } From a2d1244dcbbeb2b89a4d48e67936c67d7c2016f1 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Mon, 5 Oct 2026 22:55:57 +0200 Subject: [PATCH 3/3] chore(devex): re-run CI CI sat queued behind the runner backlog; an empty commit starts it again.