Skip to content

feat: render PR comments in the checks panel#1959

Merged
jschwxrz merged 8 commits into
generalaction:mainfrom
arnestrickmann:arne/eng-1071-render-pr-comments-in-the-right-sidebar
May 11, 2026
Merged

feat: render PR comments in the checks panel#1959
jschwxrz merged 8 commits into
generalaction:mainfrom
arnestrickmann:arne/eng-1071-render-pr-comments-in-the-right-sidebar

Conversation

@arnestrickmann
Copy link
Copy Markdown
Contributor

Summary

  • Fetch pull request conversation, review, and inline review comments on demand through the existing pull request RPC path
  • Render comments below checks in the right sidebar checks tab
  • Render PR comment bodies with compact markdown and opt-in sanitized HTML support for bot-authored HTML snippets

Validation

  • pnpm run format:check
  • pnpm run lint
  • pnpm run typecheck (blocked by existing src/renderer/app/app-menu-events.tsx TabManagerStore.tabs errors)
  • pnpm run test (blocked locally by better-sqlite3 native ABI mismatch after install/rebuild state)

…render-pr-comments-in-the-right-sidebar

# Conflicts:
#	src/renderer/lib/ui/markdown-renderer.tsx
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Greptile Summary

This PR adds a comments panel to the checks sidebar tab by fetching GitHub issue comments, review comments, and review bodies via three parallel REST API calls, then rendering them with a compact markdown renderer that supports sanitized HTML.

  • Backend (pr-sync-engine.ts, controller.ts): getPullRequestComments runs three paginated GitHub REST fetches in parallel, maps them to a shared PullRequestComment type, and exposes them through an RPC endpoint; error handling and type mapping are consistent with the existing pattern.
  • Frontend (checks-list.tsx, comments-list.tsx): PrChecksList gains a TanStack Query-powered comments fetch and renders both sections in the panel; a new CommentsList component handles loading/error/empty states and renders each comment with author avatar, location label, and a markdown body.
  • Shared (markdown-renderer.tsx, pull-requests.ts): MarkdownRenderer gains an allowHtml prop (default true in full variant, false in compact) so callers can opt in to rehype-raw processing independently of the variant setting.

Confidence Score: 3/5

Safe to merge after fixing the error-state masking in PrChecksList — all other changes are additive and well-structured.

When the comments query fails (network error, rate limit, etc.) and the PR has no checks, the empty-state guard fires before the full layout is rendered, so the "Unable to load comments" message in CommentsList is never reached and the user sees a misleading "No checks or comments" placeholder instead.

src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/checks-list.tsx — the empty-state guard condition needs to account for the error state.

Important Files Changed

Filename Overview
src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/checks-list.tsx Adds a comments query alongside checks in PrChecksList; the empty-state guard doesn't account for the error state, causing a misleading "No checks or comments" message when the comments fetch fails.
src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/comments-list.tsx New component that renders fetched PR comments; allowHtml is enabled for all comment authors (not just bots) and sort order is descending by updatedAt which reverses conversation chronology.
src/main/core/pull-requests/pr-sync-engine.ts Adds getPullRequestComments fetching three GitHub REST endpoints (issue comments, review comments, reviews) in parallel via Promise.all with rate-limiter and retry; mapping looks correct.
src/main/core/pull-requests/controller.ts Wires getPullRequestComments into the RPC controller; error mapping from parse failure to invalid_repository type is correct.
src/renderer/lib/ui/markdown-renderer.tsx Replaces the variant-based rehype plugin selection with an allowHtml prop (defaulting to variant === 'full'), preserving existing behaviour for all prior callers.
src/shared/pull-requests.ts Adds PullRequestComment type, comments_failed error variant, and exports getPrNumber/pullRequestErrorMessage — clean additions with no issues.

Sequence Diagram

sequenceDiagram
    participant UI as PrChecksList (renderer)
    participant RQ as TanStack Query
    participant RPC as IPC / RPC
    participant Ctrl as PullRequest Controller
    participant Eng as PrSyncEngine
    participant GH as GitHub REST API

    UI->>RQ: useQuery(pull-request-comments)
    RQ->>RPC: getPullRequestComments(repositoryUrl, prNumber)
    RPC->>Ctrl: getPullRequestComments()
    Ctrl->>Eng: getPullRequestComments()
    par Fetch in parallel
        Eng->>GH: issues.listComments (paginate)
        Eng->>GH: pulls.listReviewComments (paginate)
        Eng->>GH: pulls.listReviews (paginate)
    end
    GH-->>Eng: issueComments, reviewComments, reviews
    Eng-->>Ctrl: ok(PullRequestComment[])
    Ctrl-->>RPC: "ok({ comments })"
    RPC-->>RQ: response
    RQ-->>UI: comments[]
    UI->>UI: render ChecksList + CommentsList
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/checks-list.tsx:124-126
**Error state masked by empty-state guard**

When the comments query errors (e.g. GitHub API down, rate-limit), `commentsQuery.data` is `undefined` so `comments` is `[]`, and `commentsQuery.isLoading` is `false`. The guard condition therefore evaluates to `true` and returns the `EmptyState` ("No checks or comments") before the full layout — and `CommentsList`'s "Unable to load comments" message — is ever reached. The user sees a false "nothing here" message instead of the actual error state.

Add `!commentsQuery.isError` to the guard:
`checks.length === 0 && comments.length === 0 && !commentsQuery.isLoading && !commentsQuery.isError`

### Issue 2 of 3
src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/comments-list.tsx:61
`allowHtml` is passed unconditionally for every comment author. The PR description frames this as opt-in for bot-authored HTML snippets, but human-authored comments also go through `rehypeRaw`. While `rehypeSanitize` with `defaultSchema` is applied afterwards and blocks script injection, it does enable rendering of arbitrary HTML constructs (e.g. `<details>`, `<summary>`, embedded SVG-adjacent tags) in any user's comment body, which is broader than the described intent.

```suggestion
          <MarkdownRenderer
            content={comment.body}
            variant="compact"
            allowHtml={comment.author?.userName?.endsWith('[bot]') ?? false}
          />
```

### Issue 3 of 3
src/renderer/features/tasks/diff-view/changes-panel/components/pr-entry/comments-list.tsx:82-84
Comments are sorted descending by `updatedAt`, so the most recently edited comment appears first. For a conversation/review panel this reverses the chronological reading order — a comment edited five minutes ago on a thread from last week floats above replies posted after it. Sorting ascending by `createdAt` preserves conversation order, with the most recent activity naturally appearing at the bottom.

```suggestion
      [...comments].sort(
        (a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()
      ),
```

Reviews (1): Last reviewed commit: "ENG-1071 render PR comments in checks pa..." | Re-trigger Greptile

@arnestrickmann arnestrickmann changed the title [ENG-1071] Render PR comments in the checks panel feat: render PR comments in the checks panel May 11, 2026
@arnestrickmann arnestrickmann requested a review from jschwxrz May 11, 2026 01:51
@arnestrickmann
Copy link
Copy Markdown
Contributor Author

not merging this now, waiting for @jschwxrz's #1889 mermaid charts pr

@jschwxrz jschwxrz merged commit 49ae72b into generalaction:main May 11, 2026
1 check passed
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.

2 participants