Skip to content

feat: add topic task-room detail#1049

Open
donovan-yohan wants to merge 3 commits into
nightlyfrom
issue-1044-task-room-detail
Open

feat: add topic task-room detail#1049
donovan-yohan wants to merge 3 commits into
nightlyfrom
issue-1044-task-room-detail

Conversation

@donovan-yohan

@donovan-yohan donovan-yohan commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Closes #1044

Summary

  • adds a topic task-room detail panel in the topic sidebar with grouped session lanes, primary CTA, status card, task refs, and artifacts/surfaces
  • carries workspace id, lifecycle, updated timestamp, and artifact ids through the topic nav model for detail rendering without loading artifact payloads
  • keeps surface fetch failures scoped to the room artifact strip instead of blanking the topic list

Verification

  • PATH=/home/donovanyohan/.local/opt/node-v24.16.0-linux-x64/bin:$PATH rtk test npm test -- test/topic-nav.test.ts test/topic-sidebar-shell.test.ts — 2 files / 29 tests passed
  • PATH=/home/donovanyohan/.local/opt/node-v24.16.0-linux-x64/bin:$PATH rtk npm run check — exit 0; existing repo lint warnings only, no errors
  • PATH=/home/donovanyohan/.local/opt/node-v24.16.0-linux-x64/bin:$PATH rtk npm run build — exit 0; existing Vite chunk/dynamic-import warnings only
  • browser smoke: npm run dev on 127.0.0.1:5199, unlocked with local test PIN, verified topics sidebar renders with no console errors; topic writes were not seeded because this protected local hub denied missing context:write, so rich room states are covered by DOM tests

Notes

  • Simplify tier: 3 (multi-file frontend behavior/UI delta). Ran a three-lane simplify review pass; applied the useful local cleanup around stale-session grouping and primary surface CTA enablement.

Summary by CodeRabbit

  • New Features

    • Added a richer topic-room view with grouped session status, freshness details, and a clearer primary action area.
    • Topic navigation now shows additional topic metadata, including linked artifacts and last-updated information.
  • Bug Fixes

    • Improved handling when related data fails to load, showing a dedicated unavailable state instead of breaking the view.
    • Refined selection behavior so individual sessions in a room can still be opened directly, including disconnected or stale ones.
  • Tests

    • Added coverage for the new topic-room layout, metadata display, and error states.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@donovan-yohan, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 5 minutes and 1 second. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9896facd-164c-4a00-ae79-07521fa37e12

📥 Commits

Reviewing files that changed from the base of the PR and between 6249e7d and 908dd59.

📒 Files selected for processing (3)
  • frontend/src/components/TopicSidebarShell.css
  • frontend/src/components/TopicSidebarShell.tsx
  • test/topic-sidebar-shell.test.ts
📝 Walkthrough

Walkthrough

Replaces the generic TopicDetail selected-topic panel with a task-room view that groups linked sessions by state, renders task refs and a surface/artifact strip, and drives a state-aware primary action. TopicNavItem gains workspaceId, artifactIds, lifecycleLabel, and updatedAt fields. TopicSidebarView receives a new surfacesError prop threaded into the panel. 216 lines of CSS style the room layout.

Changes

Task-room detail view

Layer / File(s) Summary
TopicNavItem metadata extension
frontend/src/lib/state/topic-nav.ts, test/topic-nav.test.ts
TopicNavItem gains workspaceId, artifactIds, lifecycleLabel, and updatedAt; buildTopicNavModel populates them; a unit test asserts artifactIds round-trips.
Session grouping and labeling helpers
frontend/src/components/TopicSidebarShell.tsx
Adds TopicRoomSessionGroupKey, group-ordering constants, a session-to-group-key mapper, a non-empty group filter, and derived freshness/summary/anchor label helpers.
Task-room subcomponents
frontend/src/components/TopicSidebarShell.tsx
TopicRoomSessionRow computes per-session disabled reasons and calls onSelectSession; TopicRoomTaskRefs renders refs with optional links; TopicRoomSurfaceStrip renders surfaces and artifact IDs with error/empty states.
Task-room TopicDetail and TopicSidebarView wiring
frontend/src/components/TopicSidebarShell.tsx
Replaces generic TopicDetail with task-room panel (header, primary-action band, grouped sessions, surface strip). TopicSidebarView gains surfacesError?: boolean, threads it into TopicDetail, and separates surfaces-query error from the main error prop.
Topic-room CSS
frontend/src/components/TopicSidebarShell.css
Adds 216 lines: scrollable grid container, sticky header, muted typography, bordered action/status/section containers, accent primary button with disabled/hover states, session/ref/surface list grids, label truncation, and attention/warning color rules.
Component tests
test/topic-sidebar-shell.test.ts
Extends existing tests with room/primary-action assertions; adds tests for multi-session grouping, stale-session live-control disabling, global-session row selection, and surfaces-unavailable error messaging.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

  • #1044 – Build topic task-room detail view for grouped sessions and artifacts: This PR directly implements the acceptance criteria: grouped session list, artifact/surface strip, state-driven primary CTA, stale-session disabled controls, and component tests for grouping and error states.

