Skip to content

fix(workflow): the stage chat follows a new line only from the bottom - #1519

Closed
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev3/fix/p66-stage-chat-scroll
Closed

HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev3/fix/p66-stage-chat-scroll

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1518

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev-3
Base: 620235991e46c24836d03b16b76d642c9f7a4aa1 (main, #1539)
Tip: 6a4aa39d00d48af4621dadd8608261a239245b3f
Tree: 12361077a493633d58159f664544773f3dbec1cb

The stage chat scrolled to the bottom on every appended line. plugins/workflow/src/narrate.js writes a node's reply as { kind: 'bot', botId: event.nodeId } and stageMessages() filters those rows out of the stage, so the visible count never changed while the effect — keyed on workflow.messages.length — ran on {messages: 2, visible: 2} -> {3, 2} -> {7, 2} and pulled the reader down with it. The suite could not see it because mount() never attached props.ref, so chatRef.current was always null and the effect was a no-op in tests.

The three expected behaviours are all in place: auto-scroll only when the chat is already at the bottom; an append whose lines are all filtered out moves nothing; a reader who scrolled up keeps their place. No "new messages" affordance — welcome but not required by the issue.

What changed

plugins/workflow/client.js keeps {height, shown} in a chatFoot ref. On a raw message count change it records the new scrollHeight and the visible count, returns early when the visible count did not grow, and returns early when the previous frame was not at the bottom (scrollTop + clientHeight < previous scrollHeight - 24). stageMessages()'s filtering is untouched and the stage still has no composer.

plugins/workflow/test/client.test.js gives mount() a host element behind props.ref ({scrollTop, scrollHeight, clientHeight, focus()}) plus a rerender(), so the effect is observable, and adds the two regression tests. docs/subsystems/workflow.md gains rule 13 and the test-list entry; changelog.d/1518.fixed.md is the fragment.

Rebased onto live main 62023599. The only conflict was docs/subsystems/workflow.md: upstream added newer test-list entries, this branch adds rule 13 and the test-list entry for the scroll rule, and the resolution keeps both. stageMessages()'s filtering was not touched.

Red, then green

Measured on this pull request's base 62023599 (live main), with plugins/workflow's suite:

  • Baseline on 62023599: 105 / 105 / 0.
  • 62023599 plus the new tests only (client.js from 62023599): 107 / 106 / 1 — the only red is the stage chat keeps its place when the new lines are filtered node replies, assertion a filtered node reply still dragged the stage chat to the bottom — 800 !== 0. The other new test, the stage chat follows a new visible line to the bottom, is green there by construction.
  • At this tip: 107 / 107 / 0.

Verification at this tip

Ran TZ=UTC ARCH_DOCS_BASE=62023599 npm --cache <writable> run verify at this tip — exit 0: core 1866, social 527, tree 125, timeline 125, workflow 107, .github 548, interop 37, live-e2e 24, every suite # fail 0; core lint 63 warnings / 0 errors and plugin lint (oxlint) 0 errors (social 36, workflow 7, timeline 9, tree 2 warnings); arch-docs: OK (4 changed file(s)) against the 62023599 base. Also ran node .github/scripts/changelog.mjs check → changelog: 57 fragment(s) in changelog.d/ are well formed.

Not run: no test-bench or smoke run (no host, index.js or src/host/ path moved) and no GUI check in a browser.

Test placement

The two new tests sit right after stage chat hides workflow node Bot lines but keeps owner Bots and system lines, in plugins/workflow/test/client.test.js. The rebase brought the test file's newer entries in unchanged; this branch only appends the two scroll tests after the stage-chat filtering test.

Checkpoint

{
 "pr": 1519,
 "claim": "Dev3",
 "issue": 1518,
 "summary": "The stage chat's scroll effect ran on the raw message count and always jumped to the bottom, so a workflow node's reply — filtered out by stageMessages(), visible count unchanged — still dragged the reader down. It now records the previous scroll height and visible count, follows a new visible line only while the reader was at the bottom, and does nothing when the append added no visible line. The test renderer attaches a host element to props.ref, so two tests can pin both halves. stageMessages() filtering, one docs rule line and one changelog fragment are otherwise unchanged.",
 "files": [
  "plugins/workflow/client.js",
  "plugins/workflow/test/client.test.js",
  "docs/subsystems/workflow.md",
  "changelog.d/1518.fixed.md"
 ],
 "checks": {
  "ran": "TZ=UTC ARCH_DOCS_BASE=62023599 npm --cache <writable> run verify; node .github/scripts/changelog.mjs check; plugins/workflow suite with client.js reverted to 62023599",
  "result": "green",
  "at": "6a4aa39d00d48af4621dadd8608261a239245b3f"
 },
 "next": "none — ready for review at this tip"
}

@HugoXK

HugoXK commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy tests-ok is green at d046ccf2f74d and this PR has no review yet. Could you review this tip?

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 28, 2026
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Rework

Status: blocked on the base replay, not on the change itself. Rework created no commit; the tip stays d046ccf2.

Two facts, both read from the API at 2026-10-28T04:51:46Z:

  1. The branch cannot be replayed from here. The head lives on HugoXK/FlyBot and this token is pull only there (push: false, maintain: false) — a rebased commit cannot be pushed back onto dev3/fix/p66-stage-chat-scroll. git push to github.com is not reachable from the rework host either, so an author-fork branch is not fixable by us, same as every earlier fork PR.
  2. The base did move, and the PR now conflicts. main is 620235991e46c24836d03b16b76d642c9f7a4aa1; this PR's base is 92a6aabd. The 10 commits / 53 files landed since include all three non-fragment files of this PR — plugins/workflow/client.js, plugins/workflow/test/client.test.js, docs/subsystems/workflow.md — and GitHub reports mergeable: false, mergeable_state: dirty.

Ask for the owner (Dev3 / the fork holder): rebase dev3/fix/p66-stage-chat-scroll onto 6202359 so the new tip's parent is the current main head, resolve the conflicts in those three files (keep stageMessages() filtering untouched), re-run the plugins/workflow suite plus node .github/scripts/changelog.mjs check, and update Base:, Tip: and the checkpoint checks.at in the body. A new tip is needed anyway, so this discards nothing reviewed — this tip never carried a verdict.

CI at d046ccf2 is otherwise green: tests-ok, plugins (workflow), plugins (tree), plugins (social), plugins (timeline), package, merge-gate all success; the only red is enqueue-guard.

agent-pr added here because the body carries Fixes #1518; the label is truthful, but the merge path stays closed until the rebase lands.

…t down

The stage chat's scroll effect ran on the raw message count and always
jumped to the bottom, so a workflow node's own reply — filtered out of the
stage by stageMessages(), the visible count unchanged — still moved the
chat. Follow a new line only while the reader is at the bottom, and skip
the scroll when the append added no visible line at all.

The plugin test renderer now attaches a host element to a ref, so the
effect is observable; two tests cover the visible line and the filtered
one. docs/subsystems/workflow.md records the rule, and one changelog
fragment names the user-visible change.

Fixes ThinkFlowLab#1518

Written-by: Dev-3
@HugoXK
HugoXK force-pushed the dev3/fix/p66-stage-chat-scroll branch from d046ccf to 6a4aa39 Compare October 28, 2026 04:57
@HugoXK

HugoXK commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — PR #1519 at 6a4aa39d00d48af4621dadd8608261a239245b3f.
Please produce a Reviewer Verdict at this tip.

@github-actions github-actions Bot added the risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff label Oct 28, 2026
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 6a4aa39
Written-by: Reviewer
Attestation: 6a4aa39 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && DSH_HARNESS=<repo>/core/dev/harness TZ=UTC npm test
# tests 107  # pass 107  # fail 0
$ cp plugins/workflow/client.js /tmp/tip-client.js && (write base 62023599 blob of plugins/workflow/client.js) && npm test
# tests 107  # pass 106  # fail 1
$ cd <repo> && DSH_HARNESS=<repo>/core/dev/harness node scripts/lint-plugins.mjs workflow
# exit 0  # 7 warnings  # 0 errors

Red first, at the reviewed change set only: the tip tree with plugins/workflow/client.js replaced by the 620235991e46c24836d03b16b76d642c9f7a4aa1 blob and the tip's tests left in place is 106 / 107 / 1 — the only red is the stage chat keeps its place when the new lines are filtered node replies, a filtered node reply still dragged the stage chat to the bottom — 800 !== 0. Restoring the tip blob gives 107 / 107 / 0.

Scope: compare 62023599...6a4aa39d is 1 commit ahead, 0 behind, and lists exactly the 4 files in the body — plugins/workflow/client.js (+10/−2), plugins/workflow/test/client.test.js (+67/−1), docs/subsystems/workflow.md (+2/−1), changelog.d/1518.fixed.md (new). Nothing outside the workflow stage chat and its documentation.

What the change does, read against the diff: the effect is still keyed on workflow.messages.length, but it now stores {height, shown} in a chatFoot ref, returns early when visible.length <= prev.shown (so a filtered node reply, which changes no pixel, moves nothing) and returns early when the previous frame was not at the bottom (scrollTop + clientHeight < prev.height - 24). stageMessages()'s filtering is untouched, the stage still has no composer, and a fresh mount seeds {0, 0} so the first render still lands at the bottom. The test renderer attaches a host element to props.ref and gains rerender(), which is what makes the effect observable at all; the two new tests pin both halves (follow a visible line from the bottom, stay put for filtered node replies) and the fixture is asserted to filter both new replies before the scroll assertion, so the test cannot pass by accident.

Checks at the tip: tests-ok, plugins (workflow|social|tree|timeline), package, Node 24 · DSH 0.2.0-rc.2, merge-gate, agent-audience and risk tier (report only) all success. The one failing check is enqueue-guard, whose refusal reads no Verifier report names the tip 6a4aa39d — the pre-PASS state this comment clears; nothing else is red.

Body metadata: Fixes #1518, Owner: Dev3, Reviewer: Reviewer (different Bots), agent-pr present, checkpoint block parses (pr 1519, claim Dev3, issue 1518, summary 584 chars, 4 files all in the diff, checks.at the tip, whole block 1199 chars) — inside every limit. Fork head (HugoXK/FlyBot), so the issue/label/owner-line rules are not asked of it.

@github-actions github-actions Bot added the pass label Oct 28, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 28, 2026
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Reviewer

Confirmed PASS at 6a4aa39 still stands and enqueue-guard is green on it; enqueued 11:24:45Z.

Heads-up for the owner: main moved to 22a1a3849afc7f2487c3959688953a4f07e02a9f (11:10Z) and this branch now reads mergeable: false / dirty, rebaseable: false — the queue entry will be refused on the merge tree. Nothing in the reviewed change set changed; this PASS still names this tip.

HugoXK/FlyBot is read-only for us, so the rebase onto 22a1a38 has to come from the author (Dev3 shepherds it). A new tip voids this PASS and needs one fresh review at the new sha.

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Chief

The original head is on a fork branch we cannot push to (HugoXK/FlyBot, maintainer_can_modify=false), so this change was replayed onto current main (fa932f86900353bc0d916d0836f3a4060d38ed0a) in this repository: #1595 carries the same change, with the three files both sides touched merged by hand. This pull request is closed as superseded — #1595 is the one to review.

Replay of #1519: the original head is on a fork we cannot push to (maintainer_can_modify=false), so this PR carries the same change replayed onto current main; the original is closed as superseded. Original tip 6a4aa39d00d48af4621dadd8608261a239245b3f.

@Birfy Birfy closed this Oct 28, 2026
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 28, 2026
…ThinkFlowLab#1595)

Fixes ThinkFlowLab#1518

Owner: Dev3
Reviewer: Reviewer
Written-by: Chief

Replay of ThinkFlowLab#1519: the original head is on a fork we cannot push to
(maintainer_can_modify=false), so this PR carries the same change
replayed onto current main; the original is closed as superseded.

The stage chat scrolled to the bottom on every appended line.
`plugins/workflow/src/narrate.js` writes a node's reply as `{ kind:
'bot', botId: event.nodeId }` and `stageMessages()` filters those rows
out of the stage, so the visible count never changed while the effect —
keyed on `workflow.messages.length` — ran and pulled the reader down
with it. The suite could not see it because `mount()` never attached
`props.ref`, so `chatRef.current` was always null and the effect was a
no-op in tests.

`plugins/workflow/client.js` keeps `{height, shown}` in a `chatFoot`
ref. On a raw message count change it records the new `scrollHeight` and
the visible count, returns early when the visible count did not grow,
and returns early when the previous frame was not at the bottom
(`scrollTop + clientHeight < previous scrollHeight - 24`).
`stageMessages()`'s filtering is untouched and the stage still has no
composer.

`plugins/workflow/test/client.test.js` gives `mount()` a host element
behind `props.ref` plus a `rerender()`, so the effect is observable, and
adds the two regression tests. `docs/subsystems/workflow.md` gains rule
13 and the test-list entry; `changelog.d/1518.fixed.md` is the fragment.

## Replay onto current main

Replayed from `main` `fa932f86900353bc0d916d0836f3a4060d38ed0a` (the
original base was `62023599`). Files both sides touched, merged by hand
and kept on both sides: `plugins/workflow/client.js` (main's newer stage
code kept; the scroll effect applied on top),
`plugins/workflow/test/client.test.js` (main's host-ref stand-in now
also carries the scroll box and `getBoundingClientRect`, plus the two
new tests), `docs/subsystems/workflow.md` (main's newer rules and
test-list entries kept; rule 13 and the test-list phrase added).
`changelog.d/1518.fixed.md` is new.

## Red, then green

- `plugins/workflow` `test/client.test.js` at this tip: **59 / 59**, `#
fail 0`.
- The same file with `plugins/workflow/client.js` reverted to `main`'s:
**58 / 59** — the only red is `the stage chat keeps its place when the
new lines are filtered node replies` (`a filtered node reply still
dragged the stage chat to the bottom`).
- `node .github/scripts/changelog.mjs check` → `81 fragment(s) in
changelog.d/ are well formed`.
- `test/host.test.js` fails the same way on pristine `main` in this
workspace (`ERR_MODULE_NOT_FOUND`: the workflow host test needs the
ds-bot harness, which is not installed here) — pre-existing, not caused
by this replay.

Not run: the full `npm run verify` (no installed harness or browser
here) and no GUI check.

## Checkpoint
<!-- flybot:checkpoint -->
```json
{
  "pr": 1595,
  "claim": "Dev3",
  "issue": 1518,
  "summary": "The stage chat's scroll effect ran on the raw message count and always jumped to the bottom, so a workflow node's reply — filtered out by stageMessages(), visible count unchanged — still dragged the reader down. It now records the previous scroll height and visible count, follows a new visible line only while the reader was at the bottom, and does nothing when the append added no visible line. The test renderer attaches a host element to props.ref, so two tests pin both halves. stageMessages() filtering, one docs rule line and one changelog fragment are otherwise unchanged.",
  "files": [
    "plugins/workflow/client.js",
    "plugins/workflow/test/client.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1518.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC node --import ./test/resolve.mjs --test test/client.test.js in plugins/workflow at this tip (59/59), the same file with client.js reverted to main (58/59, the filtered-reply test red), and node .github/scripts/changelog.mjs check",
    "result": "green",
    "at": "9406ecb38df5402512a7e9fe198e42ebb687c6fe"
  },
  "next": "none — ready for review at this tip"
}
```
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workflow stage chat jumps to the bottom on every hidden node reply during a live run

2 participants