fix(ui): preserve manual scroll during streaming

[pascalandr]

Fixes #305

Summary

• stop auto-follow as soon as the user expresses an explicit upward scroll intent while the assistant is still streaming
• keep the message list from snapping back to the bottom before that upward scroll can take effect

Why

The message stream already tries to leave follow mode when the user scrolls up, but streaming renders can still re-pin the list before the upward movement is fully observed.

This makes scrolling upward feel broken while the agent is still responding.

What Changed

• packages/ui/src/components/virtual-follow-list.tsx
  • break follow mode immediately on explicit upward scroll intent
  • suppress the next automatic bottom pin so the user's scroll-up wins over the active stream

Validation

• npm run build --workspace @codenomad/ui

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted a ruthless review of the current patch for regressions and for any stronger implementation option, without broadening scope beyond this PR.

Findings

1. High: upward intent now disables follow mode even when no upward scroll actually occurs.
   File: packages/ui/src/components/virtual-follow-list.tsx:193-198

The new block flips autoScroll off immediately on any explicit upward input:

if (direction === "up" && autoScroll() && activeHoldTargetKey() === null) {
  setAutoScroll(false)
  lastObservedPinnedAtBottom = false
  suppressAutoScrollOnce = true
}

That fixes the race you were targeting, but it also changes semantics from "user scrolled up" to "user tried to scroll up".

Regression path:

• the list is still shorter than the viewport, or otherwise cannot actually move upward yet
• the user wheels upward / presses PageUp / ArrowUp
• markUserScrollIntent() turns follow off before any real scroll delta is observed
• no scroll event may fire, so nothing re-enables follow
• subsequent streamed items stop auto-following even though the viewport never left bottom

This is a behavior regression relative to the previous contract, where follow mode only broke after an actual upward movement away from bottom was observed in handleScroll() / updateScrollButtons().

Better implementation direction

A safer approach is to keep the new race fix separate from the persisted follow state:

• record a short-lived "pending upward break" flag on explicit up intent
• use that flag only to suppress the next auto-pin / render-time bottom snap
• only commit setAutoScroll(false) once handleScroll() or updateScrollButtons() confirms the viewport actually moved away from bottom

That preserves the fix for the stream race without breaking follow on non-moving input.

Validation gaps

• I did not find existing test/spec coverage for VirtualFollowList, so this regression would currently rely on manual catching.
• I attempted the PR’s stated validation command, npm run build --workspace @codenomad/ui, but the workspace in this bot environment is missing vite, so I could not complete a real build verification here.

I did not find a second issue that was as strong as the regression above. That one is significant enough that I would block on it before merging.

--
Yours,
CodeNomadBot

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/24848915679

Artifacts expire in 7 days.
Artifacts:

• pr-356-d4ecf57a53099a0ad2854ddc71b2b50ccadbe4a0-electron-linux
• pr-356-d4ecf57a53099a0ad2854ddc71b2b50ccadbe4a0-electron-linux-rpm

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted a fresh ruthless review of the current revision of this PR, focused on regressions and whether the implementation can be made safer without expanding scope.

Findings

1. High: explicit resume-to-bottom can still be blocked by stale pendingUpwardBreak state.
   File: packages/ui/src/components/virtual-follow-list.tsx:208-214, 415-420, 450-463

The previous regression was addressed well by separating "upward intent" from the persisted follow state. But the new pendingUpwardBreakUntil flag is only cleared on:

• confirmed upward break (clearPendingUpwardBreak() in updateScrollButtons())
• reset
• a later updateScrollButtons() pass where atBottom && !hasUserScrollIntent()

That leaves a concrete stale-state path:

• user expresses upward intent, so pendingUpwardBreakUntil is set
• before the 600 ms window expires, the user explicitly re-enables follow by clicking the scroll-to-bottom button or calling scrollToBottom()
• scrollToBottom() sets autoScroll(true) and jumps to bottom, but it does not clear pendingUpwardBreakUntil
• subsequent streaming updates during that window are still blocked by:
  • notifyContentRendered() returning early when hasPendingUpwardBreak() is true
  • the items().length effect skipping bottom pin when hasPendingUpwardBreak() is true
  • the followToken effect skipping bottom pin when hasPendingUpwardBreak() is true

So the component can ignore an explicit user request to resume follow for up to 600 ms. That is still a correctness issue in a streaming message list.

Better implementation direction

If the user or caller explicitly opts back into follow, that should win immediately. The minimal fix is to clear the pending flag in the explicit resume paths, for example:

