Skip to content

Rearrange some information about a PR in the right pane - #34

Merged
frenchie4111 merged 7 commits into
ness-dev:mainfrom
big-guy:pr-pane-rearrange
May 23, 2026
Merged

Rearrange some information about a PR in the right pane#34
frenchie4111 merged 7 commits into
ness-dev:mainfrom
big-guy:pr-pane-rearrange

Conversation

@big-guy

@big-guy big-guy commented May 14, 2026

Copy link
Copy Markdown
Collaborator

PR pane layout

  • #nnn rendered as a state-colored outlined pill at the start of the
    title row (border + text use success/warning/accent/danger per state).
  • Title is draggable + opens the PR in the browser on click.
  • Assignees render as an avatar stack at the left of the second row.
  • Milestone rendered as its own pill linking to the milestone page.
  • Target branch displayed in the second row (warning color when not
    the repo default); baseBranch/isDefaultBase added to PRStatus and
    populated by the GitHub fetcher.
  • Labels rendered as colored pills above linked issues with a
    luminance-aware text color.
  • Linked issues (Closes #N / GitHub link-issue UI) shown with open/
    closed state icons under the merge row.
  • Checks sorted by status (failures → pending → success/neutral) and
    then by start time.

Merge button

  • Lives in the panel header actions slot, labeled by strategy
    ("Squash+Merge" / "Merge" / "Rebase+Merge").
  • Tooltip "Merge the pull request"; confirm step "Confirm merge into
    ".
  • Disabled with red styling + GitMergeConflict icon when the PR has
    merge conflicts; tooltip "There are merge conflicts".
  • Hidden while the PR is in the merge queue.

Open / unmerged PRs

  • Show "↓ N" chip next to the target branch when behind base
    (new fetchBehindBy via GitHub's compare endpoint).

Merged PRs

  • Hide the behind chip; show the earliest local tag containing the
    merge commit (accent "vX.Y.Z") or italic "unreleased" when no tag
    yet contains it (new getFirstTagContaining git tag --contains lookup
    in main; firstReleaseTag field on PRStatus).

Merge queue

  • New fetchPRExtras GraphQL helper (5th parallel call in the detail
    fanout) returns mergeQueueEntry and closingIssuesReferences.
  • State pill shows "Queued (Nnd · ~12m)" while enqueued; merge button
    hidden while in queue.

Drag-and-drop into terminal

  • XTerminal's drop handler accepts in-app text/plain payloads in
    addition to OS file drops, wrapping in bracketed-paste markers.
  • Source panels set drag payloads:
    • Changed files / all files: "@ "
    • PR checks (with detailsUrl): URL
    • PR title + linked issues: URL
    • Commits: full commit SHA
  • Existing click handlers still work — dragging only fires on a real
    drag gesture.

Changed Files toolbar

  • Refresh moved rightmost; Review button contrast bumped.

Current:
image

With PR:
image

@big-guy
big-guy force-pushed the pr-pane-rearrange branch from 3ca371d to 2c659ba Compare May 20, 2026 19:11
@frenchie4111

frenchie4111 commented May 21, 2026

Copy link
Copy Markdown
Collaborator

this change looks great to me so far - not sure if you added it but it would be nice to auto-sort the checks so failed ones are at the top nevermind I missed your last bullet point

@big-guy
big-guy force-pushed the pr-pane-rearrange branch 3 times, most recently from 29f1a37 to 7e69451 Compare May 21, 2026 22:06
@big-guy
big-guy marked this pull request as ready for review May 21, 2026 22:17
@big-guy

big-guy commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

I played around with this a little more. I wanted to make it so the "action" (merge) button is on the header and everything else is just information.

@big-guy
big-guy force-pushed the pr-pane-rearrange branch from 7e69451 to 6b172ff Compare May 22, 2026 16:23
big-guy and others added 3 commits May 22, 2026 15:18
Every GH fetch now funnels through trackedFetch, which logs
`[github-api] METHOD /path → status (Nms)` to debug.log and bumps a
PerfMonitor counter. The HUD shows "GH API" as calls/sec alongside the
existing Store/IPC/Terminal metrics, with a toggleable chart line.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- New `expandedDiagnosticLoggingEnabled` setting (default off) gates
  the per-call `[github-api]` debug.log lines added in the previous
  commit. HUD's "GH API" rate metric is unaffected — it stays on.
- PerfMonitor now keeps a 60-bucket rolling count of GH API calls
  in the last hour, surfaced as `githubApiCallsLastHour` and shown
  as "GH (1h)" in the HUD scalar row.
- Widen the HUD from 520 to 580px (chart 500 → 560) so longer scalar
  labels like "Memory (render)" no longer wrap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fits more naturally beside the debug-log open/show buttons than in the
GitHub auth section.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR pane layout
- #nnn rendered as a state-colored outlined pill at the start of the
  title row (border + text use success/warning/accent/danger per state).
- Title is draggable + opens the PR in the browser on click.
- Assignees render as an avatar stack at the left of the second row.
- Milestone rendered as its own pill linking to the milestone page.
- Target branch displayed in the second row (warning color when not
  the repo default); baseBranch/isDefaultBase added to PRStatus and
  populated by the GitHub fetcher.
- Labels rendered as colored pills above linked issues with a
  luminance-aware text color.
- Linked issues (Closes #N / GitHub link-issue UI) shown with open/
  closed state icons under the merge row.
- Checks sorted by status (failures → pending → success/neutral) and
  then by start time.

Merge button
- Lives in the panel header actions slot, labeled by strategy
  ("Squash+Merge" / "Merge" / "Rebase+Merge").
- Tooltip "Merge the pull request"; confirm step "Confirm merge into
  <branch>".
- Disabled with red styling + GitMergeConflict icon when the PR has
  merge conflicts; tooltip "There are merge conflicts".
- Hidden while the PR is in the merge queue.

Open / unmerged PRs
- Show "↓ N" chip next to the target branch when behind base
  (new fetchBehindBy via GitHub's compare endpoint).

Merged PRs
- Hide the behind chip; show the earliest local tag containing the
  merge commit (accent "vX.Y.Z") or italic "unreleased" when no tag
  yet contains it (new getFirstTagContaining git tag --contains lookup
  in main; firstReleaseTag field on PRStatus).

Merge queue
- New fetchPRExtras GraphQL helper (5th parallel call in the detail
  fanout) returns mergeQueueEntry and closingIssuesReferences.
- State pill shows "Queued (Nnd · ~12m)" while enqueued; merge button
  hidden while in queue.

Drag-and-drop into terminal
- XTerminal's drop handler accepts in-app text/plain payloads in
  addition to OS file drops, wrapping in bracketed-paste markers.
- Source panels set drag payloads:
  - Changed files / all files: "@<path> "
  - PR checks (with detailsUrl): URL
  - PR title + linked issues: URL
  - Commits: full commit SHA
- Existing click handlers still work — dragging only fires on a real
  drag gesture.

Changed Files toolbar
- Refresh moved rightmost; Review button contrast bumped.

Internal
- Reducer + sort tests extended for the new fields; pr-poller stub
  updated to include all newly required PRStatus fields.
- usePRMergeAction extracted as a hook returning {button, errorRow}
  so the button can render in the panel header while the error row
  stays in the body.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@frenchie4111

Copy link
Copy Markdown
Collaborator

im trying to review this but ironically my gh rate limit is all used up. give me a bit but I can merge it today

@big-guy

big-guy commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

im trying to review this but ironically my gh rate limit is all used up. give me a bit but I can merge it today

I'm trying a version of this locally that uses graphql to see if that cuts down the number of GH API calls. This PR added ~50% more calls for me.

Replace the per-repo "list 100 most-recent + match by ref/sha + 5 detail
calls per PR" flow with a single aliased GraphQL request that looks up
each worktree's PR by headRefName. Two wins:

* Stale long-lived branches whose PR fell out of the top-100-most-recent
  window now show their status correctly.
* Call volume drops from ~6N+1 REST calls per repo refresh to roughly
  N+1 (one GraphQL plus per-PR behind_by compares).

Dedupes checks by name since statusCheckRollup returns an entry per
re-run, which collided with the renderer's name-keyed list.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@big-guy
big-guy force-pushed the pr-pane-rearrange branch from 6b172ff to dc49db6 Compare May 22, 2026 22:38
@big-guy

big-guy commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

The latest commit appears to drop the number of GH requests by a lot. Curious if you see the same thing.

@frenchie4111

Copy link
Copy Markdown
Collaborator

@big-guy this looks awesome, my only comment is that milestones are not a universally used feature, so it might be best for the "No milestone" state to be invisible or optional somehow

image

@frenchie4111

frenchie4111 commented May 22, 2026

Copy link
Copy Markdown
Collaborator

The latest commit appears to drop the number of GH requests by a lot. Curious if you see the same thing.

it seems like we are at N+1 now, which is much better than where we started, so I am down to merge this PR. However, from my investigation we could probably do 1 graphql call for the most recent 100 PRs and then backup to doing another call for the missing PRs. Likely that would reduce further (although graphql calls seem to be charged differently based on some points math)

@big-guy

big-guy commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

my only comment is that milestones are not a universally used feature, so it might be best for the "No milestone" state to be invisible or optional somehow

A few ideas:

  • Hide the milestone if there's no milestone set. This isn't great for how I use it, but not a big loss.
  • Only hide the milestone if there are no milestones defined for the repo.
  • Add per-repo settings for which details are included. e.g., we could drop labels, linked issues, diff stat, etc.

Do you want to merge first and I'll do one of these next? WDYT?

However, from my investigation we could probably do 1 graphql call for the most recent 100 PRs and then backup to doing another call for the missing PRs.

Yep, this sounds like a good idea, the fewer per worktree calls the better.

Adds repository.milestones(first:1){totalCount} to the batched GraphQL
request — no extra round-trip. PRs now carry a hasMilestones flag and
the PR pane skips rendering the milestone pill (or its "No milestone"
placeholder) entirely when the repo doesn't use milestones.

Tests cover all four states of the (hasMilestones × milestone) matrix
via an extracted milestoneDisplay helper.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@big-guy

big-guy commented May 23, 2026

Copy link
Copy Markdown
Collaborator Author

I ended up doing this:

Only hide the milestone if there are no milestones defined for the repo.

… the PR's head ref

`gh pr checkout` creates a synthetic local branch (e.g. `pr-37320-...`)
that doesn't match the upstream PR's `head.ref`, so a headRefName-only
GraphQL lookup misses the PR entirely. Adds a parallel `object(oid:
$sha).associatedPullRequests` sub-query that runs in the same request,
no extra round-trip. pickPRBySha already prefers SHA-matched candidates,
so the combined result resolves to the correct PR.

Tests cover the gradle/gradle scenario, the SHA-disambiguation case
where branch and SHA lookups return different PRs, dedup when both
return the same PR, and the absence of the SHA alias when the worktree
HEAD isn't a 40-char SHA.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

2 participants