Skip to content

feat(ui): show labels when focused on a session/item#269

Merged
dgershman merged 1 commit into
mainfrom
feature/crow-268-show-labels-focused
May 14, 2026
Merged

feat(ui): show labels when focused on a session/item#269
dgershman merged 1 commit into
mainfrom
feature/crow-268-show-labels-focused

Conversation

@dgershman
Copy link
Copy Markdown
Collaborator

Summary

  • Adds GitHub/GitLab label display to SessionDetailView (header), SessionRow (sidebar), and ReviewRow (review board) so users get at-a-glance label context without switching to a browser
  • Extracts a reusable LabelPillsView component from the existing TicketCard.labelRow pattern, and refactors TicketCard to use it — eliminating code duplication across 4 views
  • Adds reverse-lookup helpers on AppState (assignedIssue(for:), reviewRequest(for:), labels(forSession:)) to resolve labels from the existing AssignedIssue/ReviewRequest data without adding new fields to the Session model

Test plan

  • Select a session linked to a ticket with labels — labels appear in the detail header below the ticket title
  • Labels appear in the sidebar session row (hidden when "hide details" is toggled)
  • Review board rows now show PR labels
  • Ticket board cards still display labels correctly (refactor preserved behavior)
  • Sessions without tickets show no labels and no empty space
  • All 160 CrowCore unit tests pass (including 8 new lookup tests)

Closes #268

🤖 Generated with Claude Code

Add label display to SessionDetailView header, SessionRow sidebar, and
ReviewRow so users get at-a-glance context without switching to a browser.
Extracts a reusable LabelPillsView component and refactors TicketCard to
use it, eliminating duplication.

🤖 Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: E59F89D6-EB56-4E11-BE06-E6D6F22D5311
Copy link
Copy Markdown
Contributor

@dhilgaertner dhilgaertner left a comment

Choose a reason for hiding this comment

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

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

  • No new external input surface — labels are sourced from already-fetched AssignedIssue / ReviewRequest models and rendered via SwiftUI Text, so no injection vectors are introduced.
  • No new persisted fields, no secrets, no auth surface touched.
  • The reverse-lookup helpers are read-only and @MainActor-safe (consistent with the rest of AppState).

Concerns:

  • None.

Code Quality

Strengths:

  • The LabelPillsView extraction (Packages/CrowUI/Sources/CrowUI/CorveilTheme.swift:181) is a clean dedup of the prior TicketCard.labelRow body — same fonts, padding, capsule, overflow +N, and muted flag mapping isDone. The refactor at Packages/CrowUI/Sources/CrowUI/TicketBoardView.swift:501 is behavior-preserving.
  • Reverse lookup via existing data avoids a stale duplicate of labels on Session — good call.
  • Test coverage in Packages/CrowCore/Tests/CrowCoreTests/AppStateLookupTests.swift exercises the positive path, missing ticket URL, no-match, kind filtering, and the issue-vs-review fallback. All 160 tests pass locally.

Consider (non-blocking):

  • Packages/CrowCore/Sources/CrowCore/AppState.swift:412reviewRequest(for:) guards on session.kind == .review, but assignedIssue(for:) at line 406 does not guard on kind. A .review session that happens to have a non-nil ticketURL matching an assigned issue would fall into the issue branch of labels(forSession:) and show issue labels instead of PR labels. Probably fine in practice (review sessions typically don't carry an issue ticketURL), but worth a brief comment or a kind-aware preference if mixed states are possible.
  • Packages/CrowUI/Sources/CrowUI/CorveilTheme.swift:194Text(label) has no .lineLimit(1)/.truncationMode. GitHub allows up to 50-char labels; in narrow contexts (SessionRow sidebar, maxVisible: 2) very long labels could push siblings or wrap. This is preserved from the original TicketCard.labelRow, so not a regression — flagging only.
  • Lookups are O(n) over assignedIssues / reviewRequests per render of each row. Fine at current scale; if these collections grow large, an index keyed by URL would help.
  • Minor: the helper named pair assignedIssue(for:) / reviewRequest(for:) use for:, while labels(forSession:) uses forSession:. Not actionable (likely avoiding a label collision), but slightly inconsistent.

Summary Table

Priority Issue
Red (none)
Yellow (none)
Green Optional .lineLimit(1) on label Text for narrow contexts; consider kind-aware preference in labels(forSession:) if review sessions can carry a ticketURL; consider a URL-keyed index if assignedIssues/reviewRequests grow.

Recommendation: Approve. Scope is tight, the refactor is behavior-preserving, test coverage for the new lookups is solid, and there are no security or correctness concerns. The items above are minor consider-level polish, not blockers.

🤖 Reviewed by Crow via Claude Code

@dgershman dgershman requested a review from dhilgaertner May 14, 2026 22:54
@dgershman dgershman merged commit 5c45335 into main May 14, 2026
3 checks passed
@dgershman dgershman deleted the feature/crow-268-show-labels-focused branch May 14, 2026 23:43
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.

feat(ui): show labels when focused on a session/item

2 participants