Skip to content

fix(clients): split linked PR counts by status - #15726

Open
ashx-j wants to merge 3 commits into
pingdotgg:mainfrom
ashx-j:fix/issue-15702-split-pr-counts
Open

ashx-j wants to merge 3 commits into
pingdotgg:mainfrom
ashx-j:fix/issue-15702-split-pr-counts

Conversation

@ashx-j

@ashx-j ashx-j commented Oct 4, 2026 •

Copy link
Copy Markdown

A thread with three open and three closed PRs shows a green +6, which looks like six open PRs. This affects the sidebar, composer toolbar, and mobile thread list.

Show separate counts such as 3 open · 3 closed for unrelated linked PRs. Shared counting distinguishes ready open PRs, drafts, closed PRs, merged PRs, and pending snapshots, and excludes dismissed links. The badge uses a neutral color, long summaries truncate within the row, and individual PRs and stacks retain their existing behavior. Desktop uses the shared web components.

Fixes #15702. Alternative to #15724, which shows a neutral total. Choose one approach rather than merging both.

Validation: 115 focused tests passed across shared badge logic, web presentation/subscriptions, and mobile presentation. Targeted lint completed with pre-existing warnings; formatting and whitespace checks passed. An independent GPT-6-Astra reviewer at high reasoning found no correctness issues; an outdated sidebar comment noted during review was corrected. Full app typechecks were stopped after host memory/thread pressure prevented completion. Narrower single-threaded typechecks then passed for the shared counting logic, mobile presentation and tests, and web badge component and tests with their required ambient declarations. Browser/device verification and new before/after screenshots remain outstanding.

Implemented by GPT-6-Astra through the Codex harness.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 4, 2026
{/* An element, not bare text: bare text takes its line box from the control, which
inherits the row's size, so beside a text-sm title it sat below the other meta. */}
<span>{presentation.text}</span>
<span className="max-w-40 truncate">{presentation.text}</span>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium components/ThreadStatusIndicators.tsx:305

The badge text still expands the sidebar row instead of showing an ellipsis when it exceeds 10rem. Because this <span> is an inline child under display: contents, max-w-40 and truncate do not establish a box where those styles apply; make it inline-block (or block) before applying the width and truncation.

Suggested change
<span className="max-w-40 truncate">{presentation.text}</span>
<span className="inline-block max-w-40 truncate">{presentation.text}</span>
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ThreadStatusIndicators.tsx around line 305:

The badge text still expands the sidebar row instead of showing an ellipsis when it exceeds 10rem. Because this `<span>` is an inline child under `display: contents`, `max-w-40` and `truncate` do not establish a box where those styles apply; make it `inline-block` (or block) before applying the width and truncation.

@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a focused, tested UI bug fix that replaces ambiguous aggregate linked-PR counts with per-status summaries across existing web and mobile paths, without schema, infrastructure, security, or product-default changes. A Medium-severity unresolved layout finding indicates the web truncation may still be ineffective and remains a separate risk to address.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2e35ac0e-98ea-42d5-aa7d-74a90a92601e
📥 Commits

Reviewing files that changed from the base of the PR and between 4c58699 and 525abea.

📒 Files selected for processing (1)
  • apps/web/src/components/ThreadStatusIndicators.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/src/components/ThreadStatusIndicators.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Unrelated linked pull requests now display counts grouped by status instead of an aggregate state and total-only count. The shared resolver provides the summary, and web and mobile presentations show it with neutral styling.

Changes

Per-status Pull Request Badges

Layer / File(s) Summary
Shared status summary
packages/shared/src/threadPullRequests.ts, packages/shared/src/threadPullRequests.test.ts
The shared badge type and resolver provide counts by status for unrelated linked pull requests. Tests cover pending, draft, open, closed, and merged links.
Web and mobile badge presentations
apps/web/src/components/ThreadStatusIndicators.tsx, apps/web/src/components/ThreadStatusIndicators.test.ts, apps/web/src/components/Sidebar.tsx, apps/mobile/src/state/thread-pr-presentation.ts, apps/mobile/src/state/use-thread-pr.test.ts, apps/mobile/src/features/threads/thread-list-v2-items.tsx
Web and mobile badges display the per-status summary with neutral styling for unrelated links. Stack presentation remains distinct. Badge text is truncated, and tests cover mixed statuses and pending snapshots.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 525ab

Mixed-status linked PRs display separate counts. No actionable merge-blocking issue was found in the selected change; normal checks can proceed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5e22e

The inspected changes affect how linked pull-request statuses are displayed, rather than permissions or privileged operations. Link identity and interaction routing remain separate from the new summary. Risk is low, although an independent comparison with the previous implementation was unavailable.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated effect of snapshot-controlled status values is confined to badge text, accessibility descriptions, and styling in the inspected consumers. The new summary does not supply a navigation destination or privileged-operation argument.

Security Findings and Attack Paths

  • inferred — No introduced attack path was established from the new status summary to authority or access-control decisions. The inspected web control chooses list-versus-link behavior from badge kind and count, and the mobile presentation obtains its URL and number from the separately selected link.

Trust Boundaries and Controls

  • observed — Mobile retains the environment capability gate for linked snapshots. The status summary does not participate in fallback request parameters or the environment-scoped identity used by the presentation cache.
  • observed — The inspected web control keeps callbacks and URL handling separate from presentation text. Single-link anchors retain noopener and noreferrer; multiple-link badges invoke the list callback.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: splitting linked pull-request counts by status.
Description check ✅ Passed The description explains the problem, change, scope, and verification. It reports focused test results and clearly notes that browser/device checks and before/after screenshots remain outstanding.
Linked Issues check ✅ Passed Issue #15702 requires mixed-status linked PR counts that do not present the total as the open count. resolveThreadPullRequestBadge now builds a status summary for unrelated links, including pending …
Out of Scope Changes check ✅ Passed The shared counting change, sidebar and composer presentation, mobile presentation, truncation, tests, and sidebar comment update all support the mixed-status badge correction in #15702 or the PR's st…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 8 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Sidebar and composer PR badges show total linked count with open status for mixed states

1 participant