feat: Show PR and diff status icons on task list items#1986
Merged
charlesvien merged 9 commits intomainfrom May 4, 2026
Merged
feat: Show PR and diff status icons on task list items#1986charlesvien merged 9 commits intomainfrom
charlesvien merged 9 commits intomainfrom
Conversation
Member
Author
This was referenced May 3, 2026
8d7c1d1 to
d085988
Compare
5ba58ab to
91f2f49
Compare
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/code/src/renderer/features/sidebar/hooks/useTaskPrStatus.ts:82-84
**Transient "Has changes" flicker for tasks with `linkedBranch`**
When a task has `linkedBranch` set, the `getPrUrlForBranch` query fires but `linkedBranchPrUrl` starts as `undefined` while loading. During that window `knownPrUrl = false`, so `skipDiff = false` and the diff/sync queries run immediately. If those resolve first (likely for local git ops vs. a GitHub API call), `hasDiff` becomes `true` and the amber GitBranch icon flashes briefly before the linked PR status arrives. Adding `!!linkedBranch` to the skip condition would suppress diff queries for this case:
```typescript
const skipDiff = isCloud || !hasWorktree || knownPrUrl || knownLocalPr || !!linkedBranch;
```
The trade-off is that if `getPrUrlForBranch` returns no URL (no PR exists for the branch), `hasDiff` would stay `false`; if showing diff state for branch-less linked tasks is desired, a follow-up pass after `linkedBranchPrUrl` resolves to `null` could re-enable it.
### Issue 2 of 3
apps/code/src/renderer/features/sidebar/hooks/useTaskPrStatus.test.ts:78-93
**Index-based query mock is silently fragile**
The `queryCallIndex` counter hard-codes the assumption that `useQuery` is always called in exactly this order: `[cloudPrDetails, linkedBranchPrUrl, linkedPrDetails, localPrStatus, diffStats, syncStatus]`. If the hook ever gains a new query, reorders calls, or uses a conditional query, all the derivation tests would silently test the wrong data — passing with incorrect assertions. Consider a named/keyed mock (matching on `queryKey`) so the mock remains correct regardless of call order.
### Issue 3 of 3
apps/code/src/renderer/features/sidebar/hooks/useTaskPrStatus.test.ts:152-188
**Prefer parameterised tests for `mapPrState`**
Per project convention, multiple expectations testing the same function with different inputs should use `it.each`. Currently several tests in the `mapPrState` describe block bundle multiple `expect` calls inside a single `it`, which hides individual failure cases and bypasses idiomatic parameterisation. For example:
```typescript
it.each([
["open", true, false, "merged"],
["OPEN", true, false, "merged"],
["closed", true, false, "merged"],
[null, true, false, "merged"],
] as const)(
"mapPrState(%s, merged=%s, draft=%s) → %s",
(state, merged, draft, expected) => {
expect(mapPrState(state, merged, draft)).toBe(expected);
},
);
```
The same pattern applies to the `closed`, `draft`, and `open` cases.
Reviews (1): Last reviewed commit: "Remove comments and add derivation tests" | Re-trigger Greptile |
d085988 to
ab33331
Compare
c1c1f34 to
8196799
Compare
ab33331 to
6310a52
Compare
jonathanlab
approved these changes
May 4, 2026
Contributor
jonathanlab
left a comment
There was a problem hiding this comment.
LGTM.
nit: Not too big of a fan of doing all this in a hook, should probably just happen inside it's own service tbh.
jonathanlab
reviewed
May 4, 2026
Generated-By: PostHog Code Task-Id: 024d3719-d3db-4e67-ae44-43cb14f338cd
8196799 to
dd68bed
Compare
d5e4eca to
2f8d1ae
Compare
2f8d1ae to
9d74bcd
Compare
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
Task sidebar icons show session state (suspended, paused) which isn't useful at a glance. Users want to see work state: PR merged/open/draft/closed, uncommitted changes, or idle.
Changes
How did you test this?
Manually