Widen merged-PR recency window from 10 to 30#95
Open
0101 wants to merge 9 commits into
Open
Conversation
The dashboard fetches only the most-recently-updated closed PRs to keep the GitHub query cheap. With per_page=10, a merged PR drops out of the association as soon as 10 newer closed PRs appear, leaving its worktree showing 'No PR' despite a clean merge. Widening the window to 30 keeps merged PRs visible and associated for longer.
There was a problem hiding this comment.
Pull request overview
This PR addresses a matching gap in the Treemon dashboard's GitHub PR association. The closed-PR fetch in GithubPrStatus.fs previously requested only the 10 most-recently-updated closed PRs; once a merged PR aged past that window it dropped out of the branch-keyed PR map, and the corresponding worktree incorrectly rendered "No PR". Widening the page size to 30 keeps merged PRs associated with their worktrees for longer. The change is a single, self-contained query-parameter update on the server-side GitHub integration.
Changes:
- Increased the closed-PR fetch page size from
per_page=10toper_page=30infetchGithubPrStatuses. - Left the open-PR fetch (no
per_page) and the unrelatedactions/runsquery (per_page=10) untouched, as verified across the source.
Show a summary per file
| File | Description |
|---|---|
| src/Server/GithubPrStatus.fs | Bumps the closed-PR per_page query parameter from 10 to 30 so recently merged PRs remain matched to their worktrees. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Medium
MailboxProcessor over Map<RepoId, Map<branch, {Id;Title;Url}>> persisted atomically to data/merged-prs.json (gitignored runtime state), mirroring CanvasDocOwnership.fs. getForRepo/setForRepo are change-persisting (Decision #6, empty map drops the repo key); load() falls back to empty on absent/corrupt files. Registered in Server.fsproj and loaded at startup in Program.fs. Adds 10 tests in src/Tests/MergedPrStoreTests.fs. Includes the feature spec.
Add pure reconcileMergedPrs + private toMergedPrStatus to MergedPrStore.fs: upserts live merged PRs keeping Id/Title/Url, prunes persisted to knownBranches, overlays a reconstructed merged HasPr for known branches the live map lacks as HasPr, never overrides a live HasPr; returns effectiveMap * newPersisted (structurally equal to persisted on no-op, Decision #6). Add ReconcileMergedPrsTests (8 tests) to MergedPrStoreTests.fs. Build clean; 18/18 fast tests green.
….RefreshPr Reconcile the bounded live PR fetch with the persisted merged-PR store in the RefreshPr handler: fetchPrStatusesByRepoRoot -> livePrMap, getForRepo -> persisted, reconcileMergedPrs overlays aged-out merged branches and prunes to knownBranches, setForRepo only on change (Decision #6), then UpdatePr posts the effective map. lookupPrStatus/WorktreeApi untouched.
…ownBranches (review F7) reconcileMergedPrs now takes Set<string> option (None => skip prune; upsert+overlay still run). New pure pruneScope returns Some only when every known worktree has collected git data AND >=1 branch resolved, else None. RefreshPr computes collectedGitPaths+pruneScope and passes the option. Closes the empty/partial-knownBranches wipe and the correlated all-upstreams-None full-wipe class. Spec Decision #8 added. Verified: build 0/0, 24/24 merged-PR tests, 68/68 SchedulerTests.
…er (review F1/F2/F3) New Server.JsonStore.persist/load own the atomic temp-file+File.Move write shell and the safe-load shell; MergedPrStore, CanvasDocOwnership, and SessionManager collapse to one-line calls keeping only their unique writer body / parse fold. GlobalConfig deliberately left out (different JsonNode read-modify-write pattern).
…hes with spec (review F8) Document-only fix: refined the 'What doesn't change' bullet to note PrData feeds both the merged badge and the sync pipeline's push step (WorktreeApi.fs:391 -> SyncEngine, HasPr _ -> push); added Decision #9 recording the rationale (pre-existing, low-harm: own remote ref, never main) plus the optional SyncEngine.fs guard for future use.
…PR persistence Verifies the full MergedPrStore disk-I/O + reconcileMergedPrs path against a temp data dir: persist-on-observe, aged-out fallback, restart durability, prune, live-wins, and corruption resilience.
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.
Problem
The dashboard matches GitHub PRs to worktrees. To keep the query cheap, the closed-PR fetch in
GithubPrStatus.fsrequests only the 10 most-recently-updated closed PRs (per_page=10). Once a merged PR is pushed past the newest 10 closed PRs, the dashboard loses the association and the worktree shows "No PR" even though its PR merged fine.Change
src/Server/GithubPrStatus.fs, the closed-PR fetch now usesper_page=30instead ofper_page=10, keeping merged PRs visible and associated for longer.per_page) and the unrelatedactions/runsquery (alsoper_page=10) are untouched.Tests
dotnet build src/Server/Server.fsproj— clean (0 warnings, 0 errors).dotnet test src/Tests/Tests.fsproj --filter "Category=Fast"— 965 passed, 0 failed.dotnet test src/Tests/Tests.fsproj --filter "Category=Unit"— 851 passed, 0 failed.