Skip to content

fix(ui): keep streaming bash output pinned#556

Merged
shantur merged 1 commit into
devfrom
fix-bash-toolcall-output
Jun 16, 2026
Merged

fix(ui): keep streaming bash output pinned#556
shantur merged 1 commit into
devfrom
fix-bash-toolcall-output

Conversation

@shantur

@shantur shantur commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • notify the existing virtual follow-list path after streaming bash ANSI output updates
  • restore the inner bash output scroll immediately after direct pre updates
  • apply the same coalesced notification pattern to final ANSI output

Validation

  • npm run typecheck --workspace @codenomad/ui

Streaming bash tool output bypasses the shared markdown renderer and mutates its ANSI pre element directly, so it was not participating in the same content-render notification flow as other message parts and tool call bodies.

Update the running bash renderer to restore its inner scroll position immediately after ANSI DOM updates, then coalesce outer virtual-list render notifications through requestAnimationFrame. Final ANSI output uses the same notification path so completed ANSI output also re-measures consistently.

This preserves the existing virtual follow-list behavior without introducing new scroll APIs or forced bottom-scroll logic, and cancels pending notification frames on unmount to avoid stale callbacks.

Validation: npm run typecheck --workspace @codenomad/ui
@shantur

shantur commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

@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

codenomadbot Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for latest PR state complete.

Request understood: perform a ruthless PR-quality review of #556 for regressions and better implementation options. Assumption: this is review-only; no code changes requested.

Findings: none blocking.

What I checked:

  • Reviewed the full PR diff against origin/dev; scope is limited to packages/ui/src/components/tool-call/renderers/bash.tsx.
  • Reviewed surrounding scroll/follow infrastructure in tool-call.tsx, follow-scroll.tsx, ansi-render.tsx, and existing rendered-content notification patterns.
  • Verified the new direct-DOM bash output path calls restoreAfterRender() after mutations and schedules onContentRendered() after that, preserving the expected parent virtual-list notification order.
  • Verified cleanup cancels pending animation-frame notifications for both running and final ANSI paths.
  • Checked that the approach mirrors the existing reasoning stream notification pattern and avoids broad refactors.

Validation:

  • Initial npm run typecheck --workspace @codenomad/ui failed because dependencies were not installed in this checkout (solid-js and other workspace modules missing).
  • Ran npm install.
  • Reran npm run typecheck --workspace @codenomad/ui: passed.
  • git diff --check origin/dev...HEAD: passed.

Quality notes:

  • The implementation is slightly duplicative between RunningBashOutput, FinalAnsiOutput, and the existing reasoning stream helper, but this is acceptable at current scale and preferable to widening the PR with a shared abstraction just for this fix.
  • A more centralized long-term improvement would be adding rendered-content notifications to the generic ANSI renderer path, but this PR targets the custom bash renderer that bypasses Solid rendering with direct pre mutations. The narrow fix is reasonable.

Residual risk:

  • I did not perform a live browser reproduction of long streaming bash output pinned at bottom. Given the nature of the bug, a manual UI check would still be useful before merge if this path has been flaky.

--
Yours,
CodeNomadBot

@shantur shantur merged commit 8b5ffa1 into dev Jun 16, 2026
11 checks passed
@shantur shantur deleted the fix-bash-toolcall-output branch June 16, 2026 14:56
@github-actions

Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

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

Artifacts expire in 7 days.
Artifacts:

  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-tauri-linux
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-tauri-macos
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-tauri-windows
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-tauri-macos-arm64
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-electron-macos
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-electron-windows
  • pr-556-d8c54f8b5d275f00e331ee69121839bb55acd40d-electron-linux

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.

1 participant