Skip to content

fix(components): keep the opened-by tree on active sidebar rows - #442

Merged
lodystage[bot] merged 1 commit into
mainfrom
fix/sidebar-tree-status-end-slot
Sep 6, 2026
Merged

lodystage[bot] merged 1 commit into
mainfrom
fix/sidebar-tree-status-end-slot

Conversation

@lodystage

@lodystage lodystage Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Related issue

Problem / pressure

The sidebar's leading slot held exactly one node, and status won it. A
working, unread, or waiting-permission child dropped its ├/└
connectors, and an active opener dropped its disclosure chevron. So the
opened-by tree disappeared on precisely the rows a user is tracking: a
group of opened Sessions read as a group right up to the moment any of it
started running, and then read as a flat list with a stray indent.

Reported from the desktop sidebar, where several running rows sat between
nested idle ones and the nesting was unreadable.

Summary

Status moves from the row's leading slot to its END slot
(SidebarRowEndSlot in sidebar-row-shared.tsx), and the leading slot
now draws the opened-by tree unconditionally.

While a status shows, it REPLACES that slot's whole resting cluster —
line diff, Mergeable, worktree glyph, PR icon, and the mobile/chat
relative time — so the right edge is a single 14px mark and the title
gets the width back. Those metrics are still one hover away in the
desktop info card. A row that is idle and read is unchanged.

before   ◌  Audit archive + scope behavior      4m  +38 -4
after    ├─ Audit archive + scope behavior                  ◌

Applies to every desktop sidebar row surface through the one shared
component: Workspace groups (session-list.tsx), local-project sections
(loro-app-sidebar.tsx), the Updated / Pinned list
(sidebar-updated-session-list.tsx), and the two legacy task-list
renderers that share the same primitives.

The mobile chat rows keep their own leading-node rule for now: a
selection checkbox, an out-of-button chevron, and the edge-back swipe
zone all share that slot there, so it is a separate change.
mobile/AGENTS.md still documents the old rule, and the scoped
AGENTS.md now says not to assume the two match.

Before / after

Before After
◌ Audit archive… and ● Check mobile… render with NO trunk or elbow — the two nested rows in the middle of the group look top-level, while their idle siblings keep ├/└. The working row still shows 4m +38 -4. Every child keeps its ├/└; the working row shows a spinner at the right edge and the unread row a dot, both with the metric cluster hidden. The working opener keeps its chevron and shows its spinner at the right.

Verified in Storybook: Components/SessionList → Opened Sessions (MCP),
· Light, and · Active opener (light and dark), which already cover a
working child, an unread child, an orphan, and a child-tab opener.

Test plan

  • pnpm --filter @lody/components test — 436 files / 3198 tests pass.
  • pnpm typecheck (workspace) and pnpm check:quick (lint, i18n,
    code-collab imports, platform boundaries, public boundary) pass.
  • pnpm format.
  • Updated the tests that encoded the old rule so they now assert the new
    one, rather than deleting them:
    • session-list-opened-by.test.ts: connectors survive on working /
      unread children; the disclosure survives on a working, unread, or
      waiting opener; status renders in the end slot; and a new case pins
      that the diff numbers yield to a status.
    • sidebar-updated-opened-by.test.tsx: same guarantee for the Updated
      bucket.
    • loro-app-sidebar-worktree.test.ts: the PR icon needs a READ row now,
      plus a new unread row asserting the icon yields to the status mark.
    • spinner-rotates-in-place.test.tsx / session-list-pr-badge.test.ts:
      the spinner's square-box invariants now render through the end slot.
  • Storybook screenshots taken at 400px sidebar width in light and dark.

Not covered: no e2e run; the mobile chat lists are untouched.

Context handoff

Instructions for reviewing agents

  • Review focus: sidebar-row-shared.tsx — SessionRowLeadingSlot no longer takes the three status flags and SidebarRowEndSlot now owns them; confirm all five row renderers pass them to the end slot and none still passes them to the leading slot.
  • Decisions to challenge: a status hides the ENTIRE resting cluster, including the PR icon and CI verdict; because a never-opened Session counts as unread (sessionHasUnreadMessages with no lastReadAt), that can be many rows at once. This was the user's explicit call over the narrower "hide only the diff numbers" option.
  • Plausible failures / evidence gaps: the leading slot is now an empty reserved 14px on plain rows, so check no list lost title alignment; hover still swaps status for Archive at the right edge, so an active row shows no status while hovered (same as the pre-change leading slot); no e2e run.

Authoring context

  • User goal / directives: the sub-conversation hierarchy was unreadable because running/unread rows lost their tree lines; move read/unread/running status to the far right and hide the +/- line counts there.
  • Constraints / non-goals: desktop sidebar rows only; no change to the tree model (lib/session-opened-by-tree.ts), to the two opened-by fields, to keyboard navigation, or to the mobile chat lists.
  • Risk-bearing decisions: none touching data, authority, or compatibility — this is presentation only. The one product trade-off is the hidden PR/CI icon on unread rows, noted above.
  • Destructive or irreversible behavior: none; no migration, no persisted state, no lifecycle change.
  • Deliberately not done or tested: mobile ConversationRow keeps status on its leading node, because that slot also hosts the selection checkbox and its chevron must lift above the edge-back swipe zone — a separate change with its own screenshots. No e2e run.
  • Unknowns / confidence: high confidence in the mechanics (full component suite green, Storybook verified in light and dark); the open question is taste — how much metric information should yield to a status mark.

The leading slot held one node, and status won it: a working, unread, or
waiting child dropped its ├/└ and an active opener dropped its
disclosure. That traded away exactly the rows a user is tracking — a
group of opened Sessions read as a group right up to the moment any of
it started running, then read as a flat list with a stray indent.

Status moves to the row's END slot instead, where it replaces the
resting metric cluster (line diff, Mergeable, worktree glyph, PR icon,
mobile time) for as long as it lasts. The leading slot now draws the
tree unconditionally, so nesting is a stable fact about the list rather
than a function of activity, and the right edge stays a single 14px mark
with more room for the title. The metrics it stands in for are still in
the desktop hover info card.

Applies to every desktop sidebar row surface through the shared
`SidebarRowEndSlot`. The mobile chat rows keep their own leading-node
rule for now — a checkbox, an out-of-button chevron, and the edge-back
swipe zone share that slot there.

Model: claude-opus-5[1m]
@lodystage
lodystage Bot marked this pull request as ready for review September 6, 2026 08:55
@lodystage
lodystage Bot merged commit 9f04473 into main Sep 6, 2026
6 of 7 checks passed
@lodystage
lodystage Bot deleted the fix/sidebar-tree-status-end-slot branch September 6, 2026 08:56

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54c2c69c3e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +643 to +647
const restContent = hasSessionRowStatus({
isWaitingPermission,
isWorking,
hasUnreadMessages,
}) ? (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve the private-sharing indicator during activity

When a private task is also unread, working, or waiting for permission, this branch discards the entire restIcon; both task-list.tsx and sidebar-updated-task-list.tsx place SessionSharingIndicator inside that value. Consequently, these common states—especially a never-opened row, which is unread—remove the list's lock and its “who can open this conversation” tooltip, obscuring the session's access scope along with the intentionally suppressed metrics. Keep the privacy indicator outside the replaceable metric cluster so it remains visible beside the activity status.

Useful? React with 👍 / 👎.

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