From dd2901827112c5510dccab95cdae86c0d8b6cb07 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:25:58 +0800 Subject: [PATCH 1/4] fix(issues): anchor the working chip on issues the filter actually shows (MUL-4884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header chip showed three units at once: the number counted distinct issues, the avatar stack counted agents (with a rival "+N"), and the hover card counted tasks — under a label with no noun at all. Each figure was self-consistent; together they read as a miscount. c4209ec7c flipped the number from agents to issues but left the other three surfaces on their old units. The chip now says one thing — "N issues in progress" — where N is the number of rows clicking it leaves. Data layer: - The count is no longer re-derived from the task snapshot. The surface exposes `workingScopeIssues`: the render pipeline's own output with `workingOnly` forced on, so "chip count === row count" holds by construction instead of by convention. It previously counted running issue_ids against the PRE-filter set, so any active status/assignee/label filter — or a sub-issue hidden by the display toggle — made the chip disagree with the list it was filtering. - Chat/autopilot tasks carry issue_id "" (not null). They used to collapse into one bucket and read as a phantom issue, inflating the count by exactly one whenever any were running. They are now bucketed out and disclosed. - The list loads one page per status (50), so running work can exist past the window. The count stays list-anchored — counting rows a click cannot show would break the chip's whole promise — and the gap is stated in the hover card rather than dropped silently. UI: - Avatar stack is ambience, not a statistic: no "+N"; the tail fades and the exact roster lives in the hover card. - Hover card groups rows by issue, names both counted units in its header ("3 issues in progress · 4 tasks"), and footnotes anything excluded — only when non-zero. - Colour is two-step: idle activity is a brand tint, the filled state is reserved for "filter is ON". Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- .../agent-activity-hover-content.test.tsx | 159 ++++++++- .../agent-activity-hover-content.tsx | 302 +++++++++++++----- .../agents/components/agent-avatar-stack.tsx | 19 +- .../views/issues/components/issues-header.tsx | 14 +- .../views/issues/components/issues-page.tsx | 6 +- .../workspace-agent-working-chip.test.tsx | 267 +++++++++++++--- .../workspace-agent-working-chip.tsx | 225 +++++++------ .../views/issues/surface/issue-surface.tsx | 18 +- .../use-issue-surface-controller.test.tsx | 202 +++++++++++- .../surface/use-issue-surface-controller.ts | 5 + .../issues/surface/use-issue-surface-data.ts | 66 ++++ packages/views/locales/en/issues.json | 13 +- packages/views/locales/ja/issues.json | 9 +- packages/views/locales/ko/issues.json | 9 +- packages/views/locales/zh-Hans/issues.json | 9 +- .../my-issues/components/my-issues-header.tsx | 11 +- .../my-issues/components/my-issues-page.tsx | 3 +- 17 files changed, 1089 insertions(+), 248 deletions(-) diff --git a/packages/views/agents/components/agent-activity-hover-content.test.tsx b/packages/views/agents/components/agent-activity-hover-content.test.tsx index ae8d9569ae5..2d018964e59 100644 --- a/packages/views/agents/components/agent-activity-hover-content.test.tsx +++ b/packages/views/agents/components/agent-activity-hover-content.test.tsx @@ -2,7 +2,7 @@ import { cleanup, screen } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; -import type { AgentTask } from "@multica/core/types"; +import type { AgentTask, Issue } from "@multica/core/types"; import { renderWithI18n } from "../../test/i18n"; // The hover card renders one row per task and counts tasks, so its header @@ -54,7 +54,37 @@ vi.mock("@tanstack/react-query", async () => { return { ...actual, useQuery: () => ({ data: [] }) }; }); -import { AgentActivityHoverContent } from "./agent-activity-hover-content"; +import { + AgentActivityHoverContent, + WorkspaceAgentActivityHoverContent, +} from "./agent-activity-hover-content"; + +function makeIssue(id: string, identifier: string, title: string): Issue { + return { + id, + workspace_id: "ws-1", + number: 1, + identifier, + title, + description: null, + status: "in_progress", + priority: "none", + assignee_type: null, + assignee_id: null, + creator_type: "member", + creator_id: "user-1", + parent_issue_id: null, + project_id: null, + position: 1, + stage: null, + start_date: null, + due_date: null, + metadata: {}, + properties: {}, + created_at: "2026-06-08T08:00:00Z", + updated_at: "2026-06-08T08:00:00Z", + }; +} function makeTask(overrides: Partial): AgentTask { return { @@ -110,3 +140,128 @@ describe("AgentActivityHoverContent", () => { expect(screen.getByText("3 个 task 工作中")).toBeInTheDocument(); }); }); + +// The workspace chip can only carry one number (issues — the rows a click +// produces). This card is where the other units get stated instead of +// silently contradicting it, and where everything the number excludes is +// disclosed rather than dropped. MUL-4884. +describe("WorkspaceAgentActivityHoverContent", () => { + it("names both counted units so the chip's number stops looking wrong", () => { + // The MUL-4884 screenshot: chip says 3 while 4 agent heads show. Naming + // both units is what resolves that, rather than hiding one. + renderWithI18n( + , + ); + + expect(screen.getByText("3 issues in progress · 4 tasks")).toBeInTheDocument(); + // Rows group under their issue, mirroring what the filter does. + expect(screen.getByText("MUL-4879")).toBeInTheDocument(); + expect(screen.getByText("Counting logic looks wrong")).toBeInTheDocument(); + expect(screen.getAllByTestId("actor-avatar")).toHaveLength(4); + }); + + it("states what the number excludes rather than dropping it", () => { + renderWithI18n( + , + ); + + expect( + screen.getByText( + "1 more task has no linked issue (chat/autopilot) — not counted", + ), + ).toBeInTheDocument(); + // The list loads one page per status, so running work can sit past the + // window. Say so instead of silently under-counting. + expect( + screen.getByText( + "2 more tasks are outside the current filters or loaded range — not counted", + ), + ).toBeInTheDocument(); + }); + + it("stays quiet when there is nothing to disclose", () => { + renderWithI18n( + , + ); + + expect(screen.queryByText(/not counted/)).not.toBeInTheDocument(); + }); + + it("still discloses excluded work when nothing is counted", () => { + // Only running work is a chat task: the chip reads 0, and the card has to + // explain why rather than look broken. + renderWithI18n( + , + ); + + expect( + screen.getByText("No issues in progress right now"), + ).toBeInTheDocument(); + expect( + screen.getByText( + "1 more task has no linked issue (chat/autopilot) — not counted", + ), + ).toBeInTheDocument(); + }); + + it("renders the Chinese copy for the counted units", () => { + renderWithI18n( + , + { locale: "zh-Hans" }, + ); + + expect(screen.getByText("1 个 issue 进行中 · 2 个 task")).toBeInTheDocument(); + }); +}); diff --git a/packages/views/agents/components/agent-activity-hover-content.tsx b/packages/views/agents/components/agent-activity-hover-content.tsx index 5e5740063d3..157264d4f56 100644 --- a/packages/views/agents/components/agent-activity-hover-content.tsx +++ b/packages/views/agents/components/agent-activity-hover-content.tsx @@ -8,7 +8,7 @@ import { useWorkspaceId } from "@multica/core/hooks"; import { runtimeListOptions } from "@multica/core/runtimes/queries"; import { agentListOptions } from "@multica/core/workspace/queries"; import { deriveAgentAvailability } from "@multica/core/agents"; -import type { AgentTask } from "@multica/core/types"; +import type { AgentTask, Issue } from "@multica/core/types"; import { workloadConfig } from "../presence"; import { useT } from "../../i18n"; @@ -20,10 +20,37 @@ interface AgentActivityHoverContentProps { } /** - * Shared hover-card body for "what are these agents doing right now?" — used - * by IssueAgentActivityIndicator (per-issue) and WorkspaceAgentWorkingChip - * (workspace-wide). One row per task: agent avatar, name, status dot, - * status label, duration. + * Tick `now` once per second so duration labels update live while a hover + * card is open. setInterval only runs while the card is mounted (Base UI + * portals the content but tears it down on close), so this costs nothing + * when the card is closed. + */ +function useActivityNow(): number { + const [now, setNow] = useState(() => Date.now()); + useEffect(() => { + const id = setInterval(() => setNow(Date.now()), 1000); + return () => clearInterval(id); + }, []); + return now; +} + +/** + * O(1) agent + runtime lookups so each task row resolves without an N×M + * scan. Cheap — agents/runtimes count in tens at most. + */ +function useActivityLookups() { + const wsId = useWorkspaceId(); + const { data: agents = [] } = useQuery(agentListOptions(wsId)); + const { data: runtimes = [] } = useQuery(runtimeListOptions(wsId)); + const agentById = new Map(agents.map((a) => [a.id, a] as const)); + const runtimeById = new Map(runtimes.map((r) => [r.id, r] as const)); + return { agentById, runtimeById }; +} + +type ActivityLookups = ReturnType; + +/** + * One task row: agent avatar, name, status dot, status label, duration. * * Status colour follows the workspace's existing composition rule: * - running → brand (text-brand) @@ -32,29 +59,83 @@ interface AgentActivityHoverContentProps { * — same rule as agent-presence-indicator.tsx so users see a single, * consistent language for "agent is in trouble" vs "just enqueued". */ -export function AgentActivityHoverContent({ - tasks, -}: AgentActivityHoverContentProps) { +function AgentActivityTaskRow({ + task, + now, + agentById, + runtimeById, +}: { + task: AgentTask; + now: number; +} & ActivityLookups) { const { t } = useT("issues"); - const wsId = useWorkspaceId(); const { getActorName, getActorInitials, getActorAvatarUrl } = useActorName(); - const { data: agents = [] } = useQuery(agentListOptions(wsId)); - const { data: runtimes = [] } = useQuery(runtimeListOptions(wsId)); - // Tick `now` once per second so the per-task duration label updates - // live while the hover card is open. setInterval only runs while the - // hover card is mounted (Base UI portals the content but tears it down - // on close), so this costs nothing when the card is closed. - const [now, setNow] = useState(() => Date.now()); - useEffect(() => { - const id = setInterval(() => setNow(Date.now()), 1000); - return () => clearInterval(id); - }, []); + const agent = agentById.get(task.agent_id); + const runtime = runtimeFrom(agent?.runtime_id, runtimeById); + const availability = deriveAgentAvailability(runtime, now); + const isRunning = task.status === "running"; + // queued/dispatched both read as "queued" in the user-facing copy — + // `dispatched` is the daemon-acked sub-state of queued and not + // user-meaningful here. + const wl = isRunning ? workloadConfig.working : workloadConfig.queued; + // queued + online → muted gray (transient race, no warning); + // queued + offline/unstable → keep warning amber from workloadConfig. + // Mirrors agent-presence-indicator.tsx. + const dotClass = isRunning + ? "bg-brand" + : availability === "online" + ? "bg-muted-foreground/40" + : "bg-warning"; + const labelClass = isRunning + ? wl.textClass + : availability === "online" + ? "text-muted-foreground" + : wl.textClass; + const startedFrom = isRunning + ? (task.started_at ?? task.dispatched_at ?? task.created_at) + : task.created_at; - // Build O(1) lookups so each task row resolves agent + runtime without - // an N×M scan. Cheap — agents/runtimes count in tens at most. - const agentById = new Map(agents.map((a) => [a.id, a] as const)); - const runtimeById = new Map(runtimes.map((r) => [r.id, r] as const)); + return ( +
+ + + {getActorName("agent", task.agent_id)} + + + + + {isRunning + ? t(($) => $.agent_activity.status_running) + : t(($) => $.agent_activity.status_queued)} + + + {formatDuration(startedFrom, now)} + + +
+ ); +} + +/** + * Shared hover-card body for "what are these agents doing right now?" — used + * by IssueAgentActivityIndicator (per-issue). One row per task. + * + * The workspace-wide chip uses WorkspaceAgentActivityHoverContent below, + * which groups the same rows by issue. + */ +export function AgentActivityHoverContent({ + tasks, +}: AgentActivityHoverContentProps) { + const { t } = useT("issues"); + const now = useActivityNow(); + const { agentById, runtimeById } = useActivityLookups(); if (tasks.length === 0) return null; @@ -63,67 +144,130 @@ export function AgentActivityHoverContent({
{/* One row per task, so count tasks — not agents. A single agent can run several tasks at once, so an agent-worded header here would - disagree with the workspace chip's unique-agent count (e.g. chip - "2 working" but header "3 agents working"). */} + disagree with the row count below. */} {t(($) => $.agent_activity.hover_header_tasks, { count: tasks.length })}
- {tasks.map((task) => { - const agent = agentById.get(task.agent_id); - const runtime = runtimeFrom(agent?.runtime_id, runtimeById); - const availability = deriveAgentAvailability(runtime, now); - const isRunning = task.status === "running"; - // queued/dispatched both read as "queued" in the user-facing - // copy — `dispatched` is the daemon-acked sub-state of queued - // and not user-meaningful here. - const wl = isRunning ? workloadConfig.working : workloadConfig.queued; - // queued + online → muted gray (transient race, no warning); - // queued + offline/unstable → keep warning amber from - // workloadConfig. Mirrors agent-presence-indicator.tsx. - const dotClass = isRunning - ? "bg-brand" - : availability === "online" - ? "bg-muted-foreground/40" - : "bg-warning"; - const labelClass = isRunning - ? wl.textClass - : availability === "online" - ? "text-muted-foreground" - : wl.textClass; - const startedFrom = isRunning - ? (task.started_at ?? task.dispatched_at ?? task.created_at) - : task.created_at; - - return ( -
- - - {getActorName("agent", task.agent_id)} - - - - - {isRunning - ? t(($) => $.agent_activity.status_running) - : t(($) => $.agent_activity.status_queued)} - - - {formatDuration(startedFrom, now)} - + {tasks.map((task) => ( + + ))} +
+
+ ); +} + +interface WorkspaceAgentActivityHoverContentProps { + /** Issues the working filter leaves on screen, in list order. Each has at + * least one running task. `issues.length` IS the chip's number. */ + issues: readonly Issue[]; + /** Running tasks for those issues, keyed by issue id. */ + tasksByIssueId: ReadonlyMap; + /** Total running tasks across `issues` — the second header figure. */ + taskCount: number; + /** Running tasks with no linked issue (chat / autopilot). */ + unlinkedCount: number; + /** Running tasks whose issue is outside the current filters / loaded page. */ + outOfScopeCount: number; +} + +/** + * Hover-card body for the workspace working chip (MUL-4884). + * + * The chip can only carry one number, and the honest one is the issue count — + * it equals the rows you get when you click it. This card is where the other + * two units get explained instead of contradicting it: + * + * - the header states both counted units side by side ("3 issues · 4 + * tasks"), so "4 heads but the chip says 3" resolves instead of nagging; + * - rows group under their issue, mirroring what the filter does; + * - anything the number deliberately excludes is stated rather than + * dropped — issue-less chat/autopilot tasks, and tasks on issues outside + * the current filters or the loaded page (the list is fetched a page per + * status, so running work can exist past the window). + * + * Deliberately not a dashboard: two figures in the header, grouped rows, and + * at most two footnotes that only render when non-zero. + */ +export function WorkspaceAgentActivityHoverContent({ + issues, + tasksByIssueId, + taskCount, + unlinkedCount, + outOfScopeCount, +}: WorkspaceAgentActivityHoverContentProps) { + const { t } = useT("issues"); + const now = useActivityNow(); + const { agentById, runtimeById } = useActivityLookups(); + + const notes = ( + <> + {unlinkedCount > 0 && ( +

+ {t(($) => $.agent_activity.unlinked_note, { count: unlinkedCount })} +

+ )} + {outOfScopeCount > 0 && ( +

+ {t(($) => $.agent_activity.out_of_scope_note, { + count: outOfScopeCount, + })} +

+ )} + + ); + + if (issues.length === 0) { + return ( +
+

+ {t(($) => $.agent_activity.empty_hover)} +

+ {notes} +
+ ); + } + + return ( +
+
+ {`${t(($) => $.agent_activity.issues_in_progress, { + count: issues.length, + })} · ${t(($) => $.agent_activity.tasks_count, { count: taskCount })}`} +
+
+ {issues.map((issue) => ( +
+
+ + {issue.identifier} + {issue.title} +
+
+ {(tasksByIssueId.get(issue.id) ?? []).map((task) => ( + + ))}
- ); - })} +
+ ))}
+ {(unlinkedCount > 0 || outOfScopeCount > 0) && ( +
+ {notes} +
+ )}
); } diff --git a/packages/views/agents/components/agent-avatar-stack.tsx b/packages/views/agents/components/agent-avatar-stack.tsx index 948f6e667d0..7a69b5a7bd9 100644 --- a/packages/views/agents/components/agent-avatar-stack.tsx +++ b/packages/views/agents/components/agent-avatar-stack.tsx @@ -20,6 +20,14 @@ interface AgentAvatarStackProps { // signal a queued-only state (no running task) — same heads, weakened // visual. opacity?: "full" | "half"; + // How the tail beyond `max` is signalled. + // - `count` (default): a `+N` chip. Precise, but it reads as a second + // number — next to a count of something else (the working chip counts + // issues, these heads are agents) that lands as a contradiction. + // - `fade`: the last visible head dims instead. Says "there are more" + // without putting a rival number on screen; the exact roster lives in + // the hover card (MUL-4884). + overflow?: "count" | "fade"; className?: string; } @@ -38,6 +46,7 @@ export function AgentAvatarStack({ size = "sm", max = 3, opacity = "full", + overflow: overflowMode = "count", className, }: AgentAvatarStackProps) { const { getActorName, getActorInitials, getActorAvatarUrl } = useActorName(); @@ -45,6 +54,7 @@ export function AgentAvatarStack({ const visible = agentIds.slice(0, max); const overflow = agentIds.length - visible.length; + const fadeTail = overflowMode === "fade" && overflow > 0; const px = AVATAR_SIZE_PX[size]; // 30% overlap reads as "stacked" without obscuring the next avatar's icon. const overlap = Math.round(px * 0.3); @@ -64,7 +74,12 @@ export function AgentAvatarStack({ // Each subsequent head sits negative-margin over the previous so // the stack collapses horizontally instead of growing linearly. style={{ marginLeft: i === 0 ? 0 : -overlap }} - className="ring-2 ring-background rounded-full inline-flex" + className={cn( + "ring-2 ring-background rounded-full inline-flex", + // Dim the last head when the tail is hidden — the "there are + // more" cue in `fade` mode. + fadeTail && i === visible.length - 1 && "opacity-40", + )} > ))} - {overflow > 0 && ( + {overflowMode === "count" && overflow > 0 && ( void; @@ -765,14 +769,6 @@ export function IssuesHeader({ const toggleAgentRunningFilter = useViewStore( (s) => s.toggleAgentRunningFilter, ); - // Scope the chip to whatever issues this page is currently showing. - // /issues uses the full workspace minus Members/Agents pill filtering; - // passing the visible-issue id set lets the chip count match the list - // length when the filter is on. - const scopedIssueIds = useMemo( - () => new Set(scopedIssues.map((i) => i.id)), - [scopedIssues], - ); const SCOPE_LABEL_KEY: Record = { all: "all_label", members: "members_label", @@ -847,7 +843,7 @@ export function IssuesHeader({ s.dateFilter); @@ -22,6 +24,7 @@ function IssuesSurfaceHeader({ return ( ( + renderHeader={({ controller, workingIssues }) => ( )} diff --git a/packages/views/issues/components/workspace-agent-working-chip.test.tsx b/packages/views/issues/components/workspace-agent-working-chip.test.tsx index 036cbe023c2..c8d28f025c7 100644 --- a/packages/views/issues/components/workspace-agent-working-chip.test.tsx +++ b/packages/views/issues/components/workspace-agent-working-chip.test.tsx @@ -2,14 +2,23 @@ import { cleanup, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { AgentTask } from "@multica/core/types"; +import type { AgentTask, Issue } from "@multica/core/types"; import { renderWithI18n } from "../../test/i18n"; const mockState = vi.hoisted(() => ({ snapshot: [] as unknown[], - // Captures the agent ids handed to the avatar stack so a test can assert - // the stack still reflects distinct agents even when the count counts issues. + // Captures what the chip hands its two children so a test can assert the + // avatar stack and the hover card without reaching into their internals. avatarAgentIds: undefined as string[] | undefined, + avatarOverflow: undefined as string | undefined, + hoverProps: undefined as + | { + issues: readonly Issue[]; + taskCount: number; + unlinkedCount: number; + outOfScopeCount: number; + } + | undefined, })); vi.mock("@multica/core/hooks", () => ({ @@ -23,16 +32,29 @@ vi.mock("@multica/core/agents", () => ({ })); vi.mock("../../agents/components/agent-avatar-stack", () => ({ - AgentAvatarStack: ({ agentIds }: { agentIds: string[] }) => { + AgentAvatarStack: ({ + agentIds, + overflow, + }: { + agentIds: string[]; + overflow?: string; + }) => { mockState.avatarAgentIds = agentIds; + mockState.avatarOverflow = overflow; return
{agentIds.length}
; }, })); vi.mock("../../agents/components/agent-activity-hover-content", () => ({ - AgentActivityHoverContent: ({ tasks }: { tasks: AgentTask[] }) => ( -
{tasks.length}
- ), + WorkspaceAgentActivityHoverContent: (props: { + issues: readonly Issue[]; + taskCount: number; + unlinkedCount: number; + outOfScopeCount: number; + }) => { + mockState.hoverProps = props; + return
{props.taskCount}
; + }, })); vi.mock("@tanstack/react-query", async () => { @@ -51,7 +73,10 @@ vi.mock("@tanstack/react-query", async () => { }; }); -import { WorkspaceAgentWorkingChip } from "./workspace-agent-working-chip"; +import { + WorkspaceAgentWorkingChip, + deriveWorkingChipView, +} from "./workspace-agent-working-chip"; function makeTask(overrides: Partial): AgentTask { return { @@ -71,79 +96,249 @@ function makeTask(overrides: Partial): AgentTask { }; } +function makeIssue(id: string): Issue { + return { + id, + workspace_id: "ws-1", + number: 1, + identifier: `MUL-${id}`, + title: `Issue ${id}`, + description: null, + status: "in_progress", + priority: "none", + assignee_type: null, + assignee_id: null, + creator_type: "member", + creator_id: "user-1", + parent_issue_id: null, + project_id: null, + position: 1, + stage: null, + start_date: null, + due_date: null, + metadata: {}, + properties: {}, + created_at: "2026-06-08T08:00:00Z", + updated_at: "2026-06-08T08:00:00Z", + }; +} + beforeEach(() => { cleanup(); vi.clearAllMocks(); mockState.snapshot = []; mockState.avatarAgentIds = undefined; + mockState.avatarOverflow = undefined; + mockState.hoverProps = undefined; }); describe("WorkspaceAgentWorkingChip", () => { - it("counts distinct active issues, not running agents", () => { - // Two agents working the SAME issue: the count is about issues, so it - // must read "1", not "2" (the old unique-agent behavior). MUL-3875. + it("shows the row count the filter produces, not a snapshot re-derivation", () => { + // The screenshot case from MUL-4884: 4 running tasks, 4 distinct agents, + // 3 issues on screen. The chip says 3 — the number of rows a click + // leaves — and says it with a unit. mockState.snapshot = [ makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), - makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "issue-1" }), + makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "issue-2" }), + makeTask({ id: "t-3", agent_id: "agent-3", issue_id: "issue-3" }), + makeTask({ id: "t-4", agent_id: "agent-4", issue_id: "issue-3" }), ]; renderWithI18n( - {}} />, + {}} + workingIssues={["issue-1", "issue-2", "issue-3"].map(makeIssue)} + />, ); expect( - screen.getByRole("button", { name: /working/i }), - ).toHaveTextContent("1"); - // The avatar stack still shows both distinct agents behind that work. - expect(mockState.avatarAgentIds).toEqual(["agent-1", "agent-2"]); + screen.getByRole("button", { name: "3 issues in progress" }), + ).toBeTruthy(); + // The stack still shows every distinct agent behind that work... + expect(mockState.avatarAgentIds).toEqual([ + "agent-1", + "agent-2", + "agent-3", + "agent-4", + ]); + // ...but never as a rival "+N" number next to the issue count. The task + // and agent units are explained in the hover card, which Base UI mounts + // only on open — see agent-activity-hover-content.test.tsx. + expect(mockState.avatarOverflow).toBe("fade"); }); - it("counts each distinct issue once when agents span several issues", () => { + it("counts exactly workingIssues.length even when the filter is on", () => { + // With the filter on, workingIssues IS the rendered list. The chip must + // agree with it and nothing else. mockState.snapshot = [ makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "issue-2" }), - makeTask({ id: "t-3", agent_id: "agent-1", issue_id: "issue-3" }), ]; renderWithI18n( - {}} />, + {}} + workingIssues={["issue-1", "issue-2"].map(makeIssue)} + />, ); expect( - screen.getByRole("button", { name: /working/i }), - ).toHaveTextContent("3"); + screen.getByRole("button", { name: "2 issues in progress" }), + ).toBeTruthy(); }); - it("ignores non-running tasks and respects scopedIssueIds", () => { - mockState.snapshot = [ - makeTask({ id: "t-1", issue_id: "issue-1", status: "running" }), - makeTask({ id: "t-2", issue_id: "issue-2", status: "queued" }), - makeTask({ id: "t-3", issue_id: "issue-3", status: "running" }), - ]; + it("uses the singular unit for one issue", () => { + mockState.snapshot = [makeTask({ id: "t-1", issue_id: "issue-1" })]; renderWithI18n( {}} - scopedIssueIds={new Set(["issue-1"])} + workingIssues={[makeIssue("issue-1")]} />, ); - // Only the running task within scope counts → "1". expect( - screen.getByRole("button", { name: /working/i }), - ).toHaveTextContent("1"); + screen.getByRole("button", { name: "1 issue in progress" }), + ).toBeTruthy(); + }); + + it("shows 0 when nothing is in progress", () => { + mockState.snapshot = []; + + renderWithI18n( + {}} + workingIssues={[]} + />, + ); + + expect( + screen.getByRole("button", { name: "0 issues in progress" }), + ).toBeTruthy(); + // No heads to show when nothing is running. + expect(screen.queryByTestId("agent-avatar-stack")).toBeNull(); + }); + + // Colour is two-step on purpose: the loud filled state is reserved for + // "this filter is ON". Idle activity is a tint, so the chip reads as a + // quiet tool rather than an alert. + it("keeps the filled brand state for the active filter, not for mere activity", () => { + mockState.snapshot = [makeTask({ id: "t-1", issue_id: "issue-1" })]; + + const { rerender } = renderWithI18n( + {}} + workingIssues={[makeIssue("issue-1")]} + />, + ); + + // Activity, filter off → a tint, never the fill. + const idle = screen.getByRole("button").className; + expect(idle).toContain("bg-brand/5"); + expect(idle).not.toContain("bg-brand "); + + rerender( + {}} + workingIssues={[makeIssue("issue-1")]} + />, + ); + + // Filter on → filled. + expect(screen.getByRole("button").className).toContain("bg-brand "); }); - it("shows 0 when no agents are running", () => { + it("stays muted when there is no activity at all", () => { mockState.snapshot = []; renderWithI18n( - {}} />, + {}} + workingIssues={[]} + />, + ); + + const className = screen.getByRole("button").className; + expect(className).toContain("text-muted-foreground"); + expect(className).not.toContain("bg-brand"); + }); + + it("reports issue-less tasks to the hover card instead of counting them", () => { + // Chat / autopilot tasks carry issue_id === "" (core/types/agent.ts). + // They used to collapse into one phantom "" issue and inflate the count + // by exactly 1 (MUL-4884). + mockState.snapshot = [ + makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), + makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "" }), + makeTask({ id: "t-3", agent_id: "agent-3", issue_id: "" }), + ]; + + renderWithI18n( + {}} + workingIssues={[makeIssue("issue-1")]} + />, ); expect( - screen.getByRole("button", { name: /working/i }), - ).toHaveTextContent("0"); + screen.getByRole("button", { name: "1 issue in progress" }), + ).toBeTruthy(); + // Their agents are not in the stack either: the stack explains the count. + // (They are not dropped — deriveWorkingChipView routes them to the hover + // card's "not counted" note; see the bucket tests below.) + expect(mockState.avatarAgentIds).toEqual(["agent-1"]); + }); +}); + +describe("deriveWorkingChipView", () => { + it("buckets every running task exactly once", () => { + const view = deriveWorkingChipView( + [ + makeTask({ id: "t-1", agent_id: "a1", issue_id: "issue-1" }), + makeTask({ id: "t-2", agent_id: "a2", issue_id: "issue-1" }), + makeTask({ id: "t-3", agent_id: "a3", issue_id: "" }), + makeTask({ id: "t-4", agent_id: "a4", issue_id: "issue-offscreen" }), + makeTask({ + id: "t-5", + agent_id: "a5", + issue_id: "issue-1", + status: "queued", + }), + ], + [makeIssue("issue-1")], + ); + + expect(view.taskCount).toBe(2); + expect(view.unlinkedCount).toBe(1); + // Running work on an issue the list isn't showing (filtered out, or past + // the 50-per-status page) is disclosed rather than dropped. + expect(view.outOfScopeCount).toBe(1); + // Queued tasks are not "in progress" and land in no bucket. + expect(view.agentIds).toEqual(["a1", "a2"]); + expect(view.tasksByIssueId.get("issue-1")?.map((t) => t.id)).toEqual([ + "t-1", + "t-2", + ]); + }); + + it("dedupes agents that run several tasks at once", () => { + const view = deriveWorkingChipView( + [ + makeTask({ id: "t-1", agent_id: "a1", issue_id: "issue-1" }), + makeTask({ id: "t-2", agent_id: "a1", issue_id: "issue-2" }), + ], + [makeIssue("issue-1"), makeIssue("issue-2")], + ); + + expect(view.agentIds).toEqual(["a1"]); + expect(view.taskCount).toBe(2); }); }); diff --git a/packages/views/issues/components/workspace-agent-working-chip.tsx b/packages/views/issues/components/workspace-agent-working-chip.tsx index fc7c24a7bd5..12060b1b3e8 100644 --- a/packages/views/issues/components/workspace-agent-working-chip.tsx +++ b/packages/views/issues/components/workspace-agent-working-chip.tsx @@ -10,9 +10,9 @@ import { } from "@multica/ui/components/ui/hover-card"; import { useWorkspaceId } from "@multica/core/hooks"; import { agentTaskSnapshotOptions } from "@multica/core/agents"; -import type { AgentTask } from "@multica/core/types"; +import type { AgentTask, Issue } from "@multica/core/types"; import { AgentAvatarStack } from "../../agents/components/agent-avatar-stack"; -import { AgentActivityHoverContent } from "../../agents/components/agent-activity-hover-content"; +import { WorkspaceAgentActivityHoverContent } from "../../agents/components/agent-activity-hover-content"; import { useT } from "../../i18n"; interface WorkspaceAgentWorkingChipProps { @@ -21,117 +21,136 @@ interface WorkspaceAgentWorkingChipProps { // stays presentational and accepts both forms via plain props. value: boolean; onToggle: () => void; - // When set, only running tasks whose issue id is in this set count - // toward the chip — and toward the hover card. Lets the chip stay in - // sync with the page's visible issue scope (e.g. My Issues only shows - // "my" running tasks, not the whole workspace). When omitted, the chip - // shows workspace-wide running agents. - scopedIssueIds?: ReadonlySet; + // The rows this filter leaves on screen, computed by the surface from the + // same pipeline that renders them (see `workingScopeIssues`). The chip's + // number is this list's length — that is the whole point: the number and + // the click result cannot disagree, because they are the same list. + // + // The chip used to take the PRE-filter issue set and count distinct running + // `issue_id`s out of the task snapshot itself. That was a second derivation + // of "what's on screen" and it drifted from the real one (MUL-4884). + workingIssues: readonly Issue[]; +} + +export interface WorkingChipView { + /** Running tasks on `workingIssues`, keyed by issue id. */ + tasksByIssueId: Map; + /** Distinct agents behind the counted work — the avatar stack. */ + agentIds: string[]; + /** Total running tasks across the counted issues. */ + taskCount: number; + /** Running tasks with no linked issue (chat / autopilot). */ + unlinkedCount: number; + /** Running tasks whose issue isn't on screen (filtered out, or past the + * 50-per-status page the list loads). */ + outOfScopeCount: number; } /** - * Filter chip on the issues / my-issues header, sitting to the left of - * the Filter button. Always rendered so the filter toggle never - * disappears mid-flight (a previous design hid the chip when no agents - * were running, which trapped users in an active-but-invisible filter - * state). + * Bucket the workspace task snapshot against the issues the filter would + * show. Exported for tests: the counting rule is the entire point of this + * component, so it is a pure function rather than a hook-bound useMemo. * - * Two visual modes: + * Every running task lands in exactly one bucket: + * - no `issue_id` → unlinked (chat/autopilot; never counted) + * - issue on screen → counted, grouped under that issue + * - issue not on screen → out of scope (filtered out or past the page) * - * - Has running agents → avatar stack + count + "working" label, - * wrapped in HoverCard that lists every active task on hover. - * Brand-filled when the filter is on. + * `issue_id` is an EMPTY STRING for chat/autopilot tasks, not null — see + * packages/core/types/agent.ts. Without the guard those tasks all collapse + * into one `""` bucket and read as a phantom issue, inflating the count by + * exactly one (MUL-4884). Mirrors `deriveIssueSurfaceActivity`. + */ +export function deriveWorkingChipView( + snapshot: readonly AgentTask[], + workingIssues: readonly Issue[], +): WorkingChipView { + const onScreen = new Set(workingIssues.map((issue) => issue.id)); + const tasksByIssueId = new Map(); + const agentIds: string[] = []; + const seenAgents = new Set(); + let taskCount = 0; + let unlinkedCount = 0; + let outOfScopeCount = 0; + + for (const task of snapshot) { + if (task.status !== "running") continue; + if (!task.issue_id) { + unlinkedCount += 1; + continue; + } + if (!onScreen.has(task.issue_id)) { + outOfScopeCount += 1; + continue; + } + const bucket = tasksByIssueId.get(task.issue_id); + if (bucket) bucket.push(task); + else tasksByIssueId.set(task.issue_id, [task]); + taskCount += 1; + if (!seenAgents.has(task.agent_id)) { + seenAgents.add(task.agent_id); + agentIds.push(task.agent_id); + } + } + + return { tasksByIssueId, agentIds, taskCount, unlinkedCount, outOfScopeCount }; +} + +/** + * Filter chip on the issues / my-issues header, sitting to the left of the + * Filter button. Always rendered so the filter toggle never disappears + * mid-flight (a previous design hid the chip when no agents were running, + * which trapped users in an active-but-invisible filter state). * - * - No running agents → "0 working" label, muted when off, - * brand-filled when on. No HoverCard — there is nothing to show; - * the label IS the state. + * It says one thing: "N issues in progress" — N being exactly the rows you + * get when you click it. One number, one unit, self-verifiable. * - * Click toggles the filter in both modes. The button itself is the - * affordance — no Tooltip wrapping (the popover IS the label when there - * is one, and the label is self-explanatory when there isn't). + * The avatar stack is ambience ("who's on it"), not a second statistic: it + * carries no `+N`, because a rival number next to the main one is what made + * this chip read as broken (it counted issues while the stack counted + * agents). The precise roster, the task count, and everything the number + * excludes live in the hover card. * - * `scopedIssueIds` lets a calling header narrow the chip to a subset of - * issues — typically "what's visible on this page right now". My Issues - * uses it so the chip count matches the my-scope list; the global - * /issues page passes the All/Members/Agents-scoped set. Without it the - * chip is workspace-wide. + * Colour is two-step on purpose: idle activity is a whisper of brand, and + * the loud filled state is reserved for "this filter is ON" — the chip + * should read as a quiet tool, not an alert. */ export function WorkspaceAgentWorkingChip({ value, onToggle, - scopedIssueIds, + workingIssues, }: WorkspaceAgentWorkingChipProps) { const { t } = useT("issues"); const wsId = useWorkspaceId(); const { data: snapshot = [] } = useQuery(agentTaskSnapshotOptions(wsId)); - const { runningTasks, agentIds, issueIds } = useMemo(() => { - const running: AgentTask[] = []; - for (const task of snapshot) { - if (task.status !== "running") continue; - // When scoped, drop running tasks whose issue isn't in the visible - // set — the chip's job is to summarise what the user sees, not - // what's happening elsewhere in the workspace. - if (scopedIssueIds && !scopedIssueIds.has(task.issue_id)) continue; - running.push(task); - } - // The count tracks active *issues*, not active agents: several agents - // can work the same issue at once, and the chip answers "how many - // issues are agents working on right now?" (its filter narrows the - // list to exactly those issues). The avatar stack still shows the - // distinct agents behind that work. - const uniqueIssues = [...new Set(running.map((tk) => tk.issue_id))]; - const uniqueAgents = [...new Set(running.map((tk) => tk.agent_id))]; - return { - runningTasks: running, - agentIds: uniqueAgents, - issueIds: uniqueIssues, - }; - }, [snapshot, scopedIssueIds]); + const view = useMemo( + () => deriveWorkingChipView(snapshot, workingIssues), + [snapshot, workingIssues], + ); + + // The number. Not a re-derivation of "what's running" — the length of the + // list the click produces. + const issueCount = workingIssues.length; + const hasAgents = issueCount > 0; - const hasAgents = issueIds.length > 0; // Active (brand-filled) class — must explicitly re-pin text and bg in // every interactive state. Button's `outline` variant ships // `hover:text-foreground` + `aria-expanded:bg-muted aria-expanded:text-foreground`, - // which would otherwise repaint the brand chip back to neutral on - // hover and while the HoverCard is open. + // which would otherwise repaint the brand chip back to neutral on hover + // and while the HoverCard is open. const activeClass = value ? "border-brand bg-brand text-brand-foreground hover:bg-brand/90 hover:text-brand-foreground aria-expanded:bg-brand aria-expanded:text-brand-foreground" : hasAgents - ? "text-foreground" + ? // Idle-with-activity: a brand tint, not a fill. Enough to read as + // "something is happening here" while scanning, quiet enough that + // the filled state still means something. + "border-brand/30 bg-brand/5 text-foreground" : "text-muted-foreground"; - const label = t(($) => $.agent_activity.chip_label); - - // Idle path: no agents in scope. Still wrap in HoverCard with a - // single-line placeholder so the chip's hover behavior is consistent - // with the active state — an idle chip that does nothing on hover - // reads as broken next to an active one that pops a panel. - if (!hasAgents) { - return ( - - - 0 - {label} - - } - /> - -

- {t(($) => $.agent_activity.empty_hover)} -

- - - ); - } + const label = t(($) => $.agent_activity.issues_in_progress, { + count: issueCount, + }); return ( @@ -143,20 +162,32 @@ export function WorkspaceAgentWorkingChip({ className={`h-8 px-2 md:h-7 md:px-2.5 ${activeClass}`} onClick={onToggle} aria-pressed={value} + // The narrow layout shows the bare number, so pin the full + // sentence as the accessible name in every layout. + aria-label={label} > - - {issueIds.length} - {label} + {hasAgents && ( + + )} + {issueCount} + {label} } /> - - + + ); diff --git a/packages/views/issues/surface/issue-surface.tsx b/packages/views/issues/surface/issue-surface.tsx index c8d5a01b501..5c058f69d6d 100644 --- a/packages/views/issues/surface/issue-surface.tsx +++ b/packages/views/issues/surface/issue-surface.tsx @@ -29,6 +29,10 @@ import { export interface IssueSurfaceRenderContext { controller: IssueSurfaceController; issues: Issue[]; + /** The rows the agents-working filter would leave on screen, with this + * surface's `clientFilter` applied — headers feed it to the working chip + * so the chip's count is the post-click row count (MUL-4884). */ + workingIssues: Issue[]; } interface IssueSurfaceComponentProps extends IssueSurfaceProps { @@ -136,9 +140,18 @@ function IssueSurfaceContent({ : controller.swimlaneIssues, [clientFilter, controller.swimlaneIssues], ); + // Same clientFilter the rendered rows go through, so the chip's promise + // survives on surfaces that narrow the list locally (e.g. a search box). + const workingIssues = useMemo( + () => + clientFilter + ? controller.workingScopeIssues.filter((issue) => clientFilter(issue)) + : controller.workingScopeIssues, + [clientFilter, controller.workingScopeIssues], + ); const renderContext = useMemo( - () => ({ controller, issues }), - [controller, issues], + () => ({ controller, issues, workingIssues }), + [controller, issues, workingIssues], ); const openCreateIssue = useCallback( (defaults?: IssueCreateDefaults) => { @@ -176,6 +189,7 @@ function IssueSurfaceContent({ ) : ( diff --git a/packages/views/issues/surface/use-issue-surface-controller.test.tsx b/packages/views/issues/surface/use-issue-surface-controller.test.tsx index d9d194b8887..0f2d5092de1 100644 --- a/packages/views/issues/surface/use-issue-surface-controller.test.tsx +++ b/packages/views/issues/surface/use-issue-surface-controller.test.tsx @@ -96,20 +96,41 @@ function never() { return new Promise(() => {}); } +function makeRunningTask(id: string, agentId: string, issueId: string): AgentTask { + return { + id, + agent_id: agentId, + runtime_id: "runtime-1", + issue_id: issueId, + status: "running", + priority: 0, + dispatched_at: null, + started_at: "2026-01-01T00:00:00Z", + completed_at: null, + result: null, + error: null, + created_at: "2026-01-01T00:00:00Z", + }; +} + describe("useIssueSurfaceController", () => { let qc: QueryClient; let listIssues: ReturnType< typeof vi.fn<(params?: ListIssuesParams) => Promise> >; + let getAgentTaskSnapshot: ReturnType< + typeof vi.fn<() => Promise> + >; beforeEach(() => { qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); listIssues = vi.fn(() => never()); + getAgentTaskSnapshot = vi.fn(() => never()); setApiInstance({ listIssues, listGroupedIssues: vi.fn(() => never()), listProjects: vi.fn(() => never()), - getAgentTaskSnapshot: vi.fn(() => never()), + getAgentTaskSnapshot, getChildIssueProgress: vi.fn(() => never()), } as unknown as ApiClient); pruneIssueSurfaceViewStates([]); @@ -596,4 +617,183 @@ describe("useIssueSurfaceController", () => { "cancelled-1", ); }); + + // --- working-chip scope (MUL-4884) ------------------------------------ + // The header chip promises "N issues in progress" where N is the number of + // rows clicking it leaves. That only holds if the count comes out of the + // same filter pipeline the rows do. It used to be re-derived from the task + // snapshot against the PRE-filter issue set, so any active filter made the + // chip disagree with the list it was filtering. + + it("keeps the working scope identical to the rendered rows when the filter is on", async () => { + mockListByStatus({ + todo: [ + makeIssue({ id: "todo-1", status: "todo" }), + makeIssue({ id: "todo-2", status: "todo" }), + ], + in_progress: [makeIssue({ id: "prog-1", status: "in_progress" })], + }); + // Running work on todo-1 and prog-1; todo-2 is idle. + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "todo-1"), + makeRunningTask("t-2", "agent-2", "prog-1"), + ]); + + const store = getIssueSurfaceViewStore("project:p1"); + act(() => store.getState().toggleAgentRunningFilter()); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["list"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + await waitFor(() => + expect(result.current.workingScopeIssues.length).toBe(2), + ); + + // The chip's number IS this list's length, so this identity is the whole + // promise: what it says equals what you get. + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual( + result.current.issues.map((i) => i.id), + ); + expect(result.current.issues.map((i) => i.id).sort()).toEqual([ + "prog-1", + "todo-1", + ]); + }); + + it("predicts the post-click rows while the filter is still off", async () => { + mockListByStatus({ + todo: [ + makeIssue({ id: "todo-1", status: "todo" }), + makeIssue({ id: "todo-2", status: "todo" }), + ], + }); + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "todo-1"), + ]); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["list"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + await waitFor(() => + expect(result.current.workingScopeIssues.length).toBe(1), + ); + + // Filter off: the list still shows both rows, but the chip already + // reports the one row a click would leave. + expect(result.current.issues).toHaveLength(2); + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual([ + "todo-1", + ]); + }); + + it("narrows the working scope with the status filter, exactly like the list", async () => { + mockListByStatus({ + todo: [makeIssue({ id: "todo-1", status: "todo" })], + in_progress: [makeIssue({ id: "prog-1", status: "in_progress" })], + }); + // Both issues have running agents... + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "todo-1"), + makeRunningTask("t-2", "agent-2", "prog-1"), + ]); + + // ...but the user is only looking at `todo`. + const store = getIssueSurfaceViewStore("project:p1"); + act(() => store.getState().toggleStatusFilter("todo")); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["list"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + await waitFor(() => + expect(result.current.workingScopeIssues.length).toBe(1), + ); + + // The regression: the chip used to say 2 here (both issues have running + // agents) while clicking it produced a single row. + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual([ + "todo-1", + ]); + }); + + it("hides sub-issues from the working scope when the list hides them", async () => { + mockListByStatus({ + todo: [ + makeIssue({ id: "parent-1", status: "todo" }), + makeIssue({ id: "child-1", status: "todo", parent_issue_id: "parent-1" }), + ], + }); + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "child-1"), + ]); + + const store = getIssueSurfaceViewStore("project:p1"); + act(() => store.getState().toggleShowSubIssues()); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["list"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + // The only running work sits on a sub-issue the display toggle hides, so + // clicking the filter would leave zero rows — the chip must say 0, not 1. + expect(result.current.workingScopeIssues).toEqual([]); + }); + + it("keeps issue-less chat/autopilot tasks out of the working scope", async () => { + mockListByStatus({ + todo: [makeIssue({ id: "todo-1", status: "todo" })], + }); + // issue_id "" is how the API models a chat/autopilot task — it must not + // become a phantom row (it used to inflate the count by exactly 1). + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "todo-1"), + makeRunningTask("t-2", "agent-2", ""), + makeRunningTask("t-3", "agent-3", ""), + ]); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["list"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + await waitFor(() => + expect(result.current.workingScopeIssues.length).toBe(1), + ); + + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual([ + "todo-1", + ]); + }); }); diff --git a/packages/views/issues/surface/use-issue-surface-controller.ts b/packages/views/issues/surface/use-issue-surface-controller.ts index bc819f266ab..bad9560f92d 100644 --- a/packages/views/issues/surface/use-issue-surface-controller.ts +++ b/packages/views/issues/surface/use-issue-surface-controller.ts @@ -57,6 +57,9 @@ export interface IssueSurfaceController { projectIssues: Issue[]; issues: Issue[]; swimlaneIssues: Issue[]; + /** The rows the agents-working filter would leave on screen. Feeds the + * header chip so its count IS the post-click row count (MUL-4884). */ + workingScopeIssues: Issue[]; filteredGanttIssues: Issue[]; assigneeGroups?: IssueAssigneeGroup[]; assigneeGroupQueryKey?: QueryKey; @@ -216,6 +219,7 @@ export function useIssueSurfaceController({ const usesAssigneeBoard = effectiveViewMode === "board" && grouping === "assignee"; const usesGantt = effectiveViewMode === "gantt" && !!projectId; + const usesSwimlane = effectiveViewMode === "swimlane"; const projectFilterState = useMemo( () => ({ @@ -233,6 +237,7 @@ export function useIssueSurfaceController({ projectId, usesAssigneeBoard, usesGantt, + usesSwimlane, sort, statusFilters, priorityFilters, diff --git a/packages/views/issues/surface/use-issue-surface-data.ts b/packages/views/issues/surface/use-issue-surface-data.ts index 3018bed2e4c..216ab1a0efb 100644 --- a/packages/views/issues/surface/use-issue-surface-data.ts +++ b/packages/views/issues/surface/use-issue-surface-data.ts @@ -39,6 +39,10 @@ export interface IssueSurfaceData { projectIssues: Issue[]; issues: Issue[]; swimlaneIssues: Issue[]; + /** The rows the agents-working filter would leave on screen. See the + * `workingScopeIssues` memo for why this is a projection of the render + * pipeline rather than a second derivation from the task snapshot. */ + workingScopeIssues: Issue[]; filteredGanttIssues: Issue[]; assigneeGroups?: IssueAssigneeGroup[]; assigneeGroupQueryKey?: QueryKey; @@ -68,6 +72,7 @@ export function useIssueSurfaceData({ projectId, usesAssigneeBoard, usesGantt, + usesSwimlane, sort, statusFilters, priorityFilters, @@ -87,6 +92,7 @@ export function useIssueSurfaceData({ projectId?: string; usesAssigneeBoard: boolean; usesGantt: boolean; + usesSwimlane: boolean; sort: IssueSortParam; statusFilters: IssueStatus[]; priorityFilters: IssueFilterState["priorityFilters"]; @@ -238,6 +244,65 @@ export function useIssueSurfaceData({ ], ); + // The rows the agents-working filter leaves on screen — i.e. exactly what + // the header chip promises you get when you click it. + // + // This is deliberately a PROJECTION OF THE RENDER PIPELINE, not a second + // pass over the task snapshot: it reuses the same predicates, the same + // filter state and the same per-mode source as the rows below, with + // `workingOnly` forced on. That makes "chip count === row count" true by + // construction. The chip used to count distinct running `issue_id`s + // straight from the snapshot against the PRE-filter issue set, so any + // active status/assignee/label filter (or a sub-issue hidden by the + // display toggle) made it disagree with the list it was filtering + // (MUL-4884). + // + // Turning the filter on only adds `workingOnly` to this same pipeline, so + // the preview is the post-click list whether the filter is currently on or + // off. Mode branches mirror the render branches: gantt renders its own + // scheduled-only projection, the assignee board renders from `groups`, and + // swimlane drops the status filter. + const workingScopeIssues = useMemo(() => { + if (usesGantt) { + return applyIssueFilters( + ganttIssues, + { ...baseFilterState, workingOnly: true }, + filterContext, + ); + } + if (usesAssigneeBoard) { + return ( + filterAssigneeGroups(assigneeGroupsQuery.data?.groups, { + showSubIssues, + agentRunningFilter: true, + runningIssueIds: activity.runningIssueIds, + propertyFilters, + }) ?? [] + ).flatMap((group) => group.issues); + } + return applyIssueFilters( + surfaceIssues, + { + ...(usesSwimlane ? statuslessFilterState : baseFilterState), + workingOnly: true, + }, + filterContext, + ); + }, [ + activity.runningIssueIds, + assigneeGroupsQuery.data?.groups, + baseFilterState, + filterContext, + ganttIssues, + propertyFilters, + showSubIssues, + statuslessFilterState, + surfaceIssues, + usesAssigneeBoard, + usesGantt, + usesSwimlane, + ]); + const { data: childProgressMap = EMPTY_CHILD_PROGRESS } = useQuery( childIssueProgressOptions(wsId), ); @@ -315,6 +380,7 @@ export function useIssueSurfaceData({ projectIssues: surfaceIssues, issues, swimlaneIssues, + workingScopeIssues, filteredGanttIssues, assigneeGroups: usesAssigneeBoard ? filteredAssigneeGroups : undefined, assigneeGroupQueryKey: usesAssigneeBoard diff --git a/packages/views/locales/en/issues.json b/packages/views/locales/en/issues.json index a52d8710687..90e6ff2d048 100644 --- a/packages/views/locales/en/issues.json +++ b/packages/views/locales/en/issues.json @@ -359,9 +359,16 @@ "hover_header_queued_other": "{{count}} agents queued", "status_running": "Working", "status_queued": "Queued", - "chip_label": "working", - "empty_hover": "No agents currently working", - "filter_active_label": "Viewing only working agents" + "empty_hover": "No issues in progress right now", + "filter_active_label": "Viewing only issues in progress", + "issues_in_progress_one": "{{count}} issue in progress", + "issues_in_progress_other": "{{count}} issues in progress", + "tasks_count_one": "{{count}} task", + "tasks_count_other": "{{count}} tasks", + "unlinked_note_one": "{{count}} more task has no linked issue (chat/autopilot) — not counted", + "unlinked_note_other": "{{count}} more tasks have no linked issue (chat/autopilot) — not counted", + "out_of_scope_note_one": "{{count}} more task is outside the current filters or loaded range — not counted", + "out_of_scope_note_other": "{{count}} more tasks are outside the current filters or loaded range — not counted" }, "agent_live": { "is_working": "{{name}} is working", diff --git a/packages/views/locales/ja/issues.json b/packages/views/locales/ja/issues.json index 6f879a21677..2d1f2e29b5e 100644 --- a/packages/views/locales/ja/issues.json +++ b/packages/views/locales/ja/issues.json @@ -343,9 +343,12 @@ "hover_header_queued_other": "待機中のエージェント {{count}} 件", "status_running": "作業中", "status_queued": "待機中", - "chip_label": "作業中", - "empty_hover": "現在作業中のエージェントはありません", - "filter_active_label": "作業中のエージェントのみ表示中" + "empty_hover": "進行中の issue はありません", + "filter_active_label": "進行中の issue のみ表示中", + "issues_in_progress_other": "進行中の issue {{count}} 件", + "tasks_count_other": "task {{count}} 件", + "unlinked_note_other": "issue に紐づかない task が他に {{count}} 件あります（chat/autopilot）。カウントには含まれません", + "out_of_scope_note_other": "現在のフィルターまたは読み込み済み範囲外の task が他に {{count}} 件あります。カウントには含まれません" }, "agent_live": { "is_working": "{{name}} が作業中", diff --git a/packages/views/locales/ko/issues.json b/packages/views/locales/ko/issues.json index 335e6dda1c7..4b19aa60a26 100644 --- a/packages/views/locales/ko/issues.json +++ b/packages/views/locales/ko/issues.json @@ -343,9 +343,12 @@ "hover_header_queued_other": "대기 중인 에이전트 {{count}}개", "status_running": "작업 중", "status_queued": "대기 중", - "chip_label": "작업 중", - "empty_hover": "현재 작업 중인 에이전트가 없습니다", - "filter_active_label": "작업 중인 에이전트만 보는 중" + "empty_hover": "진행 중인 issue가 없습니다", + "filter_active_label": "진행 중인 issue만 보는 중", + "issues_in_progress_other": "진행 중인 issue {{count}}개", + "tasks_count_other": "task {{count}}개", + "unlinked_note_other": "issue에 연결되지 않은 task {{count}}개가 더 있습니다(chat/autopilot). 개수에 포함되지 않습니다", + "out_of_scope_note_other": "현재 필터 또는 불러온 범위 밖의 task {{count}}개가 더 있습니다. 개수에 포함되지 않습니다" }, "agent_live": { "is_working": "{{name}} 작업 중", diff --git a/packages/views/locales/zh-Hans/issues.json b/packages/views/locales/zh-Hans/issues.json index fd992cb8a95..829cd2799f6 100644 --- a/packages/views/locales/zh-Hans/issues.json +++ b/packages/views/locales/zh-Hans/issues.json @@ -343,9 +343,12 @@ "hover_header_queued_other": "{{count}} 个智能体排队中", "status_running": "正在工作", "status_queued": "排队中", - "chip_label": "工作中", - "empty_hover": "当前没有智能体在工作", - "filter_active_label": "正在查看工作中的智能体" + "empty_hover": "当前没有进行中的 issue", + "filter_active_label": "正在查看进行中的 issue", + "issues_in_progress_other": "{{count}} 个 issue 进行中", + "tasks_count_other": "{{count}} 个 task", + "unlinked_note_other": "另有 {{count}} 个 task 未关联 issue（chat/autopilot），不计入", + "out_of_scope_note_other": "另有 {{count}} 个 task 在当前筛选或已加载范围之外，不计入" }, "agent_live": { "is_working": "{{name}} 在工作", diff --git a/packages/views/my-issues/components/my-issues-header.tsx b/packages/views/my-issues/components/my-issues-header.tsx index 787e8a59ab3..805ae798299 100644 --- a/packages/views/my-issues/components/my-issues-header.tsx +++ b/packages/views/my-issues/components/my-issues-header.tsx @@ -1,6 +1,5 @@ "use client"; -import { useMemo } from "react"; import { ChevronDown } from "lucide-react"; import { Button } from "@multica/ui/components/ui/button"; import { @@ -23,11 +22,15 @@ import { export function MyIssuesHeader({ allIssues, + workingIssues, scope, onScopeChange, isRefreshing = false, }: { allIssues: Issue[]; + /** The rows the agents-working filter would leave on screen — the chip's + * count is this list's length, so it matches the post-click list. */ + workingIssues: Issue[]; scope: MyIssuesScope; onScopeChange: (scope: MyIssuesScope) => void; isRefreshing?: boolean; @@ -44,10 +47,6 @@ export function MyIssuesHeader({ const toggleAgentRunningFilter = useViewStore( (s) => s.toggleAgentRunningFilter, ); - const scopedIssueIds = useMemo( - () => new Set(allIssues.map((i) => i.id)), - [allIssues], - ); const scopeLabel = SCOPES.find((s) => s.value === scope)?.label ?? SCOPES[0]?.label; return ( @@ -113,7 +112,7 @@ export function MyIssuesHeader({ diff --git a/packages/views/my-issues/components/my-issues-page.tsx b/packages/views/my-issues/components/my-issues-page.tsx index 271e4445b0f..6f7414a4b38 100644 --- a/packages/views/my-issues/components/my-issues-page.tsx +++ b/packages/views/my-issues/components/my-issues-page.tsx @@ -38,9 +38,10 @@ export function MyIssuesPage() { }} modes={["board", "list", "swimlane"]} batchToolbar="list" - renderHeader={({ controller }) => ( + renderHeader={({ controller, workingIssues }) => ( Date: Thu, 16 Jul 2026 20:10:52 +0800 Subject: [PATCH 2/4] fix(issues): scope the working chip to the rows swimlane and gantt actually draw (MUL-4884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found two modes where `workingScopeIssues` still came from a different set than the one on screen — the exact drift this change set is meant to remove. Swimlane: scoped to the statusless `swimlaneIssues`, but SwimLaneView draws its cards from `issues` (status filter applied) and only uses the statusless set for LANE DISCOVERY. A status-filtered swimlane counted rows the canvas never drew. Swimlane needs no branch at all — it renders the flat filtered list like board and list do — so the special case is gone rather than corrected. Gantt: the canvas applies two rules of its own before drawing — a row needs a date to be placed, and done/cancelled hide unless `ganttShowCompleted` is on — so a done issue with a running agent counted toward the chip while the canvas refused to draw it. Those rules now live in the surface (`ganttCanvasRows`) instead of privately inside GanttView, and both `filteredGanttIssues` and the working scope go through them. Mirroring the rules in a second place would have reproduced the original bug with extra steps; hoisting them means the chip narrows the same set the canvas draws, by construction. GanttView goes back to being a renderer: it orders rows, it does not decide which ones exist. Regression tests cover both, and each was confirmed to fail against the code it guards: swimlane asserts the statusless lane source still holds the wider set, so a revert fails loudly rather than passing by luck; gantt covers the hidden done row, the undated row, and the show-completed toggle widening both the canvas and the scope together. Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- .../views/issues/components/gantt-view.tsx | 19 +-- .../use-issue-surface-controller.test.tsx | 151 ++++++++++++++++++ .../surface/use-issue-surface-controller.ts | 4 +- .../issues/surface/use-issue-surface-data.ts | 70 ++++++-- 4 files changed, 214 insertions(+), 30 deletions(-) diff --git a/packages/views/issues/components/gantt-view.tsx b/packages/views/issues/components/gantt-view.tsx index b1be5a263cc..220d8a87561 100644 --- a/packages/views/issues/components/gantt-view.tsx +++ b/packages/views/issues/components/gantt-view.tsx @@ -451,21 +451,18 @@ export function GanttView({ issues }: { issues: Issue[] }) { const today = useMemo(() => startOfDayUTC(new Date()), []); const dayPx = DAY_PX_BY_ZOOM[zoom]; - // The data source only delivers scheduled issues (server-side - // `scheduled=true`), but a row can still arrive here without a date — for - // example, a WS-driven optimistic patch that just cleared start_date / - // due_date and is waiting for the cache to refetch. Filter defensively so - // the timeline never renders a blank lane in that brief window. + // `issues` is already the canvas set: the surface applies the shared + // filters, drops undated rows, and honours `ganttShowCompleted` before + // handing it over (see `ganttCanvasRows` in use-issue-surface-data.ts). + // Those rules used to live here, which meant the header chip could count + // rows this canvas would never draw (MUL-4884). Keep this view a renderer: + // it orders rows, it does not decide which ones exist. const scheduled = useMemo(() => { - const dated = issues.filter((i) => i.start_date || i.due_date); - const filtered = showCompleted - ? dated - : dated.filter((i) => i.status !== "done" && i.status !== "cancelled"); // "position" makes no sense on a gantt — default to start_date asc when // the user hasn't picked a more specific sort. const sortField = sortBy === "position" ? "start_date" : sortBy; - return sortIssues(filtered, sortField, sortDirection); - }, [issues, showCompleted, sortBy, sortDirection]); + return sortIssues(issues, sortField, sortDirection); + }, [issues, sortBy, sortDirection]); const range = useMemo( () => computeRange(scheduled, today, zoom), diff --git a/packages/views/issues/surface/use-issue-surface-controller.test.tsx b/packages/views/issues/surface/use-issue-surface-controller.test.tsx index 0f2d5092de1..f12ec92f87f 100644 --- a/packages/views/issues/surface/use-issue-surface-controller.test.tsx +++ b/packages/views/issues/surface/use-issue-surface-controller.test.tsx @@ -796,4 +796,155 @@ describe("useIssueSurfaceController", () => { "todo-1", ]); }); + + it("scopes swimlane to the cards it draws, not its statusless lane source", async () => { + // SwimLaneView draws cards from `issues` (status filter applied) and uses + // the statusless `swimlaneIssues` only for LANE DISCOVERY. Scoping the + // chip to the statusless set counted rows the canvas never draws. + mockListByStatus({ + todo: [makeIssue({ id: "todo-1", status: "todo" })], + in_progress: [makeIssue({ id: "prog-1", status: "in_progress" })], + }); + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "todo-1"), + makeRunningTask("t-2", "agent-2", "prog-1"), + ]); + + const store = getIssueSurfaceViewStore("project:p1"); + act(() => { + store.getState().setViewMode("swimlane"); + store.getState().toggleStatusFilter("todo"); + }); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["board", "list", "swimlane"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + await waitFor(() => + expect(result.current.workingScopeIssues.length).toBe(1), + ); + + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual([ + "todo-1", + ]); + expect(result.current.issues.map((i) => i.id)).toEqual(["todo-1"]); + // The statusless lane source still carries both — so a regression back to + // it would make the assertion above fail rather than pass by accident. + expect(result.current.swimlaneIssues.map((i) => i.id).sort()).toEqual([ + "prog-1", + "todo-1", + ]); + }); + + // --- gantt canvas scope ------------------------------------------------ + // The gantt canvas draws fewer rows than the shared filters leave: a row + // needs a date, and done/cancelled hide unless `ganttShowCompleted` is on. + // Those rules live in the surface (`ganttCanvasRows`) so the chip narrows + // the same set the canvas draws. + + function mockGanttIssues(issues: Issue[]) { + listIssues.mockImplementation((params?: ListIssuesParams) => { + if (params?.scheduled === true) { + return Promise.resolve({ issues, total: issues.length }); + } + return Promise.resolve({ issues: [], total: 0 }); + }); + } + + const ganttFixture = [ + makeIssue({ + id: "gantt-open", + status: "in_progress", + start_date: "2026-01-01", + due_date: "2026-01-05", + }), + makeIssue({ + id: "gantt-done", + status: "done", + start_date: "2026-01-01", + due_date: "2026-01-05", + }), + // Scheduled server-side but momentarily dateless (e.g. a WS patch that + // just cleared both dates) — the canvas cannot place it. + makeIssue({ id: "gantt-undated", status: "in_progress" }), + ]; + + it("keeps rows the gantt canvas hides out of the working scope", async () => { + mockGanttIssues(ganttFixture); + // Every one of them has a running agent. + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "gantt-open"), + makeRunningTask("t-2", "agent-2", "gantt-done"), + makeRunningTask("t-3", "agent-3", "gantt-undated"), + ]); + + const store = getIssueSurfaceViewStore("project:p1"); + act(() => store.getState().setViewMode("gantt")); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["board", "list", "swimlane", "gantt"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => + expect(result.current.filteredGanttIssues.length).toBe(1), + ); + + // ganttShowCompleted defaults to false, so the done row and the undated + // row are not drawn — the chip must not count them either. + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual([ + "gantt-open", + ]); + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual( + result.current.filteredGanttIssues.map((i) => i.id), + ); + }); + + it("widens the gantt working scope when show-completed is turned on", async () => { + mockGanttIssues(ganttFixture); + getAgentTaskSnapshot.mockResolvedValue([ + makeRunningTask("t-1", "agent-1", "gantt-open"), + makeRunningTask("t-2", "agent-2", "gantt-done"), + makeRunningTask("t-3", "agent-3", "gantt-undated"), + ]); + + const store = getIssueSurfaceViewStore("project:p1"); + act(() => { + store.getState().setViewMode("gantt"); + store.getState().toggleGanttShowCompleted(); + }); + + const { result } = renderHook( + () => + useIssueSurfaceController({ + scope: { type: "project", projectId: "p1" }, + modes: ["board", "list", "swimlane", "gantt"], + }), + { wrapper: makeWrapper(qc, "project:p1") }, + ); + + await waitFor(() => + expect(result.current.filteredGanttIssues.length).toBe(2), + ); + + // The done row is drawn now, so it counts. The undated one still cannot + // be placed, so it still does not. + expect(result.current.workingScopeIssues.map((i) => i.id).sort()).toEqual([ + "gantt-done", + "gantt-open", + ]); + expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual( + result.current.filteredGanttIssues.map((i) => i.id), + ); + }); }); diff --git a/packages/views/issues/surface/use-issue-surface-controller.ts b/packages/views/issues/surface/use-issue-surface-controller.ts index bad9560f92d..5484e77b980 100644 --- a/packages/views/issues/surface/use-issue-surface-controller.ts +++ b/packages/views/issues/surface/use-issue-surface-controller.ts @@ -138,6 +138,7 @@ export function useIssueSurfaceController({ const propertyFilters = useViewStore((s) => s.propertyFilters); const agentRunningFilter = useViewStore((s) => s.agentRunningFilter); const showSubIssues = useViewStore((s) => s.showSubIssues); + const ganttShowCompleted = useViewStore((s) => s.ganttShowCompleted); const cardProperties = useViewStore((s) => s.cardProperties); const swimlaneGrouping = useViewStore((s) => s.swimlaneGrouping); @@ -219,7 +220,6 @@ export function useIssueSurfaceController({ const usesAssigneeBoard = effectiveViewMode === "board" && grouping === "assignee"; const usesGantt = effectiveViewMode === "gantt" && !!projectId; - const usesSwimlane = effectiveViewMode === "swimlane"; const projectFilterState = useMemo( () => ({ @@ -237,7 +237,7 @@ export function useIssueSurfaceController({ projectId, usesAssigneeBoard, usesGantt, - usesSwimlane, + ganttShowCompleted, sort, statusFilters, priorityFilters, diff --git a/packages/views/issues/surface/use-issue-surface-data.ts b/packages/views/issues/surface/use-issue-surface-data.ts index 216ab1a0efb..8359a64867c 100644 --- a/packages/views/issues/surface/use-issue-surface-data.ts +++ b/packages/views/issues/surface/use-issue-surface-data.ts @@ -34,6 +34,28 @@ const EMPTY_ISSUES: Issue[] = []; const EMPTY_CHILD_PROGRESS = new Map(); const EMPTY_PROJECTS: Project[] = []; +/** + * The rows the gantt canvas actually draws, on top of the shared filters. + * + * The canvas adds two rules of its own: a row needs a date to be placed, and + * completed work is hidden unless the user asks for it. The data source only + * delivers scheduled issues (server-side `scheduled=true`), but a row can + * still arrive without a date — e.g. a WS-driven optimistic patch that just + * cleared start_date / due_date and is waiting for the cache to refetch — so + * the date check stays defensive. + * + * These rules live HERE rather than privately inside GanttView so the header + * chip can narrow the same set the canvas draws. A view that filters its own + * rows in secret is exactly how the chip's count drifted from the list in the + * first place (MUL-4884); duplicating the rules in both places would just + * reintroduce the drift with extra steps. + */ +function ganttCanvasRows(issues: Issue[], showCompleted: boolean): Issue[] { + const dated = issues.filter((i) => i.start_date || i.due_date); + if (showCompleted) return dated; + return dated.filter((i) => i.status !== "done" && i.status !== "cancelled"); +} + export interface IssueSurfaceData { surfaceIssues: Issue[]; projectIssues: Issue[]; @@ -72,7 +94,7 @@ export function useIssueSurfaceData({ projectId, usesAssigneeBoard, usesGantt, - usesSwimlane, + ganttShowCompleted, sort, statusFilters, priorityFilters, @@ -92,7 +114,9 @@ export function useIssueSurfaceData({ projectId?: string; usesAssigneeBoard: boolean; usesGantt: boolean; - usesSwimlane: boolean; + /** Gantt's "show completed" display toggle. The canvas hides done/cancelled + * rows without it, so the working scope has to honour it too. */ + ganttShowCompleted: boolean; sort: IssueSortParam; statusFilters: IssueStatus[]; priorityFilters: IssueFilterState["priorityFilters"]; @@ -220,8 +244,12 @@ export function useIssueSurfaceData({ ); const filteredGanttIssues = useMemo( - () => applyIssueFilters(ganttIssues, baseFilterState, filterContext), - [baseFilterState, filterContext, ganttIssues], + () => + ganttCanvasRows( + applyIssueFilters(ganttIssues, baseFilterState, filterContext), + ganttShowCompleted, + ), + [baseFilterState, filterContext, ganttIssues, ganttShowCompleted], ); // The assignee-grouped board renders straight from `groups`, bypassing the @@ -259,15 +287,27 @@ export function useIssueSurfaceData({ // // Turning the filter on only adds `workingOnly` to this same pipeline, so // the preview is the post-click list whether the filter is currently on or - // off. Mode branches mirror the render branches: gantt renders its own - // scheduled-only projection, the assignee board renders from `groups`, and - // swimlane drops the status filter. + // off. + // + // Each branch below must take the SAME source the matching branch of + // IssueSurface renders: + // - gantt → the canvas set (scheduled + dated + showCompleted) + // - assignee board → the grouped response, not the flat list + // - board / list / swimlane → the flat filtered list + // + // Swimlane deliberately has no branch: SwimLaneView draws its cards from + // `issues` (status filter applied) and only uses the statusless + // `swimlaneIssues` for LANE DISCOVERY, so scoping the chip to the + // statusless set would count rows the canvas never draws. const workingScopeIssues = useMemo(() => { if (usesGantt) { - return applyIssueFilters( - ganttIssues, - { ...baseFilterState, workingOnly: true }, - filterContext, + return ganttCanvasRows( + applyIssueFilters( + ganttIssues, + { ...baseFilterState, workingOnly: true }, + filterContext, + ), + ganttShowCompleted, ); } if (usesAssigneeBoard) { @@ -282,10 +322,7 @@ export function useIssueSurfaceData({ } return applyIssueFilters( surfaceIssues, - { - ...(usesSwimlane ? statuslessFilterState : baseFilterState), - workingOnly: true, - }, + { ...baseFilterState, workingOnly: true }, filterContext, ); }, [ @@ -294,13 +331,12 @@ export function useIssueSurfaceData({ baseFilterState, filterContext, ganttIssues, + ganttShowCompleted, propertyFilters, showSubIssues, - statuslessFilterState, surfaceIssues, usesAssigneeBoard, usesGantt, - usesSwimlane, ]); const { data: childProgressMap = EMPTY_CHILD_PROGRESS } = useQuery( From 0097a91cb1c643ee38d8c448b0e465eafc219f00 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:04:09 +0800 Subject: [PATCH 3/4] feat(issues): anchor the working chip on agents, and give it real colour tiers (MUL-4884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the approved final mock. Two changes, one root cause each. SEMANTICS: the chip now counts agents, not issues. The original bug was "+1" (agents) sitting next to "3" (issues) — two units in one control. Anchoring on issues fixed which number was authoritative but left the units mismatched, so the avatar stack had to lose its +N and the label had to grow a noun to suppress the contradiction. Counting agents makes the number and the heads beside it the same list: the mismatch is gone rather than managed. It also settles the subject — only agents emit a runtime signal, so "N agents working" cannot be misread as "N issues someone is working on", which "N issues in progress" invited. Accepted trade-off: the number no longer predicts the row count of the click. One agent on two issues reads "1 agent working" and opens two rows. The chip answers WHO, the list answers WHERE — different questions, so they don't compete, and the hover card's issue grouping shows the mapping. The workingScopeIssues pipeline stays: it still decides WHICH agents count. The hover card drops both "not counted" footnotes. They explained an absence the user never perceived — chat/autopilot runs leave no row, no head, no indicator on this page — so they invented a discrepancy instead of resolving one. The empty-issue_id guard stays: that is data correctness (those agents aren't on any issue), independent of whether we narrate it. Avatar overflow returns to the component's standard +N badge; the fade variant is deleted. With an agent-anchored number, "3 shown + 1 = 4" corroborates the text instead of competing with it. COLOUR: three tiers, each a self-contained Button variant. Layering brand classes over `outline` could never work in dark mode: `outline` ships dark:bg-input/30 / dark:border-input / dark:hover:bg-input/50, tailwind-merge keeps them (different modifier group), and they win the cascade — `dark` compiles to `&:is(.dark *)`, so they outrank a bare .bg-brand on specificity, not just order. Both brand tiers were dead in dark; --brand-foreground equals --foreground there, so filter-on and filter-off rendered pixel-identical. New `brand` / `brandSubtle` variants own every state instead: hover, pressed, and aria-expanded pinned to the hover value so opening the popover doesn't read as a colour change. `brand` needs no dark: at all — the token flips per theme. `brandSubtle` runs one notch hotter in dark, where the same alpha reads weaker. Verified against the compiled stylesheet rather than by asserting class strings — a string assertion is what let the dark bug through. A cascade check resolves each tier x {default,hover,pressed,popover-open} x {light,dark} by specificity and source order; all 16 brand cells land on their intended notch, identical in both themes for `brand`. Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- packages/ui/components/ui/button.tsx | 21 +++ .../agent-activity-hover-content.test.tsx | 76 +++------- .../agent-activity-hover-content.tsx | 63 ++------- .../agents/components/agent-avatar-stack.tsx | 19 +-- .../workspace-agent-working-chip.test.tsx | 131 ++++++++---------- .../workspace-agent-working-chip.tsx | 114 +++++++-------- packages/views/locales/en/issues.json | 14 +- packages/views/locales/ja/issues.json | 9 +- packages/views/locales/ko/issues.json | 9 +- packages/views/locales/zh-Hans/issues.json | 9 +- 10 files changed, 181 insertions(+), 284 deletions(-) diff --git a/packages/ui/components/ui/button.tsx b/packages/ui/components/ui/button.tsx index d43f920308f..bb3faf2b34d 100644 --- a/packages/ui/components/ui/button.tsx +++ b/packages/ui/components/ui/button.tsx @@ -13,6 +13,27 @@ const buttonVariants = cva( default: "bg-primary text-primary-foreground [a]:hover:bg-primary/80", outline: "border-border bg-background hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:border-input dark:bg-input/30 dark:hover:bg-input/50", + // Brand-filled state for a control that is currently ON (an active + // filter, a selected toggle). Self-contained on purpose: passing + // brand classes through `className` on top of `outline` does NOT + // work — `outline` ships `dark:bg-input/30`, `hover:bg-muted` and + // `aria-expanded:bg-muted`, and those win the cascade (the `dark:` + // ones by specificity, since `dark` compiles to `&:is(.dark *)`), + // repainting the chip neutral. That is what silently killed the + // brand colour in dark mode — see MUL-4884. + // + // `brand` needs no `dark:` of its own: the --brand token already + // flips per theme, so one set of rules is correct in both. + // aria-expanded is pinned to the hover value so opening a popover + // reads as hover rather than as a colour change. + brand: + "border-brand bg-brand text-brand-foreground hover:bg-brand/90 hover:text-brand-foreground active:bg-brand/85 aria-expanded:bg-brand/90 aria-expanded:text-brand-foreground", + // Brand tint for "there is activity here" — present, but not + // claiming the loud filled state. Light and dark take their own + // opacity notches: the same alpha does not read equally against a + // white and a near-black surface, so dark runs one notch hotter. + brandSubtle: + "border-brand/28 bg-brand/7 text-foreground hover:bg-brand/12 hover:text-foreground active:bg-brand/16 aria-expanded:bg-brand/12 aria-expanded:text-foreground dark:border-brand/45 dark:bg-brand/12 dark:hover:bg-brand/18 dark:active:bg-brand/24 dark:aria-expanded:bg-brand/18", secondary: "bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground", ghost: diff --git a/packages/views/agents/components/agent-activity-hover-content.test.tsx b/packages/views/agents/components/agent-activity-hover-content.test.tsx index 2d018964e59..5370b48343c 100644 --- a/packages/views/agents/components/agent-activity-hover-content.test.tsx +++ b/packages/views/agents/components/agent-activity-hover-content.test.tsx @@ -141,14 +141,12 @@ describe("AgentActivityHoverContent", () => { }); }); -// The workspace chip can only carry one number (issues — the rows a click -// produces). This card is where the other units get stated instead of -// silently contradicting it, and where everything the number excludes is -// disclosed rather than dropped. MUL-4884. +// The workspace chip says WHO is working ("N agents working"). This card +// says WHERE: the two figures the chip does not carry, and the rows grouped +// by issue. It stays silent about work it excludes — see the component doc. +// MUL-4884. describe("WorkspaceAgentActivityHoverContent", () => { - it("names both counted units so the chip's number stops looking wrong", () => { - // The MUL-4884 screenshot: chip says 3 while 4 agent heads show. Naming - // both units is what resolves that, rather than hiding one. + it("carries the two figures the chip does not, and groups rows by issue", () => { renderWithI18n( { ]) } taskCount={4} - unlinkedCount={0} - outOfScopeCount={0} - />, + />, ); - expect(screen.getByText("3 issues in progress · 4 tasks")).toBeInTheDocument(); - // Rows group under their issue, mirroring what the filter does. + expect(screen.getByText("3 issues · 4 tasks")).toBeInTheDocument(); + // Rows group under their issue, mirroring what clicking the chip does. expect(screen.getByText("MUL-4879")).toBeInTheDocument(); expect(screen.getByText("Counting logic looks wrong")).toBeInTheDocument(); expect(screen.getAllByTestId("actor-avatar")).toHaveLength(4); }); - it("states what the number excludes rather than dropping it", () => { - renderWithI18n( - , - ); - - expect( - screen.getByText( - "1 more task has no linked issue (chat/autopilot) — not counted", - ), - ).toBeInTheDocument(); - // The list loads one page per status, so running work can sit past the - // window. Say so instead of silently under-counting. - expect( - screen.getByText( - "2 more tasks are outside the current filters or loaded range — not counted", - ), - ).toBeInTheDocument(); - }); - - it("stays quiet when there is nothing to disclose", () => { + it("says nothing about work it excludes", () => { + // Chat/autopilot runs and out-of-scope tasks leave no trace on this page, + // so a "not counted" footnote would explain an absence the user never + // perceived. The card only ever describes what IS counted. renderWithI18n( { new Map([["i-1", [makeTask({ id: "t1", issue_id: "i-1" })]]]) } taskCount={1} - unlinkedCount={0} - outOfScopeCount={0} />, ); expect(screen.queryByText(/not counted/)).not.toBeInTheDocument(); + expect(screen.getByText("1 issue · 1 task")).toBeInTheDocument(); }); - it("still discloses excluded work when nothing is counted", () => { - // Only running work is a chat task: the chip reads 0, and the card has to - // explain why rather than look broken. + it("falls back to the agent-worded empty copy when nothing is counted", () => { renderWithI18n( , ); - expect( - screen.getByText("No issues in progress right now"), - ).toBeInTheDocument(); - expect( - screen.getByText( - "1 more task has no linked issue (chat/autopilot) — not counted", - ), - ).toBeInTheDocument(); + expect(screen.getByText("No agents working right now")).toBeInTheDocument(); + expect(screen.queryByText(/not counted/)).not.toBeInTheDocument(); }); it("renders the Chinese copy for the counted units", () => { @@ -256,12 +217,11 @@ describe("WorkspaceAgentActivityHoverContent", () => { new Map([["i-1", [makeTask({ id: "t1", issue_id: "i-1" })]]]) } taskCount={2} - unlinkedCount={0} - outOfScopeCount={0} />, { locale: "zh-Hans" }, ); - expect(screen.getByText("1 个 issue 进行中 · 2 个 task")).toBeInTheDocument(); + // task / issue stay lowercase English in UI strings (conventions.zh.mdx). + expect(screen.getByText("1 个 issue · 2 个 task")).toBeInTheDocument(); }); }); diff --git a/packages/views/agents/components/agent-activity-hover-content.tsx b/packages/views/agents/components/agent-activity-hover-content.tsx index 157264d4f56..4d19e0e93c1 100644 --- a/packages/views/agents/components/agent-activity-hover-content.tsx +++ b/packages/views/agents/components/agent-activity-hover-content.tsx @@ -164,79 +164,51 @@ export function AgentActivityHoverContent({ interface WorkspaceAgentActivityHoverContentProps { /** Issues the working filter leaves on screen, in list order. Each has at - * least one running task. `issues.length` IS the chip's number. */ + * least one running task. */ issues: readonly Issue[]; /** Running tasks for those issues, keyed by issue id. */ tasksByIssueId: ReadonlyMap; /** Total running tasks across `issues` — the second header figure. */ taskCount: number; - /** Running tasks with no linked issue (chat / autopilot). */ - unlinkedCount: number; - /** Running tasks whose issue is outside the current filters / loaded page. */ - outOfScopeCount: number; } /** * Hover-card body for the workspace working chip (MUL-4884). * - * The chip can only carry one number, and the honest one is the issue count — - * it equals the rows you get when you click it. This card is where the other - * two units get explained instead of contradicting it: + * The chip says WHO is working ("N agents working"); this card says WHERE. + * The header carries the two figures the chip does not — how many issues + * that work lands on, and how many tasks it takes — and the rows group by + * issue, mirroring what clicking the chip does to the list. * - * - the header states both counted units side by side ("3 issues · 4 - * tasks"), so "4 heads but the chip says 3" resolves instead of nagging; - * - rows group under their issue, mirroring what the filter does; - * - anything the number deliberately excludes is stated rather than - * dropped — issue-less chat/autopilot tasks, and tasks on issues outside - * the current filters or the loaded page (the list is fetched a page per - * status, so running work can exist past the window). + * It says nothing about work it excludes. Chat/autopilot runs have no + * linked issue and leave no trace anywhere on this page: no row, no head, + * no indicator. A footnote about them would explain an absence the user + * never perceived — inventing a discrepancy rather than resolving one. + * Same for tasks on issues the current filters or the loaded page exclude. * - * Deliberately not a dashboard: two figures in the header, grouped rows, and - * at most two footnotes that only render when non-zero. + * Deliberately not a dashboard: two figures and grouped rows. */ export function WorkspaceAgentActivityHoverContent({ issues, tasksByIssueId, taskCount, - unlinkedCount, - outOfScopeCount, }: WorkspaceAgentActivityHoverContentProps) { const { t } = useT("issues"); const now = useActivityNow(); const { agentById, runtimeById } = useActivityLookups(); - const notes = ( - <> - {unlinkedCount > 0 && ( -

- {t(($) => $.agent_activity.unlinked_note, { count: unlinkedCount })} -

- )} - {outOfScopeCount > 0 && ( -

- {t(($) => $.agent_activity.out_of_scope_note, { - count: outOfScopeCount, - })} -

- )} - - ); - if (issues.length === 0) { return ( -
-

- {t(($) => $.agent_activity.empty_hover)} -

- {notes} -
+

+ {t(($) => $.agent_activity.empty_hover)} +

); } return (
- {`${t(($) => $.agent_activity.issues_in_progress, { + {`${t(($) => $.agent_activity.issues_count, { count: issues.length, })} · ${t(($) => $.agent_activity.tasks_count, { count: taskCount })}`}
@@ -263,11 +235,6 @@ export function WorkspaceAgentActivityHoverContent({
))} - {(unlinkedCount > 0 || outOfScopeCount > 0) && ( -
- {notes} -
- )} ); } diff --git a/packages/views/agents/components/agent-avatar-stack.tsx b/packages/views/agents/components/agent-avatar-stack.tsx index 7a69b5a7bd9..948f6e667d0 100644 --- a/packages/views/agents/components/agent-avatar-stack.tsx +++ b/packages/views/agents/components/agent-avatar-stack.tsx @@ -20,14 +20,6 @@ interface AgentAvatarStackProps { // signal a queued-only state (no running task) — same heads, weakened // visual. opacity?: "full" | "half"; - // How the tail beyond `max` is signalled. - // - `count` (default): a `+N` chip. Precise, but it reads as a second - // number — next to a count of something else (the working chip counts - // issues, these heads are agents) that lands as a contradiction. - // - `fade`: the last visible head dims instead. Says "there are more" - // without putting a rival number on screen; the exact roster lives in - // the hover card (MUL-4884). - overflow?: "count" | "fade"; className?: string; } @@ -46,7 +38,6 @@ export function AgentAvatarStack({ size = "sm", max = 3, opacity = "full", - overflow: overflowMode = "count", className, }: AgentAvatarStackProps) { const { getActorName, getActorInitials, getActorAvatarUrl } = useActorName(); @@ -54,7 +45,6 @@ export function AgentAvatarStack({ const visible = agentIds.slice(0, max); const overflow = agentIds.length - visible.length; - const fadeTail = overflowMode === "fade" && overflow > 0; const px = AVATAR_SIZE_PX[size]; // 30% overlap reads as "stacked" without obscuring the next avatar's icon. const overlap = Math.round(px * 0.3); @@ -74,12 +64,7 @@ export function AgentAvatarStack({ // Each subsequent head sits negative-margin over the previous so // the stack collapses horizontally instead of growing linearly. style={{ marginLeft: i === 0 ? 0 : -overlap }} - className={cn( - "ring-2 ring-background rounded-full inline-flex", - // Dim the last head when the tail is hidden — the "there are - // more" cue in `fade` mode. - fadeTail && i === visible.length - 1 && "opacity-40", - )} + className="ring-2 ring-background rounded-full inline-flex" > ))} - {overflowMode === "count" && overflow > 0 && ( + {overflow > 0 && ( ({ snapshot: [] as unknown[], - // Captures what the chip hands its two children so a test can assert the - // avatar stack and the hover card without reaching into their internals. + // Captures what the chip hands its children so a test can assert the + // avatar stack, the hover card and the colour tier without reaching into + // their internals. avatarAgentIds: undefined as string[] | undefined, avatarOverflow: undefined as string | undefined, + buttonVariant: undefined as string | undefined, hoverProps: undefined as - | { - issues: readonly Issue[]; - taskCount: number; - unlinkedCount: number; - outOfScopeCount: number; - } + | { issues: readonly Issue[]; taskCount: number } | undefined, })); @@ -49,14 +46,28 @@ vi.mock("../../agents/components/agent-activity-hover-content", () => ({ WorkspaceAgentActivityHoverContent: (props: { issues: readonly Issue[]; taskCount: number; - unlinkedCount: number; - outOfScopeCount: number; }) => { mockState.hoverProps = props; return
{props.taskCount}
; }, })); +// Record the variant the chip picks. The colour tier lives entirely in the +// Button variant now, so this is the assertable surface. +vi.mock("@multica/ui/components/ui/button", async () => { + const actual = + await vi.importActual( + "@multica/ui/components/ui/button", + ); + return { + ...actual, + Button: (props: React.ComponentProps) => { + mockState.buttonVariant = props.variant ?? undefined; + return ; + }, + }; +}); + vi.mock("@tanstack/react-query", async () => { const actual = await vi.importActual( @@ -129,14 +140,15 @@ beforeEach(() => { mockState.snapshot = []; mockState.avatarAgentIds = undefined; mockState.avatarOverflow = undefined; + mockState.buttonVariant = undefined; mockState.hoverProps = undefined; }); describe("WorkspaceAgentWorkingChip", () => { - it("shows the row count the filter produces, not a snapshot re-derivation", () => { + it("counts agents — the same list the avatar stack shows", () => { // The screenshot case from MUL-4884: 4 running tasks, 4 distinct agents, - // 3 issues on screen. The chip says 3 — the number of rows a click - // leaves — and says it with a unit. + // 3 issues on screen. The chip counts the agents, so its number and the + // heads beside it are one unit and cannot contradict each other. mockState.snapshot = [ makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "issue-2" }), @@ -153,27 +165,27 @@ describe("WorkspaceAgentWorkingChip", () => { ); expect( - screen.getByRole("button", { name: "3 issues in progress" }), + screen.getByRole("button", { name: "4 agents working" }), ).toBeTruthy(); - // The stack still shows every distinct agent behind that work... expect(mockState.avatarAgentIds).toEqual([ "agent-1", "agent-2", "agent-3", "agent-4", ]); - // ...but never as a rival "+N" number next to the issue count. The task - // and agent units are explained in the hover card, which Base UI mounts - // only on open — see agent-activity-hover-content.test.tsx. - expect(mockState.avatarOverflow).toBe("fade"); + // Overflow is the component's standard +N badge again: with an + // agent-anchored number, "3 shown + 1 = 4" corroborates the text instead + // of competing with it. + expect(mockState.avatarOverflow).toBeUndefined(); }); - it("counts exactly workingIssues.length even when the filter is on", () => { - // With the filter on, workingIssues IS the rendered list. The chip must - // agree with it and nothing else. + it("counts one agent once, however many issues it is working", () => { + // The accepted trade-off: the number no longer predicts the row count. + // One agent across two issues reads "1 agent working" and opens 2 rows — + // WHO vs WHERE, different questions. mockState.snapshot = [ makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), - makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "issue-2" }), + makeTask({ id: "t-2", agent_id: "agent-1", issue_id: "issue-2" }), ]; renderWithI18n( @@ -185,27 +197,11 @@ describe("WorkspaceAgentWorkingChip", () => { ); expect( - screen.getByRole("button", { name: "2 issues in progress" }), - ).toBeTruthy(); - }); - - it("uses the singular unit for one issue", () => { - mockState.snapshot = [makeTask({ id: "t-1", issue_id: "issue-1" })]; - - renderWithI18n( - {}} - workingIssues={[makeIssue("issue-1")]} - />, - ); - - expect( - screen.getByRole("button", { name: "1 issue in progress" }), + screen.getByRole("button", { name: "1 agent working" }), ).toBeTruthy(); }); - it("shows 0 when nothing is in progress", () => { + it("shows 0 when nothing is running", () => { mockState.snapshot = []; renderWithI18n( @@ -217,16 +213,18 @@ describe("WorkspaceAgentWorkingChip", () => { ); expect( - screen.getByRole("button", { name: "0 issues in progress" }), + screen.getByRole("button", { name: "0 agents working" }), ).toBeTruthy(); - // No heads to show when nothing is running. expect(screen.queryByTestId("agent-avatar-stack")).toBeNull(); }); - // Colour is two-step on purpose: the loud filled state is reserved for - // "this filter is ON". Idle activity is a tint, so the chip reads as a - // quiet tool rather than an alert. - it("keeps the filled brand state for the active filter, not for mere activity", () => { + // Colour is carried by three self-contained Button variants, never by + // brand classes layered over `outline` — layering silently loses to + // outline's `dark:` chain in dark mode (MUL-4884). Asserting the variant + // is what pins that: a className check would pass even when the colour + // never wins the cascade, which is exactly the false confidence that let + // the dark-mode bug through. + it("uses the filled brand variant only while the filter is on", () => { mockState.snapshot = [makeTask({ id: "t-1", issue_id: "issue-1" })]; const { rerender } = renderWithI18n( @@ -237,10 +235,8 @@ describe("WorkspaceAgentWorkingChip", () => { />, ); - // Activity, filter off → a tint, never the fill. - const idle = screen.getByRole("button").className; - expect(idle).toContain("bg-brand/5"); - expect(idle).not.toContain("bg-brand "); + // Activity, filter off → the tint variant. + expect(mockState.buttonVariant).toBe("brandSubtle"); rerender( { />, ); - // Filter on → filled. - expect(screen.getByRole("button").className).toContain("bg-brand "); + expect(mockState.buttonVariant).toBe("brand"); }); - it("stays muted when there is no activity at all", () => { + it("stays a plain control when nothing is running", () => { mockState.snapshot = []; renderWithI18n( @@ -265,15 +260,13 @@ describe("WorkspaceAgentWorkingChip", () => { />, ); - const className = screen.getByRole("button").className; - expect(className).toContain("text-muted-foreground"); - expect(className).not.toContain("bg-brand"); + expect(mockState.buttonVariant).toBe("outline"); }); - it("reports issue-less tasks to the hover card instead of counting them", () => { + it("keeps issue-less tasks out of the agent count", () => { // Chat / autopilot tasks carry issue_id === "" (core/types/agent.ts). - // They used to collapse into one phantom "" issue and inflate the count - // by exactly 1 (MUL-4884). + // Their agents are not working on any issue, so they must not join the + // count or the stack. mockState.snapshot = [ makeTask({ id: "t-1", agent_id: "agent-1", issue_id: "issue-1" }), makeTask({ id: "t-2", agent_id: "agent-2", issue_id: "" }), @@ -289,22 +282,21 @@ describe("WorkspaceAgentWorkingChip", () => { ); expect( - screen.getByRole("button", { name: "1 issue in progress" }), + screen.getByRole("button", { name: "1 agent working" }), ).toBeTruthy(); - // Their agents are not in the stack either: the stack explains the count. - // (They are not dropped — deriveWorkingChipView routes them to the hover - // card's "not counted" note; see the bucket tests below.) expect(mockState.avatarAgentIds).toEqual(["agent-1"]); }); }); describe("deriveWorkingChipView", () => { - it("buckets every running task exactly once", () => { + it("counts only running tasks whose issue is on screen", () => { const view = deriveWorkingChipView( [ makeTask({ id: "t-1", agent_id: "a1", issue_id: "issue-1" }), makeTask({ id: "t-2", agent_id: "a2", issue_id: "issue-1" }), + // chat/autopilot run — no linked issue makeTask({ id: "t-3", agent_id: "a3", issue_id: "" }), + // running, but its issue is filtered out / past the loaded page makeTask({ id: "t-4", agent_id: "a4", issue_id: "issue-offscreen" }), makeTask({ id: "t-5", @@ -316,13 +308,8 @@ describe("deriveWorkingChipView", () => { [makeIssue("issue-1")], ); - expect(view.taskCount).toBe(2); - expect(view.unlinkedCount).toBe(1); - // Running work on an issue the list isn't showing (filtered out, or past - // the 50-per-status page) is disclosed rather than dropped. - expect(view.outOfScopeCount).toBe(1); - // Queued tasks are not "in progress" and land in no bucket. expect(view.agentIds).toEqual(["a1", "a2"]); + expect(view.taskCount).toBe(2); expect(view.tasksByIssueId.get("issue-1")?.map((t) => t.id)).toEqual([ "t-1", "t-2", @@ -338,6 +325,8 @@ describe("deriveWorkingChipView", () => { [makeIssue("issue-1"), makeIssue("issue-2")], ); + // Two issues, two tasks, but one agent — this is the number the chip + // shows. expect(view.agentIds).toEqual(["a1"]); expect(view.taskCount).toBe(2); }); diff --git a/packages/views/issues/components/workspace-agent-working-chip.tsx b/packages/views/issues/components/workspace-agent-working-chip.tsx index 12060b1b3e8..3bc723f50d1 100644 --- a/packages/views/issues/components/workspace-agent-working-chip.tsx +++ b/packages/views/issues/components/workspace-agent-working-chip.tsx @@ -35,15 +35,12 @@ interface WorkspaceAgentWorkingChipProps { export interface WorkingChipView { /** Running tasks on `workingIssues`, keyed by issue id. */ tasksByIssueId: Map; - /** Distinct agents behind the counted work — the avatar stack. */ + /** Distinct agents behind the counted work. This IS the chip's number, + * and the avatar stack renders the same list. */ agentIds: string[]; - /** Total running tasks across the counted issues. */ + /** Total running tasks across the counted issues — the hover card's + * second figure. */ taskCount: number; - /** Running tasks with no linked issue (chat / autopilot). */ - unlinkedCount: number; - /** Running tasks whose issue isn't on screen (filtered out, or past the - * 50-per-status page the list loads). */ - outOfScopeCount: number; } /** @@ -51,15 +48,17 @@ export interface WorkingChipView { * show. Exported for tests: the counting rule is the entire point of this * component, so it is a pure function rather than a hook-bound useMemo. * - * Every running task lands in exactly one bucket: - * - no `issue_id` → unlinked (chat/autopilot; never counted) - * - issue on screen → counted, grouped under that issue - * - issue not on screen → out of scope (filtered out or past the page) + * A running task only counts when its issue is on screen. Two kinds are + * skipped, both silently — they have no visual presence on this page, so + * announcing them would explain an absence the user never noticed: + * - no `issue_id` — chat/autopilot runs, which are not issue work at all + * - issue not on screen — filtered out, or past the page the list loaded * * `issue_id` is an EMPTY STRING for chat/autopilot tasks, not null — see - * packages/core/types/agent.ts. Without the guard those tasks all collapse - * into one `""` bucket and read as a phantom issue, inflating the count by - * exactly one (MUL-4884). Mirrors `deriveIssueSurfaceActivity`. + * packages/core/types/agent.ts. The guard is data correctness, not + * presentation: without it those tasks collapse into one `""` bucket and + * their agents would join the count for work that isn't on any issue + * (MUL-4884). Mirrors `deriveIssueSurfaceActivity`. */ export function deriveWorkingChipView( snapshot: readonly AgentTask[], @@ -70,19 +69,11 @@ export function deriveWorkingChipView( const agentIds: string[] = []; const seenAgents = new Set(); let taskCount = 0; - let unlinkedCount = 0; - let outOfScopeCount = 0; for (const task of snapshot) { if (task.status !== "running") continue; - if (!task.issue_id) { - unlinkedCount += 1; - continue; - } - if (!onScreen.has(task.issue_id)) { - outOfScopeCount += 1; - continue; - } + if (!task.issue_id) continue; + if (!onScreen.has(task.issue_id)) continue; const bucket = tasksByIssueId.get(task.issue_id); if (bucket) bucket.push(task); else tasksByIssueId.set(task.issue_id, [task]); @@ -93,7 +84,7 @@ export function deriveWorkingChipView( } } - return { tasksByIssueId, agentIds, taskCount, unlinkedCount, outOfScopeCount }; + return { tasksByIssueId, agentIds, taskCount }; } /** @@ -102,18 +93,25 @@ export function deriveWorkingChipView( * mid-flight (a previous design hid the chip when no agents were running, * which trapped users in an active-but-invisible filter state). * - * It says one thing: "N issues in progress" — N being exactly the rows you - * get when you click it. One number, one unit, self-verifiable. + * It says one thing: "N agents working". The number counts agents, which is + * exactly what the avatar stack next to it shows — one control, one unit. + * Earlier versions counted issues (or tasks) beside a stack of agent heads, + * and two units sitting side by side is what made the chip read as broken + * no matter which one was "right" (MUL-4884). * - * The avatar stack is ambience ("who's on it"), not a second statistic: it - * carries no `+N`, because a rival number next to the main one is what made - * this chip read as broken (it counted issues while the stack counted - * agents). The precise roster, the task count, and everything the number - * excludes live in the hover card. + * Counting agents also settles the subject: only agents produce a runtime + * signal, so "N agents working" cannot be misread as "N issues someone is + * working on" — human work has no signal here and is not being claimed. * - * Colour is two-step on purpose: idle activity is a whisper of brand, and - * the loud filled state is reserved for "this filter is ON" — the chip - * should read as a quiet tool, not an alert. + * Accepted trade-off: the number no longer predicts the row count of the + * click. One agent on two issues reads "1 agent working" and opens two + * rows. The chip answers WHO, the list answers WHERE — different questions, + * so the two numbers do not compete; the hover card's issue grouping shows + * the mapping on the spot. + * + * Colour is three-tier and each tier is its own Button variant rather than + * classes layered over `outline` — see the `brand` / `brandSubtle` variants + * for why layering silently loses in dark mode. */ export function WorkspaceAgentWorkingChip({ value, @@ -129,27 +127,13 @@ export function WorkspaceAgentWorkingChip({ [snapshot, workingIssues], ); - // The number. Not a re-derivation of "what's running" — the length of the - // list the click produces. - const issueCount = workingIssues.length; - const hasAgents = issueCount > 0; - - // Active (brand-filled) class — must explicitly re-pin text and bg in - // every interactive state. Button's `outline` variant ships - // `hover:text-foreground` + `aria-expanded:bg-muted aria-expanded:text-foreground`, - // which would otherwise repaint the brand chip back to neutral on hover - // and while the HoverCard is open. - const activeClass = value - ? "border-brand bg-brand text-brand-foreground hover:bg-brand/90 hover:text-brand-foreground aria-expanded:bg-brand aria-expanded:text-brand-foreground" - : hasAgents - ? // Idle-with-activity: a brand tint, not a fill. Enough to read as - // "something is happening here" while scanning, quiet enough that - // the filled state still means something. - "border-brand/30 bg-brand/5 text-foreground" - : "text-muted-foreground"; + // The number and the avatar stack are the same list, so they cannot + // disagree. + const agentCount = view.agentIds.length; + const hasAgents = agentCount > 0; - const label = t(($) => $.agent_activity.issues_in_progress, { - count: issueCount, + const label = t(($) => $.agent_activity.chip_agents_working, { + count: agentCount, }); return ( @@ -157,9 +141,13 @@ export function WorkspaceAgentWorkingChip({ {hasAgents && ( - + )} - {issueCount} + {agentCount} {label} } @@ -185,8 +167,6 @@ export function WorkspaceAgentWorkingChip({ issues={workingIssues} tasksByIssueId={view.tasksByIssueId} taskCount={view.taskCount} - unlinkedCount={view.unlinkedCount} - outOfScopeCount={view.outOfScopeCount} /> diff --git a/packages/views/locales/en/issues.json b/packages/views/locales/en/issues.json index 90e6ff2d048..b07cf682b3a 100644 --- a/packages/views/locales/en/issues.json +++ b/packages/views/locales/en/issues.json @@ -359,16 +359,14 @@ "hover_header_queued_other": "{{count}} agents queued", "status_running": "Working", "status_queued": "Queued", - "empty_hover": "No issues in progress right now", - "filter_active_label": "Viewing only issues in progress", - "issues_in_progress_one": "{{count}} issue in progress", - "issues_in_progress_other": "{{count}} issues in progress", + "empty_hover": "No agents working right now", + "filter_active_label": "Viewing issues with agents working", "tasks_count_one": "{{count}} task", "tasks_count_other": "{{count}} tasks", - "unlinked_note_one": "{{count}} more task has no linked issue (chat/autopilot) — not counted", - "unlinked_note_other": "{{count}} more tasks have no linked issue (chat/autopilot) — not counted", - "out_of_scope_note_one": "{{count}} more task is outside the current filters or loaded range — not counted", - "out_of_scope_note_other": "{{count}} more tasks are outside the current filters or loaded range — not counted" + "chip_agents_working_one": "{{count}} agent working", + "chip_agents_working_other": "{{count}} agents working", + "issues_count_one": "{{count}} issue", + "issues_count_other": "{{count}} issues" }, "agent_live": { "is_working": "{{name}} is working", diff --git a/packages/views/locales/ja/issues.json b/packages/views/locales/ja/issues.json index 2d1f2e29b5e..5ef5f63ebef 100644 --- a/packages/views/locales/ja/issues.json +++ b/packages/views/locales/ja/issues.json @@ -343,12 +343,11 @@ "hover_header_queued_other": "待機中のエージェント {{count}} 件", "status_running": "作業中", "status_queued": "待機中", - "empty_hover": "進行中の issue はありません", - "filter_active_label": "進行中の issue のみ表示中", - "issues_in_progress_other": "進行中の issue {{count}} 件", + "empty_hover": "現在作業中のエージェントはありません", + "filter_active_label": "エージェントが作業中の issue を表示中", "tasks_count_other": "task {{count}} 件", - "unlinked_note_other": "issue に紐づかない task が他に {{count}} 件あります（chat/autopilot）。カウントには含まれません", - "out_of_scope_note_other": "現在のフィルターまたは読み込み済み範囲外の task が他に {{count}} 件あります。カウントには含まれません" + "chip_agents_working_other": "作業中のエージェント {{count}} 件", + "issues_count_other": "issue {{count}} 件" }, "agent_live": { "is_working": "{{name}} が作業中", diff --git a/packages/views/locales/ko/issues.json b/packages/views/locales/ko/issues.json index 4b19aa60a26..64661030f5e 100644 --- a/packages/views/locales/ko/issues.json +++ b/packages/views/locales/ko/issues.json @@ -343,12 +343,11 @@ "hover_header_queued_other": "대기 중인 에이전트 {{count}}개", "status_running": "작업 중", "status_queued": "대기 중", - "empty_hover": "진행 중인 issue가 없습니다", - "filter_active_label": "진행 중인 issue만 보는 중", - "issues_in_progress_other": "진행 중인 issue {{count}}개", + "empty_hover": "현재 작업 중인 에이전트가 없습니다", + "filter_active_label": "에이전트가 작업 중인 issue만 보는 중", "tasks_count_other": "task {{count}}개", - "unlinked_note_other": "issue에 연결되지 않은 task {{count}}개가 더 있습니다(chat/autopilot). 개수에 포함되지 않습니다", - "out_of_scope_note_other": "현재 필터 또는 불러온 범위 밖의 task {{count}}개가 더 있습니다. 개수에 포함되지 않습니다" + "chip_agents_working_other": "작업 중인 에이전트 {{count}}개", + "issues_count_other": "issue {{count}}개" }, "agent_live": { "is_working": "{{name}} 작업 중", diff --git a/packages/views/locales/zh-Hans/issues.json b/packages/views/locales/zh-Hans/issues.json index 829cd2799f6..23f73cfd141 100644 --- a/packages/views/locales/zh-Hans/issues.json +++ b/packages/views/locales/zh-Hans/issues.json @@ -343,12 +343,11 @@ "hover_header_queued_other": "{{count}} 个智能体排队中", "status_running": "正在工作", "status_queued": "排队中", - "empty_hover": "当前没有进行中的 issue", - "filter_active_label": "正在查看进行中的 issue", - "issues_in_progress_other": "{{count}} 个 issue 进行中", + "empty_hover": "当前没有智能体在工作", + "filter_active_label": "正在查看有智能体在工作的 issue", "tasks_count_other": "{{count}} 个 task", - "unlinked_note_other": "另有 {{count}} 个 task 未关联 issue（chat/autopilot），不计入", - "out_of_scope_note_other": "另有 {{count}} 个 task 在当前筛选或已加载范围之外，不计入" + "chip_agents_working_other": "{{count}} 个智能体工作中", + "issues_count_other": "{{count}} 个 issue" }, "agent_live": { "is_working": "{{name}} 在工作", From f5d0e590a76fa458dc517736667afb222a01769e Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:33:27 +0800 Subject: [PATCH 4/4] fix(issues): stop the empty-state muted text from overriding the brand tier (MUL-4884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught the colour rule breaking itself, and the test that was supposed to guard it being unable to. `filter ON + 0 agents` is a real state — the filter stays on after the last agent finishes. There the variant is `brand`, and the chip appended `text-muted-foreground` for the empty state regardless of the filter. tailwind-merge keeps the last class in a group and `className` is merged after the variant, so the muted grey WON: grey text on a brand-blue fill. The irony is exact — this change set exists because colour classes in `className` lose to a variant's `dark:` chain, and here one beat the variant instead. Either way the lesson is the same: a tier's colours only ever come from its variant. `chipAppearance` now makes that a rule with one gated exception instead of an inline ternary, and says why. The variant assertion could not catch this: the variant IS `brand` while the text is overridden. That is the same shape of false confidence as the class -string assertion it replaced, so this commit brings the cascade check into the repo instead of leaving it as something a commit message claimed. `apps/web/app/brand-variant-cascade.test.ts` compiles the real globals.css — web's and desktop's, since each defines its own `dark` variant — and resolves what a browser would paint for the merged class strings: filter declarations matching the element's classes and state, then take the winner by specificity, then source order. It asserts both tiers across default / hover / pressed / popover-open x light / dark, and pins the two ways the colour has actually been lost as executable failures: layering brand over `outline` (dark:bg-input/30 wins by specificity) and appending a colour class (wins by merge order). Both new guards were confirmed to fail against the code they guard: reverting the gate fails the chipAppearance test; giving `brand` a dark: rule fails the cascade test in both bundles. Also refreshes the comments still describing the retired issue-anchored invariant ("chip count === row count"). The scope pipeline they describe is unchanged and still load-bearing — it decides WHICH agents count — but the chip's number is agents, so it is no longer this list's length. Co-Authored-By: Claude Opus 4.8 Co-authored-by: multica-agent --- apps/web/app/brand-variant-cascade.test.ts | 280 ++++++++++++++++++ apps/web/package.json | 1 + .../views/issues/components/issues-header.tsx | 4 +- .../workspace-agent-working-chip.test.tsx | 48 ++- .../workspace-agent-working-chip.tsx | 49 ++- .../use-issue-surface-controller.test.tsx | 5 +- .../issues/surface/use-issue-surface-data.ts | 21 +- .../my-issues/components/my-issues-header.tsx | 4 +- pnpm-lock.yaml | 29 +- pnpm-workspace.yaml | 1 + 10 files changed, 396 insertions(+), 46 deletions(-) create mode 100644 apps/web/app/brand-variant-cascade.test.ts diff --git a/apps/web/app/brand-variant-cascade.test.ts b/apps/web/app/brand-variant-cascade.test.ts new file mode 100644 index 00000000000..5f588215f1d --- /dev/null +++ b/apps/web/app/brand-variant-cascade.test.ts @@ -0,0 +1,280 @@ +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import postcss from "postcss"; +import tailwind from "@tailwindcss/postcss"; +import { cn } from "@multica/ui/lib/utils"; +import { buttonVariants } from "@multica/ui/components/ui/button"; +import { beforeAll, describe, expect, it } from "vitest"; + +/** + * Does the brand chip actually render blue? + * + * This lives in apps/web because it tests the COMPILED STYLESHEET, not React + * behaviour: `globals.css` is the app's artifact, and the `dark` variant is + * defined there (`@custom-variant dark (&:is(.dark *))`) — that definition is + * what decides the outcome below. It follows the same pattern as + * font-fallback-order.test.ts, which also asserts on the app's CSS. + * + * Why simulate a cascade instead of asserting class names: a class name in + * the DOM proves nothing about the pixel. Twice on MUL-4884 the brand colour + * was present as a class and still lost: + * + * 1. Brand classes layered over the `outline` variant. tailwind-merge keeps + * `dark:bg-input/30` (different modifier group, no conflict to resolve), + * and `dark:` compiles to `&:is(.dark *)` — specificity (0,2,0) against + * a bare `.bg-brand`'s (0,1,0). The chip was grey in dark mode while + * every string assertion passed. + * 2. A colour class appended via `className` beat the variant that was + * supposed to own the colour (see the merged-class case below). + * + * Both are invisible to a `toContain("bg-brand")` test and both are caught by + * resolving what a browser would actually paint: filter the declarations that + * match the element's classes and state, then take the winner by specificity, + * then source order. + */ + +interface FlatRule { + sel: string; + prop: string; + order: number; +} + +interface ElementState { + dark?: boolean; + hover?: boolean; + active?: boolean; + expanded?: boolean; +} + +const repoRoot = resolve(process.cwd(), "../.."); + +async function compileStylesheet(entry: string): Promise { + const css = readFileSync(entry, "utf8"); + const built = await postcss([tailwind({ base: dirname(entry) })]).process(css, { + from: entry, + }); + + // Flatten Tailwind's nested output. Declarations sit under a rule OR inside + // an at-rule (`hover:` is wrapped in `@media (hover:hover)`, opacity + // modifiers in `@supports`), so collect at both levels and keep document + // order — it is the tie-breaker when specificity ties. + const rules: FlatRule[] = []; + let order = 0; + const walk = (container: postcss.Container, prefix: string) => { + container.each((node) => { + if (node.type === "decl") { + if (prefix) rules.push({ sel: prefix, prop: node.prop, order: order++ }); + } else if (node.type === "rule") { + const sel = node.selector.includes("&") + ? node.selector.replace(/&/g, prefix) + : prefix + ? prefix + node.selector + : node.selector; + walk(node, sel); + } else if (node.type === "atrule") { + walk(node, prefix); + } + }); + }; + walk(built.root, ""); + return rules; +} + +/** Selector specificity's `b` column — classes, attributes, pseudo-classes. + * `:is()` contributes its most specific argument. Nothing here reaches for + * ids or elements, so the other two columns are always 0. */ +function specificity(selector: string): number { + let count = 0; + const withoutIs = selector.replace(/:is\(([^()]*)\)/g, (_, inner: string) => { + count += Math.max( + ...inner.split(",").map((part) => (part.match(/\.[^.\s>+~:[]+/g) ?? []).length), + ); + return ""; + }); + return count + (withoutIs.match(/\\?\.[A-Za-z0-9_\\/:.\-[\]%]+/g) ?? []).length; +} + +// Tailwind escapes `/` and `:` in class selectors (`.bg-brand\/7`), so split +// on the matched (escaped) prefix and unescape only for comparison — slicing +// by the unescaped length silently mis-parses every opacity utility. +const CLASS_PREFIX = /^\.((?:[^.\s:[\\]|\\.)+)/; + +function baseClassOf(selector: string): string | null { + const m = selector.match(CLASS_PREFIX); + return m ? m[1]!.replace(/\\/g, "") : null; +} + +function matches(selector: string, classes: string[], state: ElementState) { + const m = selector.match(CLASS_PREFIX); + if (!m) return false; + const cls = m[1]!.replace(/\\/g, ""); + if (!classes.includes(cls)) return false; + const rest = selector.slice(m[0].length); + if (rest.includes(":is(.dark *)") && !state.dark) return false; + if (rest.includes(":hover") && !state.hover) return false; + if (rest.includes(":active") && !state.active) return false; + if (rest.includes('[aria-expanded="true"]') && !state.expanded) return false; + // Ignore any rule whose selector we do not model, rather than guessing. + const unmodelled = rest.replace( + /:is\(\.dark \*\)|:hover|:active|\[aria-expanded="true"\]/g, + "", + ); + return unmodelled.trim() === ""; +} + +/** The class a browser would let win for `prop` on an element carrying + * `classes` in `state`. */ +function winning( + rules: FlatRule[], + classes: string[], + state: ElementState, + prop: string, +): string | null { + const candidates = rules.filter( + (r) => r.prop === prop && matches(r.sel, classes, state), + ); + if (candidates.length === 0) return null; + candidates.sort( + (a, b) => specificity(a.sel) - specificity(b.sel) || a.order - b.order, + ); + return baseClassOf(candidates.at(-1)!.sel); +} + +const WEB_CSS = resolve(repoRoot, "apps/web/app/globals.css"); +const DESKTOP_CSS = resolve(repoRoot, "apps/desktop/src/renderer/src/globals.css"); + +// The chip's own composition: layout classes only, colour from the variant. +const brand = cn(buttonVariants({ variant: "brand", size: "sm" }), "h-8 px-2"); +const brandSubtle = cn( + buttonVariants({ variant: "brandSubtle", size: "sm" }), + "h-8 px-2", +); + +describe("brand Button variants resolve to brand colour in the real stylesheet", () => { + let rules: FlatRule[]; + + beforeAll(async () => { + rules = await compileStylesheet(WEB_CSS); + }, 60_000); + + const bg = (classes: string, state: ElementState) => + winning(rules, classes.split(/\s+/), state, "background-color"); + const text = (classes: string, state: ElementState) => + winning(rules, classes.split(/\s+/), state, "color"); + + describe("brand (filter ON — the loud filled tier)", () => { + // --brand flips per theme, so one set of rules must serve both. If a + // `dark:` rule from another variant ever creeps in, these diverge. + for (const dark of [false, true]) { + const theme = dark ? "dark" : "light"; + + it(`fills with brand and never with the neutral input token (${theme})`, () => { + expect(bg(brand, { dark })).toBe("bg-brand"); + }); + + it(`deepens one notch on hover, another when pressed (${theme})`, () => { + expect(bg(brand, { dark, hover: true })).toBe("hover:bg-brand/90"); + expect(bg(brand, { dark, hover: true, active: true })).toBe( + "active:bg-brand/85", + ); + }); + + it(`reads as hover, not as a colour change, while the popover is open (${theme})`, () => { + expect(bg(brand, { dark, expanded: true })).toBe( + "aria-expanded:bg-brand/90", + ); + }); + + it(`keeps brand-foreground text (${theme})`, () => { + expect(text(brand, { dark })).toBe("text-brand-foreground"); + }); + } + }); + + describe("brandSubtle (activity, filter OFF — the tint tier)", () => { + // Dark runs one notch hotter: the same alpha reads weaker on a near-black + // surface than on white. + it("uses the light notches in light mode", () => { + expect(bg(brandSubtle, {})).toBe("bg-brand/7"); + expect(bg(brandSubtle, { hover: true })).toBe("hover:bg-brand/12"); + expect(bg(brandSubtle, { hover: true, active: true })).toBe( + "active:bg-brand/16", + ); + expect(bg(brandSubtle, { expanded: true })).toBe("aria-expanded:bg-brand/12"); + }); + + it("uses the hotter dark notches in dark mode", () => { + expect(bg(brandSubtle, { dark: true })).toBe("dark:bg-brand/12"); + expect(bg(brandSubtle, { dark: true, hover: true })).toBe( + "dark:hover:bg-brand/18", + ); + expect(bg(brandSubtle, { dark: true, hover: true, active: true })).toBe( + "dark:active:bg-brand/24", + ); + expect(bg(brandSubtle, { dark: true, expanded: true })).toBe( + "dark:aria-expanded:bg-brand/18", + ); + }); + }); + + // The two ways the brand colour has actually been lost. Both are asserted + // as the FAILURE they are, so the reason for the current shape is executable + // rather than a comment someone can quietly undo. + describe("the mistakes this design prevents", () => { + it("shows why layering brand over `outline` cannot work in dark mode", () => { + const layered = cn( + buttonVariants({ variant: "outline", size: "sm" }), + "border-brand bg-brand text-brand-foreground", + ); + + // tailwind-merge cannot drop outline's dark: rules — different modifier + // group, so there is no conflict for it to resolve... + expect(layered).toContain("dark:bg-input/30"); + // ...and `:is(.dark *)` then outranks the bare .bg-brand. + expect(bg(layered, { dark: true })).toBe("dark:bg-input/30"); + // Light mode looks fine, which is exactly why this shipped unnoticed. + expect(bg(layered, {})).toBe("bg-brand"); + }); + + it("shows why a colour class in `className` beats the variant that owns it", () => { + // `filter ON + 0 agents` used to land here: variant `brand`, muted text + // appended for the empty state → grey text on a brand-blue fill. + const overridden = cn( + buttonVariants({ variant: "brand", size: "sm" }), + "text-muted-foreground", + ); + + expect(text(overridden, {})).toBe("text-muted-foreground"); + expect(text(overridden, { dark: true })).toBe("text-muted-foreground"); + }); + }); +}); + +describe("desktop ships the same brand cascade as web", () => { + let rules: FlatRule[]; + + beforeAll(async () => { + rules = await compileStylesheet(DESKTOP_CSS); + }, 60_000); + + // Desktop declares its own `@custom-variant dark` and its own @source globs, + // so the guarantee has to be re-proved against its bundle rather than + // assumed from web's. + it("fills the brand tier with brand in both themes", () => { + const classes = brand.split(/\s+/); + expect(winning(rules, classes, {}, "background-color")).toBe("bg-brand"); + expect(winning(rules, classes, { dark: true }, "background-color")).toBe( + "bg-brand", + ); + expect(winning(rules, classes, { dark: true }, "color")).toBe( + "text-brand-foreground", + ); + }); + + it("keeps the tint tier's dark notches", () => { + const classes = brandSubtle.split(/\s+/); + expect(winning(rules, classes, { dark: true }, "background-color")).toBe( + "dark:bg-brand/12", + ); + }); +}); diff --git a/apps/web/package.json b/apps/web/package.json index f9a33049b32..2f1b3247ca8 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -75,6 +75,7 @@ "devDependencies": { "@multica/eslint-config": "workspace:*", "@tailwindcss/postcss": "catalog:", + "postcss": "catalog:", "@testing-library/jest-dom": "catalog:", "@testing-library/react": "catalog:", "@testing-library/user-event": "catalog:", diff --git a/packages/views/issues/components/issues-header.tsx b/packages/views/issues/components/issues-header.tsx index 93a4b5574df..610a77bb0c4 100644 --- a/packages/views/issues/components/issues-header.tsx +++ b/packages/views/issues/components/issues-header.tsx @@ -750,8 +750,8 @@ export function IssuesHeader({ isRefreshing = false, }: { scopedIssues: Issue[]; - /** The rows the agents-working filter would leave on screen — the chip's - * count is this list's length, so it matches the post-click list. */ + /** The rows the agents-working filter would leave on screen. Scopes the + * chip: it counts the agents working on these rows. */ workingIssues: Issue[]; allowGantt?: boolean; dateFilter?: IssueDateFilter | null; diff --git a/packages/views/issues/components/workspace-agent-working-chip.test.tsx b/packages/views/issues/components/workspace-agent-working-chip.test.tsx index ff5ae91534e..4043ed1654d 100644 --- a/packages/views/issues/components/workspace-agent-working-chip.test.tsx +++ b/packages/views/issues/components/workspace-agent-working-chip.test.tsx @@ -86,6 +86,7 @@ vi.mock("@tanstack/react-query", async () => { import { WorkspaceAgentWorkingChip, + chipAppearance, deriveWorkingChipView, } from "./workspace-agent-working-chip"; @@ -218,12 +219,9 @@ describe("WorkspaceAgentWorkingChip", () => { expect(screen.queryByTestId("agent-avatar-stack")).toBeNull(); }); - // Colour is carried by three self-contained Button variants, never by - // brand classes layered over `outline` — layering silently loses to - // outline's `dark:` chain in dark mode (MUL-4884). Asserting the variant - // is what pins that: a className check would pass even when the colour - // never wins the cascade, which is exactly the false confidence that let - // the dark-mode bug through. + // Colour is carried by three self-contained Button variants. The tier + // rules are unit-tested against `chipAppearance` below; these two cover + // the wiring. it("uses the filled brand variant only while the filter is on", () => { mockState.snapshot = [makeTask({ id: "t-1", issue_id: "issue-1" })]; @@ -331,3 +329,41 @@ describe("deriveWorkingChipView", () => { expect(view.taskCount).toBe(2); }); }); + +// The chip's colour must come from its Button variant and nothing else. A +// colour class in `className` is appended AFTER the variant, so +// tailwind-merge keeps it and it beats the variant — the opposite of what +// "the variant owns the colour" implies. These pin the tier rules including +// the state that already got this wrong. +describe("chipAppearance", () => { + it("wears the filled brand tier while the filter is on", () => { + expect(chipAppearance(true, true).variant).toBe("brand"); + }); + + it("wears the tint tier for activity without the filter", () => { + expect(chipAppearance(false, true).variant).toBe("brandSubtle"); + }); + + it("wears the plain tier with muted text when nothing is running", () => { + const a = chipAppearance(false, false); + expect(a.variant).toBe("outline"); + // `outline` sets no text colour, so this tier supplies its own. + expect(a.className).toContain("text-muted-foreground"); + }); + + it("does not mute the text when the filter is on with 0 agents", () => { + // Real state: the filter stays on after the last agent finishes. The + // variant is `brand` here, so appending `text-muted-foreground` would + // WIN over the variant's `text-brand-foreground` and paint grey text on + // a brand-blue fill. + const a = chipAppearance(true, false); + expect(a.variant).toBe("brand"); + expect(a.className).not.toContain("text-muted-foreground"); + }); + + it("never carries a colour class for either brand tier", () => { + for (const a of [chipAppearance(true, true), chipAppearance(false, true)]) { + expect(a.className).not.toMatch(/(^|\s)(text|bg|border)-/); + } + }); +}); diff --git a/packages/views/issues/components/workspace-agent-working-chip.tsx b/packages/views/issues/components/workspace-agent-working-chip.tsx index 3bc723f50d1..8ef3a96e3d0 100644 --- a/packages/views/issues/components/workspace-agent-working-chip.tsx +++ b/packages/views/issues/components/workspace-agent-working-chip.tsx @@ -22,13 +22,13 @@ interface WorkspaceAgentWorkingChipProps { value: boolean; onToggle: () => void; // The rows this filter leaves on screen, computed by the surface from the - // same pipeline that renders them (see `workingScopeIssues`). The chip's - // number is this list's length — that is the whole point: the number and - // the click result cannot disagree, because they are the same list. + // same pipeline that renders them (see `workingScopeIssues`). // - // The chip used to take the PRE-filter issue set and count distinct running - // `issue_id`s out of the task snapshot itself. That was a second derivation - // of "what's on screen" and it drifted from the real one (MUL-4884). + // The chip does not count these — it counts the agents working ON them. The + // list decides WHICH agents count: an agent whose only running task sits on + // an issue this filter would hide is not part of the number. Taking scope + // from the render pipeline instead of re-deriving it from the task snapshot + // is what keeps that judgement in step with the list (MUL-4884). workingIssues: readonly Issue[]; } @@ -87,6 +87,31 @@ export function deriveWorkingChipView( return { tasksByIssueId, agentIds, taskCount }; } +/** + * Which colour tier the chip wears, and the only classes allowed alongside + * it. Exported so the tier rules are testable without a DOM. + * + * The tier lives entirely in the Button variant. `className` carries layout + * only — with ONE exception: the idle tier needs muted text, which `outline` + * does not set. That exception is gated on `!value`, and the gate matters: + * `filter ON + 0 agents` is a real state (the filter stays on after the last + * agent finishes), and there the variant is `brand`. Appending + * `text-muted-foreground` there does not lose to the variant — it WINS, because + * tailwind-merge keeps the last class in a group and `className` is appended + * after the variant. The result would be muted grey text on a brand-blue + * fill. Colour classes in `className` are how this component keeps breaking; + * the safe rule is that a tier's colours only ever come from its variant. + */ +export function chipAppearance( + value: boolean, + hasAgents: boolean, +): { variant: "brand" | "brandSubtle" | "outline"; className: string } { + const layout = "h-8 px-2 md:h-7 md:px-2.5"; + if (value) return { variant: "brand", className: layout }; + if (hasAgents) return { variant: "brandSubtle", className: layout }; + return { variant: "outline", className: `${layout} text-muted-foreground` }; +} + /** * Filter chip on the issues / my-issues header, sitting to the left of the * Filter button. Always rendered so the filter toggle never disappears @@ -136,18 +161,18 @@ export function WorkspaceAgentWorkingChip({ count: agentCount, }); + // Three tiers: filter ON is the loud filled state, activity without the + // filter is a tint, nothing running is a plain control. + const appearance = chipAppearance(value, hasAgents); + return ( { expect(result.current.workingScopeIssues.length).toBe(2), ); - // The chip's number IS this list's length, so this identity is the whole - // promise: what it says equals what you get. + // The scope the chip counts agents within must be the rendered list + // itself — that identity is what keeps the chip in step with the filter + // (the chip's own number counts agents, not these rows). expect(result.current.workingScopeIssues.map((i) => i.id)).toEqual( result.current.issues.map((i) => i.id), ); diff --git a/packages/views/issues/surface/use-issue-surface-data.ts b/packages/views/issues/surface/use-issue-surface-data.ts index 8359a64867c..5cb874cb237 100644 --- a/packages/views/issues/surface/use-issue-surface-data.ts +++ b/packages/views/issues/surface/use-issue-surface-data.ts @@ -273,21 +273,22 @@ export function useIssueSurfaceData({ ); // The rows the agents-working filter leaves on screen — i.e. exactly what - // the header chip promises you get when you click it. + // you get when you click the header chip. // // This is deliberately a PROJECTION OF THE RENDER PIPELINE, not a second // pass over the task snapshot: it reuses the same predicates, the same // filter state and the same per-mode source as the rows below, with - // `workingOnly` forced on. That makes "chip count === row count" true by - // construction. The chip used to count distinct running `issue_id`s - // straight from the snapshot against the PRE-filter issue set, so any - // active status/assignee/label filter (or a sub-issue hidden by the - // display toggle) made it disagree with the list it was filtering - // (MUL-4884). + // `workingOnly` forced on. Turning the filter on only adds `workingOnly` to + // this same pipeline, so the set is the post-click list whether the filter + // is currently on or off. // - // Turning the filter on only adds `workingOnly` to this same pipeline, so - // the preview is the post-click list whether the filter is currently on or - // off. + // The chip counts AGENTS, not this list's length, so these are not equal + // (one agent can hold two of these rows). What this set does decide is + // WHICH agents the chip counts — only those working on rows that survive + // the filters. Re-deriving that scope from the snapshot instead is what + // made the chip disagree with the list it was filtering: any active + // status/assignee/label filter, or a sub-issue hidden by the display + // toggle, moved the list but not the chip (MUL-4884). // // Each branch below must take the SAME source the matching branch of // IssueSurface renders: diff --git a/packages/views/my-issues/components/my-issues-header.tsx b/packages/views/my-issues/components/my-issues-header.tsx index 805ae798299..7763c1cb14b 100644 --- a/packages/views/my-issues/components/my-issues-header.tsx +++ b/packages/views/my-issues/components/my-issues-header.tsx @@ -28,8 +28,8 @@ export function MyIssuesHeader({ isRefreshing = false, }: { allIssues: Issue[]; - /** The rows the agents-working filter would leave on screen — the chip's - * count is this list's length, so it matches the post-click list. */ + /** The rows the agents-working filter would leave on screen. Scopes the + * chip: it counts the agents working on these rows. */ workingIssues: Issue[]; scope: MyIssuesScope; onScopeChange: (scope: MyIssuesScope) => void; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 235a1359cff..82fd0602561 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -632,10 +632,10 @@ importers: version: 5.6.0 fumadocs-core: specifier: ^15.5.2 - version: 15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) + version: 15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) fumadocs-mdx: specifier: ^12.0.3 - version: 12.0.3(fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) + version: 12.0.3(fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) input-otp: specifier: ^1.4.2 version: 1.4.2(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -650,7 +650,7 @@ importers: version: 1.0.1(react@19.2.3) next: specifier: ^16.2.5 - version: 16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + version: 16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) next-themes: specifier: ^0.4.6 version: 0.4.6(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -730,6 +730,9 @@ importers: jsdom: specifier: 'catalog:' version: 29.0.1(@noble/hashes@1.8.0) + postcss: + specifier: 8.5.10 + version: 8.5.10 tailwindcss: specifier: 'catalog:' version: 4.2.2 @@ -17497,7 +17500,7 @@ snapshots: transitivePeerDependencies: - supports-color - fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3): + fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3): dependencies: '@formatjs/intl-localematcher': 0.6.2 '@orama/orama': 3.1.18 @@ -17520,7 +17523,7 @@ snapshots: optionalDependencies: '@types/react': 19.2.14 lucide-react: 1.0.1(react@19.2.3) - next: 16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + next: 16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) react: 19.2.3 react-dom: 19.2.3(react@19.2.3) react-router: 7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3) @@ -17553,14 +17556,14 @@ snapshots: transitivePeerDependencies: - supports-color - fumadocs-mdx@12.0.3(fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3): + fumadocs-mdx@12.0.3(fumadocs-core@15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3): dependencies: '@mdx-js/mdx': 3.1.1 '@standard-schema/spec': 1.1.0 chokidar: 4.0.3 esbuild: 0.25.12 estree-util-value-to-estree: 3.5.0 - fumadocs-core: 15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) + fumadocs-core: 15.8.5(@types/react@19.2.14)(lucide-react@1.0.1(react@19.2.3))(next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react-dom@19.2.3(react@19.2.3))(react-router@7.14.0(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) js-yaml: 4.1.1 lru-cache: 11.2.7 mdast-util-to-markdown: 2.1.2 @@ -17573,7 +17576,7 @@ snapshots: unist-util-visit: 5.1.0 zod: 4.3.6 optionalDependencies: - next: 16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) + next: 16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3) react: 19.2.3 transitivePeerDependencies: - supports-color @@ -19605,7 +19608,7 @@ snapshots: postcss: 8.5.10 react: 19.2.3 react-dom: 19.2.3(react@19.2.3) - styled-jsx: 5.1.6(react@19.2.3) + styled-jsx: 5.1.6(@babel/core@7.29.0)(react@19.2.3) optionalDependencies: '@next/swc-darwin-arm64': 15.5.18 '@next/swc-darwin-x64': 15.5.18 @@ -19623,7 +19626,7 @@ snapshots: - '@babel/core' - babel-plugin-macros - next@16.2.6(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3): + next@16.2.6(@babel/core@7.29.0)(@opentelemetry/api@1.9.1)(@playwright/test@1.58.2)(babel-plugin-react-compiler@1.0.0)(react-dom@19.2.3(react@19.2.3))(react@19.2.3): dependencies: '@next/env': 16.2.6 '@swc/helpers': 0.5.15 @@ -19632,7 +19635,7 @@ snapshots: postcss: 8.5.10 react: 19.2.3 react-dom: 19.2.3(react@19.2.3) - styled-jsx: 5.1.6(react@19.2.3) + styled-jsx: 5.1.6(@babel/core@7.29.0)(react@19.2.3) optionalDependencies: '@next/swc-darwin-arm64': 16.2.6 '@next/swc-darwin-x64': 16.2.6 @@ -21574,10 +21577,12 @@ snapshots: dependencies: inline-style-parser: 0.2.7 - styled-jsx@5.1.6(react@19.2.3): + styled-jsx@5.1.6(@babel/core@7.29.0)(react@19.2.3): dependencies: client-only: 0.0.1 react: 19.2.3 + optionalDependencies: + '@babel/core': 7.29.0 styleq@0.1.3: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index aee3d9e6683..9f462b0c89c 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -25,6 +25,7 @@ catalog: # UI & Styling tailwindcss: "^4" "@tailwindcss/postcss": "^4" + postcss: "^8" "@tailwindcss/vite": "^4" tailwind-merge: "^3.4.0" class-variance-authority: "^0.7.1"