perf(tauri): Rust-native desktop event transport#242
Conversation
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23398482721 Artifacts expire in 7 days. |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23398502150 Artifacts expire in 7 days. |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23399446411 Artifacts expire in 7 days. |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23401824896 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23402541285 Artifacts expire in 7 days. |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23410012218 Artifacts expire in 7 days.
|
pascalandr
force-pushed
the
pagec/rust-desktop-event-transport
branch
from
5f572c2 to
4448dd0
Compare
pascalandr
changed the title
pascalandr
force-pushed
the
pagec/rust-desktop-event-transport
branch
7 times, most recently
from
74ec719 to
302e87f
Compare
|
I’d be happy to get a critical review on whether this direction makes sense upstream. |
|
Hey @pascalandr , How does this Tauri specific change effect electron and server UI? |
|
I want you to perform the gatekeeper checks for latest PR state check for regressions |
Only commit 1 is Tauri-specific. Commit 2 is intentionally cross-host: it optimizes how streaming updates are applied/rendered in the UI, so it should benefits Tauri, Electron, and the browser path alike. |
|
Am I correct in understanding that each platform will be using their native EventSource path? |
|
In this PR:
So only the Tauri transport changes here. The UI-side streaming optimizations are host-agnostic and sit below that transport choice. |
|
@pascalandr - I am sure this works well but the problem is I don't have experience with Rust / Tauri . I am more Java / Node person and the Tauri app is 100% maintained by AI and community. The only reason it came in because people wanted it so AI made it. I can't maintain Tauri only features as I can't code review them and EventSource being a core part any changes on it I will be totally reliant on AI or community for it.
Would you help me decide these please? |
|
I see but TBH, the Rust/Tauri transport is the part I cared about most, because I use the Tauri client all day long. |
|
I want you to perform the gatekeeper checks for latest PR state check for regressions
|
|
I reviewed the latest PR state as a gatekeeper review. My understanding of the request: evaluate the current PR for regressions, better implementation options, and specifically call out any knock-on effects on Electron and the server/browser UI discussed in this thread. Findings:
Knock-on effects by host:
Better implementation direction:
Validation notes:
Gatekeeper verdict: not merge-ready. Both findings above are blocking regressions, and one of them hits Electron/server directly. -- |
As of now, I want to keep native ( Tauri / Electron ) features to the bare minimum as my current process is to implement features in web and server and expect them to run on both Electron / Tauri without any changes. This helps me a lot in ways
I will need some time to work out what needs to be done for cases like this. I hope you understand. |
|
When you says not have to test multiple "platforms" do you mean Tauri / Electron or Windows/linux/Macos ? |
|
@pascalandr - Sorry I missed your comment. Multiple platforms = Tauri and Electron on different OSes ( Windows, MacOS and Linux ) |
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25625484797 Artifacts expire in 7 days. |
|
Please focus this review on multi-platform maintainability. Be especially critical about anything that increases the long-term Tauri-specific maintenance surface, leaks platform-specific behavior into shared UI/runtime paths, or makes Electron and Tauri harder to keep aligned over time. At the same time, keep in mind that the purpose of this PR is to restore a more reasonable performance balance for Tauri (especially WebView2 / GTK WebKit) compared to Electron, not to add platform-specific complexity for its own sake. Also, please treat the debug / benchmark instrumentation as non-essential review noise for now unless it hides a real architectural or behavioral problem. The main review target is the transport and integration shape, not the temporary measurement scaffolding. |
|
I reviewed the current PR state with the requested multi-platform maintainability scope. My understanding of the request: re-check whether the newest transport/settings shape keeps Tauri-specific behavior isolated, avoids leaking platform choices into shared UI/runtime paths, and preserves Electron/Tauri alignment while still allowing Tauri to recover performance on WebView2 / GTK WebKit. I treated the benchmark harness as non-essential unless it changes shipped architecture or runtime behavior. Findings:
What still looks good:
Maintainability verdict:
Validation notes:
-- |
|
@CodeNomadBot |
|
I reviewed the latest PR state again with the same multi-platform maintainability scope. My understanding of the request: verify whether the latest update fixes the two prior maintainability concerns (shared-config leakage for the Tauri fallback toggle and the hidden forced-browser override), while re-checking that the native transport remains a narrow Tauri-only integration and browser/Electron stay aligned on the existing browser Findings: No blocking or medium-severity multi-platform maintainability findings in the current state. What changed since my last review:
What still looks good:
Residual non-blocking note:
Maintainability verdict:
Validation notes:
-- |
Reintroduce the smallest Tauri-native event transport slice needed to compare it against the browser EventSource path on real sessions. Add a frontend/server benchmark hook so the desktop runtime can report end-to-end transport behavior without pulling in the broader UI performance changes from the original PR. # Conflicts: # packages/tauri-app/src-tauri/src/cli_manager.rs # Conflicts: # packages/tauri-app/src-tauri/src/main.rs
Handle named SSE ping events with explicit pong replies so the native transport survives the server heartbeat window, and remove the unused active-session fast path while keeping the benchmark harness authenticated. # Conflicts: # packages/tauri-app/src-tauri/src/cli_manager.rs
Require the explicit bench build flag before the UI harness can be activated, remove query-string command overrides, gate perf logging behind a runtime flag, and add a regression test for named SSE ping frames.
Reuse the desktop event transport session cookie for the native /api/client-connections/pong request so heartbeat updates follow the same auth contract as the SSE stream, and cover it with a request-header regression test.
Expose a device-level setting that lets Tauri users disable the native Rust desktop event transport and fall back to the browser EventSource path when debugging transport issues or comparing behavior. Persist the preference in UI settings, mirror it into localStorage so startup transport selection can read it synchronously, and restart the backend event stream immediately when the toggle changes so the new transport takes effect without a full app restart. Validation: npx tsc --noEmit --pretty -p packages/ui/tsconfig.json; npx tsc --noEmit --pretty -p packages/server/tsconfig.json; cargo test --no-run
Move the Tauri native transport fallback toggle out of the shared UI config and treat it as a true device-local preference. The setting now lives only in local state plus localStorage, so browser, Electron, and other Tauri clients do not inherit a host-specific transport choice through storage broadcasts. While updating the selector, remove the hidden forceBrowserEvents override so the visible Tauri fallback setting becomes the only shipped transport-selection path outside the benchmark build gates. Validation: npx tsc --noEmit --pretty -p packages/ui/tsconfig.json; npx tsc --noEmit --pretty -p packages/server/tsconfig.json; cargo test --no-run
pascalandr
force-pushed
the
pagec/rust-desktop-event-transport
branch
from
af6508d to
843a47c
Compare
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25761675193 Artifacts expire in 7 days.
|
The transport benchmark harness still called loadMessages with the old boolean force flag, which broke UI typecheck after the session API moved to an options object. This keeps the PR242 benchmark code compatible with the current Pagec_tauri integration baseline instead of leaving a validation-only regression on the stack.
The change is limited to the benchmark harness and preserves the existing forced reload behavior by switching to the current { force: true } call shape. This keeps the fix scoped to the branch that introduced the transport bench so downstream integrations can cherry-pick a normal commit without rewriting history.
Validation: npm run typecheck --workspace @codenomad/ui
Drop the temporary in-app benchmark harness, perf logging endpoint, and batch metrics now that the native desktop transport has been validated on Windows and Linux. Keep the shipped PR focused on the transport implementation and the user-facing Tauri fallback setting rather than the instrumentation used during evaluation. The benchmark results are preserved in the PR description as historical validation, but the benchmark code itself no longer ships in the branch. Validation: npx tsc --noEmit --pretty -p packages/ui/tsconfig.json; npx tsc --noEmit --pretty -p packages/server/tsconfig.json; cargo test --no-run # Conflicts: # packages/ui/src/transport-bench.tsx
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25959195000 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25965871529 Artifacts expire in 7 days. |
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/25965871529 Artifacts expire in 7 days. |
|
The recorded results and manual testing on Linux and Windows confirm that this restores a more reasonable performance balance for WebView2 and GTK WebKit under heavy UI load. |
|
I want you to perform the gatekeeper checks for latest PR state check for regressions |
|
I performed the latest gatekeeper review for PR #242. My understanding of the request: review the current, post-benchmark-harness-removal PR state ruthlessly for regressions, code-quality risks, and better implementation choices. I treated this as a merge gate, not just the earlier multi-platform maintainability pass. Findings:
Non-blocking notes:
Validation attempted:
Gatekeeper verdict: Do not merge yet. The remaining issue is small and localized, but it is exactly in the shared event-stream lifecycle code and can cause a newer active stream to be disconnected by a stale failed connect attempt. Add the generation guard on the -- |
Guard the async event-stream connect failure path with the same generation check used by the success callbacks so an older rejected connection attempt cannot tear down a newer healthy stream. This keeps Tauri restarts and other reconnect races from disconnecting the active transport after the local desktop transport preference changes or another connection attempt wins first. Validation: npx tsc --noEmit --pretty -p packages/ui/tsconfig.json; npx tsc --noEmit --pretty -p packages/server/tsconfig.json
|
@CodeNomadBot |
|
I performed the latest gatekeeper review for PR #242 after the follow-up fix. My understanding of the request: run the same merge-gate review as before, specifically confirming whether the stale async event-stream connect failure issue is fixed, while re-checking the transport/fallback/benchmark-removal areas for regressions. Findings: No blocking findings in the current PR state. What changed since the last gatekeeper pass:
Regression checks:
Code-quality verdict:
Validation attempted in this workspace:
Gatekeeper verdict: No objection to merge from this review, subject to the project’s normal CI/build checks passing in the fully provisioned environment. -- |
|
Please confirm this PR pay attention about multi-platform maintainability. |
|
I re-confirmed PR #242 specifically from the multi-platform maintainability angle. My understanding of the request: confirm whether this PR is acceptable after the latest gatekeeper pass, with extra attention to whether the Tauri-native transport creates long-term platform divergence, leaks Tauri behavior into shared UI/runtime code, or leaves browser/Electron harder to keep aligned. Findings: No blocking multi-platform maintainability findings in the current state. Confirmation points:
Maintainability verdict: This PR pays sufficient attention to multi-platform maintainability. It adds Tauri-specific complexity, but the complexity is isolated at the runtime boundary, justified by the measured WebView2 / GTK WebKit performance gap, and does not force browser/Electron onto a divergent shared runtime path. I have no objection to merge from the multi-platform maintainability perspective, subject to normal CI/build checks passing in the fully provisioned environment. -- |
2 participants
Summary
EventSourcepath to a native Rust desktop event transport while leaving browser and Electron unchangedevent:frames and replying tocodenomad.client.pingwith an authenticated/api/client-connections/pongEventSourcetransport without leaking that choice through shared configBenchmark
The temporary in-app benchmark harness used during validation has been removed from the final code, but the measured results are retained here for review context.
Real Tauri/WebView2 benchmark on Windows using the dedicated session:
D:\CodeNomadses_21feb15b3ffeLz3uRModK4KKnGShort command:
node -e "for (let i = 1; i <= 400; i += 1) console.log('line ' + i)"Results:
EventSourceforced in Tauri:131479.7mssawWorking=falsereachedIdle=falsebatchesReceived=84eventsReceived=84maxBatchSize=11437.4mssawWorking=truereachedIdle=truebatchesReceived=4eventsReceived=45maxBatchSize=27Long heartbeat / stale-timeout validation:
powershell -NoProfile -Command Start-Sleep -Seconds 7071689.5mssawWorking=truereachedIdle=truebatchesReceived=13eventsReceived=72maxBatchSize=25Confirmed separately afterward: the native transport also behaves better on Linux.
Validation
cargo test named_ping_event_is_routed_to_ping_channelcargo test session_cookie_is_attached_to_requestscargo test --no-runnpx tsc --noEmit --pretty -p packages/ui/tsconfig.jsonnpx tsc --noEmit --pretty -p packages/server/tsconfig.jsonNotes
EventSourcepath