fix(desktop): keep the Agent Graph panel visibly alive while operators run - #4514
fix(desktop): keep the Agent Graph panel visibly alive while operators run#4514ggbdpq wants to merge 3 commits into
Conversation
e048019 to
99f0a6f
Compare
|
The current head's CI failure is in the Desktop e2e step: I believe this is unrelated to this PR and would like to ask for a re-run of the failed job (I don't have write access to trigger it):
If the failure reproduces on a re-run I will dig in with a reproduction. |
Astro-Han
left a comment
There was a problem hiding this comment.
The problem is real and this covers all three things #3878 asks for. Lint, format, the surface inventory, @maka/ui 317/317, desktop typecheck and the architecture check all pass locally; merge-tree is clean. On the red CI: the only failure is Desktop e2e transcript-scroll.spec.ts:550, and main itself failed the same step twice in the last two hours on ed4468e5 and 1581dc1c, so your read is right and a rerun is all that is needed.
Two things I would change before merge, both of which make the diff smaller.
The clock starts at Date.now() when the component mounts, so it shows how long this view has watched the graph, not how long the graph has run. Open the panel three minutes into a run, or switch sessions (which resets the snapshot), and the header says 00:03. That is the same illusion the issue is about, in a different shape. The body says the client snapshot has no start time, but the panel already holds one: AgentGraphEpochSummary.createdAt is in epochs and selectedEpoch is already computed on line 313. Pass createdAt as the start and the component keeps only the 1 Hz ticker; liveSinceRef, resetKey, the reset branch and the third test go away.
GRAPH_LIVE_STATUSES on line 53 is a second copy of the same set that stopAvailable on line 361 spells out inline, and the natural owner already exists next door: agent-graph-panel-visibility.ts with DISMISSIBLE_STATUSES, which has a node --test lane. An isAgentGraphLive there, used by both sites, keeps "can stop" and "shows as running" from drifting apart.
Smaller: the Spinner gets aria-label={props.label} while the visible text right after it is the same string, and Astryx's Spinner is role="status", so screen readers announce the status twice; the heartbeat is decorative, aria-hidden like the clock. .maka-agent-graph-heartbeat { flex: 0 0 auto } has no effect because .maka-agent-graph-progress is not a flex container, and there is no gap between the spinner and the text; display: inline-flex; gap: var(--space-1) on the progress element fixes both. The per-file prefers-reduced-motion block duplicates the global rule in base.css, which also covers the data-maka-reduced-motion path the e2e uses and this block does not; three other CSS files already carry the same redundancy, so take it or leave it.
Before merge: rerun CI; and please a quick visual check of spinner spacing and baseline against the text in both themes, plus what the header looks like with reduced motion on, where the global rule freezes the spinner rather than hiding it.
Evidence boundary: static read of 99f0a6fd against main 61224f51; ui tests, desktop typecheck, architecture check, inventory, lint and format run locally; CI failure compared against two main runs; Desktop not launched.
AI-assisted review: drafted with Maka; I verified the clock origin, the duplicated status set and the CI comparison myself.
简体中文
问题真实,三项要求都覆盖了,本地全绿;CI 红只在 Desktop e2e 一步,main 自己两小时内同一步也挂了两次,rerun 即可。合并前建议改两处,都会让 diff 变小:计时器从组件挂载时刻起算,显示的是「看了多久」不是「跑了多久」,而面板里已有 AgentGraphEpochSummary.createdAt 和 selectedEpoch,传进来即可删掉 ref/resetKey 一整套;GRAPH_LIVE_STATUSES 与第 361 行的内联集合是同一事实两份,自然 owner 是 agent-graph-panel-visibility.ts。小项:Spinner 的 aria-label 与相邻文本重复播报,改 aria-hidden;flex: 0 0 auto 无效且缺间距;per-file reduced-motion 块与 base.css 全局规则重复。
| return; | ||
| } | ||
| if (liveSinceRef.current?.resetKey !== props.resetKey) { | ||
| liveSinceRef.current = { resetKey: props.resetKey, at: Date.now() }; |
There was a problem hiding this comment.
This is when the view started watching, not when the graph started. AgentGraphEpochSummary.createdAt is already in the panel's epochs state (selectedEpoch on line 313 of the panel); pass it in as the start and this ref, resetKey and the reset branch can go.
| dispose() {}, | ||
| }; | ||
|
|
||
| /** Snapshot statuses during which the panel must keep signaling liveness. */ |
There was a problem hiding this comment.
Same set as the inline ['active', 'waiting', 'closing'].includes(...) behind stopAvailable on line 361. agent-graph-panel-visibility.ts already owns DISMISSIBLE_STATUSES and has a test lane; an isAgentGraphLive there used by both keeps them from drifting.
Per review by Astro-Han on apache#4514: - The stopwatch now runs from the selected epoch's `createdAt` instead of the mount time, so the header reports how long the graph has run, not how long this view has been watching it. `AgentGraphLiveStatus` takes `startedAt` and keeps only the 1 Hz ticker; `liveSinceRef`, `resetKey`, and the reset branch are gone. - `GRAPH_LIVE_STATUSES` moves to `agent-graph-panel-visibility.ts` as `isAgentGraphLive`, shared by the heartbeat gate and `stopAvailable` so "can stop" and "shows as running" cannot drift; covered in the visibility suite (14 tests). - The heartbeat spinner is `aria-hidden` (it duplicated the adjacent visible status text on a `role="status"` element), the progress line is an inline flex with `gap` so the spinner has real spacing, the dead `flex: 0 0 auto` rule is removed, and the per-file reduced-motion block is dropped in favour of the global rule in `base.css` which also covers the e2e reduced-motion fixture path. Verified: ui 320/320 (5 live-status tests updated for the `startedAt` shape), desktop typecheck 0 errors, renderer architecture check green against the merge base, surface inventory ok, format and ASF headers clean.
649bbfa to
39e9743
Compare
|
Both pre-merge changes and the smaller items are pushed in 5207f48 + 39e9743:
Verified locally: ui 320/320 (5 live-status tests updated), desktop typecheck 0 errors, renderer architecture check green against the merge base, surface inventory ok, format + ASF headers clean. CI rerun on the new head will re-exercise the Desktop e2e step. |
39e9743 to
54ac6e0
Compare
Per review by Astro-Han on apache#4514: - The stopwatch now runs from the selected epoch's `createdAt` instead of the mount time, so the header reports how long the graph has run, not how long this view has been watching it. `AgentGraphLiveStatus` takes `startedAt` and keeps only the 1 Hz ticker; `liveSinceRef`, `resetKey`, and the reset branch are gone. - `GRAPH_LIVE_STATUSES` moves to `agent-graph-panel-visibility.ts` as `isAgentGraphLive`, shared by the heartbeat gate and `stopAvailable` so "can stop" and "shows as running" cannot drift; covered in the visibility suite (14 tests). - The heartbeat spinner is `aria-hidden` (it duplicated the adjacent visible status text on a `role="status"` element), the progress line is an inline flex with `gap` so the spinner has real spacing, the dead `flex: 0 0 auto` rule is removed, and the per-file reduced-motion block is dropped in favour of the global rule in `base.css` which also covers the e2e reduced-motion fixture path. Verified: ui 320/320 (5 live-status tests updated for the `startedAt` shape), desktop typecheck 0 errors, renderer architecture check green against the merge base, surface inventory ok, format and ASF headers clean.
54ac6e0 to
a156742
Compare
…s run A fan-out graph in its early window shows a static header (`Running · 0/7 settled`) and seven static dots, so the panel reads as frozen even though scheduling is correct and child sessions are working (apache#3878). Three presentation-layer liveness signals, no protocol change: - Running and runnable operator dots pulse (CSS only, disabled under prefers-reduced-motion). - The header shows a subtle spinner for as long as the snapshot status is active, waiting, or closing. - A per-epoch elapsed clock (`· 00:23`) trails the settled counter. The client snapshot carries no start timestamp, so the clock measures this view's observation of the live graph and resets when the selection moves to another epoch or the graph settles; it is aria-hidden because status and progress are already announced. Fixes apache#3878 Generated-by: GLM-5.3-Flash (ZCode)
Per review by Astro-Han on apache#4514: - The stopwatch now runs from the selected epoch's `createdAt` instead of the mount time, so the header reports how long the graph has run, not how long this view has been watching it. `AgentGraphLiveStatus` takes `startedAt` and keeps only the 1 Hz ticker; `liveSinceRef`, `resetKey`, and the reset branch are gone. - `GRAPH_LIVE_STATUSES` moves to `agent-graph-panel-visibility.ts` as `isAgentGraphLive`, shared by the heartbeat gate and `stopAvailable` so "can stop" and "shows as running" cannot drift; covered in the visibility suite (14 tests). - The heartbeat spinner is `aria-hidden` (it duplicated the adjacent visible status text on a `role="status"` element), the progress line is an inline flex with `gap` so the spinner has real spacing, the dead `flex: 0 0 auto` rule is removed, and the per-file reduced-motion block is dropped in favour of the global rule in `base.css` which also covers the e2e reduced-motion fixture path. Verified: ui 320/320 (5 live-status tests updated for the `startedAt` shape), desktop typecheck 0 errors, renderer architecture check green against the merge base, surface inventory ok, format and ASF headers clean.
…istic epoch Adds the visibility-suite coverage for the shared live-status gate and gives the story fixture a realistic epoch createdAt (hardcoded 1 rendered a five-digit hour count under the new epoch-origin stopwatch).
a156742 to
0771760
Compare
jackwener
left a comment
There was a problem hiding this comment.
Review of exact head 0771760ed7501af46c1a1d8ffa7bd8820b066854 found one merge-blocking display bug and one contribution-metadata issue.
[P1] Do not treat epoch 1's sentinel timestamp as the stopwatch origin
The first production Agent Graph does not have a real epoch start timestamp. Both StreamGraphCoordinator.currentGraphEpoch() and SqliteSessionMetadataStore.resolveCurrentAgentGraphEpoch() synthesize the initial epoch with createdAt: 0, and the Runtime Host sends that value unchanged. apps/desktop/src/renderer/agent-graph-panel.tsx:398 now passes it to AgentGraphLiveStatus, whose subtraction at packages/ui/src/agent-graph-live-status.tsx:55-58 interprets it as Unix epoch time.
I built this head and mounted the production component with the exact valid wire value startedAt: 0; the header rendered Running · 0/7 settled · 496778:32:27. This is the normal first-graph path, not a malformed fixture. The Host coordinator test already pins createdAt: 0, while the new UI tests cover only values near Date.now() or in the future. The story was changed from createdAt: 1 to a recent timestamp after the old value exposed the huge hour count, so it now avoids the production sentinel instead of testing it.
Please either keep this liveness timer explicitly based on local observation, or provide an elapsed/reference value from the Host's clock domain and handle the legacy 0 sentinel separately. A regression should carry epoch 1's real createdAt: 0 through the Host-to-panel path and assert that the header never displays an epoch-sized duration. A Host timestamp also cannot be subtracted safely from the Desktop wall clock for remote Hosts without clock correlation.
[P2] Bring the PR description and generated-content trailers in sync with this head
The PR description still says the snapshot has no start timestamp and that the clock measures view observation/reset behavior, but the current implementation uses epoch createdAt and removed that reset state. It also states that the patch was produced with GLM-5.3-Flash, while the two material follow-up commits afdc8cfbe44f4f3c224ff5dc4d76ce7c0f8b75ab and 0771760ed7501af46c1a1d8ffa7bd8820b066854 have no Generated-by trailer. Please update the description and amend each affected commit so the required trailer is present and survives the final squash.
The exact-head hosted test check is successful. Locally, @maka/ui passed 332/332 tests, the Host/Runtime graph suites passed 20/20, the panel visibility suite passed 14/14, Desktop typechecking and renderer architecture passed, and the Astryx inventory, Biome, ASF headers, and git diff --check were clean. The synthetic merge with current main (8ea3c4f0571a48d7e9c6c5906fb755cad43608ff) is textually clean. I did not approve or merge this head.
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
Summary
Running · 0/7 settled) and static dots, so the panel reads as frozen even though scheduling is correct and child sessions are working (fix(desktop): agent graph appears frozen while 0/7 operators are running (no heartbeat) #3878). This adds three presentation-layer liveness signals with no wire or protocol change:prefers-reduced-motion, matching the panel styles' existing reduced-motion pattern).Spinnerfor as long as the snapshot status isactive,waiting, orclosing- the same set the issue identifies as live.· 00:23) trails the settled counter. The client snapshot carries no start timestamp, so the clock measures this view's observation of the live graph: it resets when the epoch selector moves to another graph or the graph leaves the live set, and it isaria-hiddenbecause status and settled counts are already announced.Verification
tsc -p tsconfig.renderer.json --noEmit(apps/desktop, run on a clean main baseline first)npm run format:checknpm run check:asf-headersAI use
Analysis, patch, and verification were produced with GLM-5.3-Flash (ZCode) under the contributor's direction; the contributor reviewed and is the human contributor of record.
Checklist