Skip to content

feat(web): animate running SOP node activity - #178

Open
AaronWangCong wants to merge 2 commits into
Tencent:mainfrom
AaronWangCong:codex/node-activity-motion
Open

AaronWangCong wants to merge 2 commits into
Tencent:mainfrom
AaronWangCong:codex/node-activity-motion

Conversation

@AaronWangCong

@AaronWangCong AaronWangCong commented Nov 11, 2026 •

Copy link
Copy Markdown

What changed

The selected SOP node's inspector previously showed plain activity text, making it hard to tell whether work was still progressing. An executing node now shows a spinner beside its running status and a subtle left-to-right shimmer across its current activity text.

The animation follows the selected node and its child SOP's actual execution state. Completed nodes, human approval, pause and cancellation remain static. Streamed text updates retain the existing elements so the animation is not restarted by each log event. Light/dark themes, long activity text, reduced motion and forced colors are supported.

Scope

  • Platform core / API / runner — platform console presentation only
  • Public adapter / deployment
  • Classic
  • Portable
  • Shared behavior contract
  • Installer / CLI
  • Documentation only

Compatibility

No API, workflow execution, schema, dependency or Runner protocol changes. This PR is based directly on upstream main and is independent of the Windows contribution in #172.

Validation

  • Built the embedded chatbot and passed all 299 chatbot tests.
  • Passed the console test suite (159 checks), then the final node-inspector suite (10 checks) after adding the child pause/cancellation polling regression.
  • Passed go test -p=2 ./... -count=1 -timeout=600s and go vet ./... on Linux with Go 1.27.3 and the branch's prebuilt Gate helper.
  • Chromium visual checks using the actual inspector rendering and styles with fixture data: light/dark, spinner rotation, left-to-right shimmer, narrow long text, reduced motion and forced colors.
  • Public content check, secret scan and git diff --check.

The initial browser checks used fixture rendering. The follow-up below also exercises a real local service and a synthetic script run, without model calls or changes to existing user runs. The entire deployed public browser suite and Classic/Portable checks were not run for this presentation-only change.

Review follow-up: stylesheet caching and live UI

The reported immutable-URL defect does not reproduce through the production HTML handler. studioFileServer serves revalidated HTML and calls stampStudioIndex, which replaces handwritten CSS/JS queries with content hashes on every HTML request. The literal version in web/index.html is not the URL delivered to a browser. No production caching or version-string changes are needed.

Commit 8ae5221957b72660337c2541fa89c33fc6ee141b adds regression coverage and fresh real-service screenshots with cache-upgrade acceptance details.

  • A single Chromium context first loaded the prior stylesheet at ?v=b5623289d4bd5745, then reused it from its HTTP cache on another normal document navigation (confirmed through CDP).
  • The updated real service emitted ?v=e10036f176cdcba1; the next normal navigation requested the new URL and displayed both animations. There was no cache clear, hard refresh or Playwright request routing.
  • A real password session and shell SOP verified light/dark animation, reduced motion, a narrow viewport and cancellation updates. Only the old HTML stylesheet reference and old stylesheet bytes were simulated by a same-origin HTTP proxy; API responses were not mocked.
  • The ten node-inspector tests passed again. Three Linux Go checks passed: TestStudioIndexStampsAssetsWithContentHash, TestStudioStylesheetChangesInvalidateURLWithoutEditingIndex, and TestGinMiddlewareAndSmallViews. JavaScript syntax, public-content and whitespace checks passed.

This is evidence for re-review; it does not claim a maintainer's UI acceptance.

Safety

  • I did not include credentials, internal endpoints, production data, or organization-specific infrastructure.
  • I preserved user files and unrelated changes.
  • I updated generated Classic host bundles when applicable.
  • I documented third-party content and licensing changes when applicable.

Classic host bundles and third-party assets are unchanged.

@Robert0622

Copy link
Copy Markdown
Collaborator

LoopForge PR Review — head 570042e6fb0fe512e6b44dc9396959201ed36281

Verdict: ✅ Pass

  • Scope: presentation-only — adds a spinner on the running node's status pill and a shimmer on its activity text in the run inspector, driven by nodeInspectorIsRunning() (checks run/node running state and the node's child-run state, excluding paused/cancel-requested), gated behind prefers-reduced-motion/forced-colors. No API, schema or Runner protocol changes, matching the PR description.
  • Code review: patchRunLive now only overwrites the activity <b> text when it actually changed (b.textContent!==nr.activity) instead of always re-touching the DOM, so the shimmer animation isn't restarted by every streamed log event. childRunSignature now also hashes pauseRequested/cancelRequested, so a paused/cancelled child correctly stops the parent node's animation; nodeInspectorIsRunning cross-checks the loaded child-run state to avoid showing motion on a stale "running" parent node once the child is actually waiting/paused.
  • Tests: ran the PR's own new cases locally in this sandbox — cd webtest && node --test tests/runs-detail.test.mjs → 10/10 pass, including the 4 tests added by this PR (streamed-text DOM/element stability, terminal/paused/waiting states stay static, parent only animates while its child SOP is actually running, child pause/cancel polling toggles motion without a parent stream event). node --check web/app/runs.js also passes.
  • CI: the GitHub Actions runs for this SHA (validate, platform, e2e) are all action_required — awaiting maintainer approval to execute, not failures — so no CI signal beyond the independent GITHUB_OPENSOURCE_SCAN_PIPELINE check, which passed. The local run above is this round's only executed-test evidence; worth re-checking once the gated workflows actually run.

