tui: drill into a sidebar agent's detail card from the expanded dossier (dogfood A3)

[Hmbown]

Summary

Dogfood finding A3 on #4092 — Agents sidebar rows were truncated and not drillable. Narrow first slice of #2889 (which stays open for @aboimpinto's structured-model pass):

• Agent label row click still toggles the inline dossier (unchanged).
• The expanded dossier rows are now clickable: a new SidebarRowAction::OpenAgentDetail opens the child's transcript card (action tree, status, final summary) in the existing detail pager via open_details_pager_for_cell.
• No transcript card yet → status line points at handle_read agent:<id>/full_transcript instead of failing silently.
• The rich hover dossier (id/parent/depth/model/objective/progress) already existed and is unchanged.

Testing

• New: subagent_expanded_dossier_rows_register_open_agent_detail
• Updated 2 action-layout tests to the clickable dossier
• tui::sidebar: 90 pass; mouse module: 56 pass; fmt + CI-flag clippy clean

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 4m 25s —— View job

---

Review complete

• Gather context (read diff + changed files)
• Understand request (code review of PR tui: drill into a sidebar agent's detail card from the expanded dossier (dogfood A3) #4234)
• Review crates/tui/src/tui/sidebar.rs
• Review crates/tui/src/tui/mouse_ui.rs
• Review crates/tui/src/tui/app.rs
• Review crates/tui/src/tui/ui.rs
• Check provider/model route safety implications
• Check test coverage
• Post final review

Summary

This is a clean, well-scoped slice: SidebarRowAction::OpenAgentDetail is threaded through cleanly (new enum variant → non-exhaustive matches in as_command/is_cancel_action/agent_stop_action_for_click updated correctly), and reusing open_details_pager_for_cell instead of building a new pager path is the right call. Making activity_detail pub(crate) (ui.rs) is the minimal visibility change needed to reach it from mouse_ui.rs.

Correctness — one real bug

crates/tui/src/tui/mouse_ui.rs:421-428 (inline comment posted): the new lookup only matches SubAgentCell::Delegate:

let cell_index = app.history.iter().position(|cell| {
    matches!(cell, HistoryCell::SubAgent(SubAgentCell::Delegate(card)) if card.agent_id == agent_id)
});

Agents spawned via the fanout dispatch path (rlm_open/rlm_eval/rlm, see subagent_routing.rs:392-417) live in SubAgentCell::Fanout(FanoutCard) instead, and sidebar_agent_rows/subagent_panel_rows register the drill-in action unconditionally — there's no Delegate/Fanout distinction at the row level. So clicking the expanded dossier for a fanout worker always falls into the None branch and shows "No transcript card for {agent_id} yet…", even though the card exists.

The existing app.subagent_card_index: HashMap<String, usize> already maps agent_id → history index for both Delegate and Fanout cards (used at subagent_routing.rs:362 for the same purpose), so this can be fixed and simplified in one line:

let cell_index = app.subagent_card_index.get(&agent_id).copied();

That also drops the O(n) scan over app.history in favor of the existing O(1) map lookup.

Provider/model route safety

No provider/model prefix inference here — not applicable to this change.

Tests

• New subagent_expanded_dossier_rows_register_open_agent_detail and the two updated action-layout tests correctly verify the row-level action registration.
• Gap: nothing exercises the mouse_ui.rs handler itself (dispatching a click and checking open_details_pager_for_cell fires / the fallback message). A test with a FanoutCard-backed row would have caught the bug above — worth adding once the fix lands.

Note

I wasn't able to run cargo test/cargo clippy in this sandbox (Bash network/build commands require approval that isn't available here); the PR description states the relevant suites (tui::sidebar: 90 pass, mouse module: 56 pass) already pass locally, and this review is based on static reading of the diff plus the surrounding subagent-routing code.

[claude]

Correctness: fanout worker agents can never be drilled into.

This scan only matches SubAgentCell::Delegate, so any agent spawned through the fanout path (rlm_open/rlm_eval/rlm dispatch → SubAgentCell::Fanout(FanoutCard), see subagent_routing.rs:392-417) will never be found here, even though its transcript card genuinely exists inside the FanoutCard. Clicking the expanded dossier for such an agent always falls into the None branch and shows "No transcript card for {agent_id} yet…" — which is misleading since the card is there.

The row action is registered unconditionally in sidebar.rs::subagent_panel_rows (no Delegate/Fanout distinction), so this is reachable for real fanout workers, not just delegates.

This also reimplements — with a linear O(n) scan over app.history — a lookup that already exists and is used for exactly this purpose (both Delegate and Fanout) at subagent_routing.rs:362:

if let Some(&idx) = app.subagent_card_index.get(&agent_id) { ... }

Suggest reusing it directly instead of re-deriving the index by scanning history:

Suggested change

	let cell_index = app.history.iter().position(|cell| {
	matches!(
	cell,
	HistoryCell::SubAgent(
	crate::tui::history::SubAgentCell::Delegate(card)
	) if card.agent_id == agent_id
	)
	});
	let cell_index = app.subagent_card_index.get(&agent_id).copied();

This fixes the fanout gap and drops the linear scan, matching the existing lookup pattern used elsewhere in this file (app.subagent_card_index).

Fix this →

[claude]

Claude finished @Hmbown's task in 58s —— View job

---

Re-review after new commit

• Gather context (diff since last review)
• Check whether the fanout subagent_card_index bug flagged earlier was fixed
• Re-review any other new changes
• Post final review

Summary

The new commit (ec20d84, "tui: use the existing pager-opener import in the drill-in handler") is a small cleanup — it drops the fully-qualified crate::tui::ui::activity_detail::open_details_pager_for_cell(...) call in favor of the already-imported open_details_pager_for_cell(...), and reverts mod activity_detail; in ui.rs from pub(crate) back to private now that the wider visibility isn't needed. That's a correct, minimal simplification — no behavior change, and activity_detail is no longer reached from outside ui.rs so narrowing its visibility back down is right.

Outstanding: the fanout correctness bug from the previous review is still unfixed

crates/tui/src/tui/mouse_ui.rs:421-428 is unchanged in this commit:

let cell_index = app.history.iter().position(|cell| {
    matches!(cell, HistoryCell::SubAgent(SubAgentCell::Delegate(card)) if card.agent_id == agent_id)
});

This still only matches SubAgentCell::Delegate. Agents spawned via the fanout dispatch path (rlm_open/rlm_eval/rlm, see subagent_routing.rs:392-417) live in SubAgentCell::Fanout(FanoutCard), so clicking the expanded dossier for a fanout worker still falls into the None branch and shows "No transcript card for {agent_id} yet…" even though the card exists. app.subagent_card_index: HashMap<String, usize> (confirmed at app.rs:1837, populated for both Delegate and Fanout cards in subagent_routing.rs:354/403/414/423, and already used this way at subagent_routing.rs:362) is the existing lookup that covers both variants:

let cell_index = app.subagent_card_index.get(&agent_id).copied();

Suggest making this swap before merging — it fixes the fanout gap and replaces the O(n) app.history scan with the existing O(1) map lookup. A regression test with a FanoutCard-backed row (as noted previously) would also close the test gap on the mouse_ui.rs handler itself.

Fix this →