Skip to content

Restore the Epic PR View, and stop it from hammering GitHub - #870

Merged
hdkshingala merged 11 commits into
mainfrom
restore/epic-pr-view
Aug 1, 2026
Merged

hdkshingala merged 11 commits into
mainfrom
restore/epic-pr-view

Conversation

@hdkshingala

Copy link
Copy Markdown
Member

Brings back the Epic PR View that was reverted in #754, with the fixes the team asked for and a fetch layer that no longer treats GitHub's rate limits as someone else's problem.

What is here

The restore (a8909682) reverses #754. Three things a mechanical un-revert got wrong and that are fixed on top: the gitlink wanted to regress ~55 commits, updatePrDiffTileView no longer compiled against #809's per-kind tile view, and four restored test fixtures were missing hostCredentialMint.

Three fixes on the restored feature:

  • 95c542e7 — opening an epic no longer fetches pull requests. The background mount, changed-dot and seen-facts store are gone; a persisted presence store is fed from the panel's own stream instead. Accepted cost: the first-ever open shows no rail PR icon until the panel is revealed once.
  • 35bfc71b — a PR's chat chips collapse behind a +N popover, so a large epic no longer wraps that row into tens of lines.
  • 57544094 — an unswept PR row drops its blank title line.

The fetch layer (2ef5feed here, plus the host side in the internal PR). The panel used to fetch per epic, per poller, with no notion of a budget. It is now one host-global scheduler keyed by PR identity: one fetch and one write per PR however many epics can see it, a serial gh queue with request spacing, and freshness read from the persisted observedAt so a restart never forces a refresh.

This PR carries the part users can see. GitHub's limits are real and hitting them silently is worse than hitting them loudly, so the host now sends a notice on every pr.* frame — { kind: "rate-limited" | "backing-off", retryAt }, structure only, no prose — and the renderer turns it into an information icon beside the freshness stamp.

Notes for review

  • pr.* is unreleased surface, so notice is a free additive field: it appears in neither released-method-names nor the compat floor. Verified rather than assumed.
  • The copy says nothing about what is on screen. It used to say "showing the last data fetched", which is false for a PR paused before its first successful fetch. The freshness stamp answers that question instead, and now answers it in both states ("Not yet fetched" / "Updated Xm ago").
  • The pause is never an error state. sourceStatus stays cached while a host is paused; the notice is what explains it. Showing both lit the tile's red failure banner next to "retrying in 5s".
  • The information icon is keyboard reachable — a button inside the live region, since a focusable role="status" is exactly what jsx-a11y/no-noninteractive-tabindex rejects, and it is right to.

Verification

Format, lint and compile clean across all 5 projects. protocol 1,369 (see below) · gui-app 9,064 passing, 1 skipped · PR-focused suites 186.

Two pre-existing failures, both unrelated to this branch, which touches neither package:

  • @traycer-clients/traycer-cli — registry client > waives the floor for an unreleased CLI build asserts a waiver that a released 1.1.9 no longer gets. Version drift; git diff origin/main...HEAD -- clients/traycer-cli is empty.
  • @traycer-clients/desktop — passed on re-run, load-flaky.

The submodule worktree carries no pre-commit hook, which is why this branch had shipped an untyped vi.fn() whose mock.calls destructured as any[] (asserting nothing) and two files Prettier had never seen. All three are fixed here.

Landing

Merge this before the internal PR — the internal gitlink points at this branch.

🤖 Generated with Claude Code

Reverts #754, which removed the Epic PR View protocol and renderer
surface after the stop-ship call. The team has since reinstated the
feature, so this brings back everything #754 took out, on top of the
~55 commits that landed on main during the revert window.

Three adaptations the mechanical revert could not make:

- `updatePrDiffTileView` now re-narrows the ref before spreading.
  #809 (comm-graph tile) made a tile's `view` per-kind and re-narrowed
  the git/snapshot siblings; the restored PR helper still carried the
  old bare spread over the union, which no longer type-checks.
- Four restored PR test fixtures pass `hostCredentialMint: null`, a
  `WsStreamClientOptions` field added after the revert.
- `makeSelectActiveEpicArtifactRef` keeps main's `return active` while
  restoring the feature's generic `isTileRefRecordBacked` guard. That
  guard also covers the comm-graph tile, whose synthetic id could
  otherwise reach the persisted `lastFocusedArtifactId` - main's
  explicit `isDiffTileRef` check spans only git-diff and snapshot-diff.

`pr.*` was never part of a released protocol surface: it landed between
host-v1.1.8 and host-v1.1.9, and v1.1.9 shipped with the revert already
in. Restoring it is therefore additive against every released baseline,
and it stays off the released floor - `pr.getLocalDiff` keeps
`degrade: { kind: "unsupported" }` and the two streams stay plain v1.

Validation: compile 5/5 projects; protocol 1614/1614; gui-app 9063
passed / 1 skipped; protocol-compat gate compatible with all 7 published
baselines including cli-v1.1.9 (0 exceptions); released-baseline
handshake 30/30; guarded-files tripwire clean.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
`PrListBackgroundMount` was mounted unconditionally in the epic shell, so
opening ANY epic immediately subscribed `pr.subscribeListForEpic`, which
made the host run its bounded `gh` discovery and then sweep on a timer -
per epic pane. That was the reported epic-open slowdown.

The panel already gated itself correctly
(`!mainCollapsed && !sectionCollapsed && methodSupported`), so the fix is
to delete the one unconditional subscriber rather than add gating: PR
data is now fetched only while the Pull Requests panel is actually open.