@fx-kurumi fx-kurumi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR #178 review — head 570042e6fb0fe512e6b44dc9396959201ed36281

Recommendation: REQUEST_CHANGES (confirmed asset-cache defect). UI acceptance: blocked, not passed.

Fix the immutable stylesheet URL

web/observability.css introduces the spinner and shimmer styles, but web/index.html:16 still requests /observability.css?v=20261031metrics2. The server's internal/httpapi/gin_helpers.go:152 serves versioned CSS with Cache-Control: public, max-age=31536000, immutable.

A returning browser that has cached that URL can keep the pre-PR stylesheet after deployment. The new JavaScript classes then have no corresponding motion styles, so these users do not see the feature. Bump the asset version or use a content hash, and verify upgrade behavior with an already-cached stylesheet.

Independently executed checks

  • node --test LoopForge/webtest/tests/runs-detail.test.mjs: 10 passed, 0 failed.
  • node --check LoopForge/web/app/runs.js: passed.

Not executed / evidence limitation

I did not start the real application, open its live UI, or capture screenshots/video in this review. This environment has Chromium, but no Go or Docker in PATH and no installed Playwright/Puppeteer or ffmpeg. The DOM tests above do not establish visual or interaction acceptance. Real-service UI checks with synthetic data and fresh screenshots/video are still required before giving UI acceptance.

The prior ordinary Pass comment is not a formal APPROVED review. This report is tied to the head SHA above; a new commit requires a fresh review and new visual evidence. No production user data, credentials, or internal endpoints are included.

@fx-kurumi

Copy link
Copy Markdown
Collaborator

PR #178 re-review — head 570042e6fb0fe512e6b44dc9396959201ed36281

Correction: my earlier REQUEST_CHANGES on this head (review) was a false positive. I found no blocking issues, and the UI is verified on a live build. I could not dismiss that review with my current credentials (the org's OAuth app restriction). A maintainer can dismiss it, or I will clear it on the next review.

Withdrawn finding

I claimed that web/index.html:16 (/observability.css?v=20261031metrics2) would keep serving a cached pre-PR stylesheet. That ignores internal/httpapi/studio.go:17-31 (stampStudioIndex), which rewrites every ?v= in index.html to the first 8 bytes of the asset's SHA-256 on each request. A live server for this head serves /observability.css?v=e10036f176cdcba1, which matches sha256(web/observability.css) on this head. On main the hash is b5623289d4bd5745. The versioned URL therefore changes with the content and the immutable cache is safe. Sorry for the noise.

Live UI verification (this head, synthetic data)

  • Setup: built this SHA with Go 1.26.11 and ran it against a throwaway local MariaDB. Data was a fictional project/SOP with two script nodes; the second loops echo step-N; sleep 2. I signed in with the local acceptance login and opened #/runs/<id>, selecting the running node.
Light Dark prefers-reduced-motion: reduce
light dark reduced
▶ recording ▶ recording ▶ recording
  • Light and dark (agentflow.theme.v3=dark for dark): the status pill carries is-running and its ::before animates node-status-spin. The activity text animates node-activity-shimmer. The activity label updates in place (step-9 → step-16 …) without restarting the animation.
  • Reduced motion: both animation names compute to none. The status still reads 运行中.
  • No page errors in any run.

Executed checks

  • node --test webtest/tests/runs-detail.test.mjs: 10/10 pass.
  • node --check web/app/runs.js: ok.

Not covered

Nested child-SOP pause/cancel was covered by the PR's DOM tests only; I did not reproduce it live. CI workflows for this SHA are still action_required.

@fx-kurumi

Copy link
Copy Markdown
Collaborator

✅ 结论:Approve(通过)

审查 head 570042e:未发现阻塞问题,建议合并。

核对要点

  • nodeInspectorIsRunning(web/app/runs.js:563):运行级 pauseRequested/cancelRequested、节点非 running、子 SOP 非 running 或带暂停/取消标记、graphNodeView 已因子流程待确认/失败改写为非 running 时均返回 false;nodeId 为空时安全返回 false。首次渲染(:1117)与实时补丁(:588)共用同一判断,不会出现两条路径状态不一致。
  • patchRunLive(:588-597):每次补丁都重算 is-running 并 classList.toggle/重写 status className,所以节点完成、失败、暂停时即使不走整页重渲染也会关闭动效;活动文字只在内容变化时写入,文字仍会更新,只是相同文本不再触碰 DOM,不会重启动画。没有发现文字不更新的回归。
  • childRunSignature(:966)加入 pauseRequested/cancelRequested,子流程仅控制标记变化时 refreshChildRuns 也会触发父详情重渲染,父节点动效能及时停止/恢复。
  • CSS(web/observability.css:104-115):只用 --hui-text/--hui-secondary 令牌(亮/暗主题均有定义);@supports 包住 background-clip:text,不支持时退回纯色文字;prefers-reduced-motion: reduce 与 forced-colors: active 下实测两种动画都为 none,文字回到实色,转圈保留静态圆弧。design.md 已补对应规范。

可选(非阻塞)

  • 子 SOP 刚启动、子运行仍为 queued 时父节点不显示动效,直到子流程进入 running;若希望排队期间也提示"进行中",可把 queued 视作活动态。

验证

  • cd webtest && node --test tests/runs-detail.test.mjs → 10/10 通过(含本 PR 新增 4 条)。
  • cd webtest && node --test 'tests/**/*.test.mjs' 全量 → 160/160 通过。
  • go test -count=1 ./web/(含设计规范/资源静态检查)→ ok。
  • Chromium 截图:用 webtest 的 jsdom 夹具渲染真实 renderRunDetail 输出,配合仓库真实 CSS 在 Chromium 中加载,亮/暗主题下转圈与高光正常、长活动文字省略正常;另以 reduced_motion=reduce、forced_colors=active 校验计算样式中动画均为 none。

@fx-kurumi
fx-kurumi dismissed their stale review November 11, 2026 14:42

撤回:该 Request changes(静态资源 ?v= 未更新导致缓存)是误报——internal/httpapi/studio.go 的 stampStudioIndex 会按文件内容哈希实时改写资源版本。更正说明见 issuecomment-6795866525。

@fx-kurumi

Copy link
Copy Markdown
Collaborator

复审 head 8ae5221:通过

自上次审查(570042e,附明/暗/减少动效截图和录屏)以来只新增 8ae5221:internal/httpapi/studio_test.go 新增 45 行测试,外加 docs/node-activity-validation.md 和两张文档截图。web/ 下的界面代码没有改动,所以上次的截图和录屏仍然对应当前界面,这次没有重拍。

  • go test -count=1 ./internal/httpapi -run 'Studio|Stamp|Cache':通过,包括新增的 TestStudioStylesheetChangesInvalidateURLWithoutEditingIndex(只改 CSS、不改 index.html,下发的 URL 也会变)。
  • 新增的两张文档图只有合成 SOP 的节点检查器,没有账号、地址或真实数据。

@Robert0622

Copy link
Copy Markdown
Collaborator

LoopForge PR Review — head 8ae5221957b72660337c2541fa89c33fc6ee141b

Verdict: ✅ Pass

  • Scope: 8 files, +243/-5, platform-console-only (web/app/runs.js, web/observability.css, internal/httpapi/studio_test.go, docs/screenshots). No API, schema, or Runner-protocol changes.
  • Re-review: head moved from 570042e6fb0fe512e6b44dc9396959201ed36281 (not previously reviewed under this marker) by one additional commit that only adds docs/screenshots and a cache-upgrade regression test — no production code changed in the new commit.
  • Checked out the head SHA in an isolated worktree (no secrets/no prod access), diffed against base 63c9e46c. nodeInspectorIsRunning() correctly gates the spinner/shimmer to run.status==='running' on the exact selected node, respecting pause/cancel on both the node and its child SOP; childRunSignature() now also tracks pauseRequested/cancelRequested so patched updates re-evaluate promptly. The activity <b> text node is only updated on change, matching the "don't restart the animation on every log event" claim.
  • var(--hui-text)/var(--hui-secondary) used in the new CSS are real shared design tokens (defined in web/app/harness-ui.css, loaded on the same page as observability.css in web/index.html) — not a typo/undefined variable.
  • Ran: go test ./internal/httpapi/... -run TestStudio -v → all 5 pass (incl. the new TestStudioStylesheetChangesInvalidateURLWithoutEditingIndex). go vet ./internal/httpapi/... → clean. bash scripts/build-chatbot.sh → 47 files / 299 tests pass. node --check web/app/runs.js → syntax OK.
  • Not run: the new Chromium-based webtest/tests/runs-detail.test.mjs assertions and the documented manual Chromium visual check — no running sandbox/SOP fixture available in this isolated review environment; code review of the test file shows it exercises the same nodeInspectorIsRunning gating logic through patchRunLive.

No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants