mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
feat(web-shell): make Session Workflow dependencies navigable and quiet its chrome (#10938)
* fix(web-shell): keep the auto-sized dialog's gutter on a phone The `auto` size floors the panel at `min-w-[min(100%,560px)]` so a small plan graph does not collapse to a narrow column. tailwind-merge keeps that class alongside the base ceiling `max-w-[calc(100%-2rem)]` — different modifier groups — and below the `sm:` breakpoint min-width wins over max-width. On a 390px viewport the Plan & tasks dialog therefore rendered its rounded, ringed panel flush to both screen edges, while every fixed size (sm/md/lg/xl) kept the 1rem gutter at that same width. Floor the panel at the gutter the ceiling already reserves. * fix(web-shell): bind the DAG lane reservation to the constant that places lanes Lane spacing was stated twice in two languages: `EDGE_LANE_HEIGHT` in TS, which places each layer-spanning return lane at `lane * EDGE_LANE_HEIGHT`, and a literal `9px` in `.dagCanvas`'s bottom padding, which reserves room for them. The two are coupled only through the `--plan-edge-lanes` inline style, so raising the constant alone leaves the reservation short; with enough spanning edges the clamp pins the outer lanes together and layer-spanning edges overlap again — the exact defect the lanes exist to fix. Nothing would have caught it: the lane test mocks `offsetHeight` directly. TS now publishes the pitch as `--plan-edge-lane-height` beside the lane count, and the stylesheet reads it. Also give the graph a phone: three 240px lanes plus two 64px gutters are ~850px, so the canvas scrolled in both axes at once and the horizontal scroll hid the layer the vertical scroll was looking for. Below 720px the lane and gutter narrow. Layer semantics and the rect-measured edge routing are untouched, so this changes what fits, not what is drawn. The new stylesheet guard pins both, in the shape the other `.css.test.ts` files use, since jsdom does not compute the cascade. One test-only addition on the same component: the spanning-edge test fixed the edge's arrival column (`H 876`) but not its arrival row, and nothing else in the file asserted a spanning edge's `endY`. Substituting the return lane's `routeY` for `endY` in the path tail draws the edge ~170px below the target node while every existing assertion still passes. Pinned corner-agnostically. Refs #10866. * test(sdk): pin the skills entry in the transcript compactor keep-set `compactTaskExecutionOutput` keeps an allowlist of `task_execution` fields when it drops the bulky ones, and every neighbouring entry — `executionSummary`, `tokenCount` — is already pinned by the reducer tests. `skills` was not: removing it from the keep-set left the whole suite green, and a session restored from a persisted transcript would silently lose the skill list the live run recorded. Mutation-checked: deleting `'skills'` from the keep-set fails exactly this assertion (1 failed | 327 passed); restored, 328/328. * feat(web-shell): make workflow dependencies navigable, and let the surface state each thing once Two related passes over the Session Workflow inspector and cockpit. ## Dependencies become navigation A step's dependencies rendered as `todo.blockedBy!.join(', ')` — a run of raw Todo ids, in both the graph node and the inspector detail. An id is an address, not a label, so following an edge meant reading `step-3, step-7` and then finding those rows by eye, in a panel whose entire subject is which step waits on which. Each reference is now the step it points at — the step number the list and the graph both already show, plus its title — as a control that selects that step. The dependency list becomes the graph's navigation. Recent activity was sliced to 6 while the count beside it reported the true total, so the panel contradicted itself and the remainder was unreachable. The cap stays as a preview and gains a control that opens the rest. ## The surface states each thing once The panel had accumulated chrome that competed with its content: - Section labels were tracked-out uppercase at 10-11px, so every fixed label shouted over the step title and status it introduced. They are labels, not headlines: sentence case at a muted weight. - The cockpit spent the accent colour on a fixed "Dependency graph" eyebrow above the session title — making the one blue thing on the page the one thing that never changes, while the graph below uses that same accent to mean "this node is selected". The label moves to the subtitle line and the accent is left to mean selection. - Completed-over-total sat in a tinted pill directly above the progress bar that draws the same ratio, beside the status word that names the same state — three surfaces for one fact. The count keeps the tone as ink and drops the pill. - Runtime, tool calls, tokens and artifact kind/status were glued into single strings with middle dots, which wrapped as a unit and read out as one run of text. They are separate facts, so they are separate elements. The identity line keeps the mono face only on the short session id, which is an address the user copies or matches against a daemon log. No token or theme values change; this is the existing system applied more sparingly. Both behaviour changes are mutation-checked: reverting the dependency list to the joined id string, and restoring the silent cap, each fail exactly their new test. Refs #10866. * docs(core): say why the Todo dependency semantics are not behind the flag `todo_write`'s omitted-`blockedBy` preservation, and the `blockedBy` schema text that invites the omission, both reach sessions with Session Workflow disabled. That was a deliberate choice when the behaviour landed, but nothing in the file said so, so it reads as a missed gate — the `hasActivePlan` derivation sat bare and the only nearby comment explains the dangling-edge filter, not the gating. Record the decision and its reasoning: these are plan data-model semantics rather than presentation, and gating them on a visualization switch would give the tool two dialects, storing different plans for the same call depending on whether anyone is looking at the graph. Record the blast radius in the same place, because "reaches every session" overstates it. The merge changes an update only when the previous plan still has an unfinished item, the same id carried a non-empty `blockedBy`, and this call omits it. Plans that never used dependencies are untouched (`undefined?.filter()` is `undefined`), callers that always send the full list are unaffected, and `[]` stays the explicit clear. What does reach every session is the schema text, which is why it is named as widening this branch's exposure rather than as an independent change. Comment-only; no behaviour, no signatures. * feat(web-shell): lead the plan DAG with the step, not its status Read against the three questions a plan graph is opened with — what is running, what is blocked on what, what needs me — the node answered none of them first. **Reading order was inverted.** The node led with a status glyph, then the raw Todo id, then the status word; `nodeContent`, the only element that says what the step *is*, came third. At graph scale a reader scanned `● step-3 Blocked` before "Compare findings". An id is an address and the least useful thing at that scale, so it leaves the node face entirely — it stays in the detail panel, where it is actually copied. Content leads, at 13px/500, behind the step number the inspector list and dependency chips already use, so the three surfaces address a step identically instead of three different ways. **Status was stated three or four times per node** — glyph, status word, border tint, plus an attention badge. It is now stated once, as a 3px left rule, and only when it is worth saying: waiting is the resting state of most nodes in a real plan and carries no rule. The stylesheet already made this argument for `blocked`; extending it consistently is what stops the canvas reading as a wall of tinted boxes and gives the one running node something to stand out against. Attention folds into the same rule and outranks the status tone, because it is the state the reader is being asked to act on. Status is colour, which assistive tech cannot read, so it stays in the accessibility tree as words. **Time was invisible** in an execution trace. The node face now carries the agent count and the summed agent runtime for that step — the "is this alive or hung" signal, which is the first thing asked of a live run. Two facts with real spacing, not one middle-dot-joined string that wraps as a unit and reads out as a single run of text. **Dependencies were stated twice**, as a drawn edge and again as `Depends on step-3, step-7`. The row is now rendered only when no edge states it — above `MAX_RENDERED_PLAN_EDGES` the graph draws none, and there the row is the dependency's only statement, so deleting it outright would have lost information in exactly the case that needs it most. It renders as the same number-and-title chips the inspector uses. The input port dot goes with them: the left edge now carries the status rule, and an incoming edge already terminates in an arrowhead at the node — that arrowhead is the input marker. The output port stays, since outgoing edges leave their source unmarked. Both port dots also drop `--agent-blue-500`; every node with an edge had one, so a permanently blue dot across most of the graph left selection no colour of its own. Edge routing, the layering algorithm, the return lanes and the measure pass are untouched. This is the node face and the colour rule. Three geometry tests identified a node by its first `<span>` — the Todo id. That is now the accessible status word, so they bind to `data-plan-node-id` instead, which is the stable hook and was always the better one. Every behavioural change is mutation-checked: ungating the dependency row, deleting the accessible status word, and dropping the elapsed each fail exactly their own assertion. Design plan, including the two revisions the review pass forced, is in docs/plans/2026-09-04-session-workflow-dag-design.md. Refs #10866. * fix(web-shell): drop the unused dependencyIdsByTodo destructure ESLint flags the graph destructure binding as never used; the memo still computes and returns the map for other consumers. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-conflict/jmtlr9x6ugb * fix(web-shell): stop the activity grid from swallowing the show-all control Round-1 review findings. **Blocker.** `.showAllActivity` was a single class (0,1,0), but the button is a direct child of `.activityList`, whose base rule `.activityList > button` (0,1,1) declares the four-column activity grid and wins. The label — a bare text run that Grid wraps into an anonymous item — auto-placed into the 52px first column and wrapped to three or four lines with three empty columns beside it, and the intended padding lost too. Hover only looked right by coincidence, because the row's own hover set the same background. This is the collision handled two rules earlier for `.metrics`, missed here. Both are now pinned by the stylesheet guard so neither can regress to a bare class, and the fix resets `display` and `grid-template-columns` explicitly rather than relying on source order. **The graph's step-details panel still joined raw ids.** The node face's chips cannot be controls — that row lives inside the node's own `<button>` and a nested button is invalid HTML — but this panel sits outside it, so its references become real controls that select the step they name, which is what makes a dependency list navigation rather than text. `Depends on` and `Unblocks` both. **The narrow-viewport claim was wider than the fix.** Three 168px lanes plus two 32px gutters is ~588px, which removes horizontal scrolling between there and the 720px breakpoint but still scrolls on a 390px phone. Added a second step below 480px that gets a three-layer plan under a phone viewport, and said plainly in the comment that deeper plans still scroll — reusing the flat layout stays with #10866. The guard test is renamed to what it pins: jsdom computes no layout, so it can assert the two steps exist and narrow, not an outcome. **The lane pitch had a second copy.** The `var()` fallback restated `9px` next to the `.dagCanvas` default, so "TS owns the pitch" held only when the graph has dependencies. The default is now `0px` — the zero-lane case collapses the reservation regardless — and the guard rejects the fallback copy too. **Comment antecedent.** "the prompt change" in todoWrite named nothing a later reader could find; it now quotes the schema description it means and says why a tool schema reaches every session. Refs #10866. * fix(web-shell): keep the node's non-colour status channel, and stop clipping its port Two defects from my own review of the DAG pass, both in the class the round-1 review already caught once: invisible to jsdom, invisible to a passing unit run, and only findable by reading the stylesheet. **The status glyph was an accessibility channel, not decoration.** Making the left rule the single carrier of status dropped the glyph — but the constant it came from says exactly why it exists: "Status is carried by a glyph as well as a colour so the graph survives colour-blindness, high-contrast mode, and a greyscale screenshot." Colour plus a screen-reader-only word leaves a sighted colour-blind reader, high-contrast mode, and any greyscale capture unable to tell running from completed from needs-attention. "One status, one place" was the wrong rule; the right one is one carrier of the status *colour*, plus a redundant non-colour channel. The glyph returns for every status, in the meta line so the step's content still leads, and muted so the rule stays the only thing carrying the tone. Pinned per status, and pinned as distinct from each other, so the shape channel cannot silently collapse. **The status rule clipped the outgoing port.** Drawing it as `.node::before` needed `overflow: hidden` for the node's 8px radius to apply to it — which also clips the port dot at `right: -4px`, which is deliberately outside the box. It is a `border-left` instead: border-radius clips it natively, no overflow, and a transparent rule paints the node's own background so a resting node reads as 3px of extra left padding with no width difference between states. The stylesheet guard now rejects the overflow, the pseudo-element and a missing port offset together, since the three only fail as a set. Also drops `.nodeGlyph`'s dead sibling state: the rule is back in use, so nothing here is unused CSS. The design plan records both as corrections rather than folding them in, so a later reader sees which of its rules were wrong and why. Refs #10866. * fix(web-shell): put dependency truncation where it can actually fire Round-2 review findings that survive at this head; the round's three blockers (the node's clipped port, the show-all specificity collision, and the step-details panel's raw ids) were already fixed in the two commits after the head it reviewed. **The chips' `text-overflow: ellipsis` was dead.** It was declared on the `inline-flex` chip itself, but `text-overflow` applies to a block container's own inline content — the title sat in an anonymous flex item, so a long dependency title was clipped hard with no ellipsis. The title now has its own span carrying `overflow`, `min-width: 0` and the ellipsis, in all three places that render a reference (the node face's read-only chips and both directions of the step-details panel's links), and the stylesheet guard pins that it lives on the title rather than the box. **The comment above `.node` still described the old status behaviour** — it said status drives the accent that drives the border, which this branch stopped doing two commits ago. Rewritten to describe the border/rule split, keeping the hazard it originally recorded: with status off the border, a selected completed or blocked node can no longer pick up a second border in a clashing hue. **The compaction test's comment overclaimed.** It said every other keep-set entry is pinned by that test; it pins `skills`. Narrowed to what it does. Two coverage gaps the same round named: - The inspector test exercised only the upstream direction. Upstream and downstream render through one helper but read from different sources (`blockedBy` versus the projection's `dependentsByTodo`), so covering one did not cover the other; the downstream link is now rendered, asserted and clicked. - `planExecution.agentCount`'s singular and plural branches are separate formatter paths, and only the singular one was exercised — `1 agents` would have shipped green. Both are now pinned. Refs #10866. * fix(web-shell): drop ghost dependency controls and mark them interactive The step-details dependency buttons mapped raw selectedTodo.blockedBy, so an unresolvable step id rendered as a clickable "? <id>" control; clicking it selected a ghost id, emptied selectedTodo, and hid the details panel mid-navigation. Render from the same filtered projection the edges draw from (dependencyIdsByTodo: unknown ids, duplicates, and self-references dropped) and hide the section when nothing resolvable remains. Also add the data-plan-interactive marker to both step-details dependency button groups. The host keyboard handlers in ToolApproval and TasksStatusMessage isolate plan controls through that marker, and these were the only interactive controls in the view without it. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtmourmvi4 * fix(web-shell): let dependency titles keep their colour, and attention its ring The muted `> span` rules match every direct-child span of the chip and link — now including the title span the truncation fix added — so every dependency title rendered muted while the chip's own `color: var(--foreground)` applied to no rendered text. The title now states its own colour on the title rule, where two classes outspecific the one-class-plus-element mute. The attention rule sat before the blocked/ready rule at equal specificity, so a blocked node that also needs attention wore the muted ring instead of the attention tone. Attention is declared after the status tones so it outranks them, as its comment says. Both shapes are pinned at the source in PlanExecutionView.css.test.ts. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtn1pqbuir * fix(web-shell): dedup repeated blockedBy ids before rendering references The node-face chip row and the inspector's upstream list mapped raw model-authored blockedBy, so a repeated id emitted duplicate React keys and a duplicate chip or link. Every other blockedBy consumer already dedups with new Set, and the step-details panel reads the deduped topology projection; both surfaces now match. The dense-plan test feeds a repeated id and pins the single chip, and a new inspector test pins the single upstream link. Also pin the styles.dependencyTitle wiring on the chip and link title spans in the existing render tests: the truncation rule targets that class, so dropping the className would re-clip titles without an ellipsis while every text assertion stayed green. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtn1pqbuir * fix(web-shell): state the 480px step's true plan width in the comment The comment claimed the 480px step gets a three-layer plan under a phone viewport, but its own pinned values sum to ~396px (3×116px lanes, 2×18px gaps, 2×6px canvas padding) — still ~6px wider than the 390px phone the sibling comment names as the reference. Restate the comment with the true figure: fits phones wider than ~396px, a 390px phone still scrolls the last ~6px. Comment only; the pinned values and the stylesheet guard are unchanged. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8 * docs(plans): mark the session-workflow DAG design implemented, resolve its open questions The doc shipped in this PR declared "Status: proposal, not implemented" while the same PR ships the implementation, and left two open questions the code had already answered. Mark it implemented and record the decisions where they were made: the elapsed count renders on the node face (meta line, when greater than zero), and attention folds into the left rule via data-attention, declared after every status tone so it wins the cascade. The verification note stays open — it is about process, not a design decision. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8 * test(web-shell): pin the declarations the plan guards actually rest on The truncation guard now pins `overflow: hidden` beside `text-overflow: ellipsis` — the former is what makes the latter take effect, and `overflow` is not inherited from the chip. The two phone-width steps pin the lane width on its owner selector and add the paired gap/padding narrowing the fit rests on; dropping any of them restored the clipped-title or two-axis-scroll defect while the suite stayed green. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtngpuscjl * fix(web-shell): drop self-blocks in the inspector, keep status words beside attention The inspector's upstream dependency filter kept self-references: a todo listing its own id in blockedBy rendered a "Blocked by" chip pointing at itself, while the projection's dependentsByTodo builder and the DAG both drop self-blocks. Apply the same rule so the inspector agrees with every sibling surface. The node face's sr-only span also announced attention instead of the status word; an attention node lost its status from the accessibility tree while the aria-hidden glyph still carried it for sighted users. Announce both: the status word first, attention appended. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtnn5c4cjw * test(web-shell): witness the downstream click, attention word and cascade order - click the downstream 'Unblocks' reference in the step-details panel and assert the named node gains aria-pressed, so a dropped onClick in the selectedDependents block turns red - render a node into attention and assert its sr-only span carries the status word beside "Needs attention"; pin the inspector's self-block exclusion beside the dedup test - extend the cascade-order guard to all six status selectors, not just blocked, so regrouping any status tone below the attention rule fails Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtnn5c4cjw * test(web-shell): pin the plural agent count and the attention attribute - link two root Agent executions to one node and assert the face renders '2 agents', so an EN template that always emits `${count} agent` turns red; the count-1 side stays pinned by the existing assertion pair - assert the attention fixture's article carries data-attention='true', the hook the `.node[data-attention='true']` colour rule selects on; the 'Needs attention' word was pinned at18f9fa07, the attribute was not Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtntktgsk8 * test(web-shell): clear every .node-subject rule of overflow clipping The unclipped-port guard only inspected the bare `.node {}` rule, so an `overflow: hidden` on any of the other `.node`-targeting rules — a status variant, attention, selection, the `:has()` state, the media-scoped copies — would still cut the `right: -4px` output port and ship green. Walk every rule block (at-rule preludes unwrapped) and assert no selector whose subject is `.node` carries overflow clipping; descendant-subject rules and the legitimate chip/title clipping stay out of scope. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtntktgsk8 * docs(plans): state the shipped color mappings in the Color table The table claimed `--border` for every dependency line, `--status-running-fg` for running "and nothing else", and `--status-attention-fg` for needs attention "and nothing else". The shipped stylesheet draws edges with `--muted-foreground` and `--agent-blue-500` arrowheads, dresses in_progress in the running token, and dresses paused plus the step-details chips in the attention token. Record the mappings as shipped so a later palette retune reads the exclusivity clauses as they actually hold. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtntktgsk8 * fix(web-shell): close the cockpit review's rendering and a11y blockers (#10938) - Keep selection off the status rule: re-pin border-left-color to --node-rule in .node[data-selected='true'] so selecting a blocked/ready node no longer paints a 3px muted status bar. - Carry data-plan-interactive on the inspector's dependency links and the show-all-activity button, so the transcript's window-level keydown handlers early-return while they hold focus. - Let .dependencyList > li shrink (min-width: 0) so the dependency chip title's ellipsis actually engages instead of overflowing the column. - State dependencies as text whenever neither the aria-hidden edges nor the step-details panel can: above the edge budget, in the cockpit (showStepDetails={false}) and in document mode; drop self-references from the node-face chips like the topology builder does. - Give attention a shape channel: a muted visible sigil in the node meta row, since on paused nodes the data-attention rule re-declares the very token paused already wears and colour alone cannot tell them apart. - Count agents, not bare executions, in the node-face tally (nested subagents included; non-agent todo_id tools hidden), matching the node's own execution rows and the inspector's Subagents list. - Scope the visuals spec's runtime gate to the node face and the inspector detail separately; the page-wide locator matched both and threw a Playwright strict-mode violation. * fix(web-shell): keep plan arrowheads forward and name blockers (#10938) Two regressions wenshao measured in Chromium at this head, both introduced by this diff. R6-3: the 32px and 18px gutters are narrower than the edge router s fixed 24px shoulder, so the control point landed on the end point (32px tier, end tangent 0,0) or past it (18px tier, end tangent -14,0). orient=auto then flips the arrowhead back at its source, and the input port is gone, so the last direction cue pointed the wrong way at phone widths. The shoulder is now half the run: every gutter wide enough to afford the 24px floor already resolved to run / 2, so wide lanes keep their exact curve while the end tangent stays strictly positive at all three tiers. R7-3: with edges drawn and the details panel on, the node s accessible name stopped naming its blockers, because drawn edges are aria-hidden and the visible chip row is off there by design. A screen reader user had to activate every node to learn what blocks it. An sr-only summary on the same channel as the status word restores the words, using the chips step-number-plus-title labels, without bringing the visible row back. Evidence is unit level, not a browser capture: the geometry test reproduces wenshao s measured tangents exactly (0 at the 32px gutter, -14 at the 18px gutter) and goes red when the floor is restored; the accessible-name test goes red when the sr-only summary is dropped. vitest run PlanExecutionView + workflow: 9 files, 84 tests (was 81). Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtzb8sj51m * fix(web-shell): count node-face agents from the same sources the rows render R8-2: agentCount read only transcript subTools, while renderExecution also renders live child tasks from the task index, so a live child with no transcript entry rendered a row the tally never counted. Count the same deduped union the rows come from. Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): keep layer-skipping plan lanes out of the step they pass (#10938) wenshao's round-2 verification names two merge conditions on5f70a13. At <=480px the router gave every layer-skipping edge a fixed 24px shoulder. The 18px gutter leaves a 10px run, so the vertical segment landed inside the intervening lane: a dependency from step 3 to step 5 read as one into step 4, and the SVG paints under the nodes so the lane just disappeared into it. Derive each shoulder from the gutter measured on that side, the way the adjacent-layer edge already does, and halve the corner radius so the run into the arrowhead keeps a positive length. The 64px desktop tier has a 56px run, so min(24, 28) is still 24 and every desktop path stays byte-identical; the new witness pins the exact desktop path string. The node-face agent tally dedups by toolUseId exactly as the rows do, but nothing exercised the overlap. Add a fixture where a live child task and a transcript subTool share one toolUseId and assert it counts once (2 agents beside 2 rows); dropping the filter reads 3 agents. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtzjtfna1y * ci(web-shell): re-fire the head CI lanes that never scheduled The Qwen Code CI, Web-shell Visuals and review-lane runs created ford91a0f7eat 2026-09-13T08:50:29Z never scheduled a single job: a repo-wide Actions queue jam in that window left 59 runs stuck at status=queued with zero jobs, and both cancel and rerun are refused on them (cancel reports 'completed', rerun reports 'already running'). Runs created after ~09:30Z schedule normally, so these are dead rather than slow. No content change. This empty commit only re-fires pull_request:synchronize so the lanes actually run on the exact tree wenshao validated in round 3 ('merge once CI has actually run green on this head'). Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmtzq8wyy28 --------- Co-authored-by: yiliang114 <[email protected]> Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: yiliang114 <[email protected]> Co-authored-by: qwen-code-dev-bot <[email protected]>
This commit is contained in:
13 files changed
+2240
-111
No files matched your search
@@ -0,0 +1,216 @@
|
||||
# Session Workflow DAG — design plan
|
||||
|
||||
Status: implemented by this change. Written before code, reviewed against the
|
||||
brief once, revised. The revision — and the two corrections found during
|
||||
implementation — are recorded at the bottom rather than silently folded in.
|
||||
|
||||
## Brief
|
||||
|
||||
An execution trace of a dependency-ordered plan, watched live while an agent
|
||||
works through it. Reader: a developer. The three questions they arrive with,
|
||||
in order: **what is running now**, **what is blocked on what**, **what needs
|
||||
me**.
|
||||
|
||||
This is a product surface, not a greenfield page: it inherits the Web Shell
|
||||
token system, light/dark, CSS modules, and a stylesheet-source test
|
||||
convention. So the palette is not up for invention — the plan spends its
|
||||
freedom on structure and on the _rule_ governing existing tokens.
|
||||
|
||||
## What is wrong today
|
||||
|
||||
Read against the three questions, not as taste:
|
||||
|
||||
1. **Reading order is inverted.** The node leads with a status glyph, then the
|
||||
raw Todo id, then the status word; `nodeContent` — the only element that
|
||||
says what the step _is_ — is third. At graph scale the reader scans
|
||||
`● step-3 Blocked` before "Compare findings". An id is an address, and it
|
||||
is the least useful thing at the largest scale.
|
||||
2. **Status is stated three or four times per node**: the glyph, the status
|
||||
word, the border tint, and an "attention" badge when it applies. On a 240px
|
||||
node, most of the visible elements are about status.
|
||||
3. **Dependencies are stated twice** — as a drawn edge, and again as
|
||||
"Depends on step-3, step-7" in raw ids.
|
||||
4. **Every node looks equally important.** In a real trace at most one node is
|
||||
running and a few need attention; the rest are done or waiting. Same border
|
||||
weight, same fill, same radius on all of them — the graph does not answer
|
||||
"where do I look".
|
||||
5. **Time is invisible.** This is an execution trace, and the layout encodes
|
||||
dependency order but never elapsed time.
|
||||
6. **The port dots are always `--agent-blue-500`** — the selection colour —
|
||||
on every node that has edges. The accent is spent on something that never
|
||||
changes, so selection has to shout over it.
|
||||
|
||||
## Color
|
||||
|
||||
No new values; a rule for the existing tokens.
|
||||
|
||||
| role | token | used for |
|
||||
| ----- | ----------------------- | ------------------------------------------------------------------------------------------ |
|
||||
| ink | `--foreground` | step content — the only full-contrast text |
|
||||
| quiet | `--muted-foreground` | counts, elapsed, ids |
|
||||
| rule | `--border` | node borders; dependency lines are `--muted-foreground` with `--agent-blue-500` arrowheads |
|
||||
| live | `--status-running-fg` | running and in_progress |
|
||||
| done | `--status-done-fg` | completed, and nothing else |
|
||||
| wait | `--status-attention-fg` | needs attention, and paused (plus the step-details chips) |
|
||||
| pick | `--agent-blue-500` | selection and focus only, never decoration |
|
||||
|
||||
**The rule: a node carries at most one status colour, in exactly one place,
|
||||
and only when the status is worth saying.** Waiting/blocked is the resting
|
||||
state of most nodes in any real plan, so it carries none — the stylesheet
|
||||
already makes this argument for `blocked`; this extends it consistently
|
||||
instead of applying it to one status.
|
||||
|
||||
## Type
|
||||
|
||||
One family (the app's). Three sizes inside a node:
|
||||
|
||||
- **content** 13px / 500 — promoted to the first line
|
||||
- **meta** 11px / 400 quiet — one line beneath
|
||||
- **number** 11px tabular — the step's index
|
||||
|
||||
No status word on the node face in the default case; the left rule says it.
|
||||
No mono on the node face — the id is an address, so it belongs where it is
|
||||
copied (the detail panel), not on every card at graph scale.
|
||||
|
||||
## Layout
|
||||
|
||||
Two candidates, then the pick.
|
||||
|
||||
**A — content-first, status as a left rule**
|
||||
|
||||
```
|
||||
┌──────────────────────────────┐
|
||||
│▌ 3 Compare findings │ ▌ 3px status rule, absent when waiting
|
||||
│ 2 agents 1m14s │
|
||||
└──────────────────────────────┘
|
||||
```
|
||||
|
||||
**B — status as a filled number column**
|
||||
|
||||
```
|
||||
┌───┬──────────────────────────┐
|
||||
│ 3 │ Compare findings │ number cell filled with the status tone
|
||||
│ │ 2 agents 1m14s │
|
||||
└───┴──────────────────────────┘
|
||||
```
|
||||
|
||||
**Pick: A, carrying B's step number.** The number has to appear on the node
|
||||
because the inspector list and the new dependency chips both address steps by
|
||||
number — three surfaces, one identity, or they disagree. But B spends ~24px
|
||||
of a 240px lane on a cell whose only other job is a tone the rule already
|
||||
carries.
|
||||
|
||||
Alignment is left throughout. The lanes are already a strong vertical rhythm;
|
||||
centring anything fights it.
|
||||
|
||||
The node's border stays `rule` for every status. Removing the per-status
|
||||
border tint is what stops the canvas reading as a wall of tinted boxes and
|
||||
lets the one running node actually stand out.
|
||||
|
||||
## Where the boldness goes
|
||||
|
||||
One place: **the running node.** Usually exactly one node is running, and
|
||||
"is this thing alive or hung" is the first question a developer asks of a live
|
||||
trace. It gets the only saturated treatment on the canvas — a filled left rule
|
||||
and a live elapsed count — and everything around it stays hairline and quiet.
|
||||
|
||||
Motion: none added. The elapsed count is data changing, not an animation; no
|
||||
transition on it. The existing `prefers-reduced-motion` block stands.
|
||||
|
||||
## Principles
|
||||
|
||||
1. The step's content is the node; everything else is annotation.
|
||||
2. One status, one place, and only when it is worth saying.
|
||||
3. The edge is the dependency statement — do not restate it as text.
|
||||
4. The step number is one identity across graph, list, and dependency chip.
|
||||
5. Exactly one thing on the canvas is loud: what is running now.
|
||||
|
||||
## Explicitly not changing
|
||||
|
||||
Edge routing, the layering algorithm, the return-lane mechanics, and the
|
||||
measure pass. They work, they are heavily tested, and touching them would turn
|
||||
a node-face redesign into a second oversized PR. This proposal is the node
|
||||
face plus the colour rule.
|
||||
|
||||
## Review against the brief
|
||||
|
||||
Worked through the generic-default calibration; two findings, one of which
|
||||
changed the plan.
|
||||
|
||||
- Cream/serif/terracotta, or near-black with an acid accent: not applicable,
|
||||
the tokens are the product's.
|
||||
- Hairline rules everywhere: I _am_ moving toward hairlines, which is itself a
|
||||
default look. Kept, but for a subject-specific reason — the hairline
|
||||
replaces a per-status border tint, and the argument is the one the codebase
|
||||
already makes for `blocked`: most nodes in a healthy plan are resting, and
|
||||
tinting them all makes a healthy graph read as a wall of warnings. The 8px
|
||||
radius stays, so this is not a broadsheet.
|
||||
- The SaaS-card kit — identical rounded cards, one border treatment, a status
|
||||
wash on each: this is what the node is today. Dropping the status border and
|
||||
two of the three status tokens moves away from it.
|
||||
- **Numbered markers (01 / 02 / 03):** the calibration warns against these
|
||||
unless the content is genuinely a sequence. Here it is — a topologically
|
||||
ordered plan — and the number is already how the inspector addresses steps.
|
||||
Kept.
|
||||
- **`·`-joined meta strings — caught, and the plan changed.** The first draft
|
||||
of the node meta line read `step-3 · 2 agents · 1m14s`. That is on the tell
|
||||
list, and it is the exact pattern just removed from the inspector. Revised
|
||||
to two facts with real spacing and no id on the node face, matching the
|
||||
`.metrics` flex row already built. This also buys back horizontal room in a
|
||||
240px lane, which is what makes the narrow-viewport case work honestly
|
||||
rather than by shrinking type.
|
||||
|
||||
## Caveat found while planning
|
||||
|
||||
The "Depends on …" text row cannot simply be deleted. There are three layout
|
||||
states, not two:
|
||||
|
||||
- `hasDependencies === false` → flat layout, no `blockedBy` exists anywhere,
|
||||
so nothing to state. Safe.
|
||||
- `drawsDependencyEdges === true` → edges are drawn, the text is redundant.
|
||||
Remove.
|
||||
- `hasDependencies && !drawsDependencyEdges` → dependencies exist but exceed
|
||||
`MAX_RENDERED_PLAN_EDGES` (500), so **no edges are drawn**. Here the text is
|
||||
the only statement of the dependency and must stay.
|
||||
|
||||
So the row is kept and gated on `!drawsDependencyEdges`, rendered as the same
|
||||
number-and-title chips the inspector uses rather than as raw ids.
|
||||
|
||||
## Corrections found after the plan, during implementation
|
||||
|
||||
Recorded rather than folded in, because both are places the plan's own rule
|
||||
was wrong and a later reader should see why.
|
||||
|
||||
1. **"One status, one place" was too strong — it deleted an accessibility
|
||||
channel.** The plan replaced the status glyph with a colour-only left rule.
|
||||
But the glyph carried a documented purpose the plan never checked
|
||||
(`PlanExecutionView.tsx`: "Status is carried by a glyph as well as a colour
|
||||
so the graph survives colour-blindness, high-contrast mode, and a greyscale
|
||||
screenshot"). Colour plus a screen-reader-only word leaves a sighted
|
||||
colour-blind reader, high-contrast mode, and any greyscale capture with no
|
||||
way to tell running from completed. The rule is corrected to: one carrier
|
||||
of the status _colour_, plus a redundant non-colour channel. The glyph
|
||||
returns for every status, in the meta line so the content still leads, and
|
||||
muted so the rule stays the only thing carrying the tone.
|
||||
|
||||
2. **A pseudo-element rule needed clipping, and clipping cut the port.**
|
||||
Drawing the rule as `.node::before` required `overflow: hidden` for the 8px
|
||||
radius to apply — which also clips the outgoing port dot, deliberately
|
||||
positioned at `right: -4px` outside the box. The rule is a `border-left`
|
||||
instead, which border-radius clips natively and needs no overflow. Both
|
||||
halves are now pinned by the stylesheet guard, since neither is visible to
|
||||
jsdom or to a passing unit run.
|
||||
|
||||
## Open questions
|
||||
|
||||
The first two were answered by the implementation; the answers are recorded
|
||||
here so the decisions stay visible.
|
||||
|
||||
1. Does the elapsed count on a running node belong on the node face, or only
|
||||
in the inspector? — On the face: the meta line renders it whenever it is
|
||||
greater than zero.
|
||||
2. Should `attention` keep a badge, or fold into the left rule as the `wait`
|
||||
tone? — Folded into the rule: `data-attention` carries it, declared after
|
||||
every status tone so it wins the cascade.
|
||||
3. Verification: this box cannot run a browser, so the visual pass would ship
|
||||
read from CSS. Worth a real render before merge.
|
||||
@@ -0,0 +1,205 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
// jsdom does not compute the CSS cascade, so pin the stylesheet's source
|
||||
// shape instead. The DAG reserves bottom padding for its layer-spanning
|
||||
// return lanes, and that reservation has to use the same pitch that places
|
||||
// the lanes: TS owns `EDGE_LANE_HEIGHT` and the canvas placed lanes at
|
||||
// `lane * EDGE_LANE_HEIGHT`, while the stylesheet re-stated the pitch as a
|
||||
// literal `9px`. Raising the constant alone left the reservation short, the
|
||||
// clamp pinned the outer lanes together, and layer-spanning edges overlapped
|
||||
// again — the exact defect the lanes exist to fix — with no test signal,
|
||||
// because the lane test mocks `offsetHeight` directly. Strip comments so the
|
||||
// guards match declarations only, not prose about them.
|
||||
const planCss = readFileSync(
|
||||
fileURLToPath(new URL('./PlanExecutionView.module.css', import.meta.url)),
|
||||
'utf8',
|
||||
).replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
const planSource = readFileSync(
|
||||
fileURLToPath(new URL('./PlanExecutionView.tsx', import.meta.url)),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
describe('PlanExecutionView stylesheet', () => {
|
||||
it('takes the edge-lane pitch from TS instead of restating it', () => {
|
||||
const canvas = planCss.match(/\.dagCanvas\s*\{[^}]*\}/)?.[0];
|
||||
expect(canvas).toBeTruthy();
|
||||
expect(canvas).toMatch(/var\(--plan-edge-lane-height/);
|
||||
// The literal that used to stand in for the constant must not return
|
||||
// anywhere in the reservation.
|
||||
expect(canvas).not.toMatch(/--plan-edge-lanes\)\s*\*\s*9px/);
|
||||
// No second copy of the constant as a var() fallback either: the pitch
|
||||
// has one source, and the default only covers the zero-lane case.
|
||||
expect(canvas).not.toMatch(/--plan-edge-lane-height,\s*9px/);
|
||||
// TS has to actually publish it, or the fallback silently takes over.
|
||||
expect(planSource).toMatch(
|
||||
/'--plan-edge-lane-height':\s*`\$\{EDGE_LANE_HEIGHT\}px`/,
|
||||
);
|
||||
});
|
||||
|
||||
// The output port is positioned at `right: -4px`, deliberately outside the
|
||||
// node's box, so the node must not clip its overflow — and the status rule
|
||||
// must therefore be a border, which border-radius clips natively, rather
|
||||
// than a pseudo-element that would need `overflow: hidden` to be rounded.
|
||||
// Neither half of that is visible to jsdom or to a green unit run.
|
||||
it('keeps the node unclipped so its outgoing port survives', () => {
|
||||
const node = planCss.match(/(^|\n)\.node\s*\{[^}]*\}/)?.[0];
|
||||
expect(node).toBeTruthy();
|
||||
expect(node).not.toMatch(/overflow:\s*hidden/);
|
||||
expect(node).toMatch(/border-left:\s*3px solid var\(--node-rule\)/);
|
||||
expect(planCss).not.toMatch(/\.node::before\s*\{/);
|
||||
expect(planCss).toMatch(
|
||||
/\.dagCanvas\s+\.node\[data-plan-output='true'\]::after\s*\{[^}]*right:\s*-4px/,
|
||||
);
|
||||
// Clipping on ANY rule whose subject is `.node` — a status variant,
|
||||
// attention, selection, a `:has()` state, the media-scoped copies — cuts
|
||||
// the port just as surely as clipping on the base rule, and jsdom never
|
||||
// sees it. Walk every rule block and clear every `.node` subject of
|
||||
// overflow clipping; rules that merely descend from `.node` (such as
|
||||
// `.node[...] .nodeGlyph`) keep their own overflow business, as do the
|
||||
// chip and title rules that clip legitimately.
|
||||
const clipped: string[] = [];
|
||||
// At-rules wrap rules without adding a selector subject of their own, so
|
||||
// unwrap their preludes and scan the inner rules like top-level ones
|
||||
// (the leftover closing braces are lone text no block regex matches).
|
||||
const unwrapped = planCss.replace(/@[a-z-]+[^{]*\{/gi, '');
|
||||
for (const [, selectorList, body] of unwrapped.matchAll(
|
||||
/([^{}]+)\{([^}]*)\}/g,
|
||||
)) {
|
||||
if (!/overflow:\s*(hidden|clip)/.test(body)) continue;
|
||||
for (const selector of selectorList.split(',')) {
|
||||
const subject =
|
||||
selector
|
||||
.trim()
|
||||
.split(/\s*[>+~\s]\s*/)
|
||||
.pop() ?? '';
|
||||
if (/\.node(?![A-Za-z0-9-])/.test(subject)) {
|
||||
clipped.push(selector.trim());
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(clipped).toEqual([]);
|
||||
});
|
||||
|
||||
// The selected rule's `border-color` shorthand expands to
|
||||
// border-left-color and outranks the base rule's
|
||||
// `border-left: 3px solid var(--node-rule)` longhand (0,2,0 beats 0,1,0)
|
||||
// while the 3px width survives — so a selected blocked/ready node, whose
|
||||
// rule is deliberately transparent, grew a 3px muted left bar and
|
||||
// selection read as a status change. jsdom computes no cascade, so the
|
||||
// re-pin is asserted at the source.
|
||||
it('keeps selection off the status rule', () => {
|
||||
const selected = planCss.match(
|
||||
/(^|\n)\.node\[data-selected='true'\]\s*\{[^}]*\}/,
|
||||
)?.[0];
|
||||
expect(selected).toBeTruthy();
|
||||
expect(selected).toMatch(/border-left-color:\s*var\(--node-rule\)/);
|
||||
});
|
||||
|
||||
// `text-overflow` applies to a block container's own inline content, so
|
||||
// declaring it on the inline-flex chip did nothing: the title sat in an
|
||||
// anonymous flex item and was clipped with no ellipsis. jsdom computes no
|
||||
// layout, so this is pinned at the source instead.
|
||||
it('puts dependency truncation on the title, not the flex box', () => {
|
||||
for (const name of ['dependencyChip', 'dependencyLink']) {
|
||||
const rule = planCss.match(
|
||||
new RegExp(`(^|\\n)\\.${name}\\s*\\{[^}]*\\}`),
|
||||
)?.[0];
|
||||
expect(rule).toBeTruthy();
|
||||
expect(rule).toMatch(/display:\s*inline-flex/);
|
||||
expect(rule).not.toMatch(/text-overflow/);
|
||||
}
|
||||
const title = planCss.match(
|
||||
/\.dependencyChip\s+\.dependencyTitle,\s*\n\s*\.dependencyLink\s+\.dependencyTitle\s*\{[^}]*\}/,
|
||||
)?.[0];
|
||||
expect(title).toMatch(/text-overflow:\s*ellipsis/);
|
||||
// `text-overflow` only takes effect on a box whose own `overflow` is
|
||||
// hidden; overflow is not inherited, and the chip's own clipping does
|
||||
// not reach the title span, so the declaration must live on this rule.
|
||||
expect(title).toMatch(/overflow:\s*hidden/);
|
||||
expect(title).toMatch(/min-width:\s*0/);
|
||||
// The `> span` rules mute every direct-child span, the title included;
|
||||
// without its own colour the title would render muted while the chip's
|
||||
// `color: var(--foreground)` applies to no rendered text.
|
||||
expect(title).toMatch(/color:\s*var\(--foreground\)/);
|
||||
});
|
||||
|
||||
it('declares the attention tone after every status tone', () => {
|
||||
// All these selectors carry the same specificity, so the cascade lets
|
||||
// the later rule win. Attention co-occurs with every status (a linked
|
||||
// agent task can fail under any of them), so the attention rule must
|
||||
// stay declared after every status tone, or a node that needs attention
|
||||
// would wear its status ring instead of the attention tone its comment
|
||||
// promises — whichever status block a later edit regroups.
|
||||
const attentionAt = planCss.indexOf(".node[data-attention='true']");
|
||||
expect(attentionAt).toBeGreaterThan(-1);
|
||||
for (const status of [
|
||||
'completed',
|
||||
'running',
|
||||
'in_progress',
|
||||
'paused',
|
||||
'blocked',
|
||||
'ready',
|
||||
]) {
|
||||
const statusAt = planCss.indexOf(`.node[data-status='${status}']`);
|
||||
expect(statusAt).toBeGreaterThan(-1);
|
||||
expect(attentionAt).toBeGreaterThan(statusAt);
|
||||
}
|
||||
});
|
||||
|
||||
it('narrows the DAG lane at two viewport steps', () => {
|
||||
// Three 240px lanes plus two 64px gutters exceed a phone viewport, so the
|
||||
// canvas scrolled in both axes at once and the horizontal scroll hid the
|
||||
// layer the vertical scroll was looking for.
|
||||
expect(planCss).toMatch(
|
||||
/\.dagCanvas\s+\.layer\s*\{[^}]*flex:\s*0\s+0\s+var\(--plan-layer-width/,
|
||||
);
|
||||
// Pinned as the source shape, not as an outcome: jsdom computes no
|
||||
// cascade and no layout, so this cannot assert that a plan of a given
|
||||
// depth stops scrolling — only that both steps exist and narrow.
|
||||
const narrow = planCss.match(
|
||||
/@media\s*\(max-width:\s*720px\)\s*\{[\s\S]*?\n\}/,
|
||||
)?.[0];
|
||||
// The width pin names the block's owner selector too: a declaration
|
||||
// moved onto a selector `.layer` never reads from must fail the guard
|
||||
// even though the value still appears inside the media block.
|
||||
expect(narrow).toMatch(/\.dagCanvas\s*\{[^}]*--plan-layer-width:\s*168px/);
|
||||
// The fit also rests on the paired gutter narrowing: dropping it falls
|
||||
// back to the base 64px gap, putting a three-layer plan past the
|
||||
// viewport this step exists to fit.
|
||||
expect(narrow).toMatch(/gap:\s*32px/);
|
||||
const narrower = planCss.match(
|
||||
/@media\s*\(max-width:\s*480px\)\s*\{[\s\S]*?\n\}/,
|
||||
)?.[0];
|
||||
expect(narrower).toMatch(
|
||||
/\.dagCanvas\s*\{[^}]*--plan-layer-width:\s*116px/,
|
||||
);
|
||||
// Same pairing at the phone step: 3×116px lanes only fit because the
|
||||
// gap narrows to 18px and the canvas padding to 6px a side
|
||||
// (3×116 + 2×18 + 2×6 = 396px). Dropping any of the three restores
|
||||
// the two-axis scroll the step was added to remove.
|
||||
expect(narrower).toMatch(/gap:\s*18px/);
|
||||
expect(narrower).toMatch(/padding-right:\s*6px/);
|
||||
expect(narrower).toMatch(/padding-left:\s*6px/);
|
||||
});
|
||||
|
||||
it('keeps the sr-only dependency summary off screen', () => {
|
||||
// The interactive graph states its dependencies to assistive tech only,
|
||||
// because the visible chip row is deliberately off there and the drawn
|
||||
// edges are aria-hidden. That only holds while the summary shares the
|
||||
// status word's screen-reader clipping: dropping it from this selector
|
||||
// list turns the summary into a visible row — the very thing the diff
|
||||
// removed — with no test signal, since jsdom computes no cascade and the
|
||||
// component test can only see the class name.
|
||||
const srOnly = planCss.match(
|
||||
/(^|\n)\.nodeStatusText,\s*\n\s*\.nodeDependencyText\s*\{[^}]*\}/,
|
||||
)?.[0];
|
||||
expect(srOnly).toBeTruthy();
|
||||
expect(srOnly).toMatch(/position:\s*absolute/);
|
||||
expect(srOnly).toMatch(/clip-path:\s*inset\(50%\)/);
|
||||
expect(srOnly).toMatch(/width:\s*1px/);
|
||||
expect(srOnly).toMatch(/height:\s*1px/);
|
||||
expect(srOnly).toMatch(/overflow:\s*hidden/);
|
||||
});
|
||||
});
|
||||
@@ -136,7 +136,11 @@
|
||||
}
|
||||
|
||||
.dagCanvas {
|
||||
/* Both are overwritten from TS whenever the graph has dependencies; these
|
||||
defaults only cover the no-dependency case, where --plan-edge-lanes is 0
|
||||
and the reservation collapses regardless of the pitch. */
|
||||
--plan-edge-lanes: 0;
|
||||
--plan-edge-lane-height: 0px;
|
||||
position: relative;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
@@ -144,7 +148,11 @@
|
||||
gap: 64px;
|
||||
width: max-content;
|
||||
min-width: 100%;
|
||||
padding: 8px 10px calc(28px + var(--plan-edge-lanes) * 9px);
|
||||
/* Both operands come from TS (PlanExecutionView sets --plan-edge-lanes and
|
||||
--plan-edge-lane-height from EDGE_LANE_HEIGHT) so the reservation cannot
|
||||
drift from the lane placement it is reserving for. */
|
||||
padding: 8px 10px
|
||||
calc(28px + var(--plan-edge-lanes) * var(--plan-edge-lane-height));
|
||||
}
|
||||
|
||||
.dagEdges {
|
||||
@@ -218,25 +226,67 @@
|
||||
.dagCanvas .layer {
|
||||
position: relative;
|
||||
z-index: 1;
|
||||
flex: 0 0 240px;
|
||||
flex: 0 0 var(--plan-layer-width, 240px);
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
/* Status drives one accent, and the accent drives every edge treatment on the
|
||||
node: the border, and the selection ring. Before this, status set
|
||||
`border-color` while selection stacked an always-blue ring on top, so a
|
||||
completed (green) or blocked (amber) node picked up a second border in a
|
||||
clashing hue the moment it was selected. */
|
||||
/* Three 240px lanes plus two 64px gutters are ~850px, so on a phone the graph
|
||||
became a canvas that scrolls in both axes at once — the horizontal scroll
|
||||
hiding the layer the vertical scroll is looking for. Narrowing the lane and
|
||||
the gutter keeps the whole topology on the horizontal axis the graph
|
||||
actually needs, and leaves the layer semantics (and the measured edge
|
||||
routing, which reads real rects) untouched. */
|
||||
@media (max-width: 720px) {
|
||||
.dagCanvas {
|
||||
--plan-layer-width: 168px;
|
||||
gap: 32px;
|
||||
}
|
||||
}
|
||||
|
||||
/* 168px lanes still put a three-layer plan at ~588px, which scrolls on a
|
||||
390px phone and grows by a lane with every extra layer. This step brings
|
||||
a three-layer plan to ~396px (3×116px lanes, 2×18px gaps, 2×6px canvas
|
||||
padding): it fits phones wider than ~396px, and a 390px phone still
|
||||
scrolls the last ~6px. Deeper plans scroll horizontally at any phone
|
||||
width, which is the honest limit of narrowing alone — reusing the flat
|
||||
layout is tracked in #10866. */
|
||||
@media (max-width: 480px) {
|
||||
.dagCanvas {
|
||||
--plan-layer-width: 116px;
|
||||
gap: 18px;
|
||||
padding-right: 6px;
|
||||
padding-left: 6px;
|
||||
}
|
||||
}
|
||||
|
||||
/* Status now drives the left rule; the border stays neutral for every status
|
||||
and the accent is left to mean selection. Tinting every border made a
|
||||
healthy plan read as a wall of coloured boxes and left the one running node
|
||||
with nothing to stand out against — the first thing a reader of an
|
||||
execution trace looks for. The older hazard this replaces is still worth
|
||||
knowing: status used to set `border-color` while selection stacked an
|
||||
always-blue ring on top, so a completed or blocked node picked up a second
|
||||
border in a clashing hue the moment it was selected. With status off the
|
||||
border, the two can no longer collide. */
|
||||
.node {
|
||||
--node-accent: var(--border);
|
||||
--node-ring: var(--agent-blue-500);
|
||||
--node-rule: transparent;
|
||||
position: relative;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 6px;
|
||||
gap: 5px;
|
||||
min-width: 0;
|
||||
padding: 9px 10px;
|
||||
border: 1px solid var(--node-accent);
|
||||
/* Status, stated once. A border rather than a pseudo-element so the node's
|
||||
own border-radius clips it natively: clipping a ::before would have
|
||||
needed `overflow: hidden`, which also cuts the output port dot in half —
|
||||
it sits at `right: -4px`, deliberately outside the box. Transparent when
|
||||
resting, and the node's background paints under it (background-clip is
|
||||
border-box), so a resting node reads as 3px of extra left padding with
|
||||
no width difference between states. */
|
||||
border-left: 3px solid var(--node-rule);
|
||||
border-radius: 8px;
|
||||
background: var(--background);
|
||||
transition:
|
||||
@@ -244,25 +294,25 @@
|
||||
box-shadow 160ms ease;
|
||||
}
|
||||
|
||||
.dagCanvas .node::before,
|
||||
/* Only the outgoing port is drawn; an incoming edge already terminates in an
|
||||
arrowhead, which is the input marker. It also drops the accent: every node
|
||||
with an edge had one, so a permanently blue dot across most of the graph
|
||||
left selection no colour of its own. It is structure, so it takes the
|
||||
hairline. Note this sits outside the node's box, so the node must not clip
|
||||
its overflow. */
|
||||
.dagCanvas .node::after {
|
||||
display: none;
|
||||
position: absolute;
|
||||
top: 50%;
|
||||
width: 8px;
|
||||
height: 8px;
|
||||
border: 1px solid var(--agent-blue-500);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 999px;
|
||||
background: var(--background);
|
||||
content: '';
|
||||
transform: translateY(-50%);
|
||||
}
|
||||
|
||||
.dagCanvas .node[data-plan-input='true']::before {
|
||||
left: -4px;
|
||||
display: block;
|
||||
}
|
||||
|
||||
.dagCanvas .node[data-plan-output='true']::after {
|
||||
right: -4px;
|
||||
display: block;
|
||||
@@ -274,26 +324,18 @@
|
||||
real plan, so it stays neutral; painting it amber made a perfectly healthy
|
||||
graph read as a wall of warnings. */
|
||||
.node[data-status='completed'] {
|
||||
--node-accent: color-mix(in srgb, var(--status-done-fg) 55%, var(--border));
|
||||
--node-rule: var(--status-done-fg);
|
||||
--node-ring: var(--status-done-fg);
|
||||
}
|
||||
|
||||
.node[data-status='running'],
|
||||
.node[data-status='in_progress'] {
|
||||
--node-accent: color-mix(
|
||||
in srgb,
|
||||
var(--status-running-fg) 55%,
|
||||
var(--border)
|
||||
);
|
||||
--node-rule: var(--status-running-fg);
|
||||
--node-ring: var(--status-running-fg);
|
||||
}
|
||||
|
||||
.node[data-status='paused'] {
|
||||
--node-accent: color-mix(
|
||||
in srgb,
|
||||
var(--status-attention-fg) 55%,
|
||||
var(--border)
|
||||
);
|
||||
--node-rule: var(--status-attention-fg);
|
||||
--node-ring: var(--status-attention-fg);
|
||||
}
|
||||
|
||||
@@ -303,10 +345,25 @@
|
||||
--node-ring: var(--muted-foreground);
|
||||
}
|
||||
|
||||
/* Attention folds into the same rule instead of adding a fourth status
|
||||
token to the node face: one status, one place. It outranks the status
|
||||
tone because it is the state the reader is being asked to act on — so
|
||||
it is declared after every status tone: at equal specificity the cascade
|
||||
lets the later rule win, and a blocked/ready node that also needs
|
||||
attention would otherwise wear the muted ring. */
|
||||
.node[data-attention='true'] {
|
||||
--node-rule: var(--status-attention-fg);
|
||||
--node-ring: var(--status-attention-fg);
|
||||
}
|
||||
|
||||
/* Selection reads as "the same edge, turned up": the border goes to full
|
||||
strength and the ring echoes it, rather than introducing a second colour. */
|
||||
.node[data-selected='true'] {
|
||||
border-color: var(--node-ring);
|
||||
/* The shorthand above expands to border-left-color too: on blocked/ready,
|
||||
whose --node-rule stays transparent, it would repaint the 3px status
|
||||
rule in the ring tone and selection would read as a status change. */
|
||||
border-left-color: var(--node-rule);
|
||||
box-shadow: 0 0 0 2px color-mix(in srgb, var(--node-ring) 32%, transparent);
|
||||
}
|
||||
|
||||
@@ -333,10 +390,50 @@
|
||||
outline: none;
|
||||
}
|
||||
|
||||
/* Content first. The node led with a glyph, the raw Todo id and the status
|
||||
word, so the only element that says what the step *is* came third — and an
|
||||
id is an address, the least useful thing at graph scale. */
|
||||
.nodeTop {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
align-items: baseline;
|
||||
gap: 7px;
|
||||
}
|
||||
|
||||
/* The same number the inspector list and the dependency chips use, so the
|
||||
three surfaces address a step identically. */
|
||||
.nodeNumber {
|
||||
flex: none;
|
||||
color: var(--muted-foreground);
|
||||
font-size: 11px;
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
|
||||
/* The left rule is colour, which assistive tech cannot read, so the status
|
||||
stays in the accessibility tree as words. The dependency summary shares the
|
||||
channel: the interactive graph draws the dependency as an aria-hidden edge
|
||||
rather than stating it, so the node's accessible name has to carry it. */
|
||||
.nodeStatusText,
|
||||
.nodeDependencyText {
|
||||
position: absolute;
|
||||
overflow: hidden;
|
||||
width: 1px;
|
||||
height: 1px;
|
||||
padding: 0;
|
||||
border: 0;
|
||||
margin: -1px;
|
||||
clip-path: inset(50%);
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* Two facts, spaced, rather than one middle-dot-joined string: they wrap as
|
||||
a unit and read out as one run of text when glued. */
|
||||
.nodeMeta {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: 1px 12px;
|
||||
color: var(--muted-foreground);
|
||||
font-size: 11px;
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
|
||||
.nodeId {
|
||||
@@ -373,15 +470,89 @@
|
||||
}
|
||||
|
||||
.nodeContent {
|
||||
min-width: 0;
|
||||
color: var(--foreground);
|
||||
font-size: 13px;
|
||||
font-weight: 500;
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
/* Rendered only when no edge states the dependency — above
|
||||
MAX_RENDERED_PLAN_EDGES the graph draws none, and then this row is the
|
||||
only statement of it. Chips carry the step number and title, matching the
|
||||
inspector, instead of a comma-joined run of raw ids. */
|
||||
.dependencies {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
align-items: baseline;
|
||||
gap: 3px 6px;
|
||||
color: var(--muted-foreground);
|
||||
font-size: 11px;
|
||||
}
|
||||
|
||||
/* `text-overflow` applies to a block container's own inline content, so
|
||||
declaring it on the flex box did nothing — the title sat in an anonymous
|
||||
flex item and was clipped without an ellipsis. The truncation belongs on
|
||||
the title span, which is a real box. */
|
||||
.dependencyChip {
|
||||
display: inline-flex;
|
||||
max-width: 100%;
|
||||
align-items: baseline;
|
||||
overflow: hidden;
|
||||
gap: 5px;
|
||||
padding: 1px 6px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md, 6px);
|
||||
color: var(--foreground);
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* The `> span` rules below mute every direct-child span — the title
|
||||
included — and a direct declaration beats the chip's inherited colour,
|
||||
so the title states its own full-weight colour here. */
|
||||
.dependencyChip .dependencyTitle,
|
||||
.dependencyLink .dependencyTitle {
|
||||
overflow: hidden;
|
||||
min-width: 0;
|
||||
color: var(--foreground);
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.dependencyChip > span {
|
||||
color: var(--muted-foreground);
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
|
||||
/* The step-details panel's references, which sit outside the node's button
|
||||
and so can be real controls. Same face as the node's read-only chips. */
|
||||
.dependencyLink {
|
||||
display: inline-flex;
|
||||
max-width: 100%;
|
||||
align-items: baseline;
|
||||
overflow: hidden;
|
||||
gap: 5px;
|
||||
padding: 1px 6px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md, 6px);
|
||||
background: var(--background);
|
||||
color: var(--foreground);
|
||||
cursor: pointer;
|
||||
font-size: 11px;
|
||||
text-align: left;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.dependencyLink:hover {
|
||||
border-color: color-mix(in srgb, var(--agent-blue-500) 45%, var(--border));
|
||||
background: var(--accent);
|
||||
}
|
||||
|
||||
.dependencyLink > span {
|
||||
color: var(--muted-foreground);
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
|
||||
.executions {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -584,10 +755,12 @@ button.nestedExecution:not(:disabled):focus-visible {
|
||||
|
||||
/* The status glyph. Sized and aligned to the mono ID beside it so the two read
|
||||
as one identifier block rather than an icon bolted onto a label. */
|
||||
/* Muted, not `--node-ring`: the left rule is the single carrier of the
|
||||
status colour, and this is the redundant *shape* channel beside it. */
|
||||
.nodeGlyph {
|
||||
flex: 0 0 auto;
|
||||
width: 13px;
|
||||
color: var(--node-ring);
|
||||
color: var(--muted-foreground);
|
||||
font-family: var(--font-mono, monospace);
|
||||
font-size: 12px;
|
||||
font-style: normal;
|
||||
@@ -595,6 +768,23 @@ button.nestedExecution:not(:disabled):focus-visible {
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
/* The attention sigil, muted like the status glyph: the left rule stays the
|
||||
only carrier of tone, and this is the shape channel that survives forced
|
||||
colours — the data-attention rule re-declares the very token paused
|
||||
already wears, so without a mark a paused node needing attention paints
|
||||
pixel-identical to a healthy one. */
|
||||
.nodeAttentionMark {
|
||||
flex: 0 0 auto;
|
||||
width: 13px;
|
||||
color: var(--muted-foreground);
|
||||
font-family: var(--font-mono, monospace);
|
||||
font-size: 12px;
|
||||
font-style: normal;
|
||||
font-weight: 700;
|
||||
line-height: 1;
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
/* Work in flight is the only thing on the page that moves, so the eye finds it
|
||||
without a legend. */
|
||||
.node[data-status='running'] .nodeGlyph {
|
||||
|
||||
@@ -6,6 +6,7 @@ import { describe, expect, it, vi } from 'vitest';
|
||||
import type { DaemonSessionAgentTaskStatus } from '@qwen-code/sdk/daemon';
|
||||
import type { ACPToolCall, TodoItem } from '../../adapters/types';
|
||||
import { I18nProvider } from '../../i18n';
|
||||
import styles from './PlanExecutionView.module.css';
|
||||
import { TranscriptRenderModeProvider } from '../../transcriptRenderMode';
|
||||
import {
|
||||
getActiveAgents,
|
||||
@@ -14,6 +15,7 @@ import {
|
||||
layerPlanTodos,
|
||||
nestedAgentToolsForTool,
|
||||
nestedTasksForTool,
|
||||
PLAN_STATUS_GLYPH,
|
||||
PlanExecutionView,
|
||||
} from './PlanExecutionView';
|
||||
|
||||
@@ -571,7 +573,64 @@ describe('PlanExecutionView', () => {
|
||||
);
|
||||
});
|
||||
|
||||
expect(container.textContent).toContain('Depends on: research');
|
||||
// The drawn edge is the dependency statement, so the node face does not
|
||||
// restate it *visibly* — the chip row stays off. The step-details panel
|
||||
// still states it, asserted below, and the sr-only summary carries it to
|
||||
// assistive tech because the drawn edges are aria-hidden (asserted in
|
||||
// 'names the blockers in the node accessible name…').
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.querySelector(`.${styles.dependencies}`)).toBeNull();
|
||||
// Content leads the node; the step number matches the inspector list and
|
||||
// the dependency chips, and the agent count and elapsed carry the "is
|
||||
// this alive" signal onto the face.
|
||||
expect(buildNode?.textContent).toContain('Build');
|
||||
// Root agent plus the live child task below (parentAgentId set, no
|
||||
// transcript entry): the node renders two agent rows, so the face tally
|
||||
// must read 2 — counting only transcript subTools is the R8-2 divergence.
|
||||
expect(buildNode?.textContent).toContain('2 agents');
|
||||
expect(buildNode?.textContent).toContain('1m 5s');
|
||||
// The plural branch is pinned above; `1 agents` would ship green without
|
||||
// the negative assertion here. The singular branch is pinned in
|
||||
// 'the node-face agent tally matches the rows it renders'.
|
||||
const researchNode = container
|
||||
.querySelector('[data-plan-node-id="research"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).not.toContain('1 agents');
|
||||
expect(researchNode?.textContent ?? '').not.toContain('1 agents');
|
||||
// Status is colour on the left rule, so it stays in the accessibility
|
||||
// tree as words rather than being dropped. Asserted on a node with no
|
||||
// linked agent, so the word can only come from the node's own status
|
||||
// element and not from an execution row inside it.
|
||||
const verifyNode = container
|
||||
.querySelector('[data-plan-node-id="verify"]')
|
||||
?.closest('article');
|
||||
expect(
|
||||
verifyNode?.querySelector(`.${styles.nodeStatusText}`)?.textContent,
|
||||
).toBe('Blocked');
|
||||
// `verify` has no linked tool call in this fixture, so its visible face
|
||||
// is still just the number, the content and the status glyph; the two
|
||||
// sr-only spans (status word, dependency summary) are the only other text
|
||||
// the node carries.
|
||||
expect(verifyNode?.querySelector(`.${styles.nodeTop}`)?.textContent).toBe(
|
||||
'3Verify',
|
||||
);
|
||||
expect(verifyNode?.querySelector(`.${styles.nodeMeta}`)?.textContent).toBe(
|
||||
PLAN_STATUS_GLYPH.blocked,
|
||||
);
|
||||
// The glyph is the non-colour status channel: the left rule that carries
|
||||
// status visually is colour only, so without a shape beside it the graph
|
||||
// would lose status entirely under colour-blindness, high-contrast mode
|
||||
// or a greyscale screenshot. Pinned per status, not just as "a glyph".
|
||||
expect(buildNode?.textContent).toContain(PLAN_STATUS_GLYPH.running);
|
||||
expect(
|
||||
container
|
||||
.querySelector('[data-plan-node-id="research"]')
|
||||
?.closest('article')?.textContent,
|
||||
).toContain(PLAN_STATUS_GLYPH.completed);
|
||||
expect(PLAN_STATUS_GLYPH.blocked).not.toBe(PLAN_STATUS_GLYPH.running);
|
||||
expect(PLAN_STATUS_GLYPH.running).not.toBe(PLAN_STATUS_GLYPH.completed);
|
||||
expect(container.textContent).toContain('33%');
|
||||
expect(container.textContent).toContain('1 / 3');
|
||||
// 3, not 2: the strip now derives from the same source as the node
|
||||
@@ -590,8 +649,26 @@ describe('PlanExecutionView', () => {
|
||||
const details = container.querySelector('[data-plan-step-details]');
|
||||
expect(details?.textContent).toContain('Step details');
|
||||
expect(details?.textContent).toContain('Build');
|
||||
expect(details?.textContent).toContain('Depends on: research');
|
||||
expect(details?.textContent).toContain('Unblocks: verify');
|
||||
// Outside the node's own button, so these references are real controls:
|
||||
// each names the step by number and title, and selects it on click.
|
||||
const upstreamLink = details?.querySelector<HTMLButtonElement>(
|
||||
'[data-plan-dependency="research"]',
|
||||
);
|
||||
expect(upstreamLink?.tagName).toBe('BUTTON');
|
||||
// The host keyboard handlers isolate plan controls through this marker
|
||||
// (ToolApproval/TasksStatusMessage early-return on it), so these buttons
|
||||
// must carry it like every other control in the view.
|
||||
expect(upstreamLink?.hasAttribute('data-plan-interactive')).toBe(true);
|
||||
expect(upstreamLink?.textContent).toBe('1Research');
|
||||
expect(
|
||||
details?.querySelector('[data-plan-dependency="verify"]')?.textContent,
|
||||
).toBe('3Verify');
|
||||
expect(
|
||||
details
|
||||
?.querySelector('[data-plan-dependency="verify"]')
|
||||
?.hasAttribute('data-plan-interactive'),
|
||||
).toBe(true);
|
||||
expect(details?.textContent).not.toContain('Depends on: research');
|
||||
expect(details?.textContent).toContain('Subagents');
|
||||
expect(details?.textContent).toContain(
|
||||
'Current activity:Inspecting the implementation',
|
||||
@@ -606,6 +683,193 @@ describe('PlanExecutionView', () => {
|
||||
act(() => button?.click());
|
||||
expect(onOpen).toHaveBeenCalledWith(agentTool('build'));
|
||||
|
||||
// The downstream block wires its own onClick; witness it before the
|
||||
// upstream click below moves the selection off `build`. Dropping that
|
||||
// handler must turn this red.
|
||||
act(() =>
|
||||
details
|
||||
?.querySelector<HTMLButtonElement>('[data-plan-dependency="verify"]')
|
||||
?.click(),
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.querySelector('[data-plan-node-id="verify"]')
|
||||
?.getAttribute('aria-pressed'),
|
||||
).toBe('true');
|
||||
// That click selected `verify` and re-rendered the panel around it, so
|
||||
// bring the selection back to `build` before exercising the upstream
|
||||
// reference.
|
||||
act(() => {
|
||||
container
|
||||
.querySelector<HTMLButtonElement>('[data-plan-node-id="build"]')
|
||||
?.click();
|
||||
});
|
||||
expect(
|
||||
container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.getAttribute('aria-pressed'),
|
||||
).toBe('true');
|
||||
|
||||
// Last, because it moves the selection: following an upstream reference
|
||||
// selects the step it names, which is what makes the dependency list the
|
||||
// graph's navigation rather than a run of text.
|
||||
act(() =>
|
||||
details
|
||||
?.querySelector<HTMLButtonElement>('[data-plan-dependency="research"]')
|
||||
?.click(),
|
||||
);
|
||||
expect(
|
||||
container
|
||||
.querySelector('[data-plan-node-id="research"]')
|
||||
?.getAttribute('aria-pressed'),
|
||||
).toBe('true');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('drops unresolvable dependency ids from the step-details controls', () => {
|
||||
// blockedBy is model-authored todo_write output parsed without
|
||||
// normalization, so it can name a step that does not exist. Such an id
|
||||
// must not render as a control: clicking it would select a ghost id,
|
||||
// empty selectedTodo, and hide the panel mid-navigation.
|
||||
const ghostTodos = todos.map((todo) =>
|
||||
todo.id === 'build'
|
||||
? { ...todo, blockedBy: ['research', 'ghost-step'] }
|
||||
: todo,
|
||||
);
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={ghostTodos} tools={[]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
act(() => {
|
||||
container
|
||||
.querySelector<HTMLButtonElement>('[data-plan-node-id="build"]')
|
||||
?.click();
|
||||
});
|
||||
const details = container.querySelector('[data-plan-step-details]');
|
||||
expect(details).not.toBeNull();
|
||||
// The resolvable dependency stays a working control…
|
||||
expect(
|
||||
details?.querySelector('[data-plan-dependency="research"]'),
|
||||
).not.toBeNull();
|
||||
// …while the ghost id renders none at all.
|
||||
expect(
|
||||
details?.querySelector('[data-plan-dependency="ghost-step"]'),
|
||||
).toBeNull();
|
||||
expect(details?.textContent).not.toContain('ghost-step');
|
||||
// The ellipsis lives on `.dependencyTitle`; pin the class wiring so a
|
||||
// dropped className cannot re-clip titles while these text assertions
|
||||
// stay green.
|
||||
expect(
|
||||
Array.from(
|
||||
details
|
||||
?.querySelector('[data-plan-dependency="research"]')
|
||||
?.querySelectorAll('span') ?? [],
|
||||
).some((span) => span.classList.contains(styles.dependencyTitle)),
|
||||
).toBe(true);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('announces attention beside the status word, not instead of it', () => {
|
||||
// A failed descendant puts the node into attention while its own status
|
||||
// stays running. The sr-only span must keep announcing the status word
|
||||
// too — attention is additive for assistive tech, never a replacement.
|
||||
const runningRoot = task('running');
|
||||
const failedChild = task('failed', {
|
||||
id: 'agent-child',
|
||||
toolUseId: 'call-child',
|
||||
parentAgentId: runningRoot.id,
|
||||
});
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView
|
||||
todos={todos}
|
||||
tools={[agentTool('build')]}
|
||||
tasks={[runningRoot, failedChild]}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).toContain('Running');
|
||||
expect(buildNode?.textContent).toContain('Needs attention');
|
||||
expect(
|
||||
buildNode
|
||||
?.querySelector(`.${styles.nodeStatusText}`)
|
||||
?.textContent?.trim(),
|
||||
).toBe('Running, Needs attention');
|
||||
// The attribute is the hook the `.node[data-attention='true']` colour
|
||||
// rule selects on; dropping it keeps the word but silently loses the
|
||||
// attention tone the stylesheet derives from this state.
|
||||
expect(buildNode?.getAttribute('data-attention')).toBe('true');
|
||||
// Colour alone cannot carry attention: on a paused node the
|
||||
// data-attention rule re-declares the token paused already wears, so
|
||||
// without a shape channel the two paint pixel-identical. The visible
|
||||
// sigil lives in the meta row beside the status glyph; the words stay
|
||||
// in the sr-only span above.
|
||||
const attentionMark = buildNode?.querySelector(
|
||||
`.${styles.nodeMeta} .${styles.nodeAttentionMark}`,
|
||||
);
|
||||
expect(attentionMark?.textContent).toBe('!');
|
||||
expect(attentionMark?.getAttribute('aria-hidden')).toBe('true');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('pluralizes the agent count when two agents share one node', () => {
|
||||
// Every other fixture links a single agent per node, so the EN
|
||||
// template's plural branch ships unobserved: a template that always
|
||||
// emits `${count} agent` renders "2 agent" and nothing turns red.
|
||||
// Link two root agents to one step and pin the plural rendering.
|
||||
const secondBuildTool: ACPToolCall = {
|
||||
...agentTool('build'),
|
||||
callId: 'call-build-2',
|
||||
title: 'Agent build 2',
|
||||
};
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView
|
||||
todos={todos}
|
||||
tools={[agentTool('build'), secondBuildTool]}
|
||||
tasks={[
|
||||
task('running'),
|
||||
task('running', {
|
||||
id: 'agent-second',
|
||||
label: 'Second agent',
|
||||
toolUseId: 'call-build-2',
|
||||
}),
|
||||
]}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).toContain('2 agents');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
@@ -868,7 +1132,11 @@ describe('PlanExecutionView', () => {
|
||||
}
|
||||
if (this.tagName === 'ARTICLE') {
|
||||
const [left, top] =
|
||||
positions[this.querySelector('span')!.textContent!]!;
|
||||
positions[
|
||||
this.querySelector('[data-plan-node-id]')!.getAttribute(
|
||||
'data-plan-node-id',
|
||||
)!
|
||||
]!;
|
||||
return scaledRect(100 + left * 0.72, 50 + top * 0.72, 144, 57.6);
|
||||
}
|
||||
return scaledRect(0, 0, 0, 0);
|
||||
@@ -951,7 +1219,11 @@ describe('PlanExecutionView', () => {
|
||||
}
|
||||
if (this.tagName === 'ARTICLE') {
|
||||
const [left, top] =
|
||||
positions[this.querySelector('span')!.textContent!]!;
|
||||
positions[
|
||||
this.querySelector('[data-plan-node-id]')!.getAttribute(
|
||||
'data-plan-node-id',
|
||||
)!
|
||||
]!;
|
||||
return rect(100 + left, 50 + top, 200, 80);
|
||||
}
|
||||
return rect(0, 0, 0, 0);
|
||||
@@ -988,6 +1260,14 @@ describe('PlanExecutionView', () => {
|
||||
(match) => Number(match[1]),
|
||||
);
|
||||
expect(Math.max(...routedYs)).toBeGreaterThan(200);
|
||||
// ...and arrives at the target's own row, not merely at its column. The
|
||||
// `release` node sits at y 10 with height 80 inside a viewport whose
|
||||
// origin is (100, 50), so its vertical centre is 50. Without this, a tail
|
||||
// drawn from the return lane's `routeY` instead of the edge's `endY`
|
||||
// lands ~170px below the node while the column and clearance assertions
|
||||
// above both still pass.
|
||||
expect(spanningEdge).toMatch(/ 50 H 876$/);
|
||||
expect(routedYs).toContain(50);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
@@ -1086,7 +1366,11 @@ describe('PlanExecutionView', () => {
|
||||
}
|
||||
if (this.tagName === 'ARTICLE') {
|
||||
const [left, top] =
|
||||
positions[this.querySelector('span')!.textContent!]!;
|
||||
positions[
|
||||
this.querySelector('[data-plan-node-id]')!.getAttribute(
|
||||
'data-plan-node-id',
|
||||
)!
|
||||
]!;
|
||||
return rect(100 + left, 50 + top, 200, 80);
|
||||
}
|
||||
return rect(0, 0, 0, 0);
|
||||
@@ -1137,6 +1421,211 @@ describe('PlanExecutionView', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the arrowhead pointing at the target on every gutter tier', () => {
|
||||
// The narrow gutters this PR adds are tighter than the router's 24px
|
||||
// shoulder, so the control point landed on (32px tier) or past (18px
|
||||
// tier) the end point. The curve then arrives with a zero or negative x
|
||||
// tangent and `orient="auto"` flips the arrowhead back at its source —
|
||||
// measured in the browser as (0, 0) at 700px and (-14, 0) at 430/390px
|
||||
// against (28, 0) at 1440px. The input port is gone, so that arrowhead
|
||||
// is the last direction cue the graph has. Restoring the fixed
|
||||
// `Math.max(24, …)` shoulder turns the two narrow tiers red.
|
||||
const chain: TodoItem[] = [
|
||||
{ id: 'source', content: 'Source', status: 'completed' },
|
||||
{
|
||||
id: 'target',
|
||||
content: 'Target',
|
||||
status: 'pending',
|
||||
blockedBy: ['source'],
|
||||
},
|
||||
];
|
||||
// The lane width the 700px tier narrows to; only the gutter varies.
|
||||
const lane = 168;
|
||||
// gutter → 64px (≥721px), 32px (≤720px), 18px (≤480px).
|
||||
for (const gap of [64, 32, 18]) {
|
||||
const rect = (left: number, top: number, width: number, height: number) =>
|
||||
({
|
||||
x: left,
|
||||
y: top,
|
||||
left,
|
||||
top,
|
||||
width,
|
||||
height,
|
||||
right: left + width,
|
||||
bottom: top + height,
|
||||
toJSON: () => ({}),
|
||||
}) as DOMRect;
|
||||
const rectSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
|
||||
.mockImplementation(function () {
|
||||
if (this.parentElement?.hasAttribute('data-plan-workflow')) {
|
||||
return rect(100, 50, 1100, 300);
|
||||
}
|
||||
if (this.tagName === 'ARTICLE') {
|
||||
const id = this.querySelector('[data-plan-node-id]')!.getAttribute(
|
||||
'data-plan-node-id',
|
||||
)!;
|
||||
return rect(100 + (id === 'target' ? lane + gap : 0), 60, lane, 80);
|
||||
}
|
||||
return rect(0, 0, 0, 0);
|
||||
});
|
||||
const widthSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
|
||||
.mockReturnValue(1100);
|
||||
const heightSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'offsetHeight', 'get')
|
||||
.mockReturnValue(300);
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
try {
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={chain} tools={[]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const d = container
|
||||
.querySelector('[data-from="source"][data-to="target"]')
|
||||
?.getAttribute('d');
|
||||
expect(d, `${gap}px gutter drew no edge`).toBeTruthy();
|
||||
// `M startX startY C c1x c1y, c2x c2y, endX endY`.
|
||||
const n = [...d!.matchAll(/-?[\d.]+/g)].map((m) => Number(m[0]));
|
||||
expect(n).toHaveLength(8);
|
||||
// Prove the fixture really reproduced the tier: the router insets
|
||||
// both ends by 4px, so the run is the gutter minus 8 (56 / 24 / 10).
|
||||
expect(n[6] - n[0], `${gap}px gutter run`).toBe(gap - 8);
|
||||
// The end tangent is the end point minus the last control point, and
|
||||
// it is what the arrowhead orients on. Its x must be strictly
|
||||
// positive at every tier, or the head points back at the source.
|
||||
expect(n[6] - n[4], `${gap}px gutter end tangent x`).toBeGreaterThan(0);
|
||||
} finally {
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
rectSpy.mockRestore();
|
||||
widthSpy.mockRestore();
|
||||
heightSpy.mockRestore();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps a layer-skipping lane out of the step it passes', () => {
|
||||
// The other half of R6-3. The router gave every layer-skipping edge a
|
||||
// fixed 24px shoulder, which is wider than the ≤480px gutter (18px, a
|
||||
// 10px run). At 390/430px the rise sat 10px inside the intervening step,
|
||||
// so a dependency from step 1 to step 3 read as one into step 2 — and the
|
||||
// SVG paints under the nodes, so the lane simply disappeared into it.
|
||||
// Restoring `startX + 24` / `endX - 24` turns the 18px tier red.
|
||||
const skip: TodoItem[] = [
|
||||
{ id: 'one', content: 'One', status: 'completed' },
|
||||
{ id: 'two', content: 'Two', status: 'pending', blockedBy: ['one'] },
|
||||
{
|
||||
id: 'three',
|
||||
content: 'Three',
|
||||
status: 'pending',
|
||||
// The layer-skipping edge: `one` is layer 0, `three` is layer 2.
|
||||
blockedBy: ['two', 'one'],
|
||||
},
|
||||
];
|
||||
// The lane width the 700px tier narrows to; only the gutter varies.
|
||||
const lane = 168;
|
||||
// gutter → 64px (≥721px), 32px (≤720px), 18px (≤480px).
|
||||
for (const gap of [64, 32, 18]) {
|
||||
const rect = (left: number, top: number, width: number, height: number) =>
|
||||
({
|
||||
x: left,
|
||||
y: top,
|
||||
left,
|
||||
top,
|
||||
width,
|
||||
height,
|
||||
right: left + width,
|
||||
bottom: top + height,
|
||||
toJSON: () => ({}),
|
||||
}) as DOMRect;
|
||||
// One node per layer here, so the step the lane passes is all of layer 1.
|
||||
const leftOf = (layer: number) => layer * (lane + gap);
|
||||
const rectSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
|
||||
.mockImplementation(function () {
|
||||
if (this.parentElement?.hasAttribute('data-plan-workflow')) {
|
||||
return rect(100, 50, 1100, 300);
|
||||
}
|
||||
if (this.tagName === 'ARTICLE') {
|
||||
const id = this.querySelector('[data-plan-node-id]')!.getAttribute(
|
||||
'data-plan-node-id',
|
||||
)!;
|
||||
const layer = id === 'one' ? 0 : id === 'two' ? 1 : 2;
|
||||
return rect(100 + leftOf(layer), 60, lane, 80);
|
||||
}
|
||||
return rect(0, 0, 0, 0);
|
||||
});
|
||||
const widthSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'offsetWidth', 'get')
|
||||
.mockReturnValue(1100);
|
||||
const heightSpy = vi
|
||||
.spyOn(HTMLElement.prototype, 'offsetHeight', 'get')
|
||||
.mockReturnValue(300);
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
try {
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={skip} tools={[]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const d = container
|
||||
.querySelector('[data-from="one"][data-to="three"]')
|
||||
?.getAttribute('d');
|
||||
expect(d, `${gap}px gutter drew no lane`).toBeTruthy();
|
||||
// The route's four turns, as [control x, end x]: Q1/Q2 carry dropX,
|
||||
// Q3/Q4 carry riseX. Those are the two vertical segments' columns.
|
||||
const turns = [
|
||||
...d!.matchAll(/Q ([-\d.]+) [-\d.]+ ([-\d.]+) [-\d.]+/g),
|
||||
].map((match) => [Number(match[1]), Number(match[2])]);
|
||||
expect(turns).toHaveLength(4);
|
||||
const dropX = turns[0][0];
|
||||
const riseX = turns[2][0];
|
||||
const startX = lane + 4;
|
||||
const endX = leftOf(2) - 4;
|
||||
// The lane may only turn inside its own gutter, never inside layer 1.
|
||||
expect(dropX, `${gap}px drop column`).toBeGreaterThan(startX);
|
||||
expect(dropX, `${gap}px drop column`).toBeLessThan(leftOf(1));
|
||||
expect(riseX, `${gap}px rise column`).toBeGreaterThan(leftOf(1) + lane);
|
||||
expect(riseX, `${gap}px rise column`).toBeLessThan(endX);
|
||||
// The corner is halved at 18px so the run into the arrowhead keeps a
|
||||
// positive length; at zero the head flips back at its source.
|
||||
expect(Number(/H ([-\d.]+)$/.exec(d!)![1])).toBe(endX);
|
||||
expect(
|
||||
endX - turns[3][1],
|
||||
`${gap}px gutter end tangent x`,
|
||||
).toBeGreaterThan(0);
|
||||
|
||||
if (gap === 64) {
|
||||
// A 64px gutter affords the full 24px shoulder on both sides, so
|
||||
// desktop geometry is untouched. Pinned as a golden path because
|
||||
// the requirement is byte-identical, not merely non-crossing.
|
||||
expect(d).toBe(
|
||||
'M 172 50 H 190 Q 196 50 196 56 V 98 Q 196 104 202 104 ' +
|
||||
'H 430 Q 436 104 436 98 V 56 Q 436 50 442 50 H 460',
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
rectSpy.mockRestore();
|
||||
widthSpy.mockRestore();
|
||||
heightSpy.mockRestore();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('does not synchronously remeasure unchanged topology on task polling', () => {
|
||||
const rectSpy = vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect');
|
||||
const container = document.createElement('div');
|
||||
@@ -1342,6 +1831,275 @@ describe('PlanExecutionView', () => {
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('states the dependency on the node when no panel can state it', () => {
|
||||
// The drawn edge is aria-hidden and the cockpit passes
|
||||
// showStepDetails={false}, so there the chip row is the dependency's
|
||||
// only statement anywhere — restoring the bare !drawsDependencyEdges
|
||||
// gate turns this red.
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView
|
||||
todos={todos}
|
||||
tools={[]}
|
||||
tasks={[]}
|
||||
showStepDetails={false}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(container.querySelector('[data-plan-step-details]')).toBeNull();
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).toContain('Depends on');
|
||||
expect(buildNode?.textContent).toContain('Research');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('names the blockers in the node accessible name when the graph draws them', () => {
|
||||
// The other arm of the same gate. In the interactive graph the edges are
|
||||
// drawn and the visible chip row is off by design, but the edge layer is
|
||||
// aria-hidden — so the node's accessible name was the only place left to
|
||||
// state the dependency, and it stopped stating it: base read
|
||||
// `Blocked … Depends on: survey-api, read-tests`, head read
|
||||
// `Blocked 4 Compare findings and draft the migration plan`. A
|
||||
// screen-reader user had to activate every node to learn what blocks it.
|
||||
// Dropping the sr-only summary turns this red.
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
// showStepDetails defaults to true: the Plan & Review card and the
|
||||
// Plan & tasks dialog both render the graph that way.
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={todos} tools={[]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
// Nothing selected, so no details panel states the dependency either.
|
||||
expect(container.querySelector('[data-plan-step-details]')).toBeNull();
|
||||
expect(
|
||||
container.querySelector('[data-from="research"][data-to="build"]'),
|
||||
).not.toBeNull();
|
||||
|
||||
const buildButton = container.querySelector<HTMLButtonElement>(
|
||||
'[data-plan-node-id="build"]',
|
||||
);
|
||||
const summary = buildButton?.querySelector(`.${styles.nodeDependencyText}`);
|
||||
// A button's accessible name is its descendant text in DOM order, so this
|
||||
// span is what a screen reader appends after the step's own title.
|
||||
expect(summary?.textContent).toContain('Depends on:');
|
||||
// Human labels, matching the chips: step number plus title, not the id.
|
||||
expect(summary?.textContent).toContain('1 Research');
|
||||
expect(summary?.textContent).not.toContain('research');
|
||||
// It restores the words only — the visible row stays off, which is the
|
||||
// point of the diff.
|
||||
expect(buildButton?.querySelector(`.${styles.dependencies}`)).toBeNull();
|
||||
// And it reads after the step it belongs to, like the base name did.
|
||||
const announced = buildButton?.textContent ?? '';
|
||||
expect(announced.indexOf('Build')).toBeLessThan(
|
||||
announced.indexOf('Depends on:'),
|
||||
);
|
||||
// Sibling nodes with no blockers announce nothing extra.
|
||||
expect(
|
||||
container
|
||||
.querySelector('[data-plan-node-id="research"]')
|
||||
?.querySelector(`.${styles.nodeDependencyText}`),
|
||||
).toBeNull();
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('counts nested subagents in the node-face agent tally', () => {
|
||||
// The chip counts agents, like the execution rows beneath it and the
|
||||
// inspector's Subagents list — not just root executions. Reverting the
|
||||
// count to executions.length reads "1 agent" here and goes red.
|
||||
const rootTool: ACPToolCall = {
|
||||
...agentTool('build'),
|
||||
subTools: [
|
||||
{
|
||||
callId: 'call-nested',
|
||||
toolName: 'Agent',
|
||||
title: 'Nested agent',
|
||||
status: 'in_progress',
|
||||
},
|
||||
],
|
||||
};
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={todos} tools={[rootTool]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).toContain('2 agents');
|
||||
// The same node renders both agent rows the tally names.
|
||||
expect(buildNode?.textContent).toContain('Agent build');
|
||||
expect(buildNode?.textContent).toContain('Nested agent');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('the node-face agent tally matches the rows it renders', () => {
|
||||
// R8-2 witness: a live child task whose toolUseId matches no subTools
|
||||
// callId renders a row (via toolForNestedTask) while the transcript-only
|
||||
// tally never saw it — the node read "1 agent" beside two agent rows.
|
||||
const rootTool: ACPToolCall = {
|
||||
...agentTool('build'),
|
||||
subTools: [
|
||||
{
|
||||
callId: 'call-nested',
|
||||
toolName: 'Agent',
|
||||
title: 'Nested agent',
|
||||
status: 'in_progress',
|
||||
},
|
||||
],
|
||||
};
|
||||
const rootTask = task('running');
|
||||
const liveChild = task('running', {
|
||||
id: 'agent-live-child',
|
||||
label: 'Live child',
|
||||
toolUseId: 'call-live-child',
|
||||
parentAgentId: rootTask.id,
|
||||
});
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView
|
||||
todos={todos}
|
||||
tools={[rootTool, agentTool('research')]}
|
||||
tasks={[rootTask, liveChild]}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
// Three rows render: root, the transcript nested agent, and the live
|
||||
// child — and the tally counts the same three.
|
||||
expect(buildNode?.textContent).toContain('3 agents');
|
||||
expect(buildNode?.textContent).toContain('Nested agent');
|
||||
expect(buildNode?.textContent).toContain('Live child');
|
||||
// The lone root on the research node keeps the singular path pinned.
|
||||
const researchNode = container
|
||||
.querySelector('[data-plan-node-id="research"]')
|
||||
?.closest('article');
|
||||
expect(researchNode?.textContent).toContain('1 agent');
|
||||
expect(researchNode?.textContent).not.toContain('1 agents');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('counts an agent seen as both a live task and a transcript sub-tool once', () => {
|
||||
// The tally counts the same deduped union the rows render from, so an
|
||||
// agent observed through BOTH a live child task and a persisted subTools
|
||||
// entry is one agent, not two. Dropping the `!liveCallIds.has(…)` filter
|
||||
// reads "3 agents" beside two rows and turns this red (mutant M20).
|
||||
const rootTool: ACPToolCall = {
|
||||
...agentTool('build'),
|
||||
subTools: [
|
||||
{
|
||||
callId: 'call-nested',
|
||||
toolName: 'Agent',
|
||||
title: 'Nested agent',
|
||||
status: 'in_progress',
|
||||
},
|
||||
],
|
||||
};
|
||||
const rootTask = task('running');
|
||||
// The live child carries the same toolUseId as the sub-tool above: one
|
||||
// agent, two observations of it.
|
||||
const liveChild = task('running', {
|
||||
id: 'agent-live-child',
|
||||
label: 'Nested agent',
|
||||
toolUseId: 'call-nested',
|
||||
parentAgentId: rootTask.id,
|
||||
});
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView
|
||||
todos={todos}
|
||||
tools={[rootTool]}
|
||||
tasks={[rootTask, liveChild]}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const buildNode = container
|
||||
.querySelector('[data-plan-node-id="build"]')
|
||||
?.closest('article');
|
||||
expect(buildNode?.textContent).toContain('2 agents');
|
||||
// Root row plus one nested row: the tally and the rows agree because they
|
||||
// dedup the same way.
|
||||
expect(buildNode?.textContent).toContain('Nested agent');
|
||||
expect(
|
||||
buildNode?.querySelectorAll(`.${styles.nestedExecution}`),
|
||||
).toHaveLength(1);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('renders no agent tally for a non-agent tool tagged to the step', () => {
|
||||
// Declaring todo_id does not make a tool an agent: the tally hides
|
||||
// rather than printing "0 agents" beside the tool's row.
|
||||
const editTool: ACPToolCall = {
|
||||
callId: 'call-edit',
|
||||
toolName: 'edit',
|
||||
title: 'Edit package.json',
|
||||
status: 'completed',
|
||||
args: { todo_id: 'verify' },
|
||||
};
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<PlanExecutionView todos={todos} tools={[editTool]} tasks={[]} />
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const verifyNode = container
|
||||
.querySelector('[data-plan-node-id="verify"]')
|
||||
?.closest('article');
|
||||
expect(verifyNode?.textContent).toContain('Edit package.json');
|
||||
expect(verifyNode?.textContent).not.toContain('agent');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('skips SVG edge materialization for an excessively dense plan', () => {
|
||||
const denseTodos = Array.from(
|
||||
{ length: 33 },
|
||||
@@ -1358,6 +2116,13 @@ describe('PlanExecutionView', () => {
|
||||
),
|
||||
}),
|
||||
}),
|
||||
).map((todo) =>
|
||||
// blockedBy is model-authored and can repeat an id — or name the todo
|
||||
// itself; the chip row must dedup repeats and drop self-references
|
||||
// like the topology builder does.
|
||||
todo.id === 'dense-1'
|
||||
? { ...todo, blockedBy: ['dense-0', 'dense-0', 'dense-1'] }
|
||||
: todo,
|
||||
);
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
@@ -1372,13 +2137,33 @@ describe('PlanExecutionView', () => {
|
||||
|
||||
expect(container.querySelector('[data-plan-workflow]')).not.toBeNull();
|
||||
expect(container.querySelectorAll('[data-plan-edge]')).toHaveLength(0);
|
||||
expect(
|
||||
container.querySelectorAll('[data-plan-input], [data-plan-output]'),
|
||||
).toHaveLength(0);
|
||||
expect(container.querySelectorAll('[data-plan-output]')).toHaveLength(0);
|
||||
expect(container.textContent).toContain('Dense 32');
|
||||
// Lines disappearing with no explanation reads as a broken render, so the
|
||||
// skip is stated rather than silent.
|
||||
expect(container.textContent).toContain('Too many dependencies to draw');
|
||||
// With no edges drawn, the node's dependency row is the only statement of
|
||||
// the dependency, so it must survive here even though a node whose edges
|
||||
// ARE drawn drops it. Each reference reads as the step it names — number
|
||||
// and title, matching the inspector — not as a raw id.
|
||||
const denseNode = container
|
||||
.querySelector('[data-plan-node-id="dense-1"]')
|
||||
?.closest('article');
|
||||
expect(denseNode?.textContent).toContain('Depends on');
|
||||
expect(denseNode?.textContent).toContain('Dense 0');
|
||||
// The repeated id renders one chip, not two.
|
||||
expect((denseNode?.textContent ?? '').split('Dense 0').length - 1).toBe(1);
|
||||
// A self-reference never renders the step as its own blocker either: the
|
||||
// node's own title is the only 'Dense 1' in its text.
|
||||
expect((denseNode?.textContent ?? '').split('Dense 1').length - 1).toBe(1);
|
||||
// The truncation rule targets `.dependencyTitle`; pin the class wiring
|
||||
// so a dropped className cannot re-clip titles while text assertions
|
||||
// stay green.
|
||||
expect(
|
||||
Array.from(denseNode?.querySelectorAll('span') ?? []).some((span) =>
|
||||
span.classList.contains(styles.dependencyTitle),
|
||||
),
|
||||
).toBe(true);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
|
||||
@@ -62,6 +62,12 @@ const EMPTY_GRAPH_LAYOUT: PlanGraphLayout = {
|
||||
const EDGE_LANE_HEIGHT = 9;
|
||||
/** Corner radius where an orthogonal edge turns. */
|
||||
const EDGE_CORNER = 6;
|
||||
/**
|
||||
* Widest horizontal shoulder a layer-spanning edge runs before turning down
|
||||
* into its return lane. It is a ceiling, never a floor: a narrow gutter takes
|
||||
* half of its own run instead. See the router for why.
|
||||
*/
|
||||
const EDGE_SHOULDER = 24;
|
||||
|
||||
const MAX_RENDERED_PLAN_EDGES = 500;
|
||||
|
||||
@@ -560,6 +566,7 @@ export function PlanExecutionView({
|
||||
// rather than defeating itself on fresh array identities.
|
||||
const {
|
||||
todosById,
|
||||
stepNumberByTodo,
|
||||
toolsByTodo,
|
||||
unassigned,
|
||||
statesByTodo,
|
||||
@@ -579,6 +586,12 @@ export function PlanExecutionView({
|
||||
} = useMemo(() => {
|
||||
const knownIds = new Set(todos.map((todo) => todo.id));
|
||||
const todosById = new Map(todos.map((todo) => [todo.id, todo]));
|
||||
// The step number addresses a step in the inspector list and in the
|
||||
// dependency chips, so the graph shows the same number or the three
|
||||
// surfaces name the same step differently.
|
||||
const stepNumberByTodo = new Map(
|
||||
todos.map((todo, index) => [todo.id, index + 1]),
|
||||
);
|
||||
const toolsByTodo = new Map<string, ACPToolCall[]>();
|
||||
const unassigned: ACPToolCall[] = [];
|
||||
for (const tool of tools) {
|
||||
@@ -655,6 +668,7 @@ export function PlanExecutionView({
|
||||
}
|
||||
return {
|
||||
todosById,
|
||||
stepNumberByTodo,
|
||||
toolsByTodo,
|
||||
unassigned,
|
||||
statesByTodo,
|
||||
@@ -783,6 +797,9 @@ export function PlanExecutionView({
|
||||
? graphRect.height / graphElement.offsetHeight
|
||||
: 1;
|
||||
const measuredNodes = new Map<string, DOMRect>();
|
||||
// Horizontal extent of every topological layer, so an edge that skips
|
||||
// a layer can size its shoulders from the gutter it really crosses.
|
||||
const layerBounds = new Map<number, { left: number; right: number }>();
|
||||
let maxNodeBottom = 0;
|
||||
for (const [todoId, node] of nodeRefs.current) {
|
||||
const rect = node.getBoundingClientRect();
|
||||
@@ -797,6 +814,17 @@ export function PlanExecutionView({
|
||||
} as DOMRect;
|
||||
measuredNodes.set(todoId, normalizedRect);
|
||||
maxNodeBottom = Math.max(maxNodeBottom, normalizedRect.bottom);
|
||||
const layer = layerByTodoRef.current.get(todoId) ?? 0;
|
||||
const bounds = layerBounds.get(layer);
|
||||
if (bounds) {
|
||||
bounds.left = Math.min(bounds.left, normalizedRect.left);
|
||||
bounds.right = Math.max(bounds.right, normalizedRect.right);
|
||||
} else {
|
||||
layerBounds.set(layer, {
|
||||
left: normalizedRect.left,
|
||||
right: normalizedRect.right,
|
||||
});
|
||||
}
|
||||
}
|
||||
const edges: PlanEdgePath[] = [];
|
||||
const spanning: Array<{
|
||||
@@ -833,7 +861,16 @@ export function PlanExecutionView({
|
||||
});
|
||||
continue;
|
||||
}
|
||||
const controlX = startX + Math.max(24, (endX - startX) / 2);
|
||||
// Half the run, not the old fixed 24px floor. The ≤720px gutter
|
||||
// leaves a 24px run and the ≤480px gutter a 10px one, so a 24px
|
||||
// shoulder put the control point exactly on the end point (zero
|
||||
// tangent) or past it (negative tangent) — and `orient="auto"` then
|
||||
// flips the arrowhead back at its source, which is the only
|
||||
// direction cue left now that the input port is gone. Every gutter
|
||||
// wide enough to afford the floor already resolved to run / 2, so
|
||||
// wide lanes keep their exact curve while the end tangent's x stays
|
||||
// strictly positive at every tier.
|
||||
const controlX = startX + (endX - startX) / 2;
|
||||
edges.push({
|
||||
from: dependencyId,
|
||||
to: todoId,
|
||||
@@ -851,8 +888,40 @@ export function PlanExecutionView({
|
||||
maxNodeBottom + 14 + lane * EDGE_LANE_HEIGHT,
|
||||
Math.max(graphHeight - 6, maxNodeBottom + 14),
|
||||
);
|
||||
const dropX = edge.startX + 24;
|
||||
const riseX = edge.endX - 24;
|
||||
// Shoulders derived from the gutter each side actually has, the same
|
||||
// way the adjacent-layer edge already does it — not a fixed 24px. The
|
||||
// ≤480px gutter is 18px, so a 24px shoulder left a 10px run and put
|
||||
// the vertical segment inside the intervening lane: a dependency from
|
||||
// step 3 to step 5 looked like it entered step 4, and the SVG paints
|
||||
// under the nodes so the lane just vanished into it. Halving the run
|
||||
// centres the segment in its own gutter. EDGE_SHOULDER still wins
|
||||
// wherever the gutter can afford it (the 64px desktop tier has a 56px
|
||||
// run), so wide lanes keep the exact curve they had.
|
||||
const sourceLayer = layerByTodoRef.current.get(edge.from) ?? 0;
|
||||
const targetLayer = layerByTodoRef.current.get(edge.to) ?? 0;
|
||||
// An unmeasured neighbouring layer falls back to the full shoulder.
|
||||
const nextLayerLeft =
|
||||
layerBounds.get(sourceLayer + 1)?.left ?? Number.POSITIVE_INFINITY;
|
||||
const prevLayerRight =
|
||||
layerBounds.get(targetLayer - 1)?.right ?? Number.NEGATIVE_INFINITY;
|
||||
const dropShoulder = Math.min(
|
||||
EDGE_SHOULDER,
|
||||
(nextLayerLeft - 4 - edge.startX) / 2,
|
||||
);
|
||||
const riseShoulder = Math.min(
|
||||
EDGE_SHOULDER,
|
||||
(edge.endX - prevLayerRight - 4) / 2,
|
||||
);
|
||||
// The corner has to fit inside both shoulders. At 18px the shoulder
|
||||
// is 5, so the unhalved 6px radius left a zero-length final segment
|
||||
// and the arrowhead lost its direction again.
|
||||
const corner = Math.min(
|
||||
EDGE_CORNER,
|
||||
dropShoulder / 2,
|
||||
riseShoulder / 2,
|
||||
);
|
||||
const dropX = edge.startX + dropShoulder;
|
||||
const riseX = edge.endX - riseShoulder;
|
||||
const down = routeY > edge.startY ? 1 : -1;
|
||||
const up = edge.endY > routeY ? 1 : -1;
|
||||
edges.push({
|
||||
@@ -860,14 +929,14 @@ export function PlanExecutionView({
|
||||
to: edge.to,
|
||||
d:
|
||||
`M ${edge.startX} ${edge.startY} ` +
|
||||
`H ${dropX - EDGE_CORNER} ` +
|
||||
`Q ${dropX} ${edge.startY} ${dropX} ${edge.startY + EDGE_CORNER * down} ` +
|
||||
`V ${routeY - EDGE_CORNER * down} ` +
|
||||
`Q ${dropX} ${routeY} ${dropX + EDGE_CORNER} ${routeY} ` +
|
||||
`H ${riseX - EDGE_CORNER} ` +
|
||||
`Q ${riseX} ${routeY} ${riseX} ${routeY + EDGE_CORNER * up} ` +
|
||||
`V ${edge.endY - EDGE_CORNER * up} ` +
|
||||
`Q ${riseX} ${edge.endY} ${riseX + EDGE_CORNER} ${edge.endY} ` +
|
||||
`H ${dropX - corner} ` +
|
||||
`Q ${dropX} ${edge.startY} ${dropX} ${edge.startY + corner * down} ` +
|
||||
`V ${routeY - corner * down} ` +
|
||||
`Q ${dropX} ${routeY} ${dropX + corner} ${routeY} ` +
|
||||
`H ${riseX - corner} ` +
|
||||
`Q ${riseX} ${routeY} ${riseX} ${routeY + corner * up} ` +
|
||||
`V ${edge.endY - corner * up} ` +
|
||||
`Q ${riseX} ${edge.endY} ${riseX + corner} ${edge.endY} ` +
|
||||
`H ${edge.endX}`,
|
||||
});
|
||||
});
|
||||
@@ -936,6 +1005,12 @@ export function PlanExecutionView({
|
||||
const selectedDependents = selectedTodo
|
||||
? (dependentsByTodo.get(selectedTodo.id) ?? [])
|
||||
: [];
|
||||
// The same filtered projection the edges draw from: the topology builder
|
||||
// drops ids that name no step (and self-references), so no control here
|
||||
// can select a ghost id and hide this panel mid-navigation.
|
||||
const selectedDependencies = selectedTodo
|
||||
? (dependencyIdsByTodo.get(selectedTodo.id) ?? [])
|
||||
: [];
|
||||
const detailsId = `plan-step-details-${graphId}`;
|
||||
const overallProgressId = `plan-overall-progress-${graphId}`;
|
||||
|
||||
@@ -1183,7 +1258,12 @@ export function PlanExecutionView({
|
||||
ref={hasDependencies ? graphRef : undefined}
|
||||
style={
|
||||
hasDependencies
|
||||
? ({ '--plan-edge-lanes': graph.lanes } as CSSProperties)
|
||||
? ({
|
||||
'--plan-edge-lanes': graph.lanes,
|
||||
// Publish the lane pitch so .dagCanvas reserves bottom
|
||||
// padding from the same constant that places the lanes.
|
||||
'--plan-edge-lane-height': `${EDGE_LANE_HEIGHT}px`,
|
||||
} as CSSProperties)
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
@@ -1254,10 +1334,63 @@ export function PlanExecutionView({
|
||||
{layer.map((todo) => {
|
||||
const executions = toolsByTodo.get(todo.id) ?? [];
|
||||
const state = statesByTodo.get(todo.id)!;
|
||||
// Agent time this step has taken, summed across its root
|
||||
// agent tasks. It is the node's "is this alive" signal, so it
|
||||
// is on the face rather than only in the inspector.
|
||||
const nodeRuntimeMs = executions.reduce(
|
||||
(total, tool) =>
|
||||
total + (taskForTool(tool, taskIndex)?.runtimeMs ?? 0),
|
||||
0,
|
||||
);
|
||||
// Agents, not bare executions: a nested subagent counts too,
|
||||
// matching the rows this node renders and the inspector's
|
||||
// Subagents list for the same step. The runtime above stays
|
||||
// on roots, so nested time is not summed twice. Count from
|
||||
// the same two sources renderExecution draws the rows from —
|
||||
// live child tasks from the task index plus transcript
|
||||
// subTools, deduped by toolUseId exactly as the rows are —
|
||||
// otherwise a live child task with no transcript entry
|
||||
// renders a row the count never sees.
|
||||
const agentCount = executions.reduce((count, tool) => {
|
||||
const liveNested = nestedTasksFromIndex(tool, taskIndex);
|
||||
const liveCallIds = new Set(
|
||||
liveNested.flatMap(({ task }) =>
|
||||
task.toolUseId ? [task.toolUseId] : [],
|
||||
),
|
||||
);
|
||||
const transcriptOnly = nestedAgentToolsForTool(tool).filter(
|
||||
({ tool: nested }) => !liveCallIds.has(nested.callId),
|
||||
);
|
||||
return (
|
||||
count +
|
||||
(isSubAgentToolCall(tool) ? 1 : 0) +
|
||||
liveNested.length +
|
||||
transcriptOnly.length
|
||||
);
|
||||
}, 0);
|
||||
// blockedBy is model-authored and can repeat an id — or name
|
||||
// the todo itself. Dedup and drop self-references like the
|
||||
// topology builder; ghost ids stay, because above the edge
|
||||
// budget this row is the dependency's only statement.
|
||||
const faceDependencies = [
|
||||
...new Set(todo.blockedBy ?? []),
|
||||
].filter((id) => id !== todo.id);
|
||||
// Whether the visible chip row carries the dependency. It is
|
||||
// rendered whenever nothing else can: drawn edges are
|
||||
// aria-hidden, so they state it only visually; above
|
||||
// MAX_RENDERED_PLAN_EDGES no edges draw at all; and with the
|
||||
// details panel off (the cockpit) or selection disabled
|
||||
// (document mode) the panel never states it either. When this
|
||||
// is false the sr-only summary below carries the same fact to
|
||||
// assistive tech instead, so the dependency is stated exactly
|
||||
// once either way.
|
||||
const statesDependenciesVisibly =
|
||||
!drawsDependencyEdges || !showStepDetails || documentMode;
|
||||
return (
|
||||
<article
|
||||
className={styles.node}
|
||||
data-status={state.status}
|
||||
data-attention={state.attention || undefined}
|
||||
onPointerEnter={() => setHoveredTodoId(todo.id)}
|
||||
onPointerLeave={() =>
|
||||
setHoveredTodoId((current) =>
|
||||
@@ -1272,11 +1405,11 @@ export function PlanExecutionView({
|
||||
current === todo.id ? undefined : current,
|
||||
)
|
||||
}
|
||||
data-plan-input={
|
||||
(drawsDependencyEdges &&
|
||||
(dependencyIdsByTodo.get(todo.id)?.length ?? 0) > 0) ||
|
||||
undefined
|
||||
}
|
||||
// No input port: the left edge now carries the status
|
||||
// rule, and an incoming edge already terminates in an
|
||||
// arrowhead at the node — that arrowhead is the input
|
||||
// marker. Outgoing edges leave their source unmarked, so
|
||||
// the output port stays.
|
||||
data-plan-output={
|
||||
(drawsDependencyEdges &&
|
||||
(dependentsByTodo.get(todo.id)?.length ?? 0) > 0) ||
|
||||
@@ -1317,29 +1450,96 @@ export function PlanExecutionView({
|
||||
}
|
||||
disabled={documentMode}
|
||||
>
|
||||
{/* Status reaches assistive tech as words; the left
|
||||
rule that carries it visually is colour only.
|
||||
Attention is announced beside the status word,
|
||||
never instead of it. */}
|
||||
<span className={styles.nodeStatusText}>
|
||||
{t(statusKey(state.status))}
|
||||
{state.attention
|
||||
? `, ${t('planExecution.attention')}`
|
||||
: ''}
|
||||
</span>
|
||||
<div className={styles.nodeTop}>
|
||||
<span className={styles.nodeNumber}>
|
||||
{(stepNumberByTodo.get(todo.id) ?? 0) || ''}
|
||||
</span>
|
||||
<span className={styles.nodeContent}>
|
||||
{todo.content}
|
||||
</span>
|
||||
</div>
|
||||
<div className={styles.nodeMeta}>
|
||||
{/* The glyph is the non-colour status channel, kept
|
||||
for every status so the graph still survives
|
||||
colour-blindness, high-contrast mode and a
|
||||
greyscale screenshot. It moved off the first line
|
||||
so the step's content leads, and it is muted so
|
||||
the left rule remains the only carrier of the
|
||||
status *colour*. */}
|
||||
<i aria-hidden="true" className={styles.nodeGlyph}>
|
||||
{PLAN_STATUS_GLYPH[state.status]}
|
||||
</i>
|
||||
<span className={styles.nodeId}>{todo.id}</span>
|
||||
<span
|
||||
className={`${styles.nodeStatus} ${styles[state.status]}`}
|
||||
>
|
||||
{t(statusKey(state.status))}
|
||||
</span>
|
||||
{/* Attention's shape channel: on a paused node the
|
||||
data-attention rule re-declares the token paused
|
||||
already wears, so colour alone cannot tell it
|
||||
apart from healthy. */}
|
||||
{state.attention && (
|
||||
<span className={styles.attention}>
|
||||
{t('planExecution.attention')}
|
||||
<i
|
||||
aria-hidden="true"
|
||||
className={styles.nodeAttentionMark}
|
||||
>
|
||||
!
|
||||
</i>
|
||||
)}
|
||||
{agentCount > 0 && (
|
||||
<span>
|
||||
{t('planExecution.agentCount', {
|
||||
count: agentCount,
|
||||
})}
|
||||
</span>
|
||||
)}
|
||||
{nodeRuntimeMs > 0 && (
|
||||
<span>{formatRuntime(nodeRuntimeMs)}</span>
|
||||
)}
|
||||
</div>
|
||||
<div className={styles.nodeContent}>{todo.content}</div>
|
||||
{(todo.blockedBy?.length ?? 0) > 0 && (
|
||||
<div className={styles.dependencies}>
|
||||
{t('planExecution.dependsOn')}{' '}
|
||||
{todo.blockedBy!.join(', ')}
|
||||
</div>
|
||||
)}
|
||||
{statesDependenciesVisibly &&
|
||||
faceDependencies.length > 0 && (
|
||||
<div className={styles.dependencies}>
|
||||
<span>{t('planExecution.dependsOn')}</span>
|
||||
{faceDependencies.map((id) => (
|
||||
<span className={styles.dependencyChip} key={id}>
|
||||
<span>
|
||||
{(stepNumberByTodo.get(id) ?? 0) || '?'}
|
||||
</span>
|
||||
<span className={styles.dependencyTitle}>
|
||||
{todosById.get(id)?.content ?? id}
|
||||
</span>
|
||||
</span>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{/* The interactive graph draws the dependency instead
|
||||
of stating it, and drawn edges are aria-hidden — so
|
||||
without this the node's accessible name stops
|
||||
naming its blockers and a screen-reader user has to
|
||||
activate every node to find out what blocks it.
|
||||
Same sr-only channel as the status word, same
|
||||
step-number-plus-title labels as the chips, and it
|
||||
never brings the visible row back. */}
|
||||
{!statesDependenciesVisibly &&
|
||||
faceDependencies.length > 0 && (
|
||||
<span className={styles.nodeDependencyText}>
|
||||
{t('planExecution.dependsOn')}{' '}
|
||||
{faceDependencies
|
||||
.map(
|
||||
(id) =>
|
||||
`${(stepNumberByTodo.get(id) ?? 0) || '?'} ${
|
||||
todosById.get(id)?.content ?? id
|
||||
}`,
|
||||
)
|
||||
.join(', ')}
|
||||
</span>
|
||||
)}
|
||||
</button>
|
||||
{executions.length > 0 && (
|
||||
<div className={styles.executions}>
|
||||
@@ -1375,15 +1575,50 @@ export function PlanExecutionView({
|
||||
)}
|
||||
</div>
|
||||
<div className={styles.nodeContent}>{selectedTodo.content}</div>
|
||||
{(selectedTodo.blockedBy?.length ?? 0) > 0 && (
|
||||
{/* This panel is outside the node's own button, so unlike the
|
||||
chips on the node face these references can be controls: each
|
||||
one selects the step it names, which is what makes the
|
||||
dependency list the graph's navigation. */}
|
||||
{selectedDependencies.length > 0 && (
|
||||
<div className={styles.dependencies}>
|
||||
{t('planExecution.dependsOn')}{' '}
|
||||
{selectedTodo.blockedBy!.join(', ')}
|
||||
<span>{t('planExecution.dependsOn')}</span>
|
||||
{selectedDependencies.map((id) => (
|
||||
<button
|
||||
className={styles.dependencyLink}
|
||||
data-plan-interactive
|
||||
data-plan-dependency={id}
|
||||
key={id}
|
||||
onClick={() => updateSelectedTodoId(id)}
|
||||
title={todosById.get(id)?.content}
|
||||
type="button"
|
||||
>
|
||||
<span>{(stepNumberByTodo.get(id) ?? 0) || '?'}</span>
|
||||
<span className={styles.dependencyTitle}>
|
||||
{todosById.get(id)?.content ?? id}
|
||||
</span>
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{selectedDependents.length > 0 && (
|
||||
<div className={styles.dependencies}>
|
||||
{t('planExecution.unblocks')} {selectedDependents.join(', ')}
|
||||
<span>{t('planExecution.unblocks')}</span>
|
||||
{selectedDependents.map((id) => (
|
||||
<button
|
||||
className={styles.dependencyLink}
|
||||
data-plan-interactive
|
||||
data-plan-dependency={id}
|
||||
key={id}
|
||||
onClick={() => updateSelectedTodoId(id)}
|
||||
title={todosById.get(id)?.content}
|
||||
type="button"
|
||||
>
|
||||
<span>{(stepNumberByTodo.get(id) ?? 0) || '?'}</span>
|
||||
<span className={styles.dependencyTitle}>
|
||||
{todosById.get(id)?.content ?? id}
|
||||
</span>
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{selectedExecutions.length > 0 && (
|
||||
|
||||
@@ -47,12 +47,31 @@
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.identity span {
|
||||
color: var(--agent-blue-500);
|
||||
font-size: 10px;
|
||||
font-weight: 800;
|
||||
letter-spacing: 0.1em;
|
||||
text-transform: uppercase;
|
||||
/* The accent is the selection colour the graph below uses to say "this node".
|
||||
Spending it on a fixed label above the title made the one blue thing on the
|
||||
page the one thing that never changes, so the label drops to the subtitle
|
||||
line and the accent is left to mean selection. */
|
||||
.identity small {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
align-items: baseline;
|
||||
gap: 2px 10px;
|
||||
}
|
||||
|
||||
.identity small > span {
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.workspaceName {
|
||||
max-width: 28ch;
|
||||
}
|
||||
|
||||
/* The short session id is an address the user copies or matches against a
|
||||
daemon log, so it keeps the mono face the rest of the line does not need. */
|
||||
.sessionRef {
|
||||
font-family: var(--font-mono, monospace);
|
||||
}
|
||||
|
||||
/* preflight.css resets headings to `font: inherit`, so size and weight both
|
||||
@@ -60,7 +79,7 @@
|
||||
`min-width: 0` already caps the width; a fixed max-width only stranded space
|
||||
on wide viewports. */
|
||||
.identity h1 {
|
||||
margin: 4px 0 3px;
|
||||
margin: 0 0 3px;
|
||||
overflow: hidden;
|
||||
font-size: clamp(20px, 2.2vw, 28px);
|
||||
font-weight: 600;
|
||||
@@ -71,7 +90,6 @@
|
||||
|
||||
.identity small {
|
||||
color: var(--muted-foreground);
|
||||
font-family: var(--font-mono, monospace);
|
||||
font-size: 11px;
|
||||
}
|
||||
|
||||
|
||||
@@ -88,14 +88,19 @@ export function SessionWorkflowCockpit({
|
||||
{t('workflow.chatTitle')}
|
||||
</button>
|
||||
<div>
|
||||
<span>{t('workflow.inspector.graphCanvas')}</span>
|
||||
<h1 title={sessionName}>
|
||||
{sessionName || t('workflow.session.defaultTitle')}
|
||||
</h1>
|
||||
<small>
|
||||
{sessionId.slice(0, 8)} ·{' '}
|
||||
{workspaceCwd?.split('/').at(-1) ||
|
||||
t('workflow.session.workspace')}
|
||||
<span>{t('workflow.inspector.graphCanvas')}</span>
|
||||
<span
|
||||
className={styles.workspaceName}
|
||||
title={workspaceCwd || undefined}
|
||||
>
|
||||
{workspaceCwd?.split('/').at(-1) ||
|
||||
t('workflow.session.workspace')}
|
||||
</span>
|
||||
<span className={styles.sessionRef}>{sessionId.slice(0, 8)}</span>
|
||||
</small>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -29,4 +29,44 @@ describe('SessionWorkflowInspector stylesheet', () => {
|
||||
expect(inspectorCss).not.toMatch(/\.summaryHeading\s+span/);
|
||||
expect(inspectorCss).toMatch(/\.summaryHeading\s*>\s*div\s+span/);
|
||||
});
|
||||
|
||||
// `.activityList > button` (0,1,1) declares the four-column activity grid
|
||||
// for every direct-child button. A control styled with a bare class (0,1,0)
|
||||
// loses to it and has its label auto-placed into the 52px first column,
|
||||
// wrapping to three or four lines with three empty columns beside it —
|
||||
// invisible to jsdom, to the visuals suite and to a green unit run, so it
|
||||
// is pinned here at the source.
|
||||
it('lets the show-all control out-specify the activity grid', () => {
|
||||
expect(inspectorCss).toMatch(
|
||||
/\.activityList\s*>\s*button\.showAllActivity\s*\{/,
|
||||
);
|
||||
expect(inspectorCss).not.toMatch(/(^|\n)\.showAllActivity\s*\{/);
|
||||
const rule = inspectorCss.match(
|
||||
/\.activityList\s*>\s*button\.showAllActivity\s*\{[^}]*\}/,
|
||||
)?.[0];
|
||||
// It must actually undo the inherited grid, not merely win the cascade.
|
||||
expect(rule).toMatch(/display:\s*block/);
|
||||
expect(rule).toMatch(/grid-template-columns:\s*none/);
|
||||
});
|
||||
|
||||
// Same collision, already fixed for the wrapping metrics row: keep both
|
||||
// pinned so neither regresses to a bare class.
|
||||
it('lets the metrics row out-specify its single-line row styles', () => {
|
||||
expect(inspectorCss).toMatch(/\.linkedAgents\s+button\s+small\.metrics/);
|
||||
expect(inspectorCss).toMatch(/\.deliverables\s+button\s+small\.metrics/);
|
||||
});
|
||||
|
||||
// The dependency chip's parent <li> is the flex item of .dependencyList;
|
||||
// its default min-width: auto floored it at the title's full nowrap
|
||||
// width, so the chip grew out of its column and the title's ellipsis
|
||||
// never engaged. jsdom computes no layout, so the shrink floor is pinned
|
||||
// at the source.
|
||||
it('lets the dependency list item shrink so the chip ellipsis engages', () => {
|
||||
const rule = inspectorCss.match(
|
||||
/\.dependencyList\s*>\s*li\s*\{[^}]*\}/,
|
||||
)?.[0];
|
||||
expect(rule).toBeTruthy();
|
||||
expect(rule).toMatch(/min-width:\s*0/);
|
||||
expect(rule).toMatch(/max-width:\s*100%/);
|
||||
});
|
||||
});
|
||||
@@ -49,35 +49,39 @@
|
||||
gap: 3px;
|
||||
}
|
||||
|
||||
/* Section labels name what follows; they are not decoration. Tracked-out
|
||||
uppercase made every one of them shout over the content it introduces, so
|
||||
they read as chrome. Sentence case at a muted weight puts the emphasis back
|
||||
on the step title and the status beside it. Kept scoped to the heading's
|
||||
inner div so the count beside it keeps its own tone. */
|
||||
.summaryHeading > div span,
|
||||
.sectionHeading > div > span {
|
||||
color: var(--muted-foreground);
|
||||
font-size: 11px;
|
||||
font-weight: 700;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
font-weight: 500;
|
||||
}
|
||||
|
||||
.summaryHeading strong {
|
||||
font-size: 16px;
|
||||
}
|
||||
|
||||
/* Completed-over-total is a reading of the progress bar directly beneath it,
|
||||
and the status word sits to its left, so a third tinted surface said the
|
||||
same thing a third time. The count keeps the tone as ink and drops the
|
||||
pill. */
|
||||
.summaryCount {
|
||||
padding: 5px 8px;
|
||||
border-radius: 999px;
|
||||
color: var(--status-running-fg);
|
||||
background: var(--status-running-bg);
|
||||
font-size: 12px;
|
||||
font-size: 13px;
|
||||
font-variant-numeric: tabular-nums;
|
||||
font-weight: 600;
|
||||
}
|
||||
|
||||
.summaryCount[data-status='attention'] {
|
||||
color: var(--status-attention-fg);
|
||||
background: var(--status-attention-bg);
|
||||
}
|
||||
|
||||
.summaryCount[data-status='completed'] {
|
||||
color: var(--status-done-fg);
|
||||
background: var(--status-done-bg);
|
||||
}
|
||||
|
||||
.progress {
|
||||
@@ -281,6 +285,92 @@
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
/* A dependency is an edge in the graph beside it, so it is rendered as the
|
||||
step it points at — number and title — and selects that step on click. The
|
||||
previous comma-joined id list made the reader find the row by eye. */
|
||||
.dependencyList {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: 4px;
|
||||
margin: 0;
|
||||
padding: 0;
|
||||
list-style: none;
|
||||
}
|
||||
|
||||
/* The chip's parent <li> is the flex item, and its default min-width: auto
|
||||
floors it at the title's full nowrap width — the chip grew out of its
|
||||
column and the title's ellipsis never engaged. The number badge is kept
|
||||
out of the shrink by its own flex: none. */
|
||||
.dependencyList > li {
|
||||
min-width: 0;
|
||||
max-width: 100%;
|
||||
}
|
||||
|
||||
.dependencyLink {
|
||||
display: flex;
|
||||
max-width: 100%;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
padding: 3px 7px 3px 4px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md);
|
||||
cursor: pointer;
|
||||
font-size: 11px;
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
.dependencyLink > span:last-child {
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.dependencyLink:hover {
|
||||
border-color: color-mix(in srgb, var(--agent-blue-500) 45%, var(--border));
|
||||
background: var(--accent);
|
||||
}
|
||||
|
||||
.dependencyNumber {
|
||||
display: grid;
|
||||
width: 16px;
|
||||
height: 16px;
|
||||
flex: none;
|
||||
place-items: center;
|
||||
border-radius: var(--radius-sm, 4px);
|
||||
color: var(--muted-foreground);
|
||||
background: var(--muted);
|
||||
font-size: 10px;
|
||||
font-variant-numeric: tabular-nums;
|
||||
}
|
||||
|
||||
/* The summary beside the list reports the true total, so the preview cap
|
||||
needs a way out of it rather than a silent truncation.
|
||||
|
||||
Specificity note, same class of collision as `.metrics` above: this button
|
||||
is a DIRECT CHILD of `.activityList`, whose base rule
|
||||
`.activityList > button` (0,1,1) declares the four-column activity grid
|
||||
(`52px 26px minmax(0, 1fr) auto`) and `padding: 7px 4px`. A single-class
|
||||
rule loses to it, and the label — a bare text run that Grid wraps into an
|
||||
anonymous item — would auto-place into the 52px first column and wrap to
|
||||
three or four lines with three empty columns beside it. So this rule
|
||||
carries the same `.activityList >` prefix, and resets `display` and
|
||||
`grid-template-columns` explicitly rather than relying on order. */
|
||||
.activityList > button.showAllActivity {
|
||||
display: block;
|
||||
grid-template-columns: none;
|
||||
padding: 7px 8px;
|
||||
border-radius: var(--radius-md);
|
||||
color: var(--muted-foreground);
|
||||
cursor: pointer;
|
||||
font-size: 11px;
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
.activityList > button.showAllActivity:hover {
|
||||
color: var(--foreground);
|
||||
background: var(--accent);
|
||||
}
|
||||
|
||||
.linkedAgents {
|
||||
margin-top: 14px;
|
||||
padding-top: 12px;
|
||||
@@ -436,6 +526,22 @@
|
||||
font-size: 11px;
|
||||
}
|
||||
|
||||
/* Runtime, tool calls and tokens are separate facts, so they are separate
|
||||
elements with real spacing rather than one string glued with middle dots —
|
||||
which also let them wrap as a unit and be read out as one run of text. */
|
||||
/* Specificity note: `.linkedAgents button small` (0,1,2) and
|
||||
`.deliverables small` (0,1,1) both declare the single-line ellipsis shape
|
||||
these rows use, so the wrapping metrics row has to out-specify them rather
|
||||
than rely on source order. */
|
||||
.linkedAgents button small.metrics,
|
||||
.deliverables button small.metrics {
|
||||
display: flex;
|
||||
overflow: visible;
|
||||
flex-wrap: wrap;
|
||||
gap: 2px 10px;
|
||||
white-space: normal;
|
||||
}
|
||||
|
||||
.stateLabel {
|
||||
color: var(--muted-foreground);
|
||||
font-size: 11px;
|
||||
|
||||
@@ -223,4 +223,242 @@ describe('SessionWorkflowInspector', () => {
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('selects the referenced step from a dependency, by number and title', () => {
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
const onSelectedTodoIdChange = vi.fn();
|
||||
const todos: TodoItem[] = [
|
||||
{ id: 'prepare', content: 'Prepare inputs', status: 'completed' },
|
||||
{
|
||||
id: 'ship',
|
||||
content: 'Ship result',
|
||||
status: 'in_progress',
|
||||
blockedBy: ['prepare'],
|
||||
},
|
||||
];
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<SessionWorkflowInspector
|
||||
todos={todos}
|
||||
tools={[]}
|
||||
tasks={[]}
|
||||
artifacts={[]}
|
||||
selectedTodoId="ship"
|
||||
onSelectedTodoIdChange={onSelectedTodoIdChange}
|
||||
onExpandGraph={vi.fn()}
|
||||
onOpenSubagent={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const upstream = container.querySelector<HTMLButtonElement>(
|
||||
'[data-testid="workflow-dependency-prepare"]',
|
||||
);
|
||||
// The reference reads as the step it points at — the step number the list
|
||||
// and graph both show, plus its title — not the bare `prepare` id.
|
||||
expect(upstream).toBeTruthy();
|
||||
expect(upstream?.textContent).toContain('1');
|
||||
expect(upstream?.textContent).toContain('Prepare inputs');
|
||||
// The host keyboard handlers isolate plan controls through this marker
|
||||
// (TasksStatusMessage/ToolApproval early-return on it), so these
|
||||
// controls must carry it like the graph's own buttons do.
|
||||
expect(upstream?.hasAttribute('data-plan-interactive')).toBe(true);
|
||||
|
||||
onSelectedTodoIdChange.mockClear();
|
||||
act(() => upstream?.click());
|
||||
expect(onSelectedTodoIdChange).toHaveBeenCalledWith('prepare');
|
||||
|
||||
// Downstream too: the two directions render through the same helper but
|
||||
// read from different sources (`blockedBy` vs the projection's
|
||||
// `dependentsByTodo`), so covering one does not cover the other.
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<SessionWorkflowInspector
|
||||
todos={todos}
|
||||
tools={[]}
|
||||
tasks={[]}
|
||||
artifacts={[]}
|
||||
selectedTodoId="prepare"
|
||||
onSelectedTodoIdChange={onSelectedTodoIdChange}
|
||||
onExpandGraph={vi.fn()}
|
||||
onOpenSubagent={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
const downstream = container.querySelector<HTMLButtonElement>(
|
||||
'[data-testid="workflow-dependency-ship"]',
|
||||
);
|
||||
expect(downstream?.textContent).toContain('2');
|
||||
expect(downstream?.textContent).toContain('Ship result');
|
||||
expect(downstream?.hasAttribute('data-plan-interactive')).toBe(true);
|
||||
onSelectedTodoIdChange.mockClear();
|
||||
act(() => downstream?.click());
|
||||
expect(onSelectedTodoIdChange).toHaveBeenCalledWith('ship');
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('dedups a repeated dependency id in the upstream list', () => {
|
||||
// blockedBy is model-authored and can repeat an id; without the dedup
|
||||
// the list rendered two links sharing one key.
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
const todos: TodoItem[] = [
|
||||
{ id: 'prepare', content: 'Prepare inputs', status: 'completed' },
|
||||
{
|
||||
id: 'ship',
|
||||
content: 'Ship result',
|
||||
status: 'in_progress',
|
||||
blockedBy: ['prepare', 'prepare'],
|
||||
},
|
||||
];
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<SessionWorkflowInspector
|
||||
todos={todos}
|
||||
tools={[]}
|
||||
tasks={[]}
|
||||
artifacts={[]}
|
||||
selectedTodoId="ship"
|
||||
onSelectedTodoIdChange={vi.fn()}
|
||||
onExpandGraph={vi.fn()}
|
||||
onOpenSubagent={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(
|
||||
container.querySelectorAll('[data-testid="workflow-dependency-prepare"]'),
|
||||
).toHaveLength(1);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('drops a self-reference from the upstream list', () => {
|
||||
// The projection's dependentsByTodo builder skips self-blocks
|
||||
// (session-workflow-model.ts), and the DAG draws no edge for them; the
|
||||
// inspector's upstream filter must apply the same rule, or a todo
|
||||
// naming itself in blockedBy renders a chip that only re-selects the
|
||||
// already-selected step.
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
const todos: TodoItem[] = [
|
||||
{ id: 'prepare', content: 'Prepare inputs', status: 'completed' },
|
||||
{
|
||||
id: 'ship',
|
||||
content: 'Ship result',
|
||||
status: 'in_progress',
|
||||
blockedBy: ['ship', 'prepare'],
|
||||
},
|
||||
];
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<SessionWorkflowInspector
|
||||
todos={todos}
|
||||
tools={[]}
|
||||
tasks={[]}
|
||||
artifacts={[]}
|
||||
selectedTodoId="ship"
|
||||
onSelectedTodoIdChange={vi.fn()}
|
||||
onExpandGraph={vi.fn()}
|
||||
onOpenSubagent={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(
|
||||
container.querySelector('[data-testid="workflow-dependency-ship"]'),
|
||||
).toBeNull();
|
||||
expect(
|
||||
container.querySelectorAll('[data-testid="workflow-dependency-prepare"]'),
|
||||
).toHaveLength(1);
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it('keeps every activity row reachable past the preview cap', () => {
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
const todos: TodoItem[] = [
|
||||
{ id: 'build', content: 'Build', status: 'in_progress' },
|
||||
];
|
||||
const tools: ACPToolCall[] = Array.from({ length: 8 }, (_, index) => ({
|
||||
callId: `agent-${index}`,
|
||||
toolName: 'Agent',
|
||||
title: `Agent ${index}`,
|
||||
status: 'completed' as const,
|
||||
args: { todo_id: 'build' },
|
||||
}));
|
||||
const tasks = tools.map((tool, index) => ({
|
||||
kind: 'agent' as const,
|
||||
id: `task-${index}`,
|
||||
label: `Agent ${index}`,
|
||||
description: `Ran step ${index}`,
|
||||
status: 'completed' as const,
|
||||
startTime: 1_000 + index,
|
||||
endTime: 2_000 + index,
|
||||
runtimeMs: 1_000,
|
||||
isBackgrounded: true,
|
||||
toolUseId: tool.callId,
|
||||
}));
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<I18nProvider language="en">
|
||||
<SessionWorkflowInspector
|
||||
todos={todos}
|
||||
tools={tools}
|
||||
tasks={tasks}
|
||||
artifacts={[]}
|
||||
onSelectedTodoIdChange={vi.fn()}
|
||||
onExpandGraph={vi.fn()}
|
||||
onOpenSubagent={vi.fn()}
|
||||
/>
|
||||
</I18nProvider>,
|
||||
);
|
||||
});
|
||||
|
||||
const activityList = () =>
|
||||
Array.from(container.querySelectorAll('button')).filter((button) =>
|
||||
/^\d{1,2}:\d{2}/.test(button.textContent?.trim() ?? ''),
|
||||
);
|
||||
const beforeExpand = activityList().length;
|
||||
const showAll = container.querySelector<HTMLButtonElement>(
|
||||
'[data-testid="workflow-activity-show-all"]',
|
||||
);
|
||||
// The count beside the list reports the true total, so the cap has to be
|
||||
// an invitation rather than a silent truncation.
|
||||
expect(beforeExpand).toBe(6);
|
||||
expect(showAll?.textContent).toContain('8');
|
||||
// Same host-keyboard isolation as the dependency links above.
|
||||
expect(showAll?.hasAttribute('data-plan-interactive')).toBe(true);
|
||||
|
||||
act(() => showAll?.click());
|
||||
expect(activityList().length).toBe(8);
|
||||
expect(
|
||||
container.querySelector('[data-testid="workflow-activity-show-all"]'),
|
||||
).toBeNull();
|
||||
|
||||
act(() => root.unmount());
|
||||
container.remove();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useMemo } from 'react';
|
||||
import { useEffect, useMemo, useState } from 'react';
|
||||
import type {
|
||||
DaemonSessionArtifact,
|
||||
DaemonSessionTaskStatus,
|
||||
@@ -22,6 +22,12 @@ import {
|
||||
} from './session-workflow-model';
|
||||
import styles from './SessionWorkflowInspector.module.css';
|
||||
|
||||
/**
|
||||
* Activity rows shown before the list asks to be expanded. The summary beside
|
||||
* it reports the true total, so the remainder must stay reachable.
|
||||
*/
|
||||
const ACTIVITY_PREVIEW_COUNT = 6;
|
||||
|
||||
export interface SessionWorkflowInspectorProps {
|
||||
todos: readonly TodoItem[];
|
||||
tools: readonly ACPToolCall[];
|
||||
@@ -53,6 +59,15 @@ export function SessionWorkflowInspector({
|
||||
[tasks, todos, tools],
|
||||
);
|
||||
const defaultTodoId = getDefaultWorkflowTodoId(todos, projection);
|
||||
// Dependencies are stated as Todo ids, which are addresses, not labels. The
|
||||
// step number is what the list and the graph both show, so carry it into
|
||||
// every dependency reference.
|
||||
const stepNumberById = useMemo(() => {
|
||||
const numbers = new Map<string, number>();
|
||||
todos.forEach((todo, index) => numbers.set(todo.id, index + 1));
|
||||
return numbers;
|
||||
}, [todos]);
|
||||
const [showAllActivity, setShowAllActivity] = useState(false);
|
||||
const effectiveSelectedTodoId = projection.todosById.has(selectedTodoId ?? '')
|
||||
? selectedTodoId
|
||||
: defaultTodoId;
|
||||
@@ -84,9 +99,16 @@ export function SessionWorkflowInspector({
|
||||
const selectedTools = selectedTodo
|
||||
? (projection.agentToolsByTodo.get(selectedTodo.id) ?? [])
|
||||
: [];
|
||||
const upstream = selectedTodo?.blockedBy?.filter((id) =>
|
||||
projection.todosById.has(id),
|
||||
);
|
||||
// blockedBy is model-authored and can repeat an id — or name the todo
|
||||
// itself. Dedup before mapping so a repeated reference cannot emit a
|
||||
// duplicate key or a second link, and drop self-references with the same
|
||||
// rule the projection's dependentsByTodo builder applies, so the
|
||||
// inspector never shows a step blocked by itself.
|
||||
const upstream = selectedTodo
|
||||
? [...new Set(selectedTodo.blockedBy ?? [])].filter(
|
||||
(id) => id !== selectedTodo.id && projection.todosById.has(id),
|
||||
)
|
||||
: undefined;
|
||||
// The projection already derives this for the graph's edges; recomputing it
|
||||
// here rescanned every todo's `blockedBy` for the same answer. It also drops
|
||||
// a todo that lists itself in `blockedBy`, which the previous filter kept as
|
||||
@@ -95,6 +117,30 @@ export function SessionWorkflowInspector({
|
||||
? (projection.dependentsByTodo.get(selectedTodo.id) ?? [])
|
||||
: [];
|
||||
|
||||
// A dependency reference selects the step it names: the list of ids was
|
||||
// read-only text, so following an edge meant finding the row by eye.
|
||||
const dependencyLink = (todoId: string) => {
|
||||
const target = projection.todosById.get(todoId);
|
||||
const number = stepNumberById.get(todoId);
|
||||
return (
|
||||
<li key={todoId}>
|
||||
<button
|
||||
className={styles.dependencyLink}
|
||||
data-plan-interactive
|
||||
data-testid={`workflow-dependency-${todoId}`}
|
||||
onClick={() => onSelectedTodoIdChange(todoId)}
|
||||
title={target?.content}
|
||||
type="button"
|
||||
>
|
||||
{number !== undefined && (
|
||||
<span className={styles.dependencyNumber}>{number}</span>
|
||||
)}
|
||||
<span>{target?.content ?? todoId}</span>
|
||||
</button>
|
||||
</li>
|
||||
);
|
||||
};
|
||||
|
||||
const detail = selectedTodo && selectedState && (
|
||||
<section className={styles.detail} data-testid="workflow-step-detail">
|
||||
<div className={styles.sectionHeading}>
|
||||
@@ -111,17 +157,25 @@ export function SessionWorkflowInspector({
|
||||
<div>
|
||||
<dt>{t('workflow.dependencies.upstream')}</dt>
|
||||
<dd>
|
||||
{upstream?.length
|
||||
? upstream.join(', ')
|
||||
: t('workflow.dependencies.none')}
|
||||
{upstream?.length ? (
|
||||
<ul className={styles.dependencyList}>
|
||||
{upstream.map((id) => dependencyLink(id))}
|
||||
</ul>
|
||||
) : (
|
||||
t('workflow.dependencies.none')
|
||||
)}
|
||||
</dd>
|
||||
</div>
|
||||
<div>
|
||||
<dt>{t('workflow.dependencies.unblocks')}</dt>
|
||||
<dd>
|
||||
{downstream.length
|
||||
? downstream.map((todo) => todo.id).join(', ')
|
||||
: t('workflow.dependencies.noDownstream')}
|
||||
{downstream.length ? (
|
||||
<ul className={styles.dependencyList}>
|
||||
{downstream.map((todo) => dependencyLink(todo.id))}
|
||||
</ul>
|
||||
) : (
|
||||
t('workflow.dependencies.noDownstream')
|
||||
)}
|
||||
</dd>
|
||||
</div>
|
||||
</dl>
|
||||
@@ -168,7 +222,13 @@ export function SessionWorkflowInspector({
|
||||
task.description}
|
||||
</small>
|
||||
)}
|
||||
{metrics.length > 0 && <small>{metrics.join(' · ')}</small>}
|
||||
{metrics.length > 0 && (
|
||||
<small className={styles.metrics}>
|
||||
{metrics.map((metric) => (
|
||||
<span key={metric}>{metric}</span>
|
||||
))}
|
||||
</small>
|
||||
)}
|
||||
</span>
|
||||
{task && (
|
||||
<span className={styles.stateLabel} data-status={task.status}>
|
||||
@@ -314,7 +374,10 @@ export function SessionWorkflowInspector({
|
||||
<small>{projection.activity.length}</small>
|
||||
</summary>
|
||||
<div className={styles.activityList}>
|
||||
{projection.activity.slice(0, 6).map((task) => {
|
||||
{(showAllActivity
|
||||
? projection.activity
|
||||
: projection.activity.slice(0, ACTIVITY_PREVIEW_COUNT)
|
||||
).map((task) => {
|
||||
const tool = projection.toolsByTaskId.get(task.id);
|
||||
const at = task.endTime ?? task.startTime;
|
||||
const content = (
|
||||
@@ -359,6 +422,20 @@ export function SessionWorkflowInspector({
|
||||
{projection.activity.length === 0 && (
|
||||
<p>{t('workflow.activity.empty')}</p>
|
||||
)}
|
||||
{!showAllActivity &&
|
||||
projection.activity.length > ACTIVITY_PREVIEW_COUNT && (
|
||||
<button
|
||||
className={styles.showAllActivity}
|
||||
data-plan-interactive
|
||||
data-testid="workflow-activity-show-all"
|
||||
onClick={() => setShowAllActivity(true)}
|
||||
type="button"
|
||||
>
|
||||
{t('workflow.activity.showAll', {
|
||||
count: projection.activity.length,
|
||||
})}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
</details>
|
||||
|
||||
@@ -377,8 +454,9 @@ export function SessionWorkflowInspector({
|
||||
>
|
||||
<span className={styles.itemText}>
|
||||
<strong>{artifact.title}</strong>
|
||||
<small>
|
||||
{artifact.kind} · {artifact.status}
|
||||
<small className={styles.metrics}>
|
||||
<span>{artifact.kind}</span>
|
||||
<span>{artifact.status}</span>
|
||||
</small>
|
||||
</span>
|
||||
<ArrowUpRightIcon aria-hidden="true" />
|
||||
|
||||
@@ -179,8 +179,16 @@ for (const theme of [
|
||||
// the todo graph. A broken tool-call ↔ task linkage (e.g. a `toolUseId`
|
||||
// drift) still renders the canvas but silently drops the runtime metric
|
||||
// and the inspector's agent row. `1m 14s` is `formatRuntime(runtimeMs)`
|
||||
// for the fixture's running task — locale-independent.
|
||||
await expect(page.getByText('1m 14s')).toBeVisible();
|
||||
// for the fixture's running task — locale-independent. It prints on
|
||||
// both the node face and the inspector's agent row, so a page-wide
|
||||
// locator would match both and throw a strict-mode violation; gate each
|
||||
// surface separately, keeping both halves load-bearing.
|
||||
await expect(
|
||||
page.locator('[data-plan-node-id="inspect-package"]').getByText('1m 14s'),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
page.locator('[data-testid="workflow-step-detail"]').getByText('1m 14s'),
|
||||
).toBeVisible();
|
||||
await captureScreenshot(page, `session-workflow-cockpit-${theme}`);
|
||||
});
|
||||
}
|
||||
@@ -2904,6 +2904,8 @@ const EN: Messages = {
|
||||
'planExecution.unblocks': 'Unblocks:',
|
||||
'planExecution.unassigned': 'Unassigned executions',
|
||||
'planExecution.attention': 'Needs attention',
|
||||
'planExecution.agentCount': (v) =>
|
||||
`${v?.count ?? 0} ${Number(v?.count ?? 0) === 1 ? 'agent' : 'agents'}`,
|
||||
'planExecution.status.running': 'Running',
|
||||
'planExecution.status.paused': 'Paused',
|
||||
'planExecution.status.completed': 'Completed',
|
||||
@@ -2943,6 +2945,7 @@ const EN: Messages = {
|
||||
'workflow.dependencies.unblocks': 'Unblocks',
|
||||
'workflow.dependencies.noDownstream': 'No downstream steps',
|
||||
'workflow.activity.empty': 'No Agent runs are linked to a Todo yet.',
|
||||
'workflow.activity.showAll': (v) => `Show all ${v?.count ?? 0} runs`,
|
||||
'workflow.deliverables.title': 'Session deliverables',
|
||||
'workflow.deliverables.none': 'No artifacts have been published yet',
|
||||
'workflow.status.running': 'Running',
|
||||
@@ -6493,6 +6496,7 @@ const ZH: Messages = {
|
||||
'planExecution.unblocks': '解锁：',
|
||||
'planExecution.unassigned': '未关联的执行',
|
||||
'planExecution.attention': '需要关注',
|
||||
'planExecution.agentCount': (v) => `${v?.count ?? 0} 个 Agent`,
|
||||
'planExecution.status.running': '运行中',
|
||||
'planExecution.status.paused': '已暂停',
|
||||
'planExecution.status.completed': '已完成',
|
||||
@@ -6531,6 +6535,7 @@ const ZH: Messages = {
|
||||
'workflow.dependencies.unblocks': '解除阻塞',
|
||||
'workflow.dependencies.noDownstream': '没有下游步骤',
|
||||
'workflow.activity.empty': '还没有关联到待办的 Agent 执行。',
|
||||
'workflow.activity.showAll': (v) => `查看全部 ${v?.count ?? 0} 条执行`,
|
||||
'workflow.deliverables.title': '会话交付物',
|
||||
'workflow.deliverables.none': '尚未发布产物',
|
||||
'workflow.status.running': '运行中',
|
||||
|
||||
Reference in new issue
Block a user