feat(code): add PR diff source#1864
Conversation
a36c1e1 to
d107aac
Compare
0459081 to
82824d0
Compare
d107aac to
ced37dd
Compare
82824d0 to
1ff05ce
Compare
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/code/src/renderer/features/code-review/components/ReviewPage.tsx
Line: 38
Comment:
**Duplicate empty-array constant**
`EMPTY_PR_FILES` is structurally identical to `EMPTY_BRANCH_FILES` (both are `ChangedFile[]` initialized to `[]`). Per the OnceAndOnlyOnce simplicity rule, a single constant should serve both uses.
```suggestion
const EMPTY_CHANGED_FILES: ChangedFile[] = [];
```
Then replace both `EMPTY_BRANCH_FILES` and `EMPTY_PR_FILES` usages with `EMPTY_CHANGED_FILES`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/code/src/renderer/features/code-review/utils/resolveDiffSource.test.ts
Line: 78-86
Comment:
**Missing branch fallback test case**
The previous suite included a case where `linkedBranch` is set but `aheadOfDefault` is `0` (branch exists but nothing has been committed ahead). That case was dropped when the test was updated and is no longer covered. The surviving "falls back to local when unavailable" case only exercises `linkedBranch: null`.
Consider adding:
```ts
{
desc: "explicit branch falls back to local when no commits ahead",
configured: "branch",
hasLocalChanges: false,
linkedBranch: "feat/x",
aheadOfDefault: 0,
prSourceAvailable: false,
expected: "local",
},
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(code): add PR diff source" | Re-trigger Greptile |
| } from "./ReviewShell"; | ||
|
|
||
| const EMPTY_BRANCH_FILES: ChangedFile[] = []; | ||
| const EMPTY_PR_FILES: ChangedFile[] = []; |
There was a problem hiding this comment.
Duplicate empty-array constant
EMPTY_PR_FILES is structurally identical to EMPTY_BRANCH_FILES (both are ChangedFile[] initialized to []). Per the OnceAndOnlyOnce simplicity rule, a single constant should serve both uses.
| const EMPTY_PR_FILES: ChangedFile[] = []; | |
| const EMPTY_CHANGED_FILES: ChangedFile[] = []; |
Then replace both EMPTY_BRANCH_FILES and EMPTY_PR_FILES usages with EMPTY_CHANGED_FILES.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/code-review/components/ReviewPage.tsx
Line: 38
Comment:
**Duplicate empty-array constant**
`EMPTY_PR_FILES` is structurally identical to `EMPTY_BRANCH_FILES` (both are `ChangedFile[]` initialized to `[]`). Per the OnceAndOnlyOnce simplicity rule, a single constant should serve both uses.
```suggestion
const EMPTY_CHANGED_FILES: ChangedFile[] = [];
```
Then replace both `EMPTY_BRANCH_FILES` and `EMPTY_PR_FILES` usages with `EMPTY_CHANGED_FILES`.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| { | ||
| desc: "explicit branch falls back to local when no linked branch", | ||
| desc: "explicit branch falls back to local when unavailable", | ||
| configured: "branch", | ||
| hasLocalChanges: false, | ||
| linkedBranch: null, | ||
| aheadOfDefault: 0, | ||
| prSourceAvailable: false, | ||
| expected: "local", | ||
| }, |
There was a problem hiding this comment.
Missing branch fallback test case
The previous suite included a case where linkedBranch is set but aheadOfDefault is 0 (branch exists but nothing has been committed ahead). That case was dropped when the test was updated and is no longer covered. The surviving "falls back to local when unavailable" case only exercises linkedBranch: null.
Consider adding:
{
desc: "explicit branch falls back to local when no commits ahead",
configured: "branch",
hasLocalChanges: false,
linkedBranch: "feat/x",
aheadOfDefault: 0,
prSourceAvailable: false,
expected: "local",
},Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/code-review/utils/resolveDiffSource.test.ts
Line: 78-86
Comment:
**Missing branch fallback test case**
The previous suite included a case where `linkedBranch` is set but `aheadOfDefault` is `0` (branch exists but nothing has been committed ahead). That case was dropped when the test was updated and is no longer covered. The surviving "falls back to local when unavailable" case only exercises `linkedBranch: null`.
Consider adding:
```ts
{
desc: "explicit branch falls back to local when no commits ahead",
configured: "branch",
hasLocalChanges: false,
linkedBranch: "feat/x",
aheadOfDefault: 0,
prSourceAvailable: false,
expected: "local",
},
```
How can I resolve this? If you propose a fix, please make it concise.1ff05ce to
ce4f443
Compare
c12cfc3 to
dc2f4b7
Compare
ce4f443 to
58ec2e3
Compare
58ec2e3 to
9e9fb6d
Compare
dc2f4b7 to
d7b8cbb
Compare
9e9fb6d to
db71e1d
Compare

Problem
local tasks may have associated PRs now, but there's no way to view the PR diff
Changes
adds PR as a diff source, little bit of refactoring along the way
How did you test this?
manually