Coalesce duplicate widget tool requests

[insane66613]

Summary

• single-flight identical concurrent widget tool calls using stable argument keys
• allow sequential calls to execute normally after settlement
• suppress identical UI telemetry events within a short 250 ms burst window

Verification

• npm run build
• test-ui-event-tracking.js (5 passing cases)

Summary by CodeRabbit

• Performance

  • Coalesced identical concurrent tool requests so only one host call is executed.
  • Suppressed duplicate UI analytics events for 250ms using normalized payload keys.

• Bug Fixes

  • Ensured subsequent (non-concurrent) tool calls run normally after prior requests finish.
  • Kept distinct events distinct while filtering only immediate duplicates; bounded the dedupe cache to prevent unbounded growth.

• New Features

  • Added deterministic request key generation to improve reliable event/tool deduplication.

• Tests

  • Added integration coverage for tool-call coalescing and UI event suppression, including canonicalization edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

The tool bridge now coalesces identical concurrent calls using canonical request keys, while the UI event tracker suppresses repeated normalized events within 250 milliseconds. Integration tests cover canonicalization, sequential behavior, duplicate events, and bounded cache eviction.

Changes

Deduplication controls

Layer / File(s)	Summary
Concurrent tool-call coalescing src/ui/shared/canonical-key.ts, src/ui/shared/tool-bridge.ts, test/test-ui-event-tracking.js	Canonical serialization distinguishes supported primitive and special numeric values while normalizing object key order. The tool bridge shares identical in-flight executions and retains helper and fallback behavior; tests cover concurrent, sequential, and canonical-key cases.
UI event suppression and validation src/ui/shared/ui-event-tracker.ts, test/test-ui-event-tracking.js	UI events with the same event and normalized parameters are suppressed for 250ms, stale cache entries are evicted beyond 100 entries, and normalized parameters are passed to track_ui_event. Tests cover duplicate suppression and cache bounds.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WidgetCall
  participant createToolBridge
  participant callToolOnce
  participant HostCallTool
  WidgetCall->>createToolBridge: callTool(name, normalized args)
  createToolBridge->>callToolOnce: start one in-flight request
  callToolOnce->>HostCallTool: execute tool call
  HostCallTool-->>callToolOnce: return result
  callToolOnce-->>createToolBridge: resolve shared promise
  createToolBridge-->>WidgetCall: return shared result

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: deduplicating duplicate widget tool requests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/shared/tool-bridge.ts`:
- Around line 73-80: The deduplication keys use inconsistent serialization and
collapse distinct values such as NaN and null. In src/ui/shared/tool-bridge.ts
lines 73-80, update stableStringify to provide the shared canonical sorted-key
encoding, preserving distinct type-safe representations for non-finite numbers
and invalid runtime values or rejecting them; in
src/ui/shared/ui-event-tracker.ts line 38, replace raw normalizedParams
serialization with the same shared encoder so equivalent objects with different
insertion order produce identical keys. Ensure both utilities use this single
canonical implementation.

In `@src/ui/shared/ui-event-tracker.ts`:
- Around line 45-49: Update the recentEvents maintenance logic to enforce a hard
maximum of 100 entries: after removing stale entries in the existing sweep,
evict the oldest remaining entries until recentEvents.size is at most 100,
preserving newer timestamps and the duplicate-window behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b7018710-920b-44b8-8b57-681bb9a009e3

📥 Commits

Reviewing files that changed from the base of the PR and between 1eccc8b and d7eee51.

📒 Files selected for processing (3)

• src/ui/shared/tool-bridge.ts
• src/ui/shared/ui-event-tracker.ts
• test/test-ui-event-tracking.js

[coderabbitai]

🧹 Nitpick comments (1)

test/test-ui-event-tracking.js (1)

136-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a regression test for refreshed cache entries.

The loop at Line [144] only tests eviction of a never-revisited key. In the supplied tracker implementation, updating an existing Map entry does not move it to the end; after a key is accepted again beyond 250 ms, adding 100 unique events can evict that freshly refreshed key and allow a duplicate within the suppression window. Add a deterministic-clock or focused timing case that refreshes index: 0, adds 100 unique events, then repeats it immediately.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test-ui-event-tracking.js` around lines 136 - 150, Extend
testUiEventCacheStaysBounded with a deterministic-clock or focused timing case
that waits past the 250 ms suppression interval, tracks index: 0 to refresh it,
adds 100 unique events, then repeats index: 0 immediately. Assert the refreshed
key remains suppressed, preventing an additional call and covering cache recency
after an existing entry is accepted again.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/test-ui-event-tracking.js`:
- Around line 136-150: Extend testUiEventCacheStaysBounded with a
deterministic-clock or focused timing case that waits past the 250 ms
suppression interval, tracks index: 0 to refresh it, adds 100 unique events,
then repeats index: 0 immediately. Assert the refreshed key remains suppressed,
preventing an additional call and covering cache recency after an existing entry
is accepted again.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8fe582dd-7808-408c-b0a6-82ced118c5fc

📥 Commits

Reviewing files that changed from the base of the PR and between d7eee51 and 2e5db87.

📒 Files selected for processing (4)

• src/ui/shared/canonical-key.ts
• src/ui/shared/tool-bridge.ts
• src/ui/shared/ui-event-tracker.ts
• test/test-ui-event-tracking.js

🚧 Files skipped from review as they are similar to previous changes (1)

• src/ui/shared/ui-event-tracker.ts

[insane66613]

Current head 2e5db87896d25c081f01d6032251c51390116ac5 addresses the prior canonicalization and bounded-cache findings. The PR is ready for maintainer re-review. One automated nit remains requesting a focused regression test for refreshed cache-entry recency; it is non-blocking to the core implementation but should be considered before merge.