Skip to content

feat: server-side echo suppression for compositor slider drags#175

Merged
streamer45 merged 4 commits intomainfrom
devin/1773935440-server-side-echo-suppression
Mar 19, 2026
Merged

feat: server-side echo suppression for compositor slider drags#175
streamer45 merged 4 commits intomainfrom
devin/1773935440-server-side-echo-suppression

Conversation

@staging-devin-ai-integration
Copy link
Contributor

@staging-devin-ai-integration staging-devin-ai-integration bot commented Mar 19, 2026

Summary

Implements server-side echo suppression for compositor slider drags, eliminating the need for the client-side isSliderActiveAtom guard.

Backend (Rust):

  • Add BroadcastEvent wrapper around ApiEvent with exclude_conn_id: Option<u64> for per-connection filtering in the broadcast channel
  • Add TuneNodeSilent WebSocket action — fire-and-forget like TuneNodeAsync, but broadcasts NodeParamsChanged to all clients except the sender
  • Each WebSocket connection gets a unique monotonic ID (NEXT_CONN_ID) separate from the ACTIVE_CONNECTIONS gauge to prevent ID reuse after disconnects
  • All existing event_tx.send() calls wrapped with BroadcastEvent::to_all()

Client (TypeScript):

  • Add tuneNodeConfigSilent to useSession — sends TuneNodeSilent via sendFireAndForget()
  • Thread onConfigChangeSilent through MonitorView → pipelineGraph → CompositorNode → useCompositorLayers → compositorCommit
  • Throttled slider sends (opacity, rotation, drag/resize) use the silent commit adapter; non-throttled sends (drag-end, CRUD) use the normal adapter
  • Remove isSliderActiveAtom and all associated guards, onSliderStart/onSliderEnd props from slider widgets
  • Add throttleActiveRef in useCompositorCommit to guard against stale NodeViewDataUpdated echo-backs during slider drags (this event path is NOT suppressed by TuneNodeSilent since it comes from the engine asynchronously)

Two echo-back paths:

  1. NodeParamsChanged — broadcast from handler → suppressed by TuneNodeSilent
  2. NodeViewDataUpdated — broadcast from engine view-data task → guarded client-side by throttleActiveRef (set true during throttled sends, cleared via trailing-edge debounce)

Review & Testing Checklist for Human

  • Multi-client slider drag: Open two browser tabs on Monitor view with a compositor. Drag opacity slider in tab A — tab B should see the change in real-time. Tab A should not exhibit any flickering or value regression during the drag.
  • Concurrent edits during drag: While dragging a slider in tab A, make a change in tab B (e.g. add/remove a layer). Verify tab A receives the change from tab B (this was blocked by the old isSliderActiveAtom guard).
  • Drag-end reconciliation: After releasing a slider, verify the final committed value matches across all clients (the non-silent tunenode commit on drag-end broadcasts to all including sender).
  • Connection ID uniqueness under churn: With multiple tabs connecting/disconnecting rapidly, verify no cross-tab echo suppression occurs (i.e., changes from client B are never incorrectly suppressed for client C).

Notes

  • The throttleActiveRef debounce delay matches throttleMs (default 100ms from PARAM_THROTTLE_MS), so the guard stays active for the full throttle window and clears shortly after the last tick.
  • Design view (no sessionId) is unaffected — it doesn't have onConfigChangeSilent, so silentCommitAdapter is null and throttled sends fall back to the normal adapter.

Link to Devin session: https://staging.itsdev.in/sessions/f9f80a4c02884400a21cdda813991a11
Requested by: @streamer45


Staging: Open in Devin

Add TuneNodeSilent WebSocket action that broadcasts NodeParamsChanged to
all clients EXCEPT the sender.  This eliminates stale echo-backs during
high-frequency slider drags (opacity, rotation) without blocking changes
from other clients.

Backend:
- BroadcastEvent wrapper with optional exclude_conn_id in state.rs
- Per-connection unique ID via ACTIVE_CONNECTIONS counter in websocket.rs
- TuneNodeSilent variant in RequestPayload (crates/api/src/lib.rs)
- handle_tune_node_silent handler that broadcasts with sender exclusion
- All existing event_tx.send() calls wrapped with BroadcastEvent::to_all()

Client:
- tuneNodeConfigSilent in useSession.ts using 'tunenodesilent' action
- onConfigChangeSilent threaded through MonitorView -> pipelineGraph ->
  CompositorNode -> useCompositorLayers -> compositorCommit
- Throttled sends (slider drags) use silent adapter; non-throttled sends
  (drag-end, CRUD) use normal adapter
- Removed isSliderActiveAtom and all client-side echo-back guards
- Removed onSliderStart/onSliderEnd from OpacityControl/RotationControl

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@staging-devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

staging-devin-ai-integration[bot]

This comment was marked as resolved.

streamkit-devin and others added 3 commits March 19, 2026 16:27
ACTIVE_CONNECTIONS is decremented on disconnect, so reusing it as a
connection ID source can cause ID collisions (e.g. A=0, B=1, A
disconnects→counter=1, C connects→C=1 collides with B).

Add a separate NEXT_CONN_ID AtomicU64 that is never decremented,
ensuring connection IDs are globally unique across the server lifetime.

Also add unit tests for BroadcastEvent exclusion filtering.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…sends

TuneNodeSilent suppresses NodeParamsChanged echo-back server-side, but
NodeViewDataUpdated is still broadcast to all clients (the engine does
not track the originating connection). During slider drags, stale
view-data can overwrite the local atom's newer value.

Add a throttleActiveRef in useCompositorCommit that is set to true when
a throttled silent send fires and cleared via a trailing-edge debounce.
Both sync guards (sync-from-props and useServerLayoutSync) now check
this ref alongside dragStateRef, skipping updates while a throttled
slider send is in-flight.

Includes an integration test that verifies stale server view-data is
correctly skipped during active throttled slider sends.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Extract shared body of handle_tune_node_async and handle_tune_node_silent
into handle_tune_node_fire_and_forget(exclude_conn_id: Option<u64>).
Both thin wrappers now delegate to it with None / Some(conn_id).

This eliminates ~100 lines of duplicated code (permission checks, session
lookup, file security validation, pipeline model update, engine forwarding)
that was a maintenance hazard — any future bug fix or security patch would
have needed to be mirrored in both functions.

Also simplifies redundant assertions in BroadcastEvent test and adds a
note about the theoretical throttleActiveRef clear-timing edge case.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@streamer45 streamer45 merged commit 3b5f4cd into main Mar 19, 2026
16 checks passed
@streamer45 streamer45 deleted the devin/1773935440-server-side-echo-suppression branch March 19, 2026 20:22
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.

2 participants