The changed-dot goes with it. It existed only to make an epic-open sweep
useful, and it cannot be honest without one, so `pr-changed-dot`, the
seen-facts baseline store and the rail badge are removed rather than left
to light from data nothing collects.

Presence-gating survives, because the rail still needs to know whether an
epic has PRs at all. It moves to a much smaller `pr-presence-store`: one
persisted boolean per `(hostId, epicId)`, written from the open panel's
own stream frames and read by `isAutoVisible`. The deliberate cost is
that a first-ever open of an epic shows no PR icon until the panel is
revealed once from the rail context menu ("No PRs yet"); every later open
shows it immediately with no fetch and no flicker.

This is the renderer half. Serving several epics from one shared source,
refreshing on a TTL, and adding Octokit-style rate-limit prevention is
the separate follow-up - decision #7 ("no rate-limit infrastructure
needed") assumed one sweep, not one per open epic.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
`PrOwnerBadges` mapped every owner into a `flex-wrap` row. A PR derived
from dozens of chats therefore wrapped into tens of rows, which broke the
panel row's fixed four-band block so a list of PRs no longer scanned as a
list, and did the same to the detail card's Chats section.

Both surfaces render this one component, so capping it fixes both: three
chips inline, then a `+N` chip whose popover lists every owner as a
chats-menu-style row (kind icon + title) that opens its own tile.

Two details that are deliberate rather than incidental:

- The popover lists ALL owners, not just the hidden tail. Someone who
  opened it is hunting one specific chat and should not have to also scan
  the chips behind it.
- Nothing collapses at exactly one over the limit, where a "+1" would
  cost the same width as the chip it hides.

The popover stops click and keydown propagation: without that, activating
a row would bubble to the PR row's own handler and open the PR tile as
well as the chat.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
`prRowTitleText` returns `null` for a PR whose title has not been
observed, but the row rendered the `<p>` regardless, so the element was
present and empty. The card then read as "number badge, blank line,
branch" - reported as the PR summary sometimes not appearing.

The projection's own contract already says what to do here: a null title
means the identity token IS the label, not that the PR is untitled, and
that identity is already rendered in the number badge above. So the row
drops the element instead of leaving a gap where a title would go.

Ships with the fetch-gating change on purpose: deferring PR fetches to
panel-open makes an unswept row a normal sight rather than a rare one, so
the blank line would have become far more visible.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
The host is gaining a rate-limit-aware fetch layer that pauses PR
refreshes instead of spending requests it already knows will fail. Today
that pause is invisible: rows simply stop updating.

Adds `notice` to both `pr.*` frame families - `{ kind, retryAt }`,
structure and no prose - and renders it as an information icon in the
panel header and the detail tile header. Every word the user reads about a
pause lives in one client-side module, so the two surfaces cannot drift
into saying different things.