• scrollToBottom()
• api.setAutoScroll(true) if you want the API to have the same guarantee

That keeps the race fix while avoiding a stale guard overriding a newer user action.

Validation gaps

• I still do not find test/spec coverage for VirtualFollowList, so these timing-sensitive follow-state interactions remain unguarded.
• I re-ran npm run build --workspace @codenomad/ui, but this bot environment still cannot resolve vite, so I could not perform a real local build verification.

Net: the earlier blocker is fixed, but I would still block this revision until the stale pendingUpwardBreak path is resolved.

--
Yours,
CodeNomadBot

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted a fresh ruthless pass on the current revision of this PR, focused on regressions and on whether the implementation can be made safer without expanding scope.

Findings

1. High: manual return-to-bottom can still leave follow suppressed until the pending window expires.
   File: packages/ui/src/components/virtual-follow-list.tsx:208-214, 295-307, 416-420, 457-469

The previous blocker was addressed correctly for explicit resume paths like scrollToBottom() and api.setAutoScroll(true). But there is still a stale pendingUpwardBreakUntil path when the user returns to bottom by normal scrolling instead of using an explicit API/button path.

Concrete path:

• user expresses upward intent, so pendingUpwardBreakUntil is set in markUserScrollIntent()
• before a confirmed upward break happens, the user scrolls back to bottom manually
• in updateScrollButtons(), the user-scroll sync block can set autoScroll(true) when atBottom && !autoScroll()
• however, clearPendingUpwardBreak() only runs later when atBottom && !hasUserScrollIntent()
• during the remaining intent window, hasPendingUpwardBreak() is still true, so:
  • notifyContentRendered() returns early
  • the items().length effect skips bottom pin
  • the followToken effect skips bottom pin

So the list can be visibly back at bottom with follow re-enabled, but still ignore streamed auto-follow work for up to 600 ms because the stale pending flag keeps winning.

This is still a correctness bug in the same interaction family: a newer, real user state should override the temporary guard immediately.

Better implementation direction

The minimal safer change is to clear the pending upward break as soon as the list confirms it is back at bottom, including inside the user-scroll synchronization path.

In practice, when atBottom causes follow to resume, the pending guard should be cleared in that same branch instead of waiting for !hasUserScrollIntent().

Validation gaps

• I still do not find automated test/spec coverage for VirtualFollowList, so these timing-sensitive transitions remain unguarded.
• I re-ran npm run build --workspace @codenomad/ui, but this bot environment still cannot resolve vite, so I could not complete local build verification here.

Net: the last revision improved the implementation, but I would still block this PR until the remaining stale pendingUpwardBreak path is removed.

Side note: packages/ui/src/components/virtual-follow-list.tsx is now 566 lines, which is over the repository’s source-file warning threshold. I would not refactor for that in this PR, but it is worth tracking separately.

--
Yours,
CodeNomadBot

[pascalandr]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[github-actions]

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/24851801866

Artifacts expire in 7 days.
Artifacts: (none found on this run)

[codenomadbot]

Gatekeeper review for the latest PR state.

What I understood: you wanted a fresh ruthless pass on the current revision of this PR, focused on regressions and on whether there is a safer implementation available without expanding scope.

Findings

I do not currently see a blocking correctness issue in the latest revision.

The previous regressions I called out have been addressed in the current patch:

• upward intent now suppresses auto-pin immediately without persisting follow-off until real upward movement is confirmed
• explicit resume paths clear the pending upward-break guard
• manual user return-to-bottom also clears the pending upward-break guard in the same user-scroll sync path

That leaves the follow-state transitions internally consistent for the race this PR is targeting.

Residual risks / gaps

• I still do not find automated test/spec coverage for VirtualFollowList, so this remains a timing-sensitive interaction change that is protected mainly by manual reasoning.
• I re-ran npm run build --workspace @codenomad/ui, but this bot environment still cannot resolve vite, so I could not complete local build verification here.
• packages/ui/src/components/virtual-follow-list.tsx is now 569 lines, which is above the repo’s source-file warning threshold. I would not refactor for that in this PR, but it is worth tracking separately.

Gatekeeper verdict

From a code-review perspective, I would no longer block this PR on the implementation itself.

If CI is green in the real project environment, this looks mergeable to me.

--
Yours,
CodeNomadBot

[shantur]

@pascalandr - Do you still see this issue happening in latest develop? Can you please check and confirm as I remember working on it recently

[pascalandr]

I think #375 is the better direction.
It supersedes #356 and moves the scroll-follow logic to a cleaner model. It is a larger change, but I think that is justified here.