Public: anyone can read this; the crew can change it.

Asks still read per pull once a pull has a check result #2789

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Home no longer reads per pull once a pull has a check result. With the seed below (twelve open pulls a round), `GET /` runs 92 statements for one round and 92 for four. Before this change it ran 108 and 207. The type check is clean, and the 42 test files covering the touched modules pass (526 tests, none failing).

shipped production is serving it deploy 652cd9c

opened this

Home no longer reads per pull once a pull has a check result. With the seed below (twelve open pulls a round), GET / runs 92 statements for one round and 92 for four. Before this change it ran 108 and 207. The type check is clean, and the 42 test files covering the touched modules pass (526 tests, none failing).

Built on #2780. test/asks-counts.test.ts, activeRunsFor and handoversOf come from #2780, which hasn't merged yet. This branch merges #2780's head (0ede4db9); the four commits after it are this change. Merge #2780 first.

Grouped helpers (ids in, Map out, one query per chunk), each beside its singular read:

  • core/reviews.ts: reviewSummariesFor and listReviewsFor. reviewSummary and the grouped helper now count verdicts through one shared function, summarize.
  • core/decisions.ts: latestDecisionsFor.
  • core/runs.ts: automaticRunsSinceFor, which takes a separate start time for each pull.
  • core/holds.ts: lastReleasesFor.
  • core/agents.ts: crewWithRole. It is used by the new reviewersForRepos (crew/review.ts), and reviewersOf now calls it too.
  • crew/guard.ts: the hand-over tail is now handoversSince. It reads approvals, people's words and the crew's rounds once for the whole set. handoverOf and handoversOf both go through it. Who may direct the crew is asked once per repository, for everyone who spoke there.
  • crew/review.ts: reviewAttemptsFor and reviewWaitsFor. reviewWaitsFor returns, per pull, the reviewer it waits for, the person it waits for, and its review history. waitsForPerson and waitsForReviewer are now one-pull calls of it, so there is still one rule. It accepts hand-overs the caller has already read, so asks doesn't read them twice.
  • web/studio/asks.ts: lastHumanWords replaces lastHumanWord, which had no other caller and is deleted. askCandidatesFor now reads tries, stops, last human words and review waits once, for every pull that could still become a card.

One fix outside the issue's list. consequencesOf read a person's pull's author once per pull. That kept home's count growing with green pulls opened by a person. It now reads the authors once with getUsersByIds and remembers canWrite per repository and author.

Tests (test/asks-counts.test.ts):

  • Each round now seeds pulls that are:

- red, with tries, a lifted stop and a person's word since; - stopped; - green: a person's and the crew's, assisted and autonomous; - handed over after three rounds.

  • The seed's crew accounts now get agent rows. Without them no repository had a reviewer, so the reviewer branch never ran.
  • Seeded pulls and comments are backdated by a minute or two. Otherwise the strict > comparisons could see two rows from the same millisecond and drop one; I saw this happen with the hand-over pull.
  • New tests:

- the stopped, look and hand-over cards render; - each grouped helper matches its singular read on every seeded pull.

  • One assertion narrowed: the "hand-overs once" filter matched every statement starting SELECT r.issue_id. With handed-over pulls seeded, that also caught the new rounds query, so it now matches only the hand-over count. The test now also checks that rounds, verdicts, tries and reviewers are each read once.

Not done:

  • isStalePull, staleGreen and mergesAgainSince get no grouped helper. They read an in-memory cache filled by the stale-ask sweep and run no SQL, so they cost home no statements.
  • Parked-issue asks still read per issue: getIssueById, latestDecision(…, "hand") and listComments. They are issues, not pulls, so they're outside this issue.

To try: bun test test/asks-counts.test.ts.

🤖 Generated with Claude Code

Closes #2781

commented

Home no longer reads per pull once a pull has a check result. With the seed below (twelve open pulls a round), GET / runs 92 statements for one round and 92 for four. Before this change it ran 108 and 207. The type check is clean, and the 42 test files covering the touched modules pass (526 tests, none failing).

Built on #2780. test/asks-counts.test.ts, activeRunsFor and handoversOf come from #2780, which hasn't merged yet. This branch merges #2780's head (0ede4db9); the four commits after it are this change. Merge #2780 first.

Grouped helpers (ids in, Map out, one query per chunk), each beside its singular read:

  • core/reviews.ts: reviewSummariesFor and listReviewsFor. reviewSummary and the grouped helper now count verdicts through one shared function, summarize.
  • core/decisions.ts: latestDecisionsFor.
  • core/runs.ts: automaticRunsSinceFor, which takes a separate start time for each pull.
  • core/holds.ts: lastReleasesFor.
  • core/agents.ts: crewWithRole. It is used by the new reviewersForRepos (crew/review.ts), and reviewersOf now calls it too.
  • crew/guard.ts: the hand-over tail is now handoversSince. It reads approvals, people's words and the crew's rounds once for the whole set. handoverOf and handoversOf both go through it. Who may direct the crew is asked once per repository, for everyone who spoke there.
  • crew/review.ts: reviewAttemptsFor and reviewWaitsFor. reviewWaitsFor returns, per pull, the reviewer it waits for, the person it waits for, and its review history. waitsForPerson and waitsForReviewer are now one-pull calls of it, so there is still one rule. It accepts hand-overs the caller has already read, so asks doesn't read them twice.
  • web/studio/asks.ts: lastHumanWords replaces lastHumanWord, which had no other caller and is deleted. askCandidatesFor now reads tries, stops, last human words and review waits once, for every pull that could still become a card.

One fix outside the issue's list. consequencesOf read a person's pull's author once per pull. That kept home's count growing with green pulls opened by a person. It now reads the authors once with getUsersByIds and remembers canWrite per repository and author.

Tests (test/asks-counts.test.ts):

  • Each round now seeds pulls that are:

- red, with tries, a lifted stop and a person's word since; - stopped; - green: a person's and the crew's, assisted and autonomous; - handed over after three rounds.

  • The seed's crew accounts now get agent rows. Without them no repository had a reviewer, so the reviewer branch never ran.
  • Seeded pulls and comments are backdated by a minute or two. Otherwise the strict > comparisons could see two rows from the same millisecond and drop one; I saw this happen with the hand-over pull.
  • New tests:

- the stopped, look and hand-over cards render; - each grouped helper matches its singular read on every seeded pull.

  • One assertion narrowed: the "hand-overs once" filter matched every statement starting SELECT r.issue_id. With handed-over pulls seeded, that also caught the new rounds query, so it now matches only the hand-over count. The test now also checks that rounds, verdicts, tries and reviewers are each read once.

Not done:

  • isStalePull, staleGreen and mergesAgainSince get no grouped helper. They read an in-memory cache filled by the stale-ask sweep and run no SQL, so they cost home no statements.
  • Parked-issue asks still read per issue: getIssueById, latestDecision(…, "hand") and listComments. They are issues, not pulls, so they're outside this issue.

To try: bun test test/asks-counts.test.ts.

🤖 Generated with Claude Code

commented

Home's asks now read every pull with a check result in grouped queries. With the seed below, GET / runs 92 statements for one round of twelve open pulls and 92 for four rounds; before this change it ran 108 and 207. The type check is clean, and the 42 test files covering the touched modules pass (526 tests, none failing).

Merge with main. #2780 has landed on main, squashed. On main, src/crew/guard.ts, src/web/studio/asks.ts and test/asks-counts.test.ts are identical to #2780's head, which is what this branch was built on. The conflicts were therefore resolved by taking this branch's side of all three, which already contains #2780.

Grouped helpers (ids in, Map out, one query per chunk), each beside its singular read:

  • core/reviews.ts: reviewSummariesFor and listReviewsFor. reviewSummary and the grouped helper count verdicts through one shared function, summarize.
  • core/decisions.ts: latestDecisionsFor.
  • core/runs.ts: automaticRunsSinceFor, which takes a separate start time for each pull.
  • core/holds.ts: lastReleasesFor.
  • core/agents.ts: crewWithRole. It backs the new reviewersForRepos (crew/review.ts), and reviewersOf uses it too.
  • crew/guard.ts: the hand-over tail is now handoversSince. It reads approvals, people's words and the crew's rounds once for the whole set. handoverOf and handoversOf both go through it. Who may direct the crew is asked once per repository, for everyone who spoke there.
  • crew/review.ts: reviewAttemptsFor and reviewWaitsFor. reviewWaitsFor returns, per pull, the reviewer it waits for, the person it waits for, and its review history. waitsForPerson and waitsForReviewer are now one-pull calls of it, so there is still one rule. It accepts hand-overs the caller has already read, so they aren't read twice.
  • web/studio/asks.ts: lastHumanWords replaces lastHumanWord, which had no other caller and is deleted. askCandidatesFor reads tries, stops, last human words and review waits once, for every pull that could still become a card.

One fix outside the issue's list. consequencesOf read a person's pull's author once per pull, which kept home growing with green pulls a person opened. It now reads the authors once with getUsersByIds and remembers canWrite per repository and author.

Tests (test/asks-counts.test.ts):

  • Each round now seeds pulls that are:

- red, with tries, a lifted stop and a person's word since; - stopped; - green: a person's and the crew's, assisted and autonomous; - handed over after three rounds.

  • The seed's crew accounts get agent rows. Without them no repository had a reviewer, so the reviewer branch never ran.
  • Seeded pulls and comments are backdated by a minute or two. Otherwise the strict > comparisons could drop a row created in the same millisecond, which happened to the hand-over pull.
  • New tests:

- the stopped, look and hand-over cards render; - each grouped helper matches its singular read on every seeded pull.

  • One assertion narrowed: the "hand-overs once" filter matched any statement starting SELECT r.issue_id. With handed-over pulls in the seed it also caught the new rounds query, so it now matches only the hand-over count. The test now also checks that rounds, verdicts, tries and reviewers are each read once.

Not done:

  • isStalePull, staleGreen and mergesAgainSince get no grouped helper: they read an in-memory cache filled by a sweep and run no SQL, so they cost home no statements.
  • Parked-issue asks still read per issue (getIssueById, latestDecision(…, "hand"), listComments). They are issues, not pulls, so they're outside this issue.

To try: bun test test/asks-counts.test.ts.

🤖 Generated with Claude Code

Approved

Read the diff against the list. Home now reads reviews, verdicts, tries, stops, releases, reviewers and hand-overs once per set, and each singular read (waitsForPerson, waitsForReviewer, reviewAttempts, handoverOf) goes through the same rule as its grouped version. The tests seed the check-result states and assert that each grouped read equals its singular read and runs once. No new dependency, no auth or output surface touched, no dead code. I did not open the preview or the screenshots, so I have not seen it work by eye.

Two things I would do next:

  1. reviewAttemptsFor filters the whole chunk of head runs for every pull (O(n²) within a chunk of 400). Bucket the rows by issue_id first, as the review runs are.
  2. The CHUNK = 400 constant is now declared in five modules. Share one exported constant so the SQLite variable bound lives in one place.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs