Skip to content

Revalidate PR number when opening prompt picker#506

Merged
PureWeen merged 2 commits intomainfrom
fix/prompt-pr-revalidation
Apr 5, 2026
Merged

Revalidate PR number when opening prompt picker#506
PureWeen merged 2 commits intomainfrom
fix/prompt-pr-revalidation

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

@PureWeen PureWeen commented Apr 4, 2026

When the user opens the prompt picker, invalidate the PrLinkService cache and re-fetch the current branch's PR. This ensures the autofilled PR context is accurate after branch switches.

Also fixes WsBridgeIntegrationTests build error (ChangeModelAsync signature changed with new reasoningEffort parameter).

Testing: All 3,129 tests pass.

@PureWeen
Copy link
Copy Markdown
Owner Author

PureWeen commented Apr 4, 2026

🔍 Multi-Model Code Review — PR #506

Revalidate PR number when opening prompt picker

CI Status

⚠️ No CI checks reported on this branch


Findings

🟡 MODERATE — Popup blocked ~1–2s on every open (3/3 reviewers)

File: PolyPilot/Components/ExpandedSessionView.razorShowPromptsPopup (new code ~line 995–1001)

await FetchPrUrlAsync(dir) spawns two subprocesses (git rev-parse + gh pr view, ~1–2s) synchronously in the critical path of ShowPromptsPopup. The popup cannot display until this completes. There is no spinner or disabled state — the user clicks and nothing visibly happens for 1–2s.

The existing pattern in OnParametersSet deliberately uses fire-and-forget (_ = FetchPrUrlAsync(dir)) to avoid this exact latency problem. This PR introduces a regression by blocking.

Suggestion: Use fire-and-forget (_ = FetchPrUrlAsync(dir)) and show the popup immediately with whatever _prLabel is already cached. The popup opens instantly; if the PR number changes, a StateHasChanged() call in the callback will update it.


🟡 MODERATE — _prLabel not reset when branch has no PR (3/3 reviewers)

File: PolyPilot/Components/ExpandedSessionView.razorFetchPrUrlAsync method (~line 829–835)

When GetPrUrlForDirectoryAsync returns null (branch has no associated PR), _prUrl is set to null but _prLabel retains its old value (e.g., "#506"). The downstream code in ShowPromptsPopup that injects _prLabel[1..] into the popup JS context has no null-URL guard — so a stale PR number from the previous branch gets autofilled.

Fix: Add else { _prLabel = "PR"; } in FetchPrUrlAsync when _prUrl is null.


🟢 MINOR — Unconditional cache invalidation defeats caching (3/3 reviewers)

File: PolyPilot/Components/ExpandedSessionView.razor (~line 995)

Invalidate(dir) is called on every prompt picker open, even if the branch hasn't changed since the last check. This completely bypasses the 5-minute TTL cache in PrLinkService, spawning unnecessary subprocesses.

Suggestion: Compare the current branch to a stored value and only invalidate on actual branch change. Or use a shorter TTL check instead of unconditional invalidation.


✅ No issues — WsBridge test fix (3/3 reviewers)

Named parameter ct: correctly adapts to the new ChangeModelAsync signature with the nullable reasoning effort parameter. Clean, correct fix.


Re-Review (after d7e9ca21d)

Branch rebased on latest origin/main (1f4f99f7d). Fixes committed as d7e9ca21d.

Previous Finding Status

# Finding Status
1 🟡 Popup blocked ~1–2s on every open FIXED — Changed await FetchPrUrlAsync(dir) to _ = FetchPrUrlAsync(dir) (fire-and-forget). Popup opens instantly with cached _prLabel; background fetch updates via StateHasChanged if PR number changes.
2 🟡 _prLabel not reset when branch has no PR FIXED — Added else { _prLabel = "PR"; } in FetchPrUrlAsync when _prUrl is null. Stale PR numbers are now cleared.
3 🟢 Unconditional cache invalidation ADDRESSED — Kept unconditional invalidation (intentional: branch can change without directory changing), but added detailed comment explaining why. Fire-and-forget eliminates the user-visible latency cost.

Test Results

3,155 tests pass (0 failed, 0 skipped)

Updated Recommendation

Approve — all findings addressed. Popup no longer blocks, stale PR labels cleared, cache invalidation explained.

PureWeen and others added 2 commits April 5, 2026 17:31
When the user opens the prompt picker, invalidate the PrLinkService cache
and re-fetch the current branch's PR number. This ensures the autofilled
PR context is accurate even after branch switches.

Also fix WsBridgeIntegrationTests: ChangeModelAsync gained a
reasoningEffort parameter, so pass CancellationToken as named arg.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ve comments

- Change await FetchPrUrlAsync to fire-and-forget so the prompt picker
  opens instantly instead of blocking ~1-2s on subprocess spawns
- Reset _prLabel to "PR" when FetchPrUrlAsync returns null (no PR for
  branch), preventing stale PR numbers from being autofilled
- Add detailed comment explaining why unconditional invalidation is
  needed (branch can change without directory changing)

3,155 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen force-pushed the fix/prompt-pr-revalidation branch from 6716f81 to d7e9ca2 Compare April 5, 2026 22:35
@PureWeen PureWeen merged commit 312f341 into main Apr 5, 2026
@PureWeen PureWeen deleted the fix/prompt-pr-revalidation branch April 5, 2026 22:38
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.

1 participant