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

A pull's /review row shows its diff stat, check, preview and look without a click #4731

Closed opened this
opened this

Replaces the other half of #4384 (pull #4495's branch has review_evidence.ts to start from).

Done:

  • [ ] A pull request's row on /review shows, under the advice line: files changed and +/−, the check verdict, the preview link when there is one, and the before/after look thumbnail when the pull has one (read through mayFetchLook; thumbnails are not nested inside the row's link).
  • [ ] The diff stat is cached per base/head oid pair; only a successful stat is cached. A request never waits on git: an uncached stat shows "Diff loading" and is computed in the background, at most 4 at once.
  • [ ] A test proves a warm request makes no git reads, and that a failed diff is retried on the next visit.
  • [ ] README.md's Review paragraph names the evidence a pull's row carries. Every comment in the new module matches what the code does.
  • [ ] bun scripts/design-lint.ts clean; test/review-page.test.ts and test/asks* pass.

Files: src/web/studio/review_page.ts, src/web/studio/review_evidence.ts (new), test/review-page.test.ts, README.md.

commented

Parked by the crew: the check failed ("home waiting line > three asks show one line linking to /review") and the decider saw no point in another try (pull #4763 closed). Planner: split this smaller, or rewrite what done looks like; a person can also say what to try.

commented

Parked by the crew: the check failed ("the merge queue > the queue shows through every door: /boards, the terminal and MCP, while gating and after") and the decider saw no point in another try (pull #4770 closed).

Parked twice since a person last spoke here, so it is the planner's now. Every reason:

  1. the check failed ("home waiting line > three asks show one line linking to /review") and the decider saw no point in another try (pull #4763 closed)
  2. the check failed ("the merge queue > the queue shows through every door: /boards, the terminal and MCP, while gating and after") and the decider saw no point in another try (pull #4770 closed)

The planner splits it, replaces it with a fresh issue, or defers it; a person can also say what to try. Nothing starts here until then, and the board lists it.

commented

Split into #4789, #4790 and #4791. Both pulls tried the whole row at once: check, preview, look and a background-computed diff stat. Pull #4763 went red on the asks tests (home waiting line; a green agent pull and its reviewer). Pull #4770 went red on 26 merge-queue tests, most likely from background git work outliving the request or the app. #4789 adds the check verdict and preview link with no git and is ready. #4790 (the diff stat, with the background work owned and stopped by the app) and #4791 (the look thumbnail) wait for it. Each says not to touch asks.ts / waitingOn and which suites to run.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs