Feature Request: Read PR Comments (#31) by aaryan-rampal · Pull Request #32 · bb-cli/bb-cli

aaryan-rampal · 2026-02-25T22:12:58Z

I am writing this PR to address this issue I raised. I added support for inline and general comments. The way it is formatted is in markdown, maybe a bit too fancy but I think it looks good. That can be changed easily, let me know if you'd like it to be simpler (though I do think .md comments with a --json flag would be the best solution). I also added a drive by fix for git worktree support (use git rev-parse instead of assuming .git is a directory [not the case for worktrees]). Anyways, that's what I had to say, here's an additional AI summary of the important points:

AI SUMMARY

What changed

Added bb pr show as a new PR command entry point in src/Actions/Pr.php
Implemented comment retrieval/formatting flow in src/Actions/Pr.php, including:
- show()
- displayComments()
- fetchComments()
- fetchInlineComments()
- formatGeneralComment()
- formatInlineComment()
- formatTimestamp()
Added support for showing:
- General PR comments
- Inline comments with file + line metadata
Added optional filtering controls (limit/unresolved handling) in the command path
Drive-by fix in bin/bb: switched git-repo detection to git rev-parse --git-dir so worktrees are supported (instead of assuming .git is a directory)
Updated changelog entry for the new command in CHANGELOG.md
Usage
bb pr show <pr_id> [limit] [unresolved]
Example output:

## Pull Request Comments (PR #31)
### General Comments
foo-user (2 hours ago):
Can we rename this helper before merge? bar baz qux.
build-bot (1 hour ago):
Pipeline warning in stage "lint". Random note here.
### Inline Code Comments
File: src/Actions/Pr.php:411
bar-reviewer (45 minutes ago):
nit: maybe extract this into a small formatter function.
File: bin/bb:128
foo-dev (30 minutes ago):
good catch on worktree handling, this fixes local setup issues.

Copilot

Pull request overview

Adds a new bb pr show command to display pull request comments (general + inline) and improves local repo detection to support git worktrees.

Changes:

Added bb pr show entry point and implemented fetching/formatting/display of general and inline PR comments.
Added git worktree-compatible repo detection by using git rev-parse --git-dir instead of checking for a .git/ directory.
Documented the new command in CHANGELOG.md.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File	Description
src/Actions/Pr.php	Introduces the `show` command and supporting helpers to fetch/filter/format PR comments.
bin/bb	Updates git-repo detection to work with worktrees.
CHANGELOG.md	Notes the new `bb pr show` command and its usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T22:17:32Z

src/Actions/Pr.php

+        $comments = $this->fetchComments($prId, $limit);
+        $inlineComments = $this->fetchInlineComments($prId, $limit);
+


displayComments() calls both fetchComments() and fetchInlineComments(), but both methods hit the same /pullrequests/{id}/comments endpoint. This doubles the API calls and also means the pagelen limit applies to the combined stream, so one section can show fewer than limit items even when more exist. Consider fetching once and partitioning into general vs inline in-memory (and/or paginating until each bucket reaches the requested limit).

Smart, will look into this

Copilot · 2026-02-25T22:17:32Z

src/Actions/Pr.php

+        if ($diff->d > 7) {
+            return $date->format('M d, Y');
+        } elseif ($diff->d > 0) {
+            return $diff->d . ' day' . ($diff->d > 1 ? 's' : '') . ' ago';
+        } elseif ($diff->h > 0) {
+            return $diff->h . ' hour' . ($diff->h > 1 ? 's' : '') . ' ago';
+        } else {
+            return $diff->i . ' minute' . ($diff->i > 1 ? 's' : '') . ' ago';


formatTimestamp() uses $diff->d for thresholds, but DateInterval::d is the day component (0–31), not the total number of days. For timestamps older than a month this can produce incorrect relative output (e.g., 40 days ago may look like only a few days). Use $diff->days (total days) for these comparisons, and also consider $diff->invert to avoid reporting future timestamps as "... ago".

Suggested change

if ($diff->d > 7) {

return $date->format('M d, Y');

} elseif ($diff->d > 0) {

return $diff->d . ' day' . ($diff->d > 1 ? 's' : '') . ' ago';

} elseif ($diff->h > 0) {

return $diff->h . ' hour' . ($diff->h > 1 ? 's' : '') . ' ago';

} else {

return $diff->i . ' minute' . ($diff->i > 1 ? 's' : '') . ' ago';

// If the timestamp is in the future, avoid showing a misleading "ago" string.

if ($diff->invert === 0 && $diff->days > 7) {

return $date->format('M d, Y');

} elseif ($diff->invert === 0 && $diff->days > 0) {

return $diff->days . ' day' . ($diff->days > 1 ? 's' : '') . ' ago';

} elseif ($diff->invert === 0 && $diff->h > 0) {

return $diff->h . ' hour' . ($diff->h > 1 ? 's' : '') . ' ago';

} elseif ($diff->invert === 0) {

return $diff->i . ' minute' . ($diff->i > 1 ? 's' : '') . ' ago';

} else {

// For future timestamps, fall back to an absolute date.

return $date->format('M d, Y');

Copilot · 2026-02-25T22:17:33Z

src/Actions/Pr.php

+            if (array_get($comment, 'type') === 'pullrequest_comment' &&
+                !empty(array_get($comment, 'inline.path')) &&
+                !empty(array_get($comment, 'content.raw', ''))) {
+                $inlineComments[] = $comment;


fetchInlineComments() filters out entries with empty content.raw. This will also drop deleted inline comments (which are still relevant context and are handled explicitly for general comments). Consider including deleted inline comments and letting the formatter render a placeholder (similar to formatGeneralComment()), rather than silently omitting them.

src/Actions/Pr.php

aaryan-rampal · 2026-02-25T23:38:22Z

To be honest, show is weird UX to just show comments. I think show should be PR description and a git diff --stat style output and changing the view comments to another flag. Unfortunately, I believe it's pretty finicky to add a show <pr-id> --comments in this project without bigger structural changes.

semiherdogan · 2026-02-27T09:18:05Z

Hi,
Thank you for the PR, this is a valuable addition.
Below is my review.

File-by-File Review

1. `src/Actions/Pr.php` (+234 lines)

Architecture Concern: Pr class is too large

The Pr class has grown from ~335 lines to 569 lines. The new show command adds 7 private methods (displayComments, fetchAllComments, formatGeneralComment, formatTimestamp, extractCommitHash, formatInlineComment) alongside 1 public method (show). This is a significant amount of new logic.

Recommendation: Extract the show/comments functionality into a dedicated class. Suggested approach:

Create src/Actions/PrDetails.php (or PrShow.php) as a new action class extending Base.
Register it in bin/bb as a new action (e.g., 'pr-details' => PrDetails::class).
Move the 7 private helper methods into that class.
This keeps Pr focused on PR lifecycle operations (list, create, merge, approve, decline) and puts comment display in its own class.

Private utility methods that should move to `helpers.php`

Several of the new private methods are generic utilities, not PR-specific:

formatTimestamp($dateString) — Converts a datetime string to relative time. This is a general-purpose utility with no dependency on $this or PR data. Should be a standalone function in src/utils/helpers.php.
extractCommitHash($comment) — Extracts a commit hash from an associative array by trying multiple field paths. While it has a specific field list, the pattern (try multiple keys, return first match) is generic. Could be moved to helpers as a utility, or kept in the new class if preferred.

Methods that should stay in the new class (not in helpers)

displayComments — Orchestrates output formatting; tightly coupled to the display logic.
fetchAllComments — Makes API calls via $this->makeRequest(); must stay in a Base subclass.
formatGeneralComment / formatInlineComment — PR-comment-specific formatting; belong in the new PR details class.

Bug: Filtering applied after slicing

$comments = array_slice($commentsData['general'], 0, $limit);
$inlineComments = array_slice($commentsData['inline'], 0, $limit);

// Filter by unresolved status if requested
if ($unresolved) {
    $comments = array_values(array_filter($comments, function ($c) {
        return !(array_get($c, 'resolved') ?? false);
    }));
}

The array_slice (limit) is applied before the unresolved filter. If the user asks for 10 unresolved comments but there are 50 comments total with only 3 unresolved in the first 10, they'll get 3 results instead of up to 10. The filter should be applied before the slice.

Fix: Swap order — filter first, then slice.

Bug: `formatTimestamp` relative time logic is inverted

$diff = $now->diff($date);

// Handle future timestamps
if ($diff->invert === 1) {
    return $date->format('M d, Y');
}

DateInterval::$invert is 1 when the result is negative, meaning $date is in the past (since $now->diff($date) yields negative when $date < $now). The comment says "Handle future timestamps" but the condition actually catches past timestamps. This means:

Past timestamps → formatted as absolute date (the "future" branch fires).
Future timestamps → formatted as relative time (the relative branch fires).

This is backwards. The invert === 1 check should trigger relative-time formatting, not absolute.

Minor: `resolved` field path

return !(array_get($c, 'resolved') ?? false);

According to Bitbucket API, the comment resolution status is under resolution.type or resolution object, not a top-level resolved boolean. This may silently never filter anything. Worth verifying against actual API responses.

Minor: No pagination for comments

fetchAllComments only reads the first page of /pullrequests/{id}/comments. Bitbucket paginates responses (default 10-50 items). PRs with many comments will have truncated results. A next page URL check would be needed for completeness.

Minor: `$limit` validation accepts strings from CLI

The $limit parameter comes from $argv as a string. is_numeric catches this, but $limit < 1 and $limit > 100 are string comparisons until the (int) cast on line 362. This works in PHP due to type juggling but is fragile. Consider casting earlier.

2. `bin/bb` (+2 lines, -1 line)

// Before
if (!is_dir(getcwd().'/.git')) {

// After
$gitDir = trim((string) exec('git rev-parse --git-dir 2>/dev/null'));
if ($gitDir === '') {

Good change. git rev-parse --git-dir correctly detects git repositories even when the CWD is a subdirectory of the repo (where .git folder wouldn't exist). This is a real bug fix.

Summary of Findings

#	Severity	File	Issue
1	High	`Pr.php`	Class too large — extract into `PrDetails` or similar
2	High	`Pr.php`	Move `formatTimestamp` (and optionally `extractCommitHash`) to `helpers.php`
3	Medium	`Pr.php`	`formatTimestamp` invert logic is backwards (past vs future)
4	Medium	`Pr.php`	Filter-then-slice order bug (unresolved filter applied after limit)
5	Low	`Pr.php`	`resolved` field path may not match Bitbucket API schema
6	Low	`Pr.php`	No pagination support for comments endpoint

Recommended Next Steps

Create a new PrDetails (or PrShow) action class to house the show/comments functionality.
Move formatTimestamp to helpers.php as a standalone function.
Fix the formatTimestamp invert logic.
Fix filter-before-slice ordering for the unresolved flag.
Verify the resolved field path against actual Bitbucket API responses.
Squash your commits please.

Thanks again for the PR! Please address the above points before merging.

@dinncer Once these issues are addressed, I’m okay with merging.

Add PR comment viewing with detailed display and optimizations - Create new PrDetails class for fetching and displaying PR comments - Support both general comments and inline code comments with file paths - Optimize API calls by fetching all comments once and partitioning in-memory - Add format_relative_timestamp helper for human-readable time display - Fix timestamp calculation bug for dates older than 1 month - Handle future timestamps and missing data gracefully - Register 'show' command in AVAILABLE_COMMANDS - Update CHANGELOG with new feature

aaryan-rampal · 2026-03-03T20:38:21Z

Done, refactored to PrDetails.php
Done in helpers.php
Done in helpers.php
Done
After some testing, made some changes to unresolved filtering. It only applies to inline comments. Passing in true for unresolved shows ALL general comments + all UNRESOLVED inline comments.
Done

Also, removed the limit parameter. This makes it more gh-like, with a chronologically sorted comments output. It fetches all comments, so no need to worry about user handling pagination.

Up next after this (possibly in another PR), nicer viewing of the output (possibly render markdown like how gh does). Also, add adding + editing + deleting comments functionality.

dinncer · 2026-03-04T07:51:29Z

@aaryan-rampal

Thanks for addressing the review comments and updating the PR.

The changes look good now and I appreciate you taking the time to go through the feedback and adjust the implementation. Thanks for the contribution!

We'll include this in the upcoming 1.4.0 release @semiherdogan

Copilot AI review requested due to automatic review settings February 25, 2026 22:12

Copilot started reviewing on behalf of aaryan-rampal February 25, 2026 22:13 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

semiherdogan requested a review from dinncer February 27, 2026 07:19

aaryan-rampal force-pushed the main branch from 4c4ad70 to 16d89b6 Compare March 3, 2026 20:18

aaryan-rampal force-pushed the main branch from 16d89b6 to 9ed9366 Compare March 3, 2026 20:36

semiherdogan approved these changes Mar 4, 2026

View reviewed changes

dinncer approved these changes Mar 4, 2026

View reviewed changes

dinncer merged commit 39e5d00 into bb-cli:main Mar 4, 2026

dinncer mentioned this pull request Mar 4, 2026

Feature Request: Add functionality to read comments on PRs #31

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature Request: Read PR Comments (#31)#32

Feature Request: Read PR Comments (#31)#32
dinncer merged 1 commit intobb-cli:mainfrom
aaryan-rampal:main

aaryan-rampal commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

aaryan-rampal Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Uh oh!

aaryan-rampal commented Feb 25, 2026

Uh oh!

semiherdogan commented Feb 27, 2026

Uh oh!

aaryan-rampal commented Mar 3, 2026 •

edited

Loading

Uh oh!

dinncer commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		$comments = $this->fetchComments($prId, $limit);
		$inlineComments = $this->fetchInlineComments($prId, $limit);

Conversation

aaryan-rampal commented Feb 25, 2026

AI SUMMARY

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

aaryan-rampal Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aaryan-rampal commented Feb 25, 2026

Uh oh!

semiherdogan commented Feb 27, 2026

File-by-File Review

1. src/Actions/Pr.php (+234 lines)

Architecture Concern: Pr class is too large

Private utility methods that should move to helpers.php

Methods that should stay in the new class (not in helpers)

Bug: Filtering applied after slicing

Bug: formatTimestamp relative time logic is inverted

Minor: resolved field path

Minor: No pagination for comments

Minor: $limit validation accepts strings from CLI

2. bin/bb (+2 lines, -1 line)

Summary of Findings

Recommended Next Steps

Uh oh!

aaryan-rampal commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dinncer commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1. `src/Actions/Pr.php` (+234 lines)

Private utility methods that should move to `helpers.php`

Bug: `formatTimestamp` relative time logic is inverted

Minor: `resolved` field path

Minor: `$limit` validation accepts strings from CLI

2. `bin/bb` (+2 lines, -1 line)

aaryan-rampal commented Mar 3, 2026 •

edited

Loading