The open pull request list refreshes again on a busy repo, and says so when it couldn't - #111
Merged
Merged
Conversation
… request, so a busy repo's list refreshes again, and a list that couldn't refresh says so - The list was `gh pr list --json ...,statusCheckRollup`, which asks for every check on every pull request's head commit. On a repo with 83 open pull requests and 30 to 60 checks each, GitHub answers that with an HTTP 504 every time. The call failed on every beat, the last good list stayed on screen, new pull requests never showed and merged ones never left. - pull_request::list now runs one `gh api graphql` query with `gh pr list`'s own fields and order, and reads checks from the rollup's `state` on the last commit. That is GitHub's own pass / fail / pending word, computed server side. On cli/cli it matched the old per-check fold on all 62 open pull requests. `gh pr view` and the detail fetch still fold every check, since those ask about one pull request. - A failed list lookup is now remembered per project (App::open_prs_failed). The PULL REQUESTS MODAL shows "couldn't refresh (^r retries)" on its own row under the filter, over the last rows that worked, or "couldn't ask GitHub" when there are none. It is a row, not a title suffix, so a narrow modal can't cut it off. The next answer clears it. - Tests cover the GraphQL answer shape, the rollup word mapping, the query asking for no check contexts, the failure mark being set and cleared, and the modal notice with its hit area. - Docs (how-it-works, keys, configuration) describe the query and the notice. Closes #106 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…query, and a pr-list-stale scene shows a list that couldn't refresh - nebula now asks for the open pull request list with `gh api graphql` instead of `gh pr list`. Without this, every PR scene's stand-in gh would fail that call and the grid and modal would render "couldn't ask GitHub". - The stub hands back fixtures/pr-list.json, still in `gh pr list` rows, as the GraphQL answer. A row's statusCheckRollup checks become the rollup state on its last commit, so the existing fixtures and scenes keep working unchanged. - fixtures/pr-list.ok-count holding N answers the first N list calls and fails every later one the way GitHub's 504 did. - New scene pr-list-stale reuses pr-row-conflicts's fixtures with an ok-count of 1: `v`, then Ctrl+r, shows the modal saying "couldn't refresh (^r retries)" over the rows it kept. Refs #106 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #106. On a busy repo the open pull request list stopped updating, because
gh pr listasked GitHub for every CI check on every pull request and GitHub timed out every time. The list now asks for GitHub's single pass / fail / pending verdict per pull request, which comes back in about 2 seconds. When a refresh does fail, the PULL REQUESTS MODAL now says so.Contents: 🐛 Symptom · 🔍 Cause · ✅ Fix · 📸 Before / After · 🔁 State ·⚠️ Risk · 🔧 Technical overview · 🧪 Proof · 📝 Notes
🐛 Symptom
The report came from a private repo with 83 open pull requests, averaging 31 checks each and up to 58. On nebula v0.40.2,
vlisted 72 pull requests. The 15 newest never showed up, and 4 that had merged or closed stayed listed.pr-cache/pull-requests.jsongot a freshsaved_atevery few minutes, but its list never changed, and nothing on screen said it was stale.🔍 Cause
The list ran
gh pr list --json …,statusCheckRollup.ghfills that field with every check context on each pull request's head commit, which on that repo is thousands of entries. GitHub's GraphQL API answered withHTTP 504after about 11 s on every call. nebula treats a failed lookup as a one-off: it keeps the last list, retires nothing, and backs off for up to 10 minutes. That is right for a flaky network, but this call never succeeded, so the list stayed on its last good answer indefinitely. The failure was silent because nothing recorded that the last ask had failed.✅ Fix
statusCheckRollup { state }on each pull request's last commit. That is the ✓ / ✗ / pending GitHub already shows on the pull request page.gh pr listgave.failingfor failed checks,conflictsfor merge conflicts.v) showscouldn't refresh (^r retries)in the warn color on its own row under the filter.couldn't ask GitHub (^r retries)instead of the misleadingno open pull requests.refreshing…as before. The next answer clears the notice.gh pr view) and the reading pane's detail fetch still fold every check. Each asks about a single pull request, which never timed out.📸 Before / After
The same scene on both builds. The list answers once at boot, then every later ask fails the way GitHub's 504 did, and
Ctrl+rasks again inside the modal. The footer'srefreshing pull requests…is theCtrl+rpress's own flash in both shots.🔁 State
flowchart TD T["OPEN PRS beat / Ctrl+r"] --> Q{"which call?"} Q -- before --> OLD["gh pr list --json …,statusCheckRollup<br/>every check on every PR"] OLD -- "HTTP 504 after ~11 s, every time" --> MISS["answer: None"] MISS --> KEEP["keep last list, back off ≤ 10 min"] KEEP --> SILENT["modal looks current"] Q -- after --> NEW["gh api graphql LIST_QUERY<br/>statusCheckRollup { state }"] NEW -- "~2 s" --> ROWS["rows + health"] ROWS --> CLEAR["open_prs_failed cleared"] NEW -- "still fails (no gh, offline)" --> MARK["open_prs_failed marked"] MARK --> NOTE["modal: couldn't refresh (^r retries)"] classDef bad fill:#fdd,stroke:#c33,color:#000 classDef good fill:#dfd,stroke:#3a3,color:#000 class OLD,SILENT bad class NEW,NOTE goodVerdict: 🟢 Low risk. The change only swaps which
ghsubcommand runs, uses a constant query, and adds one warning row to the modal.ghthe TUI already runs, with a constant query.{owner}/{repo}are filled in byghfrom the checkout, the same waygh pr listresolves its repo, and no user text reaches the call. A repoghcan't resolve, or a GraphQL error, exits non-zero and reads as "couldn't ask", as before.cli/cli). The draw adds oneHashSetlookup.open_prs_failedfollowspr_detail_failed's pattern: a set of keys whose last ask failed, cleared by the next answer and pruned with the projects. The call goes through the samegh()helper andTIMEOUT, and the notice row is drawn with the modal's existing row helpers.Rollback:
git revertthe merge bringsgh pr listback. Nothing persisted changes shape (theOpenPrrows and the PR CACHE are untouched). The screenshots onpr-assetsstay.🔧 Technical overview
crates/nebula-tui/src/pull_request.rs:listnow runsgh api graphql -F owner={owner} -F repo={repo} -F limit=100 -f query=LIST_QUERY. The query hasgh pr list's own fields pluscommits(last: 1) { … statusCheckRollup { state } }.parse_listreads rows fromdata.repository.pullRequests.nodes.healthreads the rollupstatewhen a node carries it (SUCCESSpasses,FAILURE/ERRORfail,PENDING/EXPECTEDare pending,nullis absent). Otherwise it falls back to the old per-check fold, whichgh pr viewand the detail payload still use.crates/nebula-tui/src/event_loop.rs:note_open_prs_answermarks or clearsApp::open_prs_failed(declared incrates/nebula-tui/src/app.rs). Removed projects are pruned from it the same way asopen_prs.crates/nebula-tui/src/pr_modal.rsdraws the notice row and shifts the rows' hit area under it, so clicks still land on the right row.scripts/shot/bin/ghanswers the GraphQL call from the samepr-list.jsonfixtures, converting each row's checks into a rollupstate, so every existing PR scene renders as before.pr-list.ok-countmakes it fail after N answers, and the newpr-list-stalescene uses that.cli/cli: 0 disagreements.refreshing…torefres. The one thing this has to say must not be the part that gets clipped.🧪 Proof
a_list_rows_checks_are_the_rollup_stateandthe_list_query_asks_for_the_rollup_state_aloneincrates/nebula-tui/src/pull_request.rs. The second pins that the query asks for no check contexts.a_list_that_could_not_be_refreshed_says_soincrates/nebula-tui/src/pr_modal.rs: the notice, the kept rows, the shifted hit area,refreshing…during a retry,couldn't ask GitHubwith no rows, and the notice clearing.the_open_pr_list_backs_off_when_empty_and_survives_a_failed_callincrates/nebula-tui/src/event_loop.rsnow also checks that the failure mark is set and then cleared.list()againstcli/clireturned 62 rows in 3.0 s, withPassing 41 · Failing 5 · Absent 16and 30 conflicting. The reporter's private repo was not available, so the 83-PR case is covered by the reporter's own timing of this query (all 83 in 2.0–2.5 s), not by a run here.origin/main(5938529) plus this change:cargo fmt --checkandmake lint(clippy) are green.cargo test --workspace --no-fail-fast: 1685 passed, 3 failed.e2e_tuitests that also fail on untouched main (nebula_open_from_inside_a_session_raises_the_file_tabs,tui_drag_past_the_pane_top_autoscrolls_and_copies_the_run; theirstart_sessionwaits on an old footer).branch_switch::tests::stash_and_switch_leaves_the_changes_in_a_named_stash, hit a transientrun git: No such file or directoryand passed 3 of 3 on rerun. It is in code this PR doesn't touch.📝 Notes
main. It merges cleanly ontoorigin/main, and that merged tree is what the gate ran on.🤖 Generated with Claude Code