Freshness is now stated in both directions. A PR that has never been
fetched reads "Not yet fetched" rather than rendering nothing at all,
which is what made the earlier tooltip wording ("showing the last data
fetched") a claim it could not support. The detail tile's stamp moves out
of the context card - hidden below 1180px - and into the header, so a
narrow tile no longer explains a pause with nothing on screen saying how
old the rows it is pausing actually are.

The tooltip trigger is a button inside the live region rather than a
focusable status span. The tooltip is the only place the full explanation
exists, and a hover-only trigger handed it to pointer users while leaving
sighted keyboard users looking at an icon they could not open.

Also fixes three things this branch shipped unchecked, because the
submodule worktree carries no pre-commit hook: an untyped `vi.fn()` whose
`mock.calls` destructured as `any[]` and therefore asserted nothing, and
two files Prettier had never seen.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 343f76a4-ddfe-478e-afc2-e8d133397ae3

📥 Commits

Reviewing files that changed from the base of the PR and between 5aa20c6 and df9d304.

📒 Files selected for processing (2)
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-row.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-stream-test-fixtures.ts

Summary by CodeRabbit

  • New Features
    • Added a Pull Requests panel with repository grouping, status indicators, freshness notices, visibility controls, and refresh support.
    • Added detailed pull-request views for overview, conversations, files, checks, commits, and attention items.
    • Added local diff views with collapsible files, stale-data warnings, and unavailable-state messaging.
    • Added pull-request tiles, GitHub links, owner badges, quote-to-chat actions, and keyboard-accessible navigation.
    • Added sidebar context-menu controls for showing, hiding, and resetting panels.
  • Bug Fixes
    • Improved streamed Markdown handling for balanced HTML containers and streaming boundaries.
    • Added clearer messaging when pull-request data sources are unavailable.
  • Accessibility
    • Enhanced tab navigation, labels, announcements, contrast, and keyboard interactions.

Walkthrough

Added Pull Request host contracts, shared subscriptions, persisted presence, sidebar visibility controls, renderer-only canvas tiles, detail and local-diff views, PR projections, quote actions, Markdown block handling, shared styling, and extensive tests.

Changes

Pull Request platform and data flow

Layer / File(s) Summary
Host contracts and subscriptions
protocol/src/host/*, clients/gui-app/src/hooks/pr/*
Added versioned PR list, detail, and local-diff contracts. Added shared WebSocket sessions, cache updates, refresh handling, notices, retries, and tab-bound host resolution.
PR projections and state
clients/gui-app/src/lib/pr/*, clients/gui-app/src/stores/epics/pr-*.ts, clients/gui-app/src/lib/query-keys/*
Added PR grouping, check classification, conversation projection, attention queues, quote payloads, deterministic tile identities, detail tabs, presence persistence, and query keys.
Sidebar and canvas integration
clients/gui-app/src/components/epic-canvas/sidebar/*, clients/gui-app/src/stores/epics/canvas/*, clients/gui-app/src/components/epic-canvas/renderers/*
Added the Pull Requests panel, visibility overrides, rail context menus, PR detail and diff tile schemas, renderers, active-tile selectors, and persisted diff actions.
PR detail presentation
clients/gui-app/src/components/epic-canvas/pr/*
Added PR list rows, detail headers, tabs, checks, commits, conversations, files, queues, owner badges, source notices, local diffs, avatars, and quote-target selection.
Validation and supporting UI changes
clients/gui-app/**/__tests__/*, protocol/src/host/__tests__/*, clients/gui-app/src/markdown/*, clients/gui-app/src/components/worktree/*
Added coverage for PR behavior, accessibility, persistence, canvas integration, protocol validation, and Markdown HTML containers. Added shared PR state-pill styling and accent surface tokens.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Sidebar as Pull Requests panel
  participant ListStream as PR list subscription
  participant Presence as PR presence store
  participant Canvas as Epic Canvas
  participant DetailStream as PR detail subscription
  participant LocalDiff as Local diff RPC

  Sidebar->>ListStream: Subscribe for epic PRs
  ListStream-->>Sidebar: Snapshot or update frame
  Sidebar->>Presence: Record PR presence
  Sidebar->>Canvas: Open PR detail tile
  Canvas->>DetailStream: Subscribe to PR detail
  DetailStream-->>Canvas: Detail snapshot and updates
  Canvas->>LocalDiff: Request base-to-head diff
  LocalDiff-->>Canvas: Diff or unavailable response
Loading

Possibly related PRs

Suggested labels: protocol-compat-override

Poem

A rabbit checks each diff with care,
Pull Request tiles bloom everywhere.
Tabs hop, streams flow,
Quotes find chat below,
While panels align with a tidy stare.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.95% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: restoring the Epic PR View and reducing excessive GitHub fetching.
Description check ✅ Passed The description directly explains the restored PR view, rate-limit-aware fetching, UI changes, fixes, and validation results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch restore/epic-pr-view

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 39

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx`:
- Around line 143-236: The duplicated MockStreamSession and MockWsStreamClient
fixtures should be centralized into one shared fixture module. In
clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx
lines 143-236, replace the local classes with imports from that module and use
PrSubscribeDetailServerFrame as the emitFrame type; in
clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-actions.test.tsx
lines 37-129, import the same classes and use PrSubscribeListForEpicServerFrame.
Ensure both tests share the fixture while preserving their respective frame
types.
- Around line 715-718: Update the tab-click loop in the PR detail body test to
select each interactive tab with screen.getByRole("tab", { name: ... }) using
the current tab name, while retaining shellClasses() and its existing
getByTestId-based class assertions unchanged.

In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-commits.test.tsx`:
- Around line 20-23: Update the navigator setup in the PR detail commits test to
preserve the existing navigator object and define or replace only its clipboard
property. Remove the spread-based replacement object, keeping all
prototype-provided navigator members such as userAgent and language intact while
retaining the writeText stub.

In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-files-tab.test.tsx`:
- Around line 232-239: In the test around the external-link assertion, replace
the zero-delay setTimeout wrapped in act with waitFor that retries until
openExternalLink has been called once with the expected pull-request files URL.
Add waitFor to the `@testing-library/react` import and remove the now-unused act
import.
- Around line 176-199: Update the diff-opener queries in the affected tests to
use screen.getByRole<HTMLButtonElement>("button", { name: /Open diff/ }) instead
of getByTestId("pr-detail-open-diff"), and assert the button’s native disabled
property. Retain getByTestId only for non-interactive assertions such as row
counts.

In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-tab-strip.test.tsx`:
- Around line 14-15: Update the fixture values in the PR detail tab strip tests
so counts.checks and blocking.checks differ, including the corresponding values
in the additional affected cases, allowing assertions to verify that the Checks
tab uses the blocking count rather than the plain count.

In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-actions.test.tsx`:
- Around line 206-218: Reset the module-level subscription registry in this
suite’s beforeEach and afterEach alongside resetCanvas and queryClient.clear.
Use the existing usePrListSubscription testing reset helper, matching the
__resetPrDetailSubscriptionsForTesting pattern, so each test starts and ends
without shared live subscription entries.

In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-body.test.tsx`:
- Around line 36-38: Update PrPanelBody’s PR tile creation to derive hostId from
the current tab scope rather than useReactiveActiveHostId(), using the
appropriate tab-scoped host resolution without useTabHostId() unless
TabHostProvider is present. Adjust the test mock and assertions to cover an
active host differing from the tab’s bound host, ensuring the persisted tile
uses the tab host.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-body.tsx`:
- Around line 477-489: Update oldestObservedAt to include
data.reviewThreads.observedAt in the reduced observation list, ensuring stale
review-thread data contributes to the header staleness calculation. Also update
the associated doc comment count to reflect the additional section.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-commits.tsx`:
- Around line 49-82: Update the commit header hint condition in the component
rendering the commits summary to use props.commits.isTruncated alongside the
count comparison, matching the footer’s truncation state. Preserve the existing
“Showing the last N” text and count behavior while ensuring it appears whenever
the commits contract indicates truncation, including when totalCount is null.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-conversation.tsx`:
- Around line 601-615: Update PrOlderOnGitHub to use useRunnerOpenExternalLink
for props.href when a RunnerHost is bound, preventing native navigation in that
case. Preserve the existing target="_blank" anchor behavior when no RunnerHost
is available, following the pattern in pr-detail-files-tab.tsx.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-files-tab.tsx`:
- Around line 141-164: Update PrFilesGitHubFooterLink to guard duplicate clicks
using openExternalLink.isPending, matching PrDetailGitHubLink: prevent the
action while the request is pending, apply the pending disabled behavior, and
render inline AgentSpinningDots without changing the existing link label.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-header.tsx`:
- Around line 154-170: Remove the obsolete first JSDoc block immediately above
PrDetailMergeLine, retaining only the second block that documents the function’s
author and branch-flow content.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-queue.tsx`:
- Around line 215-253: In the headline rendering section, collapse the
duplicated TooltipWrapper instances into one shared wrapper with the existing
label, positioning props, and surrounding structure. Keep the conditional
distinction only in its child, rendering the non-interactive paragraph when
item.detailsUrl is null and the clickable button with openDetails otherwise.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-detail-tone.ts`:
- Around line 72-105: Update prChecksTone and formatPrChecksValue to handle
totals not represented by passed, failing, or pending: after the existing
failing and pending checks, return "none" and label the value as the count of
checks (for example, "4 checks") when the remaining count is nonzero. Preserve
the current empty, failing, pending, and fully passed behavior, matching
prChecksSummary’s fallback semantics.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-local-diff-body.tsx`:
- Around line 87-96: Update the GitHub anchors in the component, including the
“View the full diff on GitHub” link and the truncation-notice anchor, to use
useRunnerOpenExternalLink when a RunnerHost is bound. Keep target="_blank" and
native anchor behavior when no RunnerHost is available; the fallback handler
must be a no-op and must not call preventDefault.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-owner-label.tsx`:
- Around line 316-345: Extract the duplicated owner-resolution logic from
PrOwnerBadge and PrOwnerRow into a shared local hook such as
usePrOwnerResolution, including the gated chat and terminal-agent lookups, host
fallback, label, icon, and openTileInEpic behavior. Have the hook return {
label, hostId, Icon, openOwner }, and update both components to consume it while
preserving their existing callbacks and inputs.
- Around line 169-177: Update the overflow chip aria-label and the popover
heading in the PrOwnerLabel component to use a noun that correctly covers both
“chats” and “terminal-agent” owners, preferably a kind-neutral “owners” label or
one derived from the present owner kinds. Update the corresponding assertion in
pr-owner-label.test.tsx to match the revised wording.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-panel-body.tsx`:
- Around line 371-377: Update the warning branch in the banner className to use
the existing theme warning token classes, matching the token pair defined by
PR_TONE_SURFACE_CLASS.pending and PR_TONE_CHIP_CLASS.pending instead of fixed
amber classes. Leave the error tone unchanged.

In `@clients/gui-app/src/components/epic-canvas/pr/pr-source-notice.tsx`:
- Around line 29-46: Update the role="status" region in the PR source notice
component to include the notice message as visually hidden text, so live-region
announcements use text content rather than aria-label; preserve the button’s
aria-label for the focus path. Update the corresponding pr-source-notice test
assertion to verify the hidden message text instead of the region’s aria-label.

In `@clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx`:
- Around line 495-499: Use useTabHostId() instead of useReactiveActiveHostId()
for tab-bound host resolution across EpicLeftPanelHost and
EpicLeftPanelLoadingHost in
clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx:495-499
(also update the same pattern at line 547), EpicLeftPanelRailContent in
clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar-rail.tsx:176-179,
and PrPanelActionsLive in
clients/gui-app/src/components/epic-canvas/pr/pr-panel-actions.tsx:39-48; also
replace the same active-host lookup in the corresponding PR panel body if
present, so all per-tab surfaces use the tab’s bound host identity.

In `@clients/gui-app/src/hooks/pr/use-pr-detail-subscription.ts`:
- Around line 75-131: Extract the duplicated subscription registry and lifecycle
logic into one generic shared helper, parameterized by method name, frame type,
cache-key builder, and frame-to-data mapper. In
clients/gui-app/src/hooks/pr/use-pr-detail-subscription.ts#L75-L131, retain
detail-specific schema, query keys, mapper, and methodSupported handling while
delegating keying, ref-counting, consumer migration, terminal retries, and reset
behavior. In clients/gui-app/src/hooks/pr/use-pr-list-subscription.ts#L48-L104,
remove the duplicated registry/key/reset implementations and use the helper,
preserving mode in the list session key.

In `@clients/gui-app/src/hooks/pr/use-pr-list-subscription.ts`:
- Around line 364-396: Introduce a shared frame-to-data mapper near
writeIntoCache, following the existing toSubscriptionData pattern in
use-pr-detail-subscription.ts, that converts PrListDataFrame into
PrListSubscriptionData. Update both writeIntoCache and replayLastEventIntoCache
to use this mapper instead of duplicating the object literal.

In `@clients/gui-app/src/hooks/pr/use-pr-local-diff.ts`:
- Around line 59-88: Move the first JSDoc block describing the local-diff query
behavior from above localDiffKeyParts to directly above usePrLocalDiffQuery,
leaving the identity-key documentation attached to localDiffKeyParts. Ensure the
hook retains documentation for host resolution, staleTime, and
E_HOST_UNSUPPORTED fallback.

In `@clients/gui-app/src/lib/persist/keys.ts`:
- Line 167: Update the static zustand stores section header comment from 21 to
24 to match the current number of kind: "static" entries, leaving the correct
scoped header and store definitions unchanged.

In `@clients/gui-app/src/lib/pr/__tests__/pr-list-projection.test.ts`:
- Around line 199-257: Add coverage in the test suite for prRowTitleText and
formatPrRowTitle, importing both helpers and using the existing item fixture.
Verify null and empty titles return null, absent titles fall back to the bare
number or branch identity, and known identity plus title formats as “identity ·
title,” preventing regression to “Untitled pull request” output.

In `@clients/gui-app/src/lib/pr/pr-attention-queue.ts`:
- Around line 239-253: Align the comment above the review-required condition
with its actual behavior: update it to state that only a standing objection
represented by blocking suppresses the row, while failing checks do not. Keep
the existing condition and review-required item logic unchanged.

In `@clients/gui-app/src/lib/pr/pr-check-groups.ts`:
- Around line 121-126: Update prCheckContextKey so it always appends the index,
including when context.detailsUrl is present, ensuring keys remain unique across
contexts sharing a target URL. Adjust the corresponding assertion in
pr-check-groups.test.ts to expect the indexed URL.

In `@clients/gui-app/src/lib/pr/pr-quote.ts`:
- Around line 79-90: Update the check label construction in the quote builder to
use formatPrCheckName instead of context.name, importing it from the existing
pr-check-groups module. Preserve the surrounding status and details URL
formatting while ensuring reusable workflow checks retain their composed
workflow, job, and event disambiguation.

In `@clients/gui-app/src/lib/pr/pr-source-notice-message.ts`:
- Around line 22-30: Update the notice message function to switch exhaustively
on notice.kind instead of using a fallback return after the rate-limited branch.
Preserve the existing messages for rate-limited and unreachable cases, and add a
never guard for any unhandled kind so future PrSourceNotice variants fail type
checking.

In `@clients/gui-app/src/lib/query-keys/pr-query-keys.ts`:
- Around line 16-17: Update the listForEpic query-key builder to accept a
named-arguments object, matching the signatures of detail and localDiff, with
hostId and epicId accessed by property. Update every listForEpic call site to
pass the named fields and preserve the generated key structure.

In `@clients/gui-app/src/stores/epics/canvas/__tests__/store.test.ts`:
- Around line 1159-1184: Update the test around openTileInTab so it actually
creates a second pane, opening right in that new pane while left remains in the
original pane; then keep assertions that the active pane’s tile is active and
the tile parked in the other pane is inactive. Alternatively, rename the test to
describe the same-pane active-tab behavior it currently exercises.

In `@clients/gui-app/src/stores/epics/canvas/canvas-selectors.ts`:
- Around line 248-262: Extract the shared active-tile resolution from
makeSelectIsActiveTile and makeSelectActiveEpicArtifactRef into one private
resolver, then update both selectors to use it so pane and active-tab lookup
rules remain consistent. Preserve makeSelectActiveEpicArtifactRef’s existing
isTileRefRecordBacked gate and makeSelectIsActiveTile’s boolean result behavior.

In `@clients/gui-app/src/stores/epics/canvas/tile-schema/pr-detail-tile.ts`:
- Around line 19-33: Both PR tile parsers accept invalid pull-request numbers;
require positive integers before building tile IDs. In
clients/gui-app/src/stores/epics/canvas/tile-schema/pr-detail-tile.ts lines
19-33, update the validation condition before prDetailTileId to reject
non-integers and values less than or equal to zero. Apply the same validation in
clients/gui-app/src/stores/epics/canvas/tile-schema/pr-diff-tile.ts lines 24-34
before prDiffTileId.

In `@clients/gui-app/src/stores/epics/left-panel-store.ts`:
- Around line 388-401: Replace the single-entry cache used by
getStoredPanelGroups with a WeakMap keyed by object inputs, so alternating
panelGroups identities reuse their previously normalized ReadonlyArray results.
Retain a separate fallback cache for non-object inputs, and preserve the
existing readPersistedPanelGroups, DEFAULT_LEFT_PANEL_GROUPS, and
normalizeLeftPanelGroups behavior.

In `@clients/gui-app/src/stores/epics/pr-presence-store.ts`:
- Line 24: Update the persistence handling for hasItemsByScopeKey so stale
entries do not accumulate indefinitely in localStorage. During rehydration,
remove entries whose hostId or epicId is no longer known, or implement a bounded
least-recently-written eviction policy while preserving current entries for
active scopes.
- Around line 89-96: Update the persistence options around
migratePrPresencePersistedState to add a merge handler that sanitizes the
rehydrated hasItemsByScopeKey through migratePrPresencePersistedState, including
when the persisted version already matches version 1. Preserve the existing
partialize and migration behavior while ensuring the sanitized state is merged
into the current store state.

In `@protocol/src/host/__tests__/pr-schemas.test.ts`:
- Around line 365-371: Update the test around prCheckStatusSchema to assert the
schema’s complete option set, including an exact six-value count or equivalent
set equality, rather than only parsing each listed status. Keep the existing
status values covered while ensuring the test fails if another option is added
without updating the client handling.

In `@protocol/src/host/pr-schemas.ts`:
- Around line 659-676: Update prGetLocalDiffRequestSchema so its linkGroupKey
field rejects empty strings and repoIdentifier.owner and repoIdentifier.repo are
validated as non-empty at this request boundary. Preserve prLinkGroupKeySchema
and prRepoIdentifierSchema unchanged because they are shared by nullable
stream-frame schemas that may allow empty identifiers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d334dca3-367e-4fd5-96f7-61e43fd70d96

📥 Commits

Reviewing files that changed from the base of the PR and between d35b3f9 and 2ef5fee.

📒 Files selected for processing (106)
  • clients/gui-app/__tests__/contrast.ts
  • clients/gui-app/src/components/epic-canvas/__tests__/epic-sidebar.test.tsx
  • clients/gui-app/src/components/epic-canvas/__tests__/left-panel-registry.test.ts
  • clients/gui-app/src/components/epic-canvas/__tests__/root-dnd-commits.test.ts
  • clients/gui-app/src/components/epic-canvas/canvas/tab-group-view.tsx
  • clients/gui-app/src/components/epic-canvas/canvas/tab-strip.tsx
  • clients/gui-app/src/components/epic-canvas/dnd/drag-overlay-chip.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-checks.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-commits.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-conversation.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-files-tab.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-header.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-queue.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-tab-strip.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-local-diff-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-owner-label.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-actions.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-row.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-source-notice.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-avatar.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-card.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-commits.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-conversation.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-files-tab.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-header.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-queue.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-sections.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-tab-strip.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-tone.ts
  • clients/gui-app/src/components/epic-canvas/pr/pr-local-diff-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-owner-label.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-panel-actions.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-panel-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-quote-target-picker.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-row.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-source-notice.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-state-pill.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/dead-tile-banner.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/pr-detail-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/pr-diff-tile.tsx
  • clients/gui-app/src/components/epic-canvas/renderers/tile-render.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-file-tree-report-issue.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-nested-focus-boundary.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-selection-mode.test.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar-rail.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/epic-sidebar.tsx
  • clients/gui-app/src/components/epic-canvas/sidebar/left-panel-registry.ts
  • clients/gui-app/src/components/worktree/worktree-pr-metadata.tsx
  • clients/gui-app/src/components/worktree/worktree-pr-state-palette.ts
  • clients/gui-app/src/hooks/pr/__tests__/use-pr-detail-subscription.test.tsx
  • clients/gui-app/src/hooks/pr/__tests__/use-pr-list-subscription.test.tsx
  • clients/gui-app/src/hooks/pr/use-pr-detail-subscription.ts
  • clients/gui-app/src/hooks/pr/use-pr-list-subscription.ts
  • clients/gui-app/src/hooks/pr/use-pr-local-diff.ts
  • clients/gui-app/src/hooks/pr/use-pr-quote-targets.ts
  • clients/gui-app/src/lib/host-rpc-policy/host-method-policy-table.ts
  • clients/gui-app/src/lib/host/stream-runtime-context.ts
  • clients/gui-app/src/lib/persist/__tests__/keys.test.ts
  • clients/gui-app/src/lib/persist/__tests__/store-persist-names.test.ts
  • clients/gui-app/src/lib/persist/keys.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-attention-queue.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-check-groups.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-conversation.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-detail-projection.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-list-projection.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-review-hunk.test.ts
  • clients/gui-app/src/lib/pr/pr-attention-queue.ts
  • clients/gui-app/src/lib/pr/pr-check-groups.ts
  • clients/gui-app/src/lib/pr/pr-conversation.ts
  • clients/gui-app/src/lib/pr/pr-detail-projection.ts
  • clients/gui-app/src/lib/pr/pr-detail-tile.ts
  • clients/gui-app/src/lib/pr/pr-diff-tile.ts
  • clients/gui-app/src/lib/pr/pr-list-projection.ts
  • clients/gui-app/src/lib/pr/pr-quote.ts
  • clients/gui-app/src/lib/pr/pr-review-hunk.ts
  • clients/gui-app/src/lib/pr/pr-source-notice-message.ts
  • clients/gui-app/src/lib/query-keys/__tests__/pr-query-keys.test.ts
  • clients/gui-app/src/lib/query-keys/index.ts
  • clients/gui-app/src/lib/query-keys/pr-query-keys.ts
  • clients/gui-app/src/markdown/__tests__/markdown-html-containers.test.tsx
  • clients/gui-app/src/markdown/traycer-markdown.tsx
  • clients/gui-app/src/markdown/use-markdown-blocks.ts
  • clients/gui-app/src/stores/epics/__tests__/left-panel-store.test.ts
  • clients/gui-app/src/stores/epics/__tests__/pr-presence-store.test.ts
  • clients/gui-app/src/stores/epics/canvas/__tests__/store.test.ts
  • clients/gui-app/src/stores/epics/canvas/__tests__/tile-schema.test.ts
  • clients/gui-app/src/stores/epics/canvas/actions.ts
  • clients/gui-app/src/stores/epics/canvas/canvas-selectors.ts
  • clients/gui-app/src/stores/epics/canvas/store.ts
  • clients/gui-app/src/stores/epics/canvas/tile-kind-types.ts
  • clients/gui-app/src/stores/epics/canvas/tile-kinds.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/index.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/pr-detail-tile.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/pr-diff-tile.ts
  • clients/gui-app/src/stores/epics/canvas/types.ts
  • clients/gui-app/src/stores/epics/left-panel-store.ts
  • clients/gui-app/src/stores/epics/pr-detail-view-store.ts
  • clients/gui-app/src/stores/epics/pr-presence-store.ts
  • protocol/src/host/__tests__/pr-schemas.test.ts
  • protocol/src/host/index.ts
  • protocol/src/host/pr-contracts.ts
  • protocol/src/host/pr-schemas.ts
  • protocol/src/host/registry.ts

Comment thread clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx Outdated
Comment thread clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx Outdated
Comment thread clients/gui-app/src/stores/epics/left-panel-store.ts Outdated
Comment thread clients/gui-app/src/stores/epics/pr-presence-store.ts
Comment thread clients/gui-app/src/stores/epics/pr-presence-store.ts
Comment thread protocol/src/host/__tests__/pr-schemas.test.ts
Comment thread protocol/src/host/pr-schemas.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-selection-mode.test.tsx (1)

605-637: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Return a full query result from the provider-list mock.

The mock returns only { data }. This can hide loading, error, and pending-state regressions in the sidebar. Use a disabled real useQuery with a non-void queryFn and initialData, or return a complete UseQueryResult.

Based on learnings, GUI sidebar tests should use a disabled real query with a non-void query function instead of a partial host-query result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-selection-mode.test.tsx`
around lines 605 - 637, Update the useProvidersListForClient mock to use a
disabled real useQuery with a non-void queryFn and the existing provider payload
as initialData, preserving the current provider/profile fixtures while exposing
complete query states to the sidebar tests.

Source: Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In
`@clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-selection-mode.test.tsx`:
- Around line 605-637: Update the useProvidersListForClient mock to use a
disabled real useQuery with a non-void queryFn and the existing provider payload
as initialData, preserving the current provider/profile fixtures while exposing
complete query states to the sidebar tests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 710a64bc-8bc1-4328-953b-6633e2092a9c

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef5fee and e65c510.

📒 Files selected for processing (4)
  • clients/gui-app/src/components/epic-canvas/sidebar/__tests__/epic-sidebar-selection-mode.test.tsx
  • clients/gui-app/src/lib/host-rpc-policy/host-method-policy-table.ts
  • clients/gui-app/src/stores/epics/canvas/store.ts
  • protocol/src/host/registry.ts

…shness

Five fixes from the CodeRabbit pass, each a thing the UI told the user that
was not true.

The paused-source notice put its sentence in the live region's `aria-label`.
A live region announces its accessible CONTENT when that content changes, so
labelling an otherwise-empty region left nothing to announce: the pause
appeared on screen and said nothing at all. The sentence is now visually
hidden text inside the region; the button keeps its own label, because a
control is named by its label.

`oldestObservedAt` omitted `reviewThreads.observedAt`, so a stale review
thread section could not drag the header's freshness hint down - the header
claimed the view was fresher than its stalest visible content.

The commits header decided "Showing the last N" from a count comparison
rather than the contract's `isTruncated`. With a null `totalCount` the
comparison is false on exactly the truncated frames, so the header claimed a
complete list while the footer offered "view all on GitHub".

`prChecksTone`/`formatPrChecksValue` treated `total` as the sum of the three
buckets. It is not - skipped, neutral and cancelled land in `total` alone -
so an all-skipped run rendered a green dot reading "0 passed".
`prChecksSummary` already got this right; these two now match it.

`prSourceNoticeMessage` fell through to the offline sentence for any
unrecognised kind. A new wire kind is now a compile error instead of
silently inheriting copy that would describe the wrong thing.

Also corrects the static-store count comment, which said 21 against 26
entries.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-source-notice.test.tsx`:
- Around line 95-99: Update the status-region assertions in the PR source notice
test to select it with screen.getByRole("status") instead of the
pr-source-notice test ID, and assert the expected text on that role-selected
element. Remove the redundant pr-source-notice-message test ID assertion while
preserving verification of the status region’s accessible contract and message
content.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5e353100-8578-4a51-b788-c51b6888ddd5

📥 Commits

Reviewing files that changed from the base of the PR and between e65c510 and 9e7ee40.

📒 Files selected for processing (8)
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-tone.test.ts
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-source-notice.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-commits.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-tone.ts
  • clients/gui-app/src/components/epic-canvas/pr/pr-source-notice.tsx
  • clients/gui-app/src/lib/persist/keys.ts
  • clients/gui-app/src/lib/pr/pr-source-notice-message.ts

Comment thread clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-source-notice.test.tsx Outdated
@hdkshingala

Copy link
Copy Markdown
Member Author

CodeRabbit triage — 8 of 39 addressed, 31 deliberately deferred

Every finding was checked against current code. Disposition below so the open threads are not mistaken for unread ones.

Fixed (6) — 9e7ee40c

Finding Why it was real
pr-source-notice.tsx — live region announces nothing A live region announces its accessible content; an aria-label on an empty region left nothing to announce. The pause was silent.
pr-detail-body.tsx — reviewThreads.observedAt excluded A stale review-thread section could not drag the freshness hint down, so the header claimed to be fresher than its stalest visible content.
pr-detail-commits.tsx — count comparison instead of isTruncated With a null totalCount the comparison is false on exactly the truncated frames: header claimed a complete list while the footer offered "view all on GitHub".
pr-detail-tone.ts — total treated as the bucket sum Skipped/neutral/cancelled land in total alone, so an all-skipped run rendered a green dot reading "0 passed". prChecksSummary already handled this; these now match it.
pr-source-notice-message.ts — non-exhaustive kind A new wire kind silently inherited the offline sentence. Now a compile error.
keys.ts — stale static-store count Stale, though the number is 26, not the 24 suggested.

Both behavioural fixes were mutation-probed: reverting the production line turns only the corresponding test red.

False positives (2)

epic-sidebar.tsx and pr-panel-body.test.tsx — "bind to the tab host". <TabHostProvider> is mounted only in tile-render.tsx and landing-terminal-tile.tsx; the sidebar and left panel are not tiles, so useTabHostId() there throws its own guard. Details in those threads. The rule is real but scoped to tiles, and the PR detail tile already honours it.

Deferred (31) — left open for maintainer triage

Refactor, test-organisation and coverage suggestions rather than defects: extract-shared-helper, centralize-fixture, select-by-role, add-coverage-for-X, doc-wording. They are reasonable, but they are preferences on an already-large reinstated changeset, and none of them changes behaviour. Left open rather than silently resolved so a human can take the ones they want.

Behavioural fixes:

Both PR tile parsers accepted any `number` for `prNumber`, so `NaN`, `0`,
negatives and fractions all survived the persistence boundary. Every tile-id
builder interpolates that value with `String(...)`, so a corrupted record
restored a tile that renders, subscribes, and can only ever report "not
found". `isPersistedPrNumber` now applies the domain the wire schema already
states. It drops only records that were always unusable: a tile written by any
released build carries a number GitHub issued, positive by construction.

GitHub anchors in the local-diff body and the conversation footer bypassed the
Runner bridge, so on a RunnerHost they navigated natively instead of opening
externally. Both now route through it, and both keep native anchor behaviour
when no RunnerHost is bound - the fallback is a no-op that must not call
`preventDefault`. The files-tab footer link additionally guards against a
duplicate request while one is pending, matching `PrDetailGitHubLink`.

`pr-presence-store` now sanitizes persisted state on every rehydration rather
than trusting what was read back, and `pr-check-groups` no longer assumes
`detailsUrl` is unique across contexts.

The rest is structure the review asked for and the tests it proved missing:
a shared ref-counted subscription registry behind both PR hooks, one
frame-to-data mapper, shared owner resolution behind the badge and the row,
one `pr-external-github-link` component, theme warning tokens in place of a
fixed amber ramp, role-based queries in the tests that were reaching for
test ids, and coverage for the null-title fallback and the check-name
composition.

Verified: `bun run compile` clean across all 5 projects, gui-app PR/store/hook
suites 756 passing across 66 files, protocol 1642 passing, lint and format
clean.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-stream-test-fixtures.ts`:
- Around line 56-59: Update the fixture’s close() method so repeated calls
return without emitting another status event after the session is already
closed. Preserve the existing closed-state assignment and single "closed"
notification on the first call, matching IStreamSession.close() idempotency.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: af735bbc-9656-4f39-8086-34e139f3611f

📥 Commits

Reviewing files that changed from the base of the PR and between 9e7ee40 and 5aa20c6.

📒 Files selected for processing (42)
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-commits.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-files-tab.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-detail-tab-strip.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-external-github-link.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-local-diff-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-owner-label.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-actions.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-panel-body.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-source-notice.test.tsx
  • clients/gui-app/src/components/epic-canvas/pr/__tests__/pr-stream-test-fixtures.ts
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-conversation.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-files-tab.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-header.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-queue.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-external-github-link.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-local-diff-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-owner-label.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-panel-body.tsx
  • clients/gui-app/src/components/epic-canvas/pr/pr-row.tsx
  • clients/gui-app/src/hooks/pr/shared-stream-subscription.ts
  • clients/gui-app/src/hooks/pr/use-pr-detail-subscription.ts
  • clients/gui-app/src/hooks/pr/use-pr-list-subscription.ts
  • clients/gui-app/src/hooks/pr/use-pr-local-diff.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-check-groups.test.ts
  • clients/gui-app/src/lib/pr/__tests__/pr-list-projection.test.ts
  • clients/gui-app/src/lib/pr/pr-attention-queue.ts
  • clients/gui-app/src/lib/pr/pr-check-groups.ts
  • clients/gui-app/src/lib/pr/pr-quote.ts
  • clients/gui-app/src/lib/query-keys/pr-query-keys.ts
  • clients/gui-app/src/stores/epics/__tests__/left-panel-store.test.ts
  • clients/gui-app/src/stores/epics/__tests__/pr-presence-store.test.ts
  • clients/gui-app/src/stores/epics/canvas/__tests__/store.test.ts
  • clients/gui-app/src/stores/epics/canvas/__tests__/tile-schema.test.ts
  • clients/gui-app/src/stores/epics/canvas/canvas-selectors.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/pr-detail-tile.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/pr-diff-tile.ts
  • clients/gui-app/src/stores/epics/canvas/tile-schema/pr-number.ts
  • clients/gui-app/src/stores/epics/left-panel-store.ts
  • clients/gui-app/src/stores/epics/pr-presence-store.ts
  • protocol/src/host/__tests__/pr-schemas.test.ts
  • protocol/src/host/pr-schemas.ts
💤 Files with no reviewable changes (1)
  • clients/gui-app/src/components/epic-canvas/pr/pr-detail-header.tsx

The guard landed in 5aa20c6 without its test, so nothing held it. Removing
`if (isPending) return;` from `PrNumberAnchor` now turns this test red on its
own; before, a double click opened the user's browser twice and no test said
so.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
`IStreamSession.close()` is idempotent; this fixture was not, so a repeat
close emitted a second "closed" status event and could drive a lifecycle the
real transport cannot produce. A hook mishandling that would fail here while
working in the app, or pass here while broken in it.

Signed-off-by: Hardik Shingala <hardik@traycer.ai>
@hdkshingala

Copy link
Copy Markdown
Member Author

@coderabbitai approve

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved and changes approved.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant