Say how much feedback is still unanswered, in the lists a reviewer scans #83

Merged

a73x merged 1 commit from worktree-agent-a92ea79c409b34466 into main

2026-08-13 07:10:32 +00:00

a73x commented

2026-08-13 07:08:08 +00:00

(Migrated from git.a73x.sh)

28fc6751 made one thread's answered-ness visible on patch show. That answers "is this thread done" one patch at a time, which is the wrong shape for the question a reviewer starts with: which patches have unanswered feedback. Answering it meant opening every patch.

PatchState::unresolved_comments is the one definition behind the number, and patch list, its --json, the dashboard and the web list all call it. Putting it on the folded state is what stops a fourth divergence of the kind 10bb2d84 argued about ids and 952390ad is arguing about timestamps. It is derived rather than serialized, so it never reaches the on-disk fold cache — the same argument that keeps looks_merged off PatchState — and it costs a pass over a Vec the list already folded, not a second walk of the DAG.

The decision the issue names

A stale claim counts as answered. A resolution keeps the revision it was pinned to, and one older than the tip is shown as resolved by Alice at r1 — 2 revisions landed since: a hint, deliberately not a state change. Counting it as unanswered would make that state change in every list view — exactly what 28fc6751 refused. It would also be unactionable: staleness is a property of the pair (claim revision, current tip), not of the thread, so a revision touching an unrelated file would re-raise feedback nobody reopened, and the only way back to zero would be to re-resolve every thread on every revision. That is the noisier question "not re-confirmed against the tip", not the one a review queue asks. The caveat stays on the surface with room to spell it out.

A withdrawn comment is not outstanding. A tombstone has no words left to answer; counting it would leave a row clearable only by "resolving" a comment that says nothing. A non_blocking comment is counted — excluding it would let the count be silenced by a flag rather than by an answer.

What each surface renders

patch list appends (N unresolved), omitted at zero like (N new) beside it. --json takes the opposite rule and always carries unresolved_comments, because a caller cannot tell an absent key from a zero. Full ids unchanged.
The dashboard puts !N between the status and the title. The list pane is ~28 columns, so a suffix would be truncated away on every real repository.
The web list gains an Unresolved column, blank at zero.

Filters

CLI --unresolved is a flag that composes with the status axis (--unresolved -a widens to closed). The dashboard binds u on the patch pane, a second axis rather than a fourth value of the status cycle. The web adds ?filter=unresolved, a fifth value of a single-select bar that implies open. The shapes differ because a flag composes and a link bar does not; the number means the same thing on all three. An empty result names the filter instead of claiming the repository is empty.

Verification

13 new integration tests plus 8 dashboard render tests, TDD throughout. A test snapshots every collab ref across patch list, --unresolved, --json and both web pages and asserts nothing moved. cargo clippy --all-targets silent.

Note: tests/tui_review_test.rs is flaky on main independently of this change — measured 4/4 failing on a clean main checkout with the same two editor-abort tests, identical to this branch.

Migrated from git-collab patche d452edbb1045a5be29e42d5b61e9ee51e5003924.

Original author: a73x dev@a73x.sh.

Fixes #63

Revision 1: 51e7d5ae1d6e5f1c76a1bb39faf36a66b0213141; base 15bc9a3c2f8c1622b6eb5157c6176452ba7569bb

`28fc6751` made one thread's answered-ness visible on `patch show`. That answers "is this thread done" one patch at a time, which is the wrong shape for the question a reviewer starts with: *which* patches have unanswered feedback. Answering it meant opening every patch. `PatchState::unresolved_comments` is the one definition behind the number, and `patch list`, its `--json`, the dashboard and the web list all call it. Putting it on the folded state is what stops a fourth divergence of the kind `10bb2d84` argued about ids and `952390ad` is arguing about timestamps. It is derived rather than serialized, so it never reaches the on-disk fold cache — the same argument that keeps `looks_merged` off `PatchState` — and it costs a pass over a `Vec` the list already folded, not a second walk of the DAG. ## The decision the issue names **A stale claim counts as answered.** A resolution keeps the revision it was pinned to, and one older than the tip is shown as `resolved by Alice at r1 — 2 revisions landed since`: a hint, deliberately not a state change. Counting it as unanswered would make that state change in every list view — exactly what 28fc6751 refused. It would also be unactionable: staleness is a property of the pair (claim revision, current tip), not of the thread, so a revision touching an unrelated file would re-raise feedback nobody reopened, and the only way back to zero would be to re-resolve every thread on every revision. That is the noisier question "not re-confirmed against the tip", not the one a review queue asks. The caveat stays on the surface with room to spell it out. **A withdrawn comment is not outstanding.** A tombstone has no words left to answer; counting it would leave a row clearable only by "resolving" a comment that says nothing. A `non_blocking` comment *is* counted — excluding it would let the count be silenced by a flag rather than by an answer. ## What each surface renders - `patch list` appends ` (N unresolved)`, omitted at zero like `(N new)` beside it. `--json` takes the opposite rule and always carries `unresolved_comments`, because a caller cannot tell an absent key from a zero. Full ids unchanged. - The dashboard puts `!N` between the status and the title. The list pane is ~28 columns, so a suffix would be truncated away on every real repository. - The web list gains an Unresolved column, blank at zero. ## Filters CLI `--unresolved` is a flag that composes with the status axis (`--unresolved -a` widens to closed). The dashboard binds `u` on the patch pane, a second axis rather than a fourth value of the status cycle. The web adds `?filter=unresolved`, a fifth value of a single-select bar that implies open. The shapes differ because a flag composes and a link bar does not; the number means the same thing on all three. An empty result names the filter instead of claiming the repository is empty. ## Verification 13 new integration tests plus 8 dashboard render tests, TDD throughout. A test snapshots every collab ref across `patch list`, `--unresolved`, `--json` and both web pages and asserts nothing moved. `cargo clippy --all-targets` silent. Note: `tests/tui_review_test.rs` is flaky on main independently of this change — measured 4/4 failing on a clean `main` checkout with the same two editor-abort tests, identical to this branch. --- Migrated from git-collab patche `d452edbb1045a5be29e42d5b61e9ee51e5003924`. Original author: a73x <dev@a73x.sh>. Fixes #63 Revision 1: `51e7d5ae1d6e5f1c76a1bb39faf36a66b0213141`; base `15bc9a3c2f8c1622b6eb5157c6176452ba7569bb`