Repository navigation
feat: add a Summary tab and prs summary, counting a window's pull requests - #15
Conversation
There was a problem hiding this comment.
PR review — failure-paths
Used: Read the head of PR 15 and traced the SQLite store, the sync and the GitHub history reads through the Summary tab and prs summary. I built nothing and ran no injected failure. The traced outcomes rest on reading the code and on the PR's own tests, which I did not run.
Standards: I ran no fleet standards search and cited no rules. I judged against the three recurring code-quality rules from my instructions: evidence from the effect, partial results carrying a signal, and a failure costing only its own unit.
Findings: 0 correctness · 0 design · 0 prose
No findings.
Findings as JSON
{
"used": "Read the head of PR 15 and traced the SQLite store, the sync and the GitHub history reads through the Summary tab and `prs summary`. I built nothing and ran no injected failure. The traced outcomes rest on reading the code and on the PR's own tests, which I did not run.",
"standards": "I ran no `fleet standards search` and cited no rules. I judged against the three recurring code-quality rules from my instructions: evidence from the effect, partial results carrying a signal, and a failure costing only its own unit.",
"findings": []
}This review does not gate merge.
There was a problem hiding this comment.
PR review — system-design
Used: Cloned PR 15 at head. Read the diff, internal/summary (store.go, sync.go) and pullrequests/github_history.go. Traced Sync, Keep and the search query construction. Did not build or run it.
Standards: No rules cited. I ran no fleet standards search, because no finding rests on a standards rule.
Findings: 1 correctness · 1 design · 0 prose
Correctness
system-design.1 internal/summary/sync.go:38 · internal/pullrequests/github_history.go:163 · internal/summary/store.go:243
Reach: A pull request is merged or closed, and a sync caches it. Someone reopens it. Every later sync asks GitHub for is:closed updated:>=…. The reopened pull request is open now, so the search never returns it. No code deletes or revalidates the row, so the cache keeps its state and closed_at. The Summary tab counts it as merged or closed in the window until the window moves past its closed_at. If it later merges, the merge is correct, but the closed_at of the old close can double-count it only until the row is overwritten. The coverage claims 'each kept pull request is as it stood at Synced or later', which this row breaks.
The closed-history cache never drops a pull request that reopens.
The incremental sync only reads pull requests that are closed now. A pull request reopened after it was cached is never read again. Its row stays as merged or closed.
The cached closed_at also stays in range, so the Summary tab's closed-unmerged and merged counts, the time-to-merge percentiles and the per-repo table include a pull request that is open. The same pull request is also counted in the open-now figures, which come from the live open listing.
Fix. Make the sync reconcile reopened pull requests. For example, read is:open updated:>=… in the same sync and delete cached rows that match. Alternatively, re-read cached rows in the window by number with a GraphQL node query. Add Store.Keep support for deletes inside the same transaction. A deleted or transferred repository's rows need the same treatment.
Design and architecture
system-design.2 internal/summary/store.go:140 · internal/summary/store.go:245 · internal/summary/sync.go:21
Reach: Two processes can write the same SQLite file. They are a running board and a prs summary run, and the code comments name both. Each reads Coverage, then makes network calls that take seconds to minutes on a first read, then commits Keep. The coverage is a read-modify-write across that gap. Process A reads no coverage and starts a 90-day backfill. Process B starts later with a 7d window and also sees no coverage. B commits From=now-7d. A then commits From=now-90d, which is fine here. In the other order, B commits last and replaces From=now-90d with From=now-7d. The 90 days of rows stay, but the next 90d window re-reads the older stretch. The store also holds Synced at the moment each process began, so a late committer can move Synced backwards.
Concurrent syncs of one cache file can narrow the coverage.
Sync reads the coverage, makes network calls, then Keep overwrites covered_from and synced_at unconditionally. _txlock=immediate only serialises the final write. It does not serialise the read-modify-write.
The result is not wrong counts, because the upserts are guarded on updated_at. It is repeated full-window GitHub searches, and each search page carries review bodies. This costs search rate limit (30 requests a minute, secondary limits) on the shared token.
Fix. Make the coverage write monotonic inside Keep: covered_from = MIN(existing, new) and synced_at = MAX(existing, new). Alternatively, take a per-file lock across Sync.
Findings as JSON
{
"used": "Cloned PR 15 at head. Read the diff, internal/summary (store.go, sync.go) and pullrequests/github_history.go. Traced Sync, Keep and the search query construction. Did not build or run it.",
"standards": "No rules cited. I ran no `fleet standards search`, because no finding rests on a standards rule.",
"findings": [
{
"number": "system-design.1",
"section": "correctness",
"sites": [
"internal/summary/sync.go:38",
"internal/pullrequests/github_history.go:163",
"internal/summary/store.go:243"
],
"reach": "A pull request is merged or closed, and a sync caches it. Someone reopens it. Every later sync asks GitHub for `is:closed updated:>=…`. The reopened pull request is open now, so the search never returns it. No code deletes or revalidates the row, so the cache keeps its `state` and `closed_at`. The Summary tab counts it as merged or closed in the window until the window moves past its `closed_at`. If it later merges, the merge is correct, but the `closed_at` of the old close can double-count it only until the row is overwritten. The coverage claims 'each kept pull request is as it stood at Synced or later', which this row breaks.",
"body": "**The closed-history cache never drops a pull request that reopens.**\n\nThe incremental sync only reads pull requests that are closed now. A pull request reopened after it was cached is never read again. Its row stays as merged or closed.\n\nThe cached `closed_at` also stays in range, so the Summary tab's closed-unmerged and merged counts, the time-to-merge percentiles and the per-repo table include a pull request that is open. The same pull request is also counted in the open-now figures, which come from the live open listing.\n\n**Fix.** Make the sync reconcile reopened pull requests. For example, read `is:open updated:>=…` in the same sync and delete cached rows that match. Alternatively, re-read cached rows in the window by number with a GraphQL node query. Add `Store.Keep` support for deletes inside the same transaction. A deleted or transferred repository's rows need the same treatment."
},
{
"number": "system-design.2",
"section": "design",
"sites": [
"internal/summary/store.go:140",
"internal/summary/store.go:245",
"internal/summary/sync.go:21"
],
"reach": "Two processes can write the same SQLite file. They are a running board and a `prs summary` run, and the code comments name both. Each reads Coverage, then makes network calls that take seconds to minutes on a first read, then commits `Keep`. The coverage is a read-modify-write across that gap. Process A reads no coverage and starts a 90-day backfill. Process B starts later with a 7d window and also sees no coverage. B commits `From=now-7d`. A then commits `From=now-90d`, which is fine here. In the other order, B commits last and replaces `From=now-90d` with `From=now-7d`. The 90 days of rows stay, but the next 90d window re-reads the older stretch. The store also holds `Synced` at the moment each process began, so a late committer can move `Synced` backwards.",
"body": "**Concurrent syncs of one cache file can narrow the coverage.**\n\n`Sync` reads the coverage, makes network calls, then `Keep` overwrites `covered_from` and `synced_at` unconditionally. `_txlock=immediate` only serialises the final write. It does not serialise the read-modify-write.\n\nThe result is not wrong counts, because the upserts are guarded on `updated_at`. It is repeated full-window GitHub searches, and each search page carries review bodies. This costs search rate limit (30 requests a minute, secondary limits) on the shared token.\n\n**Fix.** Make the coverage write monotonic inside `Keep`: `covered_from = MIN(existing, new)` and `synced_at = MAX(existing, new)`. Alternatively, take a per-file lock across `Sync`."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — fleet-patterns
Used: I cloned prs and checked out PR 15 (head 85fe478). I read store.go, github_history.go, until.go, round_post.go and the config and go.mod diffs. I ran gh search code --owner datapointchris for the shared problems: GitHub's 1,000-result search cap, PR merge statistics, percentiles, modernc SQLite, busy_timeout and user_version. Code search ran out at its rate limit after nine queries, so the p90/median, median merge and XDG_CACHE_HOME sqlite queries returned 403. I shallow-cloned projects and ypl to compare their SQLite stores.
Standards: Rules found: data.md/A timestamp is TIMESTAMPTZ in Postgres and ISO 8601 UTC text in SQLite, which the store follows with RFC 3339 UTC text. No rule settles SQLite pragmas in the DSN ("sqlite pragma DSN busy_timeout WAL", "sqlite connection pragma per connection pool txlock immediate"). No rule settles splitting a search at the 1,000-result cap ("github search 1000 result cap split"). For emptying a cache when its schema changes ("cache schema change empties rather than migrate"), only migration rules came up, and none covers a cache.
Findings: 0 correctness · 0 design · 0 prose
No findings.
Findings as JSON
{
"used": "I cloned prs and checked out PR 15 (head 85fe478). I read store.go, github_history.go, until.go, round_post.go and the config and go.mod diffs. I ran `gh search code --owner datapointchris` for the shared problems: GitHub's 1,000-result search cap, PR merge statistics, percentiles, modernc SQLite, busy_timeout and user_version. Code search ran out at its rate limit after nine queries, so the p90/median, `median merge` and `XDG_CACHE_HOME sqlite` queries returned 403. I shallow-cloned projects and ypl to compare their SQLite stores.",
"standards": "Rules found: data.md/A timestamp is TIMESTAMPTZ in Postgres and ISO 8601 UTC text in SQLite, which the store follows with RFC 3339 UTC text. No rule settles SQLite pragmas in the DSN (\"sqlite pragma DSN busy_timeout WAL\", \"sqlite connection pragma per connection pool txlock immediate\"). No rule settles splitting a search at the 1,000-result cap (\"github search 1000 result cap split\"). For emptying a cache when its schema changes (\"cache schema change empties rather than migrate\"), only migration rules came up, and none covers a cache.",
"findings": []
}This review does not gate merge.
There was a problem hiding this comment.
PR review — stale-prose
Used: I cloned the repo and checked out PR 15 at 85fe478. I pulled out every added comment, doc line, help string and schema description, about 400 lines in all. I read each one beside the code it describes: Sync, ReadGitHubHistory, githubSearches, mergeBuckets, openSamples, SummaryWindows, historyAliasesPerRequest, connectionOptions, xdgPath and IsRoundPost.
Standards: I applied pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite, read with fleet standards show. I propose no new rule.
Findings: 0 correctness · 0 design · 3 prose
Prose
stale-prose.1 internal/summary/store.go:1 · internal/summary/store.go:3
False: the package doc says the store updates with one search. It reads: "Package summary counts … from the merged and closed pull requests it keeps in SQLite, which it brings up to date with one search of what changed since the last read."
The code at the head does not make one search:
Sync(sync.go:19-45) callsReadGitHubHistoryup to twice. It makes aClosedBetweenread when the window starts beforecoverage.From, then anUpdatedBetweenread.ReadGitHubHistory(github_history.go:143-147) queues one search pergithubSearchesentry. That function (github.go:129-153) makes one search perReposPerSearchchunk of repos plus one per owner.ReadGitHubHistorythen queues another search for each page, and two more for each split pastsearchCap.
The PR body itself names "five repo chunks" on the real account.
Rewrite: "…which it brings up to date by reading what changed since its last read, and back to a window's start where the store does not reach it." Or drop the clause and leave the update to Sync's doc.
stale-prose.2 README.md:300 · README.md:301 · internal/summary/numbers.go:99 · cmd/prs/summary.schema.json:96 · cmd/prs/summary.schema.json:67 · cmd/prs/summary.schema.json:81 · README.md:309 · internal/pullrequests/github_history.go:15 · internal/pullrequests/github_history.go:16
Values the code holds are written out in prose. Each is true today and goes wrong at the next change to the constant.
- README.md:300-301: "30 days at first, and
wandWstep it through 1d, 7d, 2w, 30d and 90d." This repeatsSummaryWindowsandDefaultSummaryWindow(summary_tab.go:25-27). Rewrite: "…counts the pull requests over a window, andwandWstep it shorter and longer. The strip lists the windows and lights the one counted." - numbers.go:99: "OpenOver is the open pull requests at about 120 instants from Since to Until". cmd/prs/summary.schema.json:96 says the same: "The open pull requests at about 120 instants from since". Both repeat
openSamples = 120. Rewrite numbers.go: "OpenOver is the open pull requests at about openSamples instants…". Rewrite the schema: "The open pull requests at evenly spaced instants from since, a whole number of hours apart, then at until." - cmd/prs/summary.schema.json:67: "…by how long each took from opening: under 10 minutes, an hour, 4 hours, a day, 3 days, a week, and longer." This repeats
mergeBuckets, and each item already carrieslabel,from_secondsandto_seconds. Rewrite: "The merges within the window by how long each took from opening, in buckets shortest first, each naming its own edges." - cmd/prs/summary.schema.json:81 ("an hour for a window of two days or less") and README.md:309 ("per hour for a window of two days or less") repeat
hourlyUpTo = 48 * time.Hour. Rewrite: "an hour for a short window, else a day". Or keep the README line and cut the schema one, which already listshouranddayin itsenum. - github_history.go:15-16: "…so five searches keep a request near 2.5 MB." This sits directly over
const historyAliasesPerRequest = 5. Rewrite: "historyAliasesPerRequest bounds one history request's searches, because each page carries a review body a pull request and the request grows with every alias."
stale-prose.3 internal/pullrequests/github_history.go:16 · internal/pullrequests/github_history.go:21 · internal/pullrequests/github_history.go:22 · internal/summary/sync.go:17 · internal/summary/sync.go:18 · README.md:315
These will go stale: measured or estimated numbers with nothing to re-check them. Each is a cost or a size observed on today's account and today's GitHub pricing. No command or test sits beside any of them.
- github_history.go:16: "a few kilobytes each, so five searches keep a request near 2.5 MB." This is a size estimate that review bodies and alias count both move.
- github_history.go:21-22: "One review per pull request keeps the request a point or so a page, where its reviews whole would carry every reviewer's body." This is GitHub's rate-limit pricing, which nothing here pins.
- sync.go:17-18: "Then it reads every closed pull request updated since the last sync, which after the first is a page or so." This depends on merge rate, chunk count and refresh interval.
- README.md:315: "The first read of a window reads every pull request closed within it, a few search pages for a month." This depends on how much the owner merges. The PR body measured 509 merges in 30 days, about six pages per chunk.
Rewrites:
- github_history.go:15-16: drop the number. See the rewrite in the values finding.
- github_history.go:21-22: "One review per pull request is enough to say whether a round reviewed it, and reading every review would carry every reviewer's body."
- sync.go:17-18: "Then it reads every closed pull request updated since the last sync. Nothing is kept unless every search answered."
- README.md:315: "The first read of a window reads every pull request closed within it. Each read after asks only for those updated since the last…"
Findings as JSON
{
"used": "I cloned the repo and checked out PR 15 at 85fe478. I pulled out every added comment, doc line, help string and schema description, about 400 lines in all. I read each one beside the code it describes: `Sync`, `ReadGitHubHistory`, `githubSearches`, `mergeBuckets`, `openSamples`, `SummaryWindows`, `historyAliasesPerRequest`, `connectionOptions`, `xdgPath` and `IsRoundPost`.",
"standards": "I applied pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite, read with `fleet standards show`. I propose no new rule.",
"findings": [
{
"number": "stale-prose.1",
"section": "prose",
"sites": [
"internal/summary/store.go:1",
"internal/summary/store.go:3"
],
"reach": "",
"body": "**False: the package doc says the store updates with one search.** It reads: \"Package summary counts … from the merged and closed pull requests it keeps in SQLite, which it brings up to date with one search of what changed since the last read.\"\n\nThe code at the head does not make one search:\n- `Sync` (sync.go:19-45) calls `ReadGitHubHistory` up to twice. It makes a `ClosedBetween` read when the window starts before `coverage.From`, then an `UpdatedBetween` read.\n- `ReadGitHubHistory` (github_history.go:143-147) queues one search per `githubSearches` entry. That function (github.go:129-153) makes one search per `ReposPerSearch` chunk of repos plus one per owner.\n- `ReadGitHubHistory` then queues another search for each page, and two more for each split past `searchCap`.\n\nThe PR body itself names \"five repo chunks\" on the real account.\n\nRewrite: \"…which it brings up to date by reading what changed since its last read, and back to a window's start where the store does not reach it.\" Or drop the clause and leave the update to `Sync`'s doc."
},
{
"number": "stale-prose.2",
"section": "prose",
"sites": [
"README.md:300",
"README.md:301",
"internal/summary/numbers.go:99",
"cmd/prs/summary.schema.json:96",
"cmd/prs/summary.schema.json:67",
"cmd/prs/summary.schema.json:81",
"README.md:309",
"internal/pullrequests/github_history.go:15",
"internal/pullrequests/github_history.go:16"
],
"reach": "",
"body": "**Values the code holds are written out in prose.** Each is true today and goes wrong at the next change to the constant.\n\n- README.md:300-301: \"30 days at first, and `w` and `W` step it through 1d, 7d, 2w, 30d and 90d.\" This repeats `SummaryWindows` and `DefaultSummaryWindow` (summary_tab.go:25-27). Rewrite: \"…counts the pull requests over a window, and `w` and `W` step it shorter and longer. The strip lists the windows and lights the one counted.\"\n- numbers.go:99: \"OpenOver is the open pull requests at about 120 instants from Since to Until\". cmd/prs/summary.schema.json:96 says the same: \"The open pull requests at about 120 instants from since\". Both repeat `openSamples = 120`. Rewrite numbers.go: \"OpenOver is the open pull requests at about openSamples instants…\". Rewrite the schema: \"The open pull requests at evenly spaced instants from since, a whole number of hours apart, then at until.\"\n- cmd/prs/summary.schema.json:67: \"…by how long each took from opening: under 10 minutes, an hour, 4 hours, a day, 3 days, a week, and longer.\" This repeats `mergeBuckets`, and each item already carries `label`, `from_seconds` and `to_seconds`. Rewrite: \"The merges within the window by how long each took from opening, in buckets shortest first, each naming its own edges.\"\n- cmd/prs/summary.schema.json:81 (\"an hour for a window of two days or less\") and README.md:309 (\"per hour for a window of two days or less\") repeat `hourlyUpTo = 48 * time.Hour`. Rewrite: \"an hour for a short window, else a day\". Or keep the README line and cut the schema one, which already lists `hour` and `day` in its `enum`.\n- github_history.go:15-16: \"…so five searches keep a request near 2.5 MB.\" This sits directly over `const historyAliasesPerRequest = 5`. Rewrite: \"historyAliasesPerRequest bounds one history request's searches, because each page carries a review body a pull request and the request grows with every alias.\""
},
{
"number": "stale-prose.3",
"section": "prose",
"sites": [
"internal/pullrequests/github_history.go:16",
"internal/pullrequests/github_history.go:21",
"internal/pullrequests/github_history.go:22",
"internal/summary/sync.go:17",
"internal/summary/sync.go:18",
"README.md:315"
],
"reach": "",
"body": "**These will go stale: measured or estimated numbers with nothing to re-check them.** Each is a cost or a size observed on today's account and today's GitHub pricing. No command or test sits beside any of them.\n\n- github_history.go:16: \"a few kilobytes each, so five searches keep a request near 2.5 MB.\" This is a size estimate that review bodies and alias count both move.\n- github_history.go:21-22: \"One review per pull request keeps the request a point or so a page, where its reviews whole would carry every reviewer's body.\" This is GitHub's rate-limit pricing, which nothing here pins.\n- sync.go:17-18: \"Then it reads every closed pull request updated since the last sync, which after the first is a page or so.\" This depends on merge rate, chunk count and refresh interval.\n- README.md:315: \"The first read of a window reads every pull request closed within it, a few search pages for a month.\" This depends on how much the owner merges. The PR body measured 509 merges in 30 days, about six pages per chunk.\n\nRewrites:\n- github_history.go:15-16: drop the number. See the rewrite in the values finding.\n- github_history.go:21-22: \"One review per pull request is enough to say whether a round reviewed it, and reading every review would carry every reviewer's body.\"\n- sync.go:17-18: \"Then it reads every closed pull request updated since the last sync. Nothing is kept unless every search answered.\"\n- README.md:315: \"The first read of a window reads every pull request closed within it. Each read after asks only for those updated since the last…\""
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — useless-test
Used: I cloned the repo, checked out PR 15 at 85fe478 and read both new tests beside summary_tab.go, summary_charts.go, sync.go and tui.go. I also looked through tui_test.go and the board tests for anything covering the changed code. I built the test binary, but neither test could run: an absolute TMPDIR under my directory makes the stand-in's unix socket path too long to bind, and a relative TMPDIR fails the config's absolute-path check.
Standards: I cited testing.md § "Test behavior the user cares about, not what this commit happened to change", pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change" and testing.md § "Never assert on rendered output — assert the value it was built from", including its interpolated-value exception and its rule that output another program parses is a contract.
Findings: 0 correctness · 0 design · 0 prose
No findings.
Findings as JSON
{
"used": "I cloned the repo, checked out PR 15 at 85fe478 and read both new tests beside summary_tab.go, summary_charts.go, sync.go and tui.go. I also looked through tui_test.go and the board tests for anything covering the changed code. I built the test binary, but neither test could run: an absolute TMPDIR under my directory makes the stand-in's unix socket path too long to bind, and a relative TMPDIR fails the config's absolute-path check.",
"standards": "I cited testing.md § \"Test behavior the user cares about, not what this commit happened to change\", pull-requests.md § \"Tests the diff adds are checked for whether they test the program or the change\" and testing.md § \"Never assert on rendered output — assert the value it was built from\", including its interpolated-value exception and its rule that output another program parses is a contract.",
"findings": []
}This review does not gate merge.
There was a problem hiding this comment.
PR review — contracts
Used: Built the PR head into ../bin/prs. Ran prs summary --help and prs tui --help, then three commands against a throwaway HOME, XDG dirs, registry and a dead GH_HOST: prs tui --view summary --since 7d --json, prs summary --since 7d --json and prs tui --view summary --once --since 7d. Searched the owner's repos with gh search code for closed_refresh_every_seconds and prs tui --view. Neither search returned a hit.
Standards: cli-design.md § "A flag a command accepts is read or refused by every branch, never accepted with no effect"; cli-design.md § "One fact gets one answer on every command that reaches it: the same flag meaning, error, exit code and validation"; no rule turned up for a config key given a second job (searched "config key reused for a second purpose").
Findings: 0 correctness · 2 design · 0 prose
Design and architecture
contracts.1 cmd/prs/tui.go:156 · cmd/prs/tui.go:177 · cmd/prs/tui.go:210 · cmd/prs/summary.go:95
Reach: Any script or agent that tries prs tui --view summary --since …. The README sends readers to that command as an equivalent of prs summary. Ran prs tui --view summary --since 7d --json: it printed "window": "30d", with since 30 days back. prs summary --since 7d --json printed "window": "7d". The --once frame also drew Oct 9 15:45 to now. No existing caller was found by gh search code --owner datapointchris "prs tui --view".
prs tui --view summary accepts --since and drops it. The summary branch always uses board.DefaultSummaryWindow. --since is parsed and validated on that branch too, then it only feeds the board's recently-closed read, which the branch never prints. The exit code reflects only whether the reads succeeded.
The flag's name now means two things across commands that print the same JSON:
- On
prs tui,--sinceis how far back "recently closed" holds. - On
prs summary, it is where the counting window starts.
The new --view summary route makes both meanings meet in a single command line. The help admits the result is "for 30d", but the flag is still accepted silently. This breaks cli-design.md § "A flag a command accepts is read or refused by every branch, never accepted with no effect". It also breaks § "One fact gets one answer on every command that reaches it".
Fix: when --view summary runs with --once or --json and cmd.Flags().Changed("since") is true, refuse with exit 2. The error should say: --since on prs tui is the recently closed group's; prs summary --since sets the Summary window. Apply the same refusal to --pr, which that branch does not read either. Do not make it honor --since, because then the flag would mean two things in one command.
contracts.2 internal/board/summary_tab.go:81 · cmd/prs/summary.go:107 · cmd/prs/config.go:53 · internal/config/config.go:53 · cmd/prs/tui.go:63 · cmd/prs/config.go:114
Reach: A person who tunes closed_refresh_every_seconds in config.yml, reading prs config path --help or prs config show. Each still says the key paces only the recently-closed read. OpenSummary(dir, cfg.ClosedRefreshEvery, window) now also uses it to pace the github history sync. The README and the PR body say so, but no help text does. gh search code found no config file in the owner's repos that sets the key. The cost comes with the next change: a user who wants the two reads paced differently cannot do it without renaming the key.
The existing config key closed_refresh_every_seconds has taken on a second job, and its docs still describe only the first.
These places still say only the recently-closed read:
- The
prs config pathhelp, atcmd/prs/config.go:53: "closed_refresh_every_seconds is the same for the recently closed". - The struct comment at
internal/config/config.go:53. prs tui --help, atcmd/prs/tui.go:63.- The
closed every N secondsline inprs config show.
The key now also sets how often the Summary tab's SQLite history runs its updated:>= sync. The two reads spend GraphQL points independently, so the key's documented cost ("costs GitHub as much again") understates what one setting now spends.
Fix: pick one.
- Give the history its own key, such as
history_refresh_every_seconds, defaulting to the same value. It gets its own line inconfig showand its ownFromlayer. - Or keep one key and update all four places to say it paces both the recently-closed group and the Summary tab's closed history.
Findings as JSON
{
"used": "Built the PR head into `../bin/prs`. Ran `prs summary --help` and `prs tui --help`, then three commands against a throwaway HOME, XDG dirs, registry and a dead GH_HOST: `prs tui --view summary --since 7d --json`, `prs summary --since 7d --json` and `prs tui --view summary --once --since 7d`. Searched the owner's repos with `gh search code` for `closed_refresh_every_seconds` and `prs tui --view`. Neither search returned a hit.",
"standards": "cli-design.md § \"A flag a command accepts is read or refused by every branch, never accepted with no effect\"; cli-design.md § \"One fact gets one answer on every command that reaches it: the same flag meaning, error, exit code and validation\"; no rule turned up for a config key given a second job (searched \"config key reused for a second purpose\").",
"findings": [
{
"number": "contracts.1",
"section": "design",
"sites": [
"cmd/prs/tui.go:156",
"cmd/prs/tui.go:177",
"cmd/prs/tui.go:210",
"cmd/prs/summary.go:95"
],
"reach": "Any script or agent that tries `prs tui --view summary --since …`. The README sends readers to that command as an equivalent of `prs summary`. Ran `prs tui --view summary --since 7d --json`: it printed `\"window\": \"30d\"`, with `since` 30 days back. `prs summary --since 7d --json` printed `\"window\": \"7d\"`. The `--once` frame also drew `Oct 9 15:45 to now`. No existing caller was found by `gh search code --owner datapointchris \"prs tui --view\"`.",
"body": "**`prs tui --view summary` accepts `--since` and drops it.** The summary branch always uses `board.DefaultSummaryWindow`. `--since` is parsed and validated on that branch too, then it only feeds the board's recently-closed read, which the branch never prints. The exit code reflects only whether the reads succeeded.\n\nThe flag's name now means two things across commands that print the same JSON:\n- On `prs tui`, `--since` is how far back \"recently closed\" holds.\n- On `prs summary`, it is where the counting window starts.\n\nThe new `--view summary` route makes both meanings meet in a single command line. The help admits the result is \"for 30d\", but the flag is still accepted silently. This breaks cli-design.md § \"A flag a command accepts is read or refused by every branch, never accepted with no effect\". It also breaks § \"One fact gets one answer on every command that reaches it\".\n\n**Fix:** when `--view summary` runs with `--once` or `--json` and `cmd.Flags().Changed(\"since\")` is true, refuse with exit 2. The error should say: `--since on prs tui is the recently closed group's; prs summary --since sets the Summary window`. Apply the same refusal to `--pr`, which that branch does not read either. Do not make it honor `--since`, because then the flag would mean two things in one command."
},
{
"number": "contracts.2",
"section": "design",
"sites": [
"internal/board/summary_tab.go:81",
"cmd/prs/summary.go:107",
"cmd/prs/config.go:53",
"internal/config/config.go:53",
"cmd/prs/tui.go:63",
"cmd/prs/config.go:114"
],
"reach": "A person who tunes `closed_refresh_every_seconds` in config.yml, reading `prs config path --help` or `prs config show`. Each still says the key paces only the recently-closed read. `OpenSummary(dir, cfg.ClosedRefreshEvery, window)` now also uses it to pace the `github history` sync. The README and the PR body say so, but no help text does. `gh search code` found no config file in the owner's repos that sets the key. The cost comes with the next change: a user who wants the two reads paced differently cannot do it without renaming the key.",
"body": "**The existing config key `closed_refresh_every_seconds` has taken on a second job, and its docs still describe only the first.**\n\nThese places still say only the recently-closed read:\n- The `prs config path` help, at `cmd/prs/config.go:53`: \"closed_refresh_every_seconds is the same for the recently closed\".\n- The struct comment at `internal/config/config.go:53`.\n- `prs tui --help`, at `cmd/prs/tui.go:63`.\n- The `closed every N seconds` line in `prs config show`.\n\nThe key now also sets how often the Summary tab's SQLite history runs its `updated:>=` sync. The two reads spend GraphQL points independently, so the key's documented cost (\"costs GitHub as much again\") understates what one setting now spends.\n\n**Fix:** pick one.\n- Give the history its own key, such as `history_refresh_every_seconds`, defaulting to the same value. It gets its own line in `config show` and its own `From` layer.\n- Or keep one key and update all four places to say it paces both the recently-closed group and the Summary tab's closed history."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — code-architecture
Used: I cloned the repo at PR 15's head (85fe478) and read every changed file against the existing code: board, listing_cache, github.go, sections, rows and tui.go. I ran go build ./... and go test ./internal/... ./cmd/prs/ -run Summary, and both passed. I listed each internal package's imports to check which way the dependencies point, and no lower layer reaches a higher one.
Standards: I found three rules through fleet standards search. They are code-quality.md § "Ask whatever owns a fact, the function or enum that computes it, never duplicate its logic at a second site", code-quality.md § "A derived cache is keyed by the content and base it was built from, and a read of an unfinished source is never kept as final" and tui.md § "A tool with a CLI opens its TUI as tui, one program with a tab per view". The search "bot predicate helper" returned no rule on point.
Findings: 0 correctness · 4 design · 0 prose
Design and architecture
code-architecture.1 internal/pullrequests/github_history.go:24 · internal/pullrequests/github_history.go:38 · internal/pullrequests/github_history.go:76 · internal/pullrequests/github_history.go:216 · internal/summary/store.go:26 · internal/summary/store.go:28 · internal/summary/store.go:75 · internal/summary/sync.go:48 · internal/summary/store.go:192 · internal/summary/store.go:234 · internal/board/listing_cache.go:56
Reach: The next field the summary counts edits ten sites in step. Adding a count needs a field per pull request: additions, base branch, draft, or a second review for a 'reviewed at all' share. The PR body already prices one such addition, reviews(first: 1), which shows these fields come one at a time. Missing the hand bump of schemaVersion leaves old files open with the old columns, and every Keep then fails until someone deletes the cache.
A merged pull request's shape is written out ten times, and the cache's version is a hand-kept number.
One fact, which fields a merged or closed pull request carries, is spelled at each of these places:
- the GraphQL fragment
HistoryRow githubHistoryNodepullrequests.ClosedPullRequestclosedPullRequest()summary.Closed, which copiesClosedPullRequestfield by field exceptFirstReview→ReviewRoundclosedOf()- the
CREATE TABLEtext Keep's column list, upsertSETlist andExecargument orderStore.Closed'sSELECTlist andScanorderschemaVersion
The repo already has a rule for a cache's version. listing_cache.go:56 derives listingCacheShape by reflecting over the type it stores, so a changed field changes the key without anyone remembering. The new store does not follow that rule. schemaVersion = 1 is a literal that only a person's memory ties to the schema text. That is the case code-quality.md § "Ask whatever owns a fact…" names: the cache's version is decided at a second site, apart from the shape it versions.
Fix: take the version from the shape it names. For example, set schemaVersion from a digest of the schema text, as the listing cache does with its type, and compare it with a stored value rather than with user_version. Also drop summary.Closed in favour of pullrequests.ClosedPullRequest with a ReviewRound field filled at read time. That removes one struct and its field-by-field copy.
code-architecture.2 internal/summary/store.go:101 · internal/board/listing_cache.go:147 · internal/board/listing_cache.go:119 · internal/pullrequests/github.go:129
Reach: The next change to what identifies a scope edits three sites in step. That could be a new Scope field, such as the GitHub host for an Enterprise token or excluded repos, or a change to how slugs are normalised. If the store's key is missed, the store keeps serving rows read for a different set of searches, which is the wrong-answer case code-quality.md § "A derived cache is keyed by the content…" measures. This PR is already the second cache keyed this way: the PR body says the store keys its files "as the listing cache keys its files".
Three sites decide a scope's normal form: lower-case, sort and de-duplicate the repos and owners.
githubSearchesbuilds that form to decide what GitHub is asked for.keptScopeOfplus the digest inopenListingCachebuilds it to name the listing cache's file.summary.FileForre-implements it line for line to name the SQLite file, then digests a different JSON object ({"repos":…,"owners":…}with no provider key).
The two cache keys are meant to name "the set of repos the board reads". They are built from the same inputs by separate code, and they already produce different digests for the same scope.
Fix: move the normal form onto pullrequests.Scope. A Scope.Canonical() returning sorted, lower-cased, de-duplicated repos and owners, and a Scope.Key() digesting it, would serve all three sites. githubSearches, keptScopeOf and FileFor then call it, and a new field is added once.
code-architecture.3 cmd/prs/tui.go:123 · cmd/prs/tui.go:124 · cmd/prs/tui.go:166 · cmd/prs/tui.go:168 · cmd/prs/tui.go:177 · cmd/prs/tui.go:208 · cmd/prs/tui.go:219
Reach: The next tab edits six places in tui.go. This PR is the change from one tab to two, which shows more tabs are coming, and tui.md § "…one program with a tab per view" expects a tab per view. The validation and the help text can drift out of step with NewTabs without any test failing.
The list of tabs is decided six times in tui.go.
Adding the Summary tab meant editing each of these by hand:
- the
--viewcheck at :123 - its error message at :124
- the flag's help at :208
- the
NewTabslist at :168 - a separate
if view == board.SummaryViewbranch at :177 for--once/--json printSummaryTabat :219, which copies the board's print, exit-status and fault path for one tab
gotermboard.NewTabs already rejects an unknown open name and lists the valid names (tabs.go:46-50). Even so, the command checks the name itself and spells the list twice more.
Fix: build one []tab{name, board, print} table in tui.go. Derive the --view validation and help from its names, or let NewTabs refuse the name. Pick the --once/--json printer from the same table. Each entry would expose Snapshot/Faults through one small interface, so the board's inline fault assembly and printSummaryTab become one path.
code-architecture.4 internal/board/rows.go:406 · internal/summary/numbers.go:146 · internal/summary/numbers.go:176 · internal/summary/numbers.go:234 · internal/summary/numbers.go:239 · internal/pullrequests/github.go:278 · internal/pullrequests/github_history.go:223
Reach: Any refinement of what counts as a bot touches every site and splits the board from the summary. Examples are counting dependabot[bot] from its login, or an allow-list of apps that count as people. board and summary both split people from apps, so both must change together. Nothing in the repo makes them agree.
Two predicates for "an app opened it" each have more than one owner.
- Whether a GitHub author is an app (
Author.Typename == "Bot") is decided again inclosedPullRequest, copyinggithubPullRequest. - The rule that a missing author counts as a person's is owned by
board.isBot.summary.Countwritesx.Bot != nil && *x.Botinline four times instead of calling it.
This is code-quality.md § "Ask whatever owns a fact…".
Fix: add one pullrequests.authorIsBot(typename) used by both node mappers. Add a PullRequest.IsBot() and a ClosedPullRequest.IsBot() in pullrequests, or one helper over *bool. Call it from board/rows.go, board/sections.go and the four numbers.go sites.
Findings as JSON
{
"used": "I cloned the repo at PR 15's head (85fe478) and read every changed file against the existing code: `board`, `listing_cache`, `github.go`, `sections`, `rows` and `tui.go`. I ran `go build ./...` and `go test ./internal/... ./cmd/prs/ -run Summary`, and both passed. I listed each internal package's imports to check which way the dependencies point, and no lower layer reaches a higher one.",
"standards": "I found three rules through `fleet standards search`. They are `code-quality.md` § \"Ask whatever owns a fact, the function or enum that computes it, never duplicate its logic at a second site\", `code-quality.md` § \"A derived cache is keyed by the content and base it was built from, and a read of an unfinished source is never kept as final\" and `tui.md` § \"A tool with a CLI opens its TUI as <tool> tui, one program with a tab per view\". The search \"bot predicate helper\" returned no rule on point.",
"findings": [
{
"number": "code-architecture.1",
"section": "design",
"sites": [
"internal/pullrequests/github_history.go:24",
"internal/pullrequests/github_history.go:38",
"internal/pullrequests/github_history.go:76",
"internal/pullrequests/github_history.go:216",
"internal/summary/store.go:26",
"internal/summary/store.go:28",
"internal/summary/store.go:75",
"internal/summary/sync.go:48",
"internal/summary/store.go:192",
"internal/summary/store.go:234",
"internal/board/listing_cache.go:56"
],
"reach": "The next field the summary counts edits ten sites in step. Adding a count needs a field per pull request: additions, base branch, draft, or a second review for a 'reviewed at all' share. The PR body already prices one such addition, `reviews(first: 1)`, which shows these fields come one at a time. Missing the hand bump of `schemaVersion` leaves old files open with the old columns, and every `Keep` then fails until someone deletes the cache.",
"body": "**A merged pull request's shape is written out ten times, and the cache's version is a hand-kept number.**\n\nOne fact, which fields a merged or closed pull request carries, is spelled at each of these places:\n\n- the GraphQL fragment `HistoryRow`\n- `githubHistoryNode`\n- `pullrequests.ClosedPullRequest`\n- `closedPullRequest()`\n- `summary.Closed`, which copies `ClosedPullRequest` field by field except `FirstReview` → `ReviewRound`\n- `closedOf()`\n- the `CREATE TABLE` text\n- `Keep`'s column list, upsert `SET` list and `Exec` argument order\n- `Store.Closed`'s `SELECT` list and `Scan` order\n- `schemaVersion`\n\nThe repo already has a rule for a cache's version. `listing_cache.go:56` derives `listingCacheShape` by reflecting over the type it stores, so a changed field changes the key without anyone remembering. The new store does not follow that rule. `schemaVersion = 1` is a literal that only a person's memory ties to the `schema` text. That is the case `code-quality.md` § \"Ask whatever owns a fact…\" names: the cache's version is decided at a second site, apart from the shape it versions.\n\n**Fix:** take the version from the shape it names. For example, set `schemaVersion` from a digest of the `schema` text, as the listing cache does with its type, and compare it with a stored value rather than with `user_version`. Also drop `summary.Closed` in favour of `pullrequests.ClosedPullRequest` with a `ReviewRound` field filled at read time. That removes one struct and its field-by-field copy."
},
{
"number": "code-architecture.2",
"section": "design",
"sites": [
"internal/summary/store.go:101",
"internal/board/listing_cache.go:147",
"internal/board/listing_cache.go:119",
"internal/pullrequests/github.go:129"
],
"reach": "The next change to what identifies a scope edits three sites in step. That could be a new `Scope` field, such as the GitHub host for an Enterprise token or excluded repos, or a change to how slugs are normalised. If the store's key is missed, the store keeps serving rows read for a different set of searches, which is the wrong-answer case `code-quality.md` § \"A derived cache is keyed by the content…\" measures. This PR is already the second cache keyed this way: the PR body says the store keys its files \"as the listing cache keys its files\".",
"body": "**Three sites decide a scope's normal form: lower-case, sort and de-duplicate the repos and owners.**\n\n- `githubSearches` builds that form to decide what GitHub is asked for.\n- `keptScopeOf` plus the digest in `openListingCache` builds it to name the listing cache's file.\n- `summary.FileFor` re-implements it line for line to name the SQLite file, then digests a different JSON object (`{\"repos\":…,\"owners\":…}` with no provider key).\n\nThe two cache keys are meant to name \"the set of repos the board reads\". They are built from the same inputs by separate code, and they already produce different digests for the same scope.\n\n**Fix:** move the normal form onto `pullrequests.Scope`. A `Scope.Canonical()` returning sorted, lower-cased, de-duplicated repos and owners, and a `Scope.Key()` digesting it, would serve all three sites. `githubSearches`, `keptScopeOf` and `FileFor` then call it, and a new field is added once."
},
{
"number": "code-architecture.3",
"section": "design",
"sites": [
"cmd/prs/tui.go:123",
"cmd/prs/tui.go:124",
"cmd/prs/tui.go:166",
"cmd/prs/tui.go:168",
"cmd/prs/tui.go:177",
"cmd/prs/tui.go:208",
"cmd/prs/tui.go:219"
],
"reach": "The next tab edits six places in `tui.go`. This PR is the change from one tab to two, which shows more tabs are coming, and `tui.md` § \"…one program with a tab per view\" expects a tab per view. The validation and the help text can drift out of step with `NewTabs` without any test failing.",
"body": "**The list of tabs is decided six times in `tui.go`.**\n\nAdding the Summary tab meant editing each of these by hand:\n\n- the `--view` check at :123\n- its error message at :124\n- the flag's help at :208\n- the `NewTabs` list at :168\n- a separate `if view == board.SummaryView` branch at :177 for `--once`/`--json`\n- `printSummaryTab` at :219, which copies the board's print, exit-status and fault path for one tab\n\n`gotermboard.NewTabs` already rejects an unknown `open` name and lists the valid names (tabs.go:46-50). Even so, the command checks the name itself and spells the list twice more.\n\n**Fix:** build one `[]tab{name, board, print}` table in `tui.go`. Derive the `--view` validation and help from its names, or let `NewTabs` refuse the name. Pick the `--once`/`--json` printer from the same table. Each entry would expose `Snapshot`/`Faults` through one small interface, so the board's inline fault assembly and `printSummaryTab` become one path."
},
{
"number": "code-architecture.4",
"section": "design",
"sites": [
"internal/board/rows.go:406",
"internal/summary/numbers.go:146",
"internal/summary/numbers.go:176",
"internal/summary/numbers.go:234",
"internal/summary/numbers.go:239",
"internal/pullrequests/github.go:278",
"internal/pullrequests/github_history.go:223"
],
"reach": "Any refinement of what counts as a bot touches every site and splits the board from the summary. Examples are counting `dependabot[bot]` from its login, or an allow-list of apps that count as people. `board` and `summary` both split people from apps, so both must change together. Nothing in the repo makes them agree.",
"body": "**Two predicates for \"an app opened it\" each have more than one owner.**\n\n- Whether a GitHub author is an app (`Author.Typename == \"Bot\"`) is decided again in `closedPullRequest`, copying `githubPullRequest`.\n- The rule that a missing author counts as a person's is owned by `board.isBot`. `summary.Count` writes `x.Bot != nil && *x.Bot` inline four times instead of calling it.\n\nThis is `code-quality.md` § \"Ask whatever owns a fact…\".\n\n**Fix:** add one `pullrequests.authorIsBot(typename)` used by both node mappers. Add a `PullRequest.IsBot()` and a `ClosedPullRequest.IsBot()` in `pullrequests`, or one helper over `*bool`. Call it from `board/rows.go`, `board/sections.go` and the four `numbers.go` sites."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — G1
Used: I built prs from the PR head. I ran prs summary for 7d, 30d and 90d, prs summary --since 2026-10-01 --until 2026-10-31 with and without --json, and prs tui --view summary --once, all against real GitHub (owners: datapointchris), with config and cache pointed inside my directory. I checked each count against direct GraphQL searches and against the SQLite store's rows, and ran go test ./..., which passed.
Standards: No cited rule reaches these findings. I read user-experience.md § "A trend chart sits on the thing it describes, one subject to a chart on its own labeled axis, under the numbers that summarize it" and data.md § "A new source of data gets its row in the data catalog in the change that creates it" (a file on a box takes no row). I also searched "github search qualifier misses closed pull requests", "label names what the number counts" and "fault sized against real".
Findings: 3 correctness · 0 design · 0 prose
Correctness
G1.1 internal/summary/sync.go:27 · internal/summary/sync.go:33 · internal/pullrequests/github_history.go:100 · internal/pullrequests/github.go:160
Reach: Two flows reach it: any first prs summary or Summary tab start on a fresh cache file, and any w step past the cache's start (e.g. to 90d). Both go through Sync's closed: read. It happened on the real datapointchris account on every window I ran.
The first read misses closed-unmerged pull requests, because GitHub's closed: qualifier leaves some out and updated: does not.
I ran prs summary --since 90d --json on a fresh cache. The store held 874 rows. The search user:datapointchris is:pr is:closed updated:>=2026-08-10T15:46:02Z counts 881. The 7 missing are all CLOSED unmerged:
- dotfiles#88
- dotfiles#55
- terminal-library-fleet#14
- terminal-library-fleet#16
- terminal-library-fleet#17
- reddit-scraper#15
- theme#21
For dotfiles#88 (closedAt 2026-10-19T03:53:53Z), closed:2026-10-19T03:00:00Z..2026-10-19T05:00:00Z returns 0. is:closed updated:2026-10-19 returns it.
What the user sees:
- 90d: closed unmerged reads 29 against 36 real. Opened reads 886, and
created:>=counts 893. - 30d: closed unmerged reads 17, and 18 closed in the window (dotfiles#88 missing).
--since 2026-10-01 --until 2026-10-31: prints 15, and GitHub has 16.
The open sparkline loses these PRs as well. Merged counts matched exactly (196, 512, 845, 360).
The gap is permanent. Later syncs only ask updated:>= the last sync, so a PR closed before the first read never comes back unless someone touches it.
Fix: read every stretch with UpdatedBetween. Use updated:>=since for the first read, and the same for a read back past coverage.From. A closed PR's updatedAt is never before its closedAt, so this is a superset, and Store.Closed/Count already filter by closed_at. On this account it costs 881 rows against 874. Then ClosedBetween can go.
The board's recently-closed group has the same shape at github.go:160 (is:closed closed:>=). It predates this PR, so fixing it here is optional. The help and README sentence "reads every pull request closed within the window" would change with the fix.
G1.2 internal/summary/numbers.go:127 · internal/summary/numbers.go:141 · internal/pullrequests/github_history.go:145
Reach: unproven. It needs a PR that was closed during a sync and then reopened. None of the 15 PRs open on datapointchris now has a ReopenedEvent. Reopening is a stock GitHub action, and a PR reopened between syncs settles it.
A reopened pull request keeps counting as closed unmerged, and drops out of the open count, until it closes again.
Count treats a PR found in both the store and the open listing as closed. The reason given is that GitHub's open search lags behind a merge. Suppose a PR is closed, a sync stores it as CLOSED, and someone then reopens it.
- The open listing finds it.
closedKeysdrops it fromstillOpen.- It counts under "closed unmerged" for the window, and not under "open now" or the sparkline.
No sync repairs the row. ReadGitHubHistory searches only is:closed, so an open PR's update is never read. The row is corrected only when the PR closes or merges again.
Fix: let the open listing win when its UpdatedAt is after the stored row's UpdatedAt. The listing already carries UpdatedAt. A PR the open search still finds after merging carries the stale, earlier stamp, so the lag case keeps counting as closed.
G1.3 internal/board/summary_tab.go:430 · internal/board/summary_charts.go:85
Reach: prs summary --since 2026-10-01 --until 2026-10-31, the help's own third example, or any --until before now.
--until in the past prints the open count at the window's end under the label "open now".
prs summary --since 2026-10-01 --until 2026-10-31 prints open now 8 4 4. Fifteen PRs are open now. The 8 is the count open at Nov 1 00:00, which is what Numbers.Open means and what the row's detail and the schema say ("open at until"). The OPEN chart title has the same problem (%d now) wherever a frame is drawn with an until. A user comparing a past month against today reads the wrong number.
Fix: label the row from untilText. Keep "open now" when the window ends now, and use e.g. open at Nov 1 00:00 otherwise. Do the same for the chart title.
Findings as JSON
{
"used": "I built `prs` from the PR head. I ran `prs summary` for 7d, 30d and 90d, `prs summary --since 2026-10-01 --until 2026-10-31` with and without `--json`, and `prs tui --view summary --once`, all against real GitHub (owners: datapointchris), with config and cache pointed inside my directory. I checked each count against direct GraphQL searches and against the SQLite store's rows, and ran `go test ./...`, which passed.",
"standards": "No cited rule reaches these findings. I read `user-experience.md` § \"A trend chart sits on the thing it describes, one subject to a chart on its own labeled axis, under the numbers that summarize it\" and `data.md` § \"A new source of data gets its row in the data catalog in the change that creates it\" (a file on a box takes no row). I also searched \"github search qualifier misses closed pull requests\", \"label names what the number counts\" and \"fault sized against real\".",
"findings": [
{
"number": "G1.1",
"section": "correctness",
"sites": [
"internal/summary/sync.go:27",
"internal/summary/sync.go:33",
"internal/pullrequests/github_history.go:100",
"internal/pullrequests/github.go:160"
],
"reach": "Two flows reach it: any first `prs summary` or Summary tab start on a fresh cache file, and any `w` step past the cache's start (e.g. to 90d). Both go through `Sync`'s `closed:` read. It happened on the real datapointchris account on every window I ran.",
"body": "**The first read misses closed-unmerged pull requests, because GitHub's `closed:` qualifier leaves some out and `updated:` does not.**\n\nI ran `prs summary --since 90d --json` on a fresh cache. The store held 874 rows. The search `user:datapointchris is:pr is:closed updated:>=2026-08-10T15:46:02Z` counts 881. The 7 missing are all `CLOSED` unmerged:\n- dotfiles#88\n- dotfiles#55\n- terminal-library-fleet#14\n- terminal-library-fleet#16\n- terminal-library-fleet#17\n- reddit-scraper#15\n- theme#21\n\nFor dotfiles#88 (closedAt 2026-10-19T03:53:53Z), `closed:2026-10-19T03:00:00Z..2026-10-19T05:00:00Z` returns 0. `is:closed updated:2026-10-19` returns it.\n\nWhat the user sees:\n- **90d:** closed unmerged reads 29 against 36 real. Opened reads 886, and `created:>=` counts 893.\n- **30d:** closed unmerged reads 17, and 18 closed in the window (dotfiles#88 missing).\n- **`--since 2026-10-01 --until 2026-10-31`:** prints 15, and GitHub has 16.\n\nThe open sparkline loses these PRs as well. Merged counts matched exactly (196, 512, 845, 360).\n\nThe gap is permanent. Later syncs only ask `updated:>=` the last sync, so a PR closed before the first read never comes back unless someone touches it.\n\nFix: read every stretch with `UpdatedBetween`. Use `updated:>=since` for the first read, and the same for a read back past `coverage.From`. A closed PR's `updatedAt` is never before its `closedAt`, so this is a superset, and `Store.Closed`/`Count` already filter by `closed_at`. On this account it costs 881 rows against 874. Then `ClosedBetween` can go.\n\nThe board's recently-closed group has the same shape at `github.go:160` (`is:closed closed:>=`). It predates this PR, so fixing it here is optional. The help and README sentence \"reads every pull request closed within the window\" would change with the fix."
},
{
"number": "G1.2",
"section": "correctness",
"sites": [
"internal/summary/numbers.go:127",
"internal/summary/numbers.go:141",
"internal/pullrequests/github_history.go:145"
],
"reach": "unproven. It needs a PR that was closed during a sync and then reopened. None of the 15 PRs open on datapointchris now has a `ReopenedEvent`. Reopening is a stock GitHub action, and a PR reopened between syncs settles it.",
"body": "**A reopened pull request keeps counting as closed unmerged, and drops out of the open count, until it closes again.**\n\n`Count` treats a PR found in both the store and the open listing as closed. The reason given is that GitHub's open search lags behind a merge. Suppose a PR is closed, a sync stores it as `CLOSED`, and someone then reopens it.\n- The open listing finds it.\n- `closedKeys` drops it from `stillOpen`.\n- It counts under \"closed unmerged\" for the window, and not under \"open now\" or the sparkline.\n\nNo sync repairs the row. `ReadGitHubHistory` searches only `is:closed`, so an open PR's update is never read. The row is corrected only when the PR closes or merges again.\n\nFix: let the open listing win when its `UpdatedAt` is after the stored row's `UpdatedAt`. The listing already carries `UpdatedAt`. A PR the open search still finds after merging carries the stale, earlier stamp, so the lag case keeps counting as closed."
},
{
"number": "G1.3",
"section": "correctness",
"sites": [
"internal/board/summary_tab.go:430",
"internal/board/summary_charts.go:85"
],
"reach": "`prs summary --since 2026-10-01 --until 2026-10-31`, the help's own third example, or any `--until` before now.",
"body": "**`--until` in the past prints the open count at the window's end under the label \"open now\".**\n\n`prs summary --since 2026-10-01 --until 2026-10-31` prints `open now 8 4 4`. Fifteen PRs are open now. The 8 is the count open at Nov 1 00:00, which is what `Numbers.Open` means and what the row's detail and the schema say (\"open at until\"). The OPEN chart title has the same problem (`%d now`) wherever a frame is drawn with an `until`. A user comparing a past month against today reads the wrong number.\n\nFix: label the row from `untilText`. Keep \"open now\" when the window ends now, and use e.g. `open at Nov 1 00:00` otherwise. Do the same for the chart title."
}
]
}This review does not gate merge.
There was a problem hiding this comment.
PR review — synthesis
Used: the 14 findings of round 1, from 8 review(s), and nothing else
Standards: none, because this step reads only the findings
Findings: 0 correctness · 4 design · 0 prose
Design and architecture
synthesis.1 explains G1.2 · system-design.1
A reopened pull request is never reconciled in the closed-history cache. The sync reads only is:closed searches, so a PR that reopens after being stored keeps its closed or merged row. Both findings name this gap in Sync and ReadGitHubHistory.
synthesis.2 explains code-architecture.1 · code-architecture.2 · code-architecture.3 · code-architecture.4
One fact has several owners that can drift apart. A fact such as the shape of a cached pull request, a scope's normal form, the list of tabs, or the bot predicate is spelled out at several sites with no single owner. This is the code-quality rule on asking whatever owns a fact.
synthesis.3 explains contracts.1 · contracts.2
prs tui --view summary and the shared config key do not keep one meaning across commands. A flag or key is accepted or documented with a different meaning on the tui and summary routes, while the help text still describes only the old one.
synthesis.4 explains stale-prose.2 · stale-prose.3
Prose repeats constants and measured numbers that the code owns. These values sit in comments, README and schema text with nothing tying them to the code, and they go stale when the code changes.
One defect, raised more than once
G1.2 · system-design.1: A reopened pull request keeps its stored closed or merged row, because the sync only reads is:closed and never revalidates cached rows (internal/summary/sync.go, github_history.go).
Shapes as JSON
{
"shapes": [
{
"number": "synthesis.1",
"section": "design",
"explains": [
"G1.2",
"system-design.1"
],
"body": "A reopened pull request is never reconciled in the closed-history cache. The sync reads only `is:closed` searches, so a PR that reopens after being stored keeps its closed or merged row. Both findings name this gap in `Sync` and `ReadGitHubHistory`."
},
{
"number": "synthesis.2",
"section": "design",
"explains": [
"code-architecture.1",
"code-architecture.2",
"code-architecture.3",
"code-architecture.4"
],
"body": "One fact has several owners that can drift apart. A fact such as the shape of a cached pull request, a scope's normal form, the list of tabs, or the bot predicate is spelled out at several sites with no single owner. This is the code-quality rule on asking whatever owns a fact."
},
{
"number": "synthesis.3",
"section": "design",
"explains": [
"contracts.1",
"contracts.2"
],
"body": "`prs tui --view summary` and the shared config key do not keep one meaning across commands. A flag or key is accepted or documented with a different meaning on the tui and summary routes, while the help text still describes only the old one."
},
{
"number": "synthesis.4",
"section": "design",
"explains": [
"stale-prose.2",
"stale-prose.3"
],
"body": "Prose repeats constants and measured numbers that the code owns. These values sit in comments, README and schema text with nothing tying them to the code, and they go stale when the code changes."
}
],
"duplicates": [
{
"defect": "A reopened pull request keeps its stored closed or merged row, because the sync only reads `is:closed` and never revalidates cached rows (internal/summary/sync.go, github_history.go).",
"findings": [
"G1.2",
"system-design.1"
]
}
]
}This review does not gate merge.
PR review round 1Ran: G1, useless-test, code-architecture, fleet-patterns, system-design, stale-prose, failure-paths, contracts, synthesis
Round record{
"grammar": 6,
"round": 1,
"repo": "datapointchris/prs",
"pr": 15,
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"experiment": "none",
"dispatch": "20261108T154055Z-pr-review",
"roster": "44e66c70a21627e0922b633d0e7db86363a01f6c",
"harness": "6c387842ffdad77075f35f64a0c672d7ca178c86",
"machine": "queue",
"started": "2026-11-08T15:42:06Z",
"finished": "2026-11-08T15:54:38Z",
"reviewers": [
{
"label": "G1",
"agent": "pr-reviewer",
"model": "opus",
"effort": "high",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/general.md",
"prompt_sha": "00271016dd7d",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 604,
"tokens": {
"input": 82,
"cache_write": 84258,
"cache_read": 2161939,
"output": 23262
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 82,
"cache_write": 84258,
"cache_read": 2161939,
"output": 23262
}
},
"session": "01a1bbd4-318c-7eb4-84d3-c824840004da",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818544965",
"review_id": 5818544965,
"findings": {
"correctness": 3,
"design": 0,
"prose": 0
},
"numbers": [
"G1.1",
"G1.2",
"G1.3"
],
"rules": {
"proposed": 1,
"keys": [
"rule/data/a-store-filled-from-a-search-reads-it-by-the-field-every-change-moves-and-filters-locally-by-the-field-it-counts"
],
"filed": true,
"why": "",
"path": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records/G1.rules.json"
},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:43:56Z",
"finished": "2026-11-08T15:54:07Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "useless-test",
"agent": "pr-useless-test-reviewer",
"model": "opus",
"effort": "high",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/useless-test.md",
"prompt_sha": "f0b52bbf8a48",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 224,
"tokens": {
"input": 30,
"cache_write": 29369,
"cache_read": 402929,
"output": 4847
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 30,
"cache_write": 29369,
"cache_read": 402929,
"output": 4847
}
},
"session": "01a1bbd2-51a7-7912-a389-154264123ba4",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818511035",
"review_id": 5818511035,
"findings": {
"correctness": 0,
"design": 0,
"prose": 0
},
"numbers": [],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:06Z",
"finished": "2026-11-08T15:46:01Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "code-architecture",
"agent": "pr-code-architecture-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "283f4014efc3",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 281,
"tokens": {
"input": 26,
"cache_write": 57724,
"cache_read": 494231,
"output": 7877
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 26,
"cache_write": 57724,
"cache_read": 494231,
"output": 7877
}
},
"session": "01a1bbd2-3c84-7b95-8d80-cbe711e6ca8c",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818514018",
"review_id": 5818514018,
"findings": {
"correctness": 0,
"design": 4,
"prose": 0
},
"numbers": [
"code-architecture.1",
"code-architecture.2",
"code-architecture.3",
"code-architecture.4"
],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:07Z",
"finished": "2026-11-08T15:46:55Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "fleet-patterns",
"agent": "pr-fleet-patterns-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "efa99c642e6d",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 118,
"tokens": {
"input": 14,
"cache_write": 19707,
"cache_read": 122511,
"output": 3502
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 14,
"cache_write": 19707,
"cache_read": 122511,
"output": 3502
}
},
"session": "01a1bbd2-85f0-7dda-b948-ec8bf272139f",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818505339",
"review_id": 5818505339,
"findings": {
"correctness": 0,
"design": 0,
"prose": 0
},
"numbers": [],
"rules": {
"proposed": 1,
"keys": [
"rule/data/a-go-sqlite-store-sets-its-pragmas-and-_txlockimmediate-in-the-dsn-never-through-exec"
],
"filed": true,
"why": "",
"path": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records/fleet-patterns.rules.json"
},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:11Z",
"finished": "2026-11-08T15:44:18Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "system-design",
"agent": "pr-system-design-reviewer",
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "bc2eb8615f41",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 76,
"tokens": {
"input": 14,
"cache_write": 18396,
"cache_read": 133753,
"output": 3149
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 14,
"cache_write": 18396,
"cache_read": 133753,
"output": 3149
}
},
"session": "01a1bbd2-4734-707d-bda2-9d86da975408",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818502726",
"review_id": 5818502726,
"findings": {
"correctness": 1,
"design": 1,
"prose": 0
},
"numbers": [
"system-design.1",
"system-design.2"
],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:06Z",
"finished": "2026-11-08T15:43:28Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "stale-prose",
"agent": "pr-stale-prose-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "4359b874a436",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 111,
"tokens": {
"input": 20,
"cache_write": 32911,
"cache_read": 200658,
"output": 5935
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 20,
"cache_write": 32911,
"cache_read": 200658,
"output": 5935
}
},
"session": "01a1bbd3-5032-7fcc-b43f-7f1572228124",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818507187",
"review_id": 5818507187,
"findings": {
"correctness": 0,
"design": 0,
"prose": 3
},
"numbers": [
"stale-prose.1",
"stale-prose.2",
"stale-prose.3"
],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:55Z",
"finished": "2026-11-08T15:44:51Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "failure-paths",
"agent": "pr-failure-paths-reviewer",
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "4239ed2be477",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 68,
"tokens": {
"input": 12,
"cache_write": 31299,
"cache_read": 131925,
"output": 3316
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 12,
"cache_write": 31299,
"cache_read": 131925,
"output": 3316
}
},
"session": "01a1bbd2-4252-7c71-9eb0-c0a63307f404",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818502294",
"review_id": 5818502294,
"findings": {
"correctness": 0,
"design": 0,
"prose": 0
},
"numbers": [],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:06Z",
"finished": "2026-11-08T15:43:21Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "contracts",
"agent": "pr-contracts-reviewer",
"model": "opus",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/persona.md",
"prompt_sha": "3cf737a7c111",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 242,
"tokens": {
"input": 18,
"cache_write": 30419,
"cache_read": 217783,
"output": 5978
},
"tokens_from": "result",
"tokens_by_model": {
"claude-opus-5-5": {
"input": 18,
"cache_write": 30419,
"cache_read": 217783,
"output": 5978
}
},
"session": "01a1bbd2-8934-768c-829a-026a75d9e20b",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818512313",
"review_id": 5818512313,
"findings": {
"correctness": 0,
"design": 2,
"prose": 0
},
"numbers": [
"contracts.1",
"contracts.2"
],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:42:11Z",
"finished": "2026-11-08T15:46:24Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
},
{
"label": "synthesis",
"agent": null,
"model": "sonnet",
"effort": "medium",
"experiment": "none",
"arm": "default",
"reason": "STATIC",
"prompt_profile": "pr-reviewer-prompts/synthesis.md",
"prompt_sha": "8832c4b84e09",
"status": "ran",
"why": "",
"attempts": 1,
"seconds": 10,
"tokens": {
"input": 2,
"cache_write": 10243,
"cache_read": 1499,
"output": 586
},
"tokens_from": "result",
"tokens_by_model": {
"claude-sonnet-5-5": {
"input": 2,
"cache_write": 10243,
"cache_read": 1499,
"output": 586
}
},
"session": "01a1bbde-0634-7817-8616-51b667dba3e2",
"worklog": "1331.1",
"worklog_refused": "",
"claude_code": "2.1.319",
"review_url": "https://github.com/datapointchris/prs/pull/15#pullrequestreview-5818546590",
"review_id": 5818546590,
"findings": {
"correctness": 0,
"design": 4,
"prose": 0
},
"numbers": [
"synthesis.1",
"synthesis.2",
"synthesis.3",
"synthesis.4"
],
"rules": {},
"head": "85fe478ed971265419dcddc307b897a7e9ed8d19",
"kept": false,
"records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr15-round1-20261108T154055Z/records",
"started": "2026-11-08T15:54:24Z",
"finished": "2026-11-08T15:54:38Z",
"account": "live",
"api_key_source": "none",
"machine": "queue"
}
],
"rules": {
"proposed": 2,
"keys": [
"rule/data/a-go-sqlite-store-sets-its-pragmas-and-_txlockimmediate-in-the-dsn-never-through-exec",
"rule/data/a-store-filled-from-a-search-reads-it-by-the-field-every-change-moves-and-filters-locally-by-the-field-it-counts"
],
"unfiled": []
},
"kept": []
} |
…uests There was no way to see how many pull requests merged or closed over a stretch, how long merging took, or how many a review round reviewed, short of a search per question. The Summary tab sits beside the board, and w and W step its window through 1d, 7d, 2w, 30d and 90d. It counts the pull requests open now, and those opened, merged and closed unmerged within the window, the median and p90 time to merge, and the share of merges whose first review was a review round's, each split into a person's and an app's, then each repo's merges. Three charts sit above: merges per day as columns, open pull requests across the window as a sparkline, and merges by how long they took. prs summary prints the same numbers, and --json in the shape summary.schema.json gives. The open pull requests are the board's own listings, so the tab adds no open read. The merged and closed are GitHub's, kept in a SQLite file for each set of repos under $XDG_CACHE_HOME/prs/closed-pull-requests. The first read of a window reads every pull request closed within it. Each read after asks only for those updated since the last, and a window past what the file holds reads back to its start. The coverage and the rows land in one transaction, so a read with a failed search keeps nothing. A search past GitHub's 1,000-result cap is split in halves of its stretch until each fits. Each pull request's first review comes with it in the same search, so the share of round-reviewed merges costs no read of its own. The merges chart draws through gotermboard's column series, from the pull request that adds it, because Bars labels every bar and draws nothing once bars outnumber cells. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbb1-d463-75ae-b742-e7f404d052c2 Worklog: 1331.1
The column series' branch moved onto gotermboard's main, past its v0.36.0 release, so prs pins the branch's new head. The pin moves to the tag the column series' release cuts. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbb1-d463-75ae-b742-e7f404d052c2 Worklog: 1331.1
…ened one is lost GitHub's closed: qualifier leaves out some pull requests that closed within its range, where updated: over the same stretch finds them with identical stamps. A pull request is never last updated before it closed, so every history read and the board's recently closed read now ask by updated: and drop what closed outside the window. The first 90-day read of one account held 874 rows against 881 that GitHub counts. The history reads every state. A reopened pull request comes back as a newer open record, which stands over its closed row, and Count lets the record updated last stand between the open listing and the store, as the board does. A listing record counts as open only while it stands open. The store's coverage only widens, earliest From and latest Synced, because a board and a prs summary can sync one file at once and the later commit would otherwise narrow it. Its version is a digest of its schema, so no hand-kept number can drift from the tables. One record type, pullrequests.HistoryRecord, runs from the read to the count, with the caller saying what a review round's post is. Scope.Names is the one normal form of a scope's repos and owners, for the searches and both caches' names, and PullRequest.IsBot and HistoryRecord.IsBot own whether an app opened one. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbd1-f143-7a61-a97b-5a1d6252c50e Worklog: 1331.1
prs summary --until in the past printed the count open at the window's end under "open now", and the OPEN chart titled it the same, while the pull requests open now numbered differently. Both now read "open at" the instant --until names, and "now" where the window ends now. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbd1-f143-7a61-a97b-5a1d6252c50e Worklog: 1331.1
… printed tab ignores The list of tabs was spelled in the --view check, its error, its help, the NewTabs list and a branch for printing the Summary tab. tuiTabs now holds each tab's name, the flags its --once and --json read, and how it opens on the board into what gotermboard draws, what --json prints and why either exits 1, and each of those reads that one list. prs tui --view summary --since 7d --json printed the 30d window, because --since on prs tui sets the board's recently closed, which the Summary tab's print never reads. A printed tab now refuses with exit 2 any flag it does not read, --since and --pr on the Summary tab, and says that prs summary sets the window it counts. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbd1-f143-7a61-a97b-5a1d6252c50e Worklog: 1331.1
…e code holds closed_refresh_every_seconds paces the Summary tab's read of what changed in its history as well as the recently closed read, and prs config path, prs config show, prs tui --help and the config's comments now say so. The prose about the Summary tab named its windows, buckets, sample count, request size and search pages, each a value the code holds or a cost measured on one account. It now says what each is for, and the help names the windows from the list the tab steps through. The help, README and CLAUDE.md say the history reads by update time, keeps every state, and widens its coverage only. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbd1-f143-7a61-a97b-5a1d6252c50e Worklog: 1331.1
PullRequest.IsBot owns the rule that a pull request whose listing does not say, or whose account is gone, counts as a person's. The board's own isBot repeated it, and the rows and the groups now call IsBot instead. Agent-Name: claude-code-unattended Agent-Session: 362722ae-0371-4e0a-ba15-da7877021ccb Agent-Run: 01a1bbd1-f143-7a61-a97b-5a1d6252c50e Worklog: 1331.1
fc95946 to
0f078d1
Compare
The Summary tab's chart draws each reading as a column until the next, which gotermboard first releases in v0.39.0. Agent-Name: claude-code Agent-Session: 60bee577-11ae-4c13-9517-75a25f6211a6 Worklog: 2608.1
There was no way to see how many pull requests merged or closed over a stretch, how long merging took, or how many a Claude review round reviewed, short of writing a search for each question. The 30 days to Nov 8 held 509 merges across 52 repos, and nothing in prs counted them.
This adds a Summary tab beside the board, and
prs summary, which prints the same numbers.wandWstep the tab's window through 1d, 7d, 2w, 30d and 90d.What to look at
internal/summary/sync.goandstore.go. Every read asks byupdated:, neverclosed:, and keeps every state, so the store's coverage claims every pull request updated since its start. Check thatKeepwrites rows and coverage in one transaction, that the coverage only widens, and thatSynckeeps nothing when a search fails. After the first read, onlyupdated:>=the last sync is asked, with two minutes of overlap.internal/pullrequests/github_history.goReadGitHubHistory. Check the paging by cursor, and the split into halves of a stretch where a search passes GitHub's 1,000-result cap. Both halves read the middle instant, so check the store keeps one row of a pull request read twice.internal/summary/numbers.goCount. Where the open listing and the store both hold a pull request, the record updated last stands, as on the board. GitHub's open search still finds one for a while after it merges, and one reopened since the store read it is newer in the listing. Percentiles join the two nearest ranks.internal/board/summary_tab.go. The tab'sSourcesare the board's own open listings plus one history read, so the tab adds no open read. Its look is the board's, so the margin holds still across tabs.How it was verified
go test ./...passes.TestTheSummaryTabCountsEachWindowChartsItAndPaintsFromItsCacheOnTheNextStartdrives the built binary on a pty against the stand-in GitHub, with seven closed pull requests placed back from now. It reads 30d's counts, merged4 3 1and median14.0h 24.0h 4.0h, and every chart's title and a column. Then it steps to 7d, merged3 2 1, and back to 30d. A second start with GitHub answering 502 still draws 30d's counts from the cache, and names the failed read.TestSummaryJSONHoldsItsSchemaAndReadsOnlyWhatChangedOrWhatTheCacheLacksholdsprs summary --jsontocmd/prs/summary.schema.jsonfor 30d, 1d and 90d. The 1d run asks only from the last run's instant. Before the 90d run the stand-in reopens #36, and closed unmerged falls from 2 to 1. The 90d run asks anupdated:A..Brange back to its start.prs summary --since 30d --until <3 days back>labels its open rowopen at Nov 6 00:00, and without--untilit readsopen now.prs tui --view summary --since 7d --jsonexits 2 and namesprs summary --since.closed:2026-10-19T03:00:00Z..2026-10-19T05:00:00Zon dotfiles finds nothing, andis:closed updated:over the same two hours finds dotfiles#88, closed and updated at 03:53:53Z.prs summary --since 30d --jsonagainst the account, before the reads moved toupdated:, took 11.3 s and 40 MB. The GraphQL budget read 4,924 remaining before and 4,919 after, so the run spent at most 5 points, the open listing included. A dry run prices one history request at 5 points, one an alias. It counted 509 merged and 12 closed unmerged. A directis:pr is:merged merged:>=search over the same five repo chunks counted 511 a few minutes later. The two extra merged at 15:30:27Z and 15:31:04Z, at and after the sync's instant, so the counts agree at that instant, and the next sync'supdated:>=read takes both.What changes
prs tuihas two tabs,pull-requestsandsummary, andtabmoves between them.--view summaryopens on the Summary tab. With--onceor--json, it prints the tab's frame or whatprs summary --jsonprints for 30d, and refuses--sinceand--pr, which only the board reads.prs summary [--since 30d] [--until …] [--json]is new.--untiltakes--since's forms, and a day takes in the whole of it.$XDG_CACHE_HOME/prs/closed-pull-requests/, one file per set of repos. The first read of a window reads every pull request updated within it. Each read after asks for what changed since the last, everyclosed_refresh_every_seconds, which now paces both closed reads.is:closed updated:>=and drops what closed before--since, becauseclosed:>=left some out.modernc.org/sqlite, whichprojectsalready uses.Decisions, and what they rejected
The round share reads each pull request's first review in the history search itself,
reviews(first: 1). That adds about 1 point a page and a few kilobytes a pull request. Reading every review would carry eight reviewers' bodies apiece, and a detail read per pull request would cost a point each.Any version of a round's marker counts, through
reviewrounds.IsRoundPost, with the author check round posts already pass. A merge reviewed under an older grammar was still reviewed.One store file per set of repos, as the listing cache keys its files. A registry edit starts a new file and rereads the window, a few points, and no file holds rows of repos the board does not read.
The store is a cache, so a schema change empties it rather than migrating, and the next read refills it.
Split on the cap by halving the stretch, rather than by month up front. It holds for any window and any rate of merging. Rejected: a split by calendar month, which a month with over 1,000 closes would still overflow.
The merges chart uses gotermboard's new column series, feat(chart): draw a time series' readings as columns, each until the next gotermboard#14.
Barslabels every bar and draws nothing once bars outnumber cells, andStackedAreadraws a slope between days.Read by
updated:rather thanclosed:. A pull request is never last updated before it closed, so the read is a superset, andCountfilters byclosed_at. On the reviewer's 90-day read it costs 881 rows against 874. Rejected: keepingclosed:and patching its misses, which nothing here can find.A reopened pull request stays in the store as an open row rather than being deleted. The upsert keeps the record updated last, so a slower sync committing after a faster one cannot bring back a closed row a delete removed.
closed_refresh_every_secondspaces both closed reads. Both read closed pull requests at about the same cost. Rejected: a second key, which would be a new config surface for a split no one has asked for.Risk and rollback
prs pins gotermboard at datapointchris/gotermboard#14's head, an untagged commit. The pin moves to the tag that pull request's release cuts before this merges. A revert leaves the cache file behind, which nothing else reads.
What this does not do
The review
#15 (comment)
G1.1fixed — ee3c966. Every history read and the board's recently closed read ask byupdated:. The first read and a read back past the coverage useupdated:>=sinceandupdated:since..from.G1.2fixed — ee3c966.Countlets the record updated last stand between the open listing and the store, and counts a listing record as open only while it stands open.G1.3fixed — 767e7f3. The row and the OPEN chart readopen at <instant>where--untilends the window, andopen nowotherwise.system-design.1fixed — ee3c966. The history reads every state, so a reopened pull request's newer open record overwrites its closed row. A deleted or transferred repo's rows stay, because its merges did happen within the window.system-design.2fixed — ee3c966.Keepwrites the coverage as the earliestFromand the latestSynced. Every read asks for every update since aFromolder than any sync's start, so the later sync read every change since the earlier one began.code-architecture.1fixed — ee3c966.schemaVersionis a digest of the schema text.summary.Closedand its field copy are gone, andpullrequests.HistoryRecordruns from the read to the count, withReviewRoundset at read time by the predicate the caller passes. The fragment, node struct, columns and scan stay one per layer, because GraphQL anddatabase/sqleach need their own spelling, and the digest catches a schema edit.code-architecture.2fixed — ee3c966.Scope.Namesis the one normal form, andgithubSearches,keptScopeOfandFileForcall it. The two caches keep their own digests, because one names every provider's scopes and the other GitHub's alone, so the listing cache's files keep their names.code-architecture.3fixed — fbb3e67.tuiTabsholds each tab's name, printed flags and opener, and--view, its help,NewTabsand the printed path read it.code-architecture.4fixed — ee3c966, 0f078d1.authorIsBotserves both node mappers, andPullRequest.IsBotandHistoryRecord.IsBotown the nil rule. The board's rows and groups callpr.IsBot(), and its ownisBotis gone.contracts.1fixed — fbb3e67. A printed tab refuses with exit 2 any flag it does not read,--sinceand--pron the Summary tab.contracts.2fixed — 1bc1137. One key, andprs config path,prs config show,prs tui --helpand the config's comments say it paces both closed reads.stale-prose.1fixed — ee3c966. The package doc says it reads what changed, and back to a window's start.stale-prose.2fixed — ee3c966, 1bc1137. Each value is named by what it is for, and the help names the windows fromSummaryWindows. The README keeps "per hour for a window of two days or less", and the schema's copy goes, as the finding offered.stale-prose.3fixed — ee3c966, 1bc1137. The sizes, prices and page counts are gone from the comments, help and README.synthesis.1fixed — same assystem-design.1.synthesis.2fixed — same ascode-architecture.1,code-architecture.2,code-architecture.3andcode-architecture.4.synthesis.3fixed — same ascontracts.1andcontracts.2.synthesis.4fixed — same asstale-prose.2andstale-prose.3.Session 362722ae-0371-4e0a-ba15-da7877021ccb wrote this.