Possibly related PRs

  • donovan-yohan/relay-ide#1030: Introduced the flag-gated TopicSidebarShell and the initial TopicNavItem shape that this PR extends with workspaceId/artifactIds.
  • donovan-yohan/relay-ide#1033: Also modifies TopicSidebarView and the topic-nav model in the same files touched here.
  • donovan-yohan/relay-ide#1042: Changes topicShellEnabled feature-flag defaults that control whether TopicSidebarShell (rewritten here) is rendered.

Poem

🐇 Hoppity-hop, the rooms are alive,
Sessions grouped so tasks can thrive.
Artifacts pinned, refs in a row,
Primary action steals the show.
No more scattered terminal mess—
One task room, one cozy nest! 🏠

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and matches the main change: adding a topic task-room detail view.
Linked Issues check ✅ Passed The changes add a grouped task-room detail view with header, session lanes, artifact strip, CTA states, and tests covering the requested behaviors.
Out of Scope Changes check ✅ Passed The modified files all support the topic task-room feature, navigation metadata, or its tests, with no clear unrelated additions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-1044-task-room-detail

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors the topic detail panel into a comprehensive 'Task Room' view, introducing new components, CSS styles, and state models to support grouped sessions, task references, and artifact/surface strips. Feedback on the changes includes addressing a potential reverse tabnabbing vulnerability by adding noopener to window.open, and optimizing the optional onSelectSession callback using conditional prop spreading to avoid unnecessary closures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

topSurface.openMode === 'direct' &&
topSurface.target.startsWith('http')
) {
window.open(topSurface.target, '_blank', 'noreferrer');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Using window.open with _blank without noopener can expose the application to reverse tabnabbing vulnerabilities, where the newly opened window can access and manipulate the original window via window.opener. Please include noopener in the window features string.

Suggested change
window.open(topSurface.target, '_blank', 'noreferrer');
window.open(topSurface.target, '_blank', 'noopener,noreferrer');

Comment on lines +355 to +360
<button
type="button"
className="topic-room-session__button"
title={`open exact session ${session.selectKey}`}
onClick={() => onSelectSession?.(session.selectKey)}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In accordance with the repository's general rules, optional function props should use the conditional prop spreading pattern to avoid creating unnecessary closures when the prop is not provided. This matches the pattern already established in SessionChildRow.

      <button
        type="button"
        className="topic-room-session__button"
        title={`open exact session ${session.selectKey}`}
        {...(onSelectSession
          ? { onClick: () => onSelectSession(session.selectKey) }
          : {})}
      >
References
  1. When exactOptionalPropertyTypes is enabled in TypeScript, use conditional prop spreading (e.g., {...(prop ? { prop } : {})}) for optional props that do not accept explicit undefined. This pattern is also preferred for optional function props to avoid creating unnecessary closures when the prop is not provided.

@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: 2

🤖 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 `@frontend/src/components/TopicSidebarShell.css`:
- Around line 500-506: The focus styles for the room controls in
TopicSidebarShell.css remove the native outline on .topic-room__primary,
.topic-room-ref-list a, and the session-button group while only using the hover
background, so keyboard focus is not distinct. Update the shared focus-visible
rules for those selectors to restore a clear, visible focus indicator (for
example, a dedicated outline/ring that differs from hover) while keeping hover
styling separate, and ensure the related session button styles in the same
stylesheet follow the same pattern.

In `@frontend/src/components/TopicSidebarShell.tsx`:
- Around line 1336-1337: The shell still uses the surfaces fetch to gate
rendering, so a slow surfaces request can block the topic list/task-room even
though error handling was already split out. Update TopicSidebarShell’s
loading/render gating around topicsQuery and surfacesQuery so the room can
render while surfacesQuery is pending, and keep surfaces-specific state isolated
to the strip/empty state logic rather than the top-level shell gate.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26b63b8d-828a-4355-b62c-b120344734c4

📥 Commits

Reviewing files that changed from the base of the PR and between 8751feb and 6249e7d.

📒 Files selected for processing (5)
  • frontend/src/components/TopicSidebarShell.css
  • frontend/src/components/TopicSidebarShell.tsx
  • frontend/src/lib/state/topic-nav.ts
  • test/topic-nav.test.ts
  • test/topic-sidebar-shell.test.ts

Comment thread frontend/src/components/TopicSidebarShell.css
Comment thread frontend/src/components/TopicSidebarShell.tsx
@donovan-yohan

Copy link
Copy Markdown
Owner Author

Fugu review — changes requested

Exact head reviewed: 6249e7d28efb7f039225933035a087eb117c0928

Blockers

  1. frontend/src/components/TopicSidebarShell.tsx:1331 — surface loading still blocks the whole topic shell. The PR claims surface failures/loading are isolated to the artifact strip, but loading is still true when surfacesQuery.isLoading && !surfacesQuery.data, so a slow /workspace-surfaces request hides the topic list/task-room instead of rendering the room with empty/partial surface state. This directly violates the Build topic task-room detail view for grouped sessions and artifacts #1044 empty/loading/error/offline acceptance path.

  2. frontend/src/components/TopicSidebarShell.css:500 and frontend/src/components/TopicSidebarShell.css:550 — the new room controls remove the native focus outline and replace focus-visible with the same hover background. Keyboard users cannot reliably tell whether the primary CTA, task-ref links, or session rows are focused. The existing focus test only covers .topic-search__input, not the new room controls.

Bot triage

  • CodeRabbit focus-visible concern: blocker, verified in CSS.
  • CodeRabbit surface loading isolation concern: blocker, verified in TopicSidebarShell loading gate.
  • Gemini window.open noopener: follow-up/minor. Current noreferrer is generally treated as implying noopener, but adding noopener,noreferrer explicitly in both direct-open paths would remove ambiguity.
  • Gemini optional onSelectSession closure: non-blocking style/efficiency nit; no behavioral issue found.
  • CodeRabbit docstring coverage warning / generated walkthrough noise: noise for this TS/React PR.

Evidence checked

  • PR metadata and exact head match.
  • Changed files reviewed: TopicSidebarShell.tsx, TopicSidebarShell.css, topic-nav.ts, topic-nav.test.ts, topic-sidebar-shell.test.ts.
  • Issue Build topic task-room detail view for grouped sessions and artifacts #1044 acceptance read.
  • Bot surfaces checked: review summaries, inline PR comments, issue comments.
  • Local targeted test attempt failed before execution because this worktree's installed dependencies do not include lucide-react; package.json/package-lock.json do include it and GitHub checks are green, so I am not treating that as a PR blocker.

Verdict: not approved until the two blockers above are fixed on a new PR head.

@donovan-yohan

Copy link
Copy Markdown
Owner Author

Fugu review — approved

Exact head reviewed: 908dd599e095b02803a41b3af9667792aa3d7c78

Verdict

Approved. The remediation fixes the two prior blockers without expanding the PR scope.

Findings

  • Blockers: none.
  • Concerns: none blocking. SurfaceButton anchors still use rel="noreferrer" rather than spelling noopener,noreferrer; for anchor target="_blank", noreferrer provides opener isolation in modern browsers, and the previously flagged imperative desktop/mobile window.open paths now explicitly use noopener,noreferrer.
  • Nits: Gemini's optional onSelectSession closure suggestion in TopicRoomSessionRow is style/efficiency only; no behavioral or type-safety issue found.

Evidence checked

  • PR metadata: base nightly, open/mergeable, head matches 908dd599e095b02803a41b3af9667792aa3d7c78.
  • Reviewed remediation delta 6249e7d..908dd59 and full changed-file coverage: frontend/src/components/TopicSidebarShell.tsx, frontend/src/components/TopicSidebarShell.css, frontend/src/lib/state/topic-nav.ts, test/topic-nav.test.ts, test/topic-sidebar-shell.test.ts.
  • Verified TopicSidebarShell top-level loading now depends only on topicsQuery; surfacesLoading is passed into TopicSidebarView/TopicRoomSurfaceStrip and renders as strip-local surfaces loading… state.
  • Verified focus-visible styles for primary task-room controls, task-ref links, and task-room session buttons use an accent outline, offset, and shadow distinct from hover.
  • Verified desktop and mobile imperative direct surface opens call window.open(..., '_blank', 'noopener,noreferrer').

Commands run

  • npm install — passed; installed missing worktree dependencies.
  • rtk npm test -- test/topic-sidebar-shell.test.ts test/topic-nav.test.ts — passed, 33 tests.
  • rtk npm run check — passed, 0 errors; existing repo lint warnings only.
  • rtk npm run build — passed; existing Vite dynamic-import/chunk-size warnings only.
  • gh pr checks 1049 --watch=false — CodeRabbit, ci, e2e, and scan-commits all passing.

Bot feedback + learning

  • Bot surfaces checked: review summaries, inline PR comments, issue comments.
  • CodeRabbit focus-visible blocker: already fixed, verified in CSS and tests.
  • CodeRabbit surface-loading blocker: already fixed, verified in shell gating and tests.
  • Gemini window.open noopener: fixed in both imperative paths; anchor noreferrer path is non-blocking as above.
  • Gemini optional closure suggestion: non-blocking style nit.
  • CodeRabbit rate-limit/docstring/walkthrough output: noise.
  • Learning retained: no — no new durable issue class beyond existing Relay review checks.

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.

Build topic task-room detail view for grouped sessions and artifacts

1 participant