Skip to content

feat: show stack position in the pull request list - #1041

Merged
mariusvniekerk merged 4 commits into
mainfrom
t3code/show-stack-counts-in-pr-list
Sep 4, 2026
Merged

feat: show stack position in the pull request list#1041
mariusvniekerk merged 4 commits into
mainfrom
t3code/show-stack-counts-in-pr-list

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Stacked pull requests were indistinguishable from standalone ones in the sidebar. You had to open each one to learn whether it was part of a stack and where it sat. Each list row now carries the stack icon with the pull request's position and stack size, placed just before the CI cluster, so a stack reads at a glance from the list.

  • Sidebar PR rows show a stack indicator such as "4/7" when the pull request belongs to a stack with more than one visible member. The hover title and accessible name read "Stacked: 4/7".

  • The detail view's stack chip drops the word "Stacked" and shows only the icon and count. The accessible name keeps the full wording.

  • The pull list API returns a per-row stack placement so the sidebar does not fetch stack context for every row. Members hidden as removed upstream are excluded and the rest renumbered, matching the detail stack context.

  • Also splits the workspace recency typography e2e test into desktop and phone tests. It made four dev-server page loads inside the 30 second per-test budget and failed its first attempt in every CI job since it landed, passing only on retry.

Sidebar row with the new indicator before the CI cluster, and the shortened detail chip (synthetic fixture data):

stack-indicator-row.png

stack-indicator.png

🤖 Generated with Claude Code (claude-fable-5-1)

@roborev-ci

roborev-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

roborev: Combined Review (b1c36a0)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 3m51s

@roborev-ci

roborev-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

roborev: Combined Review (f6d1202)

Verdict: One medium-severity issue requires attention; otherwise, no security concerns were identified.

Medium

  • internal/db/queries_stacks.go:385-400 — The query binds every merge-request ID twice. Because /pulls has no effective result cap, sufficiently large lists can exceed SQLite’s bind-parameter limit and cause the entire request to return an internal error, even when no stack exists. Batch the IDs or bind them once through a CTE while preserving stack window calculations across all members.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 4m4s

Docs Test and others added 3 commits September 4, 2026 12:00
Maintainers working through a stack had to open each pull request to learn
whether it was stacked and where it sat. The sidebar showed workspace, review,
and CI indicators but nothing about stacks, so a stacked pull request looked
the same as a standalone one.

Each list row now carries a compact stack indicator: the stack icon followed
by the pull request's position and the stack size, for example "4/7". It sits
just before the CI cluster in the row's indicator group. The list API returns
this placement directly so the sidebar does not fetch the full stack context
for every row. Only stacks with more than one visible member are reported,
matching when the detail view shows its stack chip. Members hidden as removed
upstream are excluded and the remaining members are renumbered, so the list
and the detail view always agree on position and size.

The detail view's stack chip drops the word "Stacked" from its visible label.
The icon already says that, so the chip now shows just the position and size.
The accessible name keeps the full wording for screen readers.

Generated with Claude Code (claude-fable-5-1)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The stack-status e2e test looked up the detail chip with a page-wide button
role query for "Stacked: 2/2". The sidebar row is also a button, and its
accessible name now includes the new stack indicator label, so the query
matched both elements. The row's label reflects the list response and does
not change when the detail refresh replaces the stack, which made the "no
longer 2/7" assertion fail in CI.

Assert the chip's accessible name through its test id instead, which is the
element the test is about. The testing context notes the row-name behavior so
future chip assertions are scoped from the start.

Generated with Claude Code (claude-fable-5-1)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…udget

The workspace recency typography test failed its first attempt in every e2e
mock job since it landed, on main and on pull requests, and only passed on
retry. It made four full page loads in one test. The 30 second limit applies
to the whole test, and each load goes through the Vite dev server, which
slows several-fold while 14 workers share it. On retry the server is quieter,
so the same test passed and the run reported it as flaky.

Split it into a desktop test and a phone test with two loads each. The
assertions are unchanged. The testing context records the budget rule so new
tests keep their navigation count small.

Generated with Claude Code (claude-fable-5-1)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@mariusvniekerk
mariusvniekerk force-pushed the t3code/show-stack-counts-in-pr-list branch from f6d1202 to b62bf78 Compare September 4, 2026 16:08
@roborev-ci

roborev-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

roborev: Combined Review (b62bf78)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 3m45s

The pull list endpoint has no size cap. The stack placement lookup expanded
the merge request IDs into placeholders and bound each one twice, so a list
of about 16,000 pull requests would exceed SQLite's 32,766 bound-variable
limit and turn the whole list response into a 500. The sibling worktree link
lookup already batches for the same reason.

Bind the IDs once as a JSON array through json_each instead. The database
test now includes an ID set of 70,000 to prove the lookup survives beyond the
limit.

Also add a phone-viewport e2e case for the sidebar stack indicator. The
indicator is enabled on phone rows, but coverage ran only at desktop width,
so the row's fit beside the review, CI, and time indicators was unchecked.

Generated with Claude Code (claude-fable-5-1)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

roborev: Combined Review (666b232)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 5m12s

@mariusvniekerk
mariusvniekerk merged commit 7fe95bd into main Sep 4, 2026
17 checks passed
@mariusvniekerk
mariusvniekerk deleted the t3code/show-stack-counts-in-pr-list branch September 4, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant