Skip to content

fix: a program prs hands the terminal to says why it failed - #12

Merged
datapointchris merged 2 commits into
mainfrom
gotermboard-hand-over
Nov 8, 2026
Merged

datapointchris merged 2 commits into
mainfrom
gotermboard-hand-over

Conversation

@datapointchris

@datapointchris datapointchris commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

A failed editor or nvim run from prs showed only exit status 1 on the footer, because prs's handover dropped the program's stderr. prs also kept its own copies of the handover, the tmux window and the last-line reading, which another board on the same library had copied and fixed separately.

gotermboard now owns all three (datapointchris/gotermboard#9). prs calls Model.HandOver, gotermboard.OpenWindow and gotermboard.LastLine, and deletes handOver, handedBack, openWindow and lastLine.

What to look at

  1. internal/board/actions.go windowAction — the config's command is a shell command, which tmux runs whole through its shell. So prs hands OpenWindow the config's needs, and its own check at launch goes away. Offering the key still reads config.WindowAction.Missing.
  2. internal/board/work.go Receive — gotermboard.HandedBack now only reads the providers again. The model runs each key's own follow-up after Receive, so nothing here calls it.
  3. internal/board/actions.go diff and comment — each failure now says gotermboard.LastLine(stderr, err). The noDiffview exit code is still checked first.
  4. go.mod — gotermboard is pinned at the commit that adds these exports, a pseudo-version, until gotermboard#9 merges and tags a release. gotermstyle moves to v0.4.0 with it.

How it was verified

  • go test ./... passes with no test changed. It drives the built binary through d, c and r against stand-in programs, and pins r's tmux arguments, new-window -n "review widget #12" -c <checkout> "claude '/review-pr 12'".
  • A throwaway tuidrive test set EDITOR to a stand-in that wrote two lines to stderr and exited 1, then pressed c on a row. The footer read the editor ended with E325: the stand-in editor found a swap file, so nothing was posted; what it wrote is in …. The test was not committed.

What changes

  • A failed c or d names the last line the editor or nvim wrote to stderr, rather than its exit status alone. A program that wrote nothing still shows its exit status.
  • A program prs hands the terminal to has a pipe on fd 2, drained to the terminal as bytes arrive. nvim and an editor draw on stdout, so neither changes.
  • A child that program leaves running keeps writing to the terminal, and the board does not wait for it to exit.
  • A window action's refusals come from gotermboard. A missing program reads not on PATH: claude, so no window opened for review widget #12. Outside tmux it reads review widget #12 opens in a tmux window, and prs is not running inside tmux. A checkout that is gone is refused, where tmux would have opened the window in another directory.

Risk and rollback

  • This must not merge while go.mod pins the pseudo-version. Once gotermboard#9 merges and its release is tagged, the pin moves to that release in this branch.
  • A revert restores prs's own copies and the pin it had before.

Session b907ef76-acb0-4393-8a6d-92c06fe1dcfb wrote this.

datapointchris added a commit that referenced this pull request Nov 8, 2026
…ument (#11)

A Claude review round posts a review per reviewer, a synthesis review
and a round comment on a pull request. prs showed them only as long
bodies in Comments. Nobody could tell from the list whether a round had
run, whether it was still running, or what it found, without opening
each pull request and reading every review.

This reads each round from the reviews and comments the detail read
already fetches, so it costs no GitHub read of its own:

- A `CLAUDE` cell after the review cell gives the newest round. Once its
round comment posts, the cell is its finding counts, as `claude 5
correctness · 3 design · 4 prose`. It adds `1 not run` or `1 unread`
where a reviewer did not run or its review could not be read, and reads
`claude did not run` where none ran. Before the round comment, it reads
`claude running` while reviews post, up to an hour after the last, then
`claude unfinished`. It reads `claude reviewed` where the newest
review's marker names a version prs does not read. The column draws only
while a row fills it.
- A Review section after Description says where the round stands, then
the synthesis's shapes and duplicates, then every finding under a
heading for its section, read from each review's JSON block. A skipped
reviewer reads as skipped, apart from one that did not run. Each earlier
round gets a line. A post of an unknown version shows whole. Comments
leaves out exactly the posts Review shows.
- A marked post counts in a round only where an app on the repo or
someone who can write to it posted it. Anyone else's shows whole with
why, and moves no cell.
- A window action marked `runs_review_round` reads running while its
tmux window is open, until a round finishes after prs opened it.

## What to look at

1. `internal/reviewrounds/rounds.go`: `Read`, `outsiderWhy` and
`jsonBlock`. Check that the block is the last json fence, matched by
lines, that an outsider's post never reaches a round, and that the
marker's version gates the rest.
2. `internal/board/review_rounds.go`: `roundStateOf`, which the cell and
the Review section both read, and `roundWindowOf` with
`roundWindows.open`, the window rule. A session keeps its window after
its round, so a window prs opened counts until a round finishes after
the opening. One prs did not open counts only where no round has
finished.
3. `internal/board/rows.go`: `rowColumns`, each column with the cell it
draws, the optional `CLAUDE` one among them.
4. `cmd/prs/testdata/review-round.json`, a real round copied from this
repo's PR 7. Machine names, home paths and a tracker id are replaced,
and each author carries the type and association GitHub sends.

## How it was verified

- `go test ./...` passes under the commit hook.
`TestEachRowReadsItsNewestReviewRoundAndItsDocumentHoldsTheRound` drives
the built binary on a pty against the stand-in GitHub:
- Widget #13 carries PR 7's whole round, a G1 finding that quotes
`<details>`, and an outsider's forged round 99.
- Gadget #5 carries the first three reviews, stamped two hours old, with
no round comment.
  - Widget #12 carries G1's review under a `v7` marker.
- The test waits for each row's cell and reads the whole document `d`
hands nvim for all three. It then presses `r`, waits for `claude
running`, empties the stand-in tmux's window list and waits for the
counts to return.
- With `jsonBlock` anchored on the last `<details>` again, the test
times out waiting for #13's counts. It does the same with the author
check removed.
- A throwaway test, deleted after, printed the cell and lines for four
more states. A round none ran read `claude did not run`. A skipped
synthesis beside a reviewer not posted read `claude 1 design · 1 not
run`, with "synthesis skipped: fewer than two findings to share a
cause." A round whose round comment fell outside the comments read read
its counts and "may have finished". A round posting ten minutes ago read
`claude running`.
- Run on PR 7's live round, the parser read 12 findings, 5 correctness ·
3 design · 4 prose, with two shapes and one duplicate. That matches the
round comment's table less the synthesis's two.
- This branch's `prs config show` reads the dotfiles template with the
new key as `key r claude review offered, runs review rounds`. The
installed prs refuses it: `field runs_review_round not found in type
config.WindowAction`.

## What changes

- Rows on repos a round reviewed gain a `CLAUDE` column between `REVIEW`
and `AGE`. A board with no round never draws it. `prs list` never does,
because it reads no detail.
- The document gains a Review section, and the newest round's posts
leave the Conversation. Earlier rounds' posts stay there.
- The line on a merged, closed or gone row names review among the
sections it did not read.
- The detail query asks for each review's and comment's author type and
association.
- The threads file `d` hands nvim gains `author_type` and
`author_association` on each comment, both required in
`diff-threads.schema.json`. A reader holding the file to the earlier
schema refuses it, because its comment object allows no other keys.
- `runs_review_round` is a new window action key. Every earlier prs
refuses a config holding it, so the dotfiles template that adds it to
`r` merges after this releases.
- Inside tmux, where a window action runs review rounds, prs asks tmux
for its window names every five seconds.

## Decisions, and what they rejected

- **No read of its own.** The detail query already fetches
`reviews(last: 100)` with bodies and `comments(last: 50)`. A shared
per-PR cache for details, diffs and rounds was rejected, because rounds
add no read to share.
- **A round's post counts only from a Bot, or from an OWNER, MEMBER or
COLLABORATOR.** The review automation posts as a Bot whose association
is NONE, and the person running it as OWNER. So association alone would
drop every review. A config allow-list of authors was rejected: every
user would fill it in before any round read, and GitHub already sends
what decides it.
- **An hour of quiet ends running.** A reviewer posts once it ends, so a
running round's posts are never further apart than its slowest
reviewer's run. Running only while a window prs saw is open was
rejected, because a round started anywhere else has no window.
- **A round whose round comment fell outside the read shows its
counts.** The reviews hold the findings whatever the round comment says.
`claude reviewed` would hide them.
- **`runs_review_round` on the window action.** The window's name is
config, so prs needs to know which action runs a round. Hard-coding
`review {repo} #{number}` would tie prs to one config. A top-level
`review_round_window` key was rejected, because it would repeat the
action's `window`.
- **A window counts only until a round finishes after prs opened it.**
tmux reports no window creation time, and a Claude session keeps its
window after its round. Taking an open window alone as running would
show `claude running` on a finished round for as long as the window
stayed open.
- **Counts sum each reviewer's findings.** The synthesis's shapes are
left out and duplicates are not subtracted, so the cell matches the
reviews' own Findings lines.
- **Smallest change that would work**: the cell and the section without
the window watch. On PR 7's round, the first review landed about four
minutes after its dispatch, and the row would read nothing for that
time.

## Risk and rollback

A revert removes the cell, the section and the key. A config holding
`runs_review_round` is then refused, so the dotfiles template change
reverts with it.

## What this does not do

- Read a round on a recently closed row, which reads no detail.
- Show a round started outside a marked window before its first reviewer
posts. Nothing on GitHub marks it until then.
- Read a grammar before v6. A v5 round's posts show whole.
- Put rounds in `--json`, which reads no detail.
- Share one reader of the grammar with the other programs that read it.
They differ on a reviewer posting twice in a round, a round with two
round comments, and which versions they read. A shared public reader is
filed.

## The review

#11 (comment)

- `G1.1` fixed — 2d4b8c6. `jsonBlock` takes the last line of three or
more backticks and `json`, then reads to the line of the same backticks.
JSON escapes a newline inside a string, so no line of a finding's text
can open a fence, whatever it quotes. The test's widget #13 carries a G1
finding quoting `<details>`.
- `G1.2` fixed — same as `synthesis.2`.
- `G1.3` fixed — same as `synthesis.1`.
- `G1.4` fixed — same as `stale-prose.2`.
- `stale-prose.1` fixed — 2d4b8c6. The three sentences now say a window
running a round shows Review before the detail read lands, in the words
proposed.
- `stale-prose.2` fixed — 2d4b8c6. CLAUDE.md reads "a post whose marker
names a version other than `reviewrounds.Version` shows whole rather
than read wrong."
- `stale-prose.3` fixed — 2d4b8c6. `withRounds` says each pull request's
cut is in `withRoundOf`, and one line above that switch says why each
gets its cut.
- `code-architecture.1` fixed — same as `synthesis.3`.
- `code-architecture.2` fixed — same as `synthesis.3`.
- `code-architecture.3` fixed — same as `synthesis.3`.
- `code-architecture.4` fixed — same as `synthesis.3`.
- `system-design.1` fixed — 2d4b8c6. The detail read asks for each
author's `__typename` and `authorAssociation`. A marked post counts only
where its author is a Bot, which is an app installed on the repo, or is
an OWNER, MEMBER or COLLABORATOR. Any other shows whole with why, moves
no cell, and never reads as the newest review. The test's widget #13
carries an outsider's round 99. The proposed config allow-list was
rejected, because GitHub already sends what decides it and a stranger's
first run would have none.
- `system-design.2` fixed — same as `synthesis.1`.
- `contracts.1` fixed — same as `synthesis.1`.
- `contracts.2` fixed — same as `synthesis.2`.
- `fleet-patterns.1` deferred — one public reader of the grammar, shared
by every program that reads it, is filed. It needs a new public module,
and the existing reader cannot be imported as it stands. Until then prs
keeps the newest review and the newest round comment, as the grammar's
writer does. It reads only v6, the rule settled for prs, and takes the
last json fence, as the writer does.
- `synthesis.1` fixed — 2d4b8c6. `roundStateOf` decides the round's
state once, and the cell and Review both read it. A round with no round
comment reads running while a window runs it, or for an hour after its
newest post. After that it reads `claude unfinished`, and Review says it
stopped before its round comment posted and that running a review again
continues it. Where the pull request has more comments than the read
holds, the round reads its counts, and Review says it may have finished.
An earlier round there no longer reads "never posted its round comment".
Gadget #5 in the test reads `claude unfinished`.
- `synthesis.2` fixed — 2d4b8c6. `Finish` keeps each reviewer's status.
A skipped reviewer reads "synthesis skipped: why", apart from "X did not
run: why". A round none ran reads `claude did not run` in yellow, and
its earlier line says none of its reviewers ran. A reviewer not run and
a review not read add `1 not run` and `1 unread` to the counts, and turn
a cell with no correctness finding yellow.
- `synthesis.3` fixed — 2d4b8c6. Each fact has one home. `windowOn`
builds a window's name and checks its provider, for both the key that
opens it and the watch that finds it. `reviewrounds.Sections` holds each
section's name, heading and whether it blocks a merge, and the board's
heading switch and `"correctness"` literal are gone. `reviewShows` is
the set Review draws, and its `bodies` the set Comments leaves out.
`rowColumns` holds each column with its cell, and `claudeAt` is gone.

Session e999b398-dbff-4616-bec4-8a433b70f632.
datapointchris added a commit that referenced this pull request Nov 8, 2026
…fig's needs

gotermboard's handover now drains a program's stderr through a pipe of its
own, so a child the program leaves running keeps writing to the terminal
rather than dying of SIGPIPE when the board comes back. OpenWindow takes
the programs a shell command runs as needs, so a window action passes its
config's needs and prs's own launch-time check goes away. A missing
program now reads "not on PATH: claude, so no window opened for review
widget #12", and a checkout that is gone is refused rather than opened in
another directory.

Agent-Name: claude-code-unattended
Agent-Session: b907ef76-acb0-4393-8a6d-92c06fe1dcfb
Agent-Run: 01a1ba32-a39f-79b5-aff6-be075373452b
Worklog: 2563.1
A failed editor or nvim run from prs showed only "exit status 1", because
prs's copy of the handover dropped the program's stderr. gotermboard now
owns the handover, the tmux window and the last-line reading, so prs calls
those and deletes its own copies.

Model.HandOver keeps the program's stderr as it still reaches the terminal,
and the comment key and d each say its last line where it fails. The
board's Receive reads the providers again on gotermboard.HandedBack, and
the model runs the key's own follow-up after. A window action still checks
its config's needs itself, because its command is a shell command tmux runs
whole, and OpenWindow looks up only a program given as its own argument.

gotermboard is pinned at the commit that adds these, and gotermstyle moves
with it to v0.4.0.

Agent-Name: claude-code-unattended
Agent-Session: b907ef76-acb0-4393-8a6d-92c06fe1dcfb
Agent-Run: 01a1ba24-d1b7-7852-8fe8-3682ac36a8af
Worklog: 2563.1
…fig's needs

gotermboard's handover now drains a program's stderr through a pipe of its
own, so a child the program leaves running keeps writing to the terminal
rather than dying of SIGPIPE when the board comes back. OpenWindow takes
the programs a shell command runs as needs, so a window action passes its
config's needs and prs's own launch-time check goes away. A missing
program now reads "not on PATH: claude, so no window opened for review
widget #12", and a checkout that is gone is refused rather than opened in
another directory.

Agent-Name: claude-code-unattended
Agent-Session: b907ef76-acb0-4393-8a6d-92c06fe1dcfb
Agent-Run: 01a1ba32-a39f-79b5-aff6-be075373452b
Worklog: 2563.1
@datapointchris
datapointchris merged commit 3edd3b4 into main Nov 8, 2026
2 checks passed
@datapointchris
datapointchris deleted the gotermboard-hand-over branch November 8, 2026 08:22
datapointchris added a commit that referenced this pull request Nov 8, 2026
…oss the width (#13)

The board drew each pull request on one line: its state, checks, review
and round, its age and
diff, its repo and number, then its title wrapping under itself. Its
branch and checkout sat
dim under the row, hidden until `f` showed them. The columns packed to
the left, so a wide
screen left most of its width empty, and nothing in the shape said which
field was which.

The first version of prs drew each pull request as a two-line block,
ruled apart from the
next, a glyph on each field, its columns spread across a screen with a
margin. This puts that
look back. The keys, the groups, the state words, the side panel and the
document are as they
were.

Session 69e19fe4-568c-41ef-9af1-316d086cd270 wrote this.

```text
 STATUS                        TITLE                           DIFF                        AGE        BRANCH
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 ▾ people · 3
      open                     Export the ledger as CSV           —                         9d           feature/ledger-export
      checks not run              ~/ledger      #4      review none

┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
      behind                   Cache the widget lookups          12    76    941            7d           add-cache
      checks passing              ~/widget      #12      review required

 ▾ bots · 1
      conflicts                Count cache hits                   3    25      2            5d           cache-metrics
      checks failing              ~/widget      #13   #12      review 2 posted      bot acme-automation
```

That is the board at 150 columns on the test fixtures, glyphs blanked
because a browser has no
Nerd Font.

## What to look at

1. `internal/board/rows.go`, `boardColumns` and `secondLine`: which
field sits on which line.
The first line is state, title, diff, age and branch. The second is
checks under the state,
then the checkout, the number with any stacked parent, the review, the
newest Claude round
   and the app that opened it.
2. `internal/board/rows.go`, `Look`: the margin is `sideMargin` from
`marginFrom` columns and
narrows below that. It also narrows from `gotermboard.PanelFrom` columns
up, so the side
panel, which gotermboard measures inside the margin, shows wherever it
would with none.
3. `internal/board/rows.go`, `numberCell`, `stackCell` and `botCell`:
the board's second line
   and `prs list`'s title line place the same cells.
4. `internal/board/table.go`: `prs list` keeps its one-line table
through `listColumns`, and
   builds its own title line now that a board row carries no `WrapLast`.
5. `cmd/prs/tui_test.go`: `HOME` is now the stand-in's directory, so a
checkout draws as
`~/widget` the way a real one reads, rather than as a long temporary
path that cut the
   second line short.

## How it was verified

- `go test ./...` passes. The TUI tests drive the built binary on a pty
against the stand-in
  GitHub and Bitbucket clients.
- The board was rendered on the stand-ins at 140, 150, 160, 170, 184,
300, 340 and 378
columns, and compared by eye with a capture of the first version at 150.
No line passes the
screen's edge at any width, by two separate measurements of each frame.
At 170 the panel
  draws beside the list inside a 5-column margin.
- `prs list --json` and `prs tui --json` still validate against
`cmd/prs/list.schema.json`.

## What changes

- **Each pull request takes two lines, with a dotted rule between
rows.** Groups keep their
  headers, counts and folds.
- **The columns spread across the width.** The diff's three counts keep
together, each
  right-aligned, so the signs stack.
- **The board keeps a margin**, with a rule under the column titles and
another over the keys.
The margin narrows on a narrow screen, and gives way to the side panel,
which shows from
  160 columns as before.
- **The cursor's two lines are bold on the theme's selection color**,
and rows draw no `▸`.
  Group headers keep theirs.
- **The branch is a column, and the checkout is always on the row**, so
`f` has nothing to show
and leaves the footer and `?`. The base a pull request merges into is in
its document.
- **A checkout reads `~`-rooted only inside the home directory at a path
segment**, so a
  sibling of home keeps its whole path.
- **`prs list` keeps its one-line table.** Its number and bot marks take
two spaces after their
  glyph, as every other field with words does.
- **gotermboard is pinned at the head of
datapointchris/gotermboard#10**, which adds the look.
The pin moves to the release that pull request cuts before this one
merges.

## Decisions, and what they rejected

- **Each line keeps one kind of field.** The status column holds the
state on the first line and
the checks on the second. Review, the round and the bot follow the
number on the second line.
*Rejected:* stacking the state, the checks and the review in the status
column, which makes
every row three lines where the first version's were two. *Rejected:*
ordering the words by
which one blocks the merge, which would give a line a different meaning
on each row.
- **The stack's `↑ #12` sits beside the number**, because a cell takes
one color and the title's
  cell no longer carries a line of marks after it.
- **The checkout reads `~`-rooted, or as owner/name with `not cloned
here` where none is.**
- **Rows draw no chevron**, because every row opens its document, so a
chevron would mark
  nothing apart. The first version had none.
- **The margin gives way to the side panel**, so a screen from 160 to
183 columns keeps the
panel and a narrower margin. *Rejected:* keeping 12 columns a side and
losing the panel there,
which also left `prs tui --once` at its default 160 columns with no
document.

## What this does not do

It adds no key and changes no key's behavior.

## The review

Round 1:
#13 (comment)

- **G1.1** fixed. `Look` narrows the margin from `gotermboard.PanelFrom`
columns up to half of
what the screen has past it, so the width inside the margin never drops
under the panel's
threshold.
`TestATerminalWideEnoughForTheSidePanelKeepsItInsideTheMargin` checks
every width
from `PanelFrom` to twice it, the terminals from 160 columns the finding
names. A render at
  170 draws the panel inside a 5-column margin.
- **G1.2** fixed. The README says the margin gives way to the panel
without its numbers, and
  CLAUDE.md names `PanelFrom` and the margin rather than 184.
- **code-architecture.1** fixed. `numberCell`, `stackCell` and `botCell`
sit beside
`checksCell` and `reviewCell`, and both layouts place them. The round
stays an optional
column in `prs list` and a field the second line skips where empty: a
table drops an empty
column, and a line of fields drops an empty field. CLAUDE.md says a
field both show has one
  builder.
- **code-architecture.2** fixed — same as G1.2.
- **stale-prose.1** fixed — same as G1.2 for the numbers. CLAUDE.md's
field list now names
  `boardColumns` and `secondLine` and keeps where a new field goes.
- **stale-prose.2** fixed, with the rewrite the finding gives.
- **fleet-patterns.1** fixed. `homeAsTilde` cuts at `home` plus the
separator, writes home
itself as `~`, and leaves the path whole where home is unknown.
*Rejected:* moving the helper
into a library prs and projects share. gotermboard and gotermstyle draw
terminals and hold no
path helpers, and the rule asks for the segment boundary rather than one
copy.
- **synthesis.1** fixed through G1.2, code-architecture.2 and
stale-prose.1.
datapointchris added a commit that referenced this pull request Nov 8, 2026
…those with i (#14)

After a review, a fixer pushes its fixes to the same branch, and whoever
reads the pull request next needs only what that push changed. The board
gave no sign that a reviewed branch had moved, and `d` opened only the
whole diff. GitHub records the commit each review was submitted against,
and the detail read already fetches it beside the branch's commits, so
this costs no read of its own.

This adds a field to each row's second line that counts the commits
since the newest review, and a key, `i`, that opens only those commits
in nvim's diff.

## What to look at

- `internal/board/actions.go`: `diffSinceReview` and `openDiff`. Check
the order of the cases in `diffSinceReview`'s chooser, which runs after
the fetch. The fetched head is the truth, so a branch force-pushed after
the detail read still opens the whole diff.
- `internal/pullrequests/fetch.go`: `HeadHolds` and `BaseMergedSince`.
Check that each git exit is read as yes, no or failure, and that only a
failure stops nvim.
- `internal/pullrequests/detail.go`: `NewestReview` and `SinceReview`.
Check the case where the reviewed commit is missing from a commit list
the read cut at 100, which reads as unknown rather than rebased.
- `internal/board/diff_files.go`: the since-review threads file leaves
out each `LEFT` thread and counts them. Check that against
`cmd/prs/diff-threads.schema.json` and the nvim panel that places
threads by side.
- `cmd/prs/tui_test.go`:
`TestARowCountsTheCommitsSinceItsReviewAndIDiffsOnlyThoseOrTheWholeDiffWithWhy`,
and `withPushesOf`, which stages the cases on the stand-in GitHub.
- `51efac4` is a separate fix to an existing test's wait, which raced
under the parallel suite.

## How it was verified

- `go test ./... -count=1` and `golangci-lint run`, green, then the pty
suite twice more in a row, green.
- The new test drives the built binary on a pty against the stand-in
GitHub, git and nvim. It covers four rows:
- Widget #12's head is two commits past mona's review. `i` runs
`DiffviewOpen 9e8d7c6…..refs/remotes/origin/pr/12`, and its threads file
reads `since_review` with one `LEFT` thread left out.
  - Gadget #5 has no review, so `i` opens the whole diff.
- Widget #13's review read a commit the stand-in git does not have, so
`i` opens the whole diff and names that commit.
- Ledger #4 is on Bitbucket, where `i` fetches nothing and starts no
nvim.
- The test failed when the row's text was broken, and when the Bitbucket
refusal was made to open the diff instead.
- `HeadHolds` and `BaseMergedSince` ran once against a real git
repository in a throwaway test, then deleted. It covered six branch
shapes: linear with the base moved, base merged in, base merged in and
moved again, reviewed after a merge, rebased, and a commit the checkout
lacks. Outside a repository, both returned git's `not a git repository`
line as an error.
- The branch where nothing was pushed since the review was run once with
a throwaway test, which saw the footer's line and no nvim start, then
deleted.

## What changes

- A GitHub row's second line reads `2 commits since review` or `1 commit
since review` after its newest review and its Claude round. It reads
`rebased since review` where the branch does not hold the reviewed
commit. A row nobody reviewed, or whose head is the reviewed commit,
reads nothing there.
- `i` on a row fetches its head as `d` does and opens nvim's diff from
the reviewed commit to the head.
- `i` opens the whole diff where no review was submitted, where the head
does not hold the reviewed commit, or where the base was merged into the
branch since the review. The footer says why once nvim exits.
- Where git cannot say whether the head holds the reviewed commit, `i`
opens nothing and the footer names git's failure.
- Where nothing was pushed since, the footer says so and nvim does not
start.
- `i` refuses on a Bitbucket row and on a recently closed one, because
prs reads neither's reviews, and before a GitHub row's detail lands.
- The threads file `i` writes reads `"diff": "since_review"`, names the
reviewed commit as `base`, holds only the `RIGHT` threads, and counts
the rest in `threads_left_out`. The one `d` writes reads `"diff":
"whole"` and `"threads_left_out": 0`. The document file beside it still
lists every thread.
- A merge after `i` pins the head `i` fetched, as one after `d` does.
- `i` is prs's own key now. A config whose window action binds `i` is
refused, and `prs tui` does not start until the config moves it.

## Decisions, and what they rejected

- **`i` for the key** — it stands for interdiff, the diff between two
versions of one change. `D` deletes in other terminal tools, and `f`,
`s`, `e`, `n`, `u`, `M` and `C` mean other things there. No common tool
binds a letter to this diff, so none was copied.
- **The fetched head decides, through git** — the row counts from the
detail read, which can be older than the head `i` fetches. Asking the
detail alone would diff from a commit a force-push since the read took
off the branch. `git rev-parse --verify --quiet <commit>^{commit}` finds
a commit missing from the checkout, and `git merge-base --is-ancestor`
answers for one present.
- **A base merge is found by where each side meets the base** — `git
merge-base <reviewed> origin/<base>` and `git merge-base <head>
origin/<base>` differ only where the base came into the branch after the
review.
- **A rebased branch opens the whole diff** — `git range-diff` would
compare the two versions of the commits across a rebase, but the whole
diff with the reason on the footer is the agreed behavior.
- **The threads file drops `LEFT` threads for `i`** — a `LEFT` thread's
line counts in the base branch's copy, and the left pane of this diff is
the reviewed commit's copy, so an nvim config placing it there would put
it on the wrong line. `diff` says which left side `base` is, and
`threads_left_out` says how many were dropped.
- **Smallest change that would work** — `DiffviewOpen
<reviewed>..<head>` with no check that the head holds the reviewed
commit. On a rebased branch git either refuses the unknown commit or
diffs two unrelated trees, so it is ruled out.

## What this does not do

- It adds nothing to `prs list` or either `--json` output, because
neither reads a pull request's detail. `prs tui --help` now says so.
- It does not count commits for Bitbucket rows, which have no detail
read.
- A merge from the base counts as one commit since review on the row.
`i` then names the merge as why it opened the whole diff.

## The review

Round 1:
#14 (comment).
Every fix is in `3f2cb47`.

- **G1.1** fixed — `i` on a recently closed row, or one no read lists,
says prs reads none of its reviews and points at `d`. "Still being read"
shows only while a read is under way.
- **G1.2** fixed — `BaseMergedSince` compares where the reviewed commit
and the head each meet `origin/<base>`, and `i` opens the whole diff
where they differ. Its reach is unproven, so it has no committed test. A
throwaway test against real git covered it. The row's `base merged since
review` was not built: the detail read holds no commit parents, and the
count of one pushed commit is true.
- **G1.3** fixed — prose says "the diff keys" or "a diff key" rather
than listing `d` and `i`, in CLAUDE.md, README, the threads schema and
the code comments.
- **stale-prose.1** fixed — README and `sinceReviewCell`'s comment say a
blank means prs could not tell, or there is nothing to count.
- **stale-prose.2** fixed — `keyTable`'s comment and CLAUDE.md drop the
claims about what other tools bind. Each says only that `D` is left
unbound because it deletes elsewhere.
- **stale-prose.3** fixed — a review GitHub names no commit for opens
the whole diff with its own reason, apart from the gone-commit one.
- **useless-test.1** fixed — the `?` overlay checks and the `D` check
are deleted, and so is "D binds nothing" in the test's comment.
- **useless-test.2** fixed — the footer waits match the values each line
carries: `bitbucket.*#4`, `diff of widget #12.*9e8d7c6`, `whole diff of
gadget #5` and `whole diff of widget #13.*5ca1ab1`. The Bitbucket
refusal is pinned by the fetch and nvim it does not run.
- **failure-paths.1** fixed — same as synthesis.1.
- **fleet-patterns.1** fixed — same as synthesis.1.
- **synthesis.1** fixed — `HeadHolds` returns `(bool, error)`. A missing
commit and `--is-ancestor`'s exit 1 read as no. Any other exit opens
nothing and names git's line on the footer.
- **contracts.1** fixed — the threads file gains `diff` and
`threads_left_out`, both required in the schema, so `base` reads one way
for each kind of diff.
- **contracts.2** fixed — README § "Keys of your own" says a window
action on a key prs binds is refused and stops `prs tui`. It names `?`
and `prs config show` as where the bound keys are read, rather than
listing letters a release can add to. This body names `i` as a key a
config can no longer bind. *Rejected:* skipping a clashing window action
with a footer warning, because the action would vanish where the refusal
names it.
- **contracts.3** fixed — `prs tui --help` says `--once` and `--json`
read no pull request's detail, and names the review round, the commits
since review and the document sections that leave out.

Session: 3689c9b4-3208-4138-b764-71b0ed5b83a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant