[krassx]

Summary

• New domain `UiTracker` in `@bugsee/trackers/browser`: one per-window singleton (registry + `rebind`, merge-not-drop) owning orientation, screenSize, viewport, colorScheme, reducedMotion, displayMode and frameRate stages, with per-stage listener activation (emitter gains `listenerCount` + `onListenersChanged`).
• Replaces the per-signal Orientation/ScreenSize/Fps trackers; the orientation, screen_size and fps traces and the orientation_changed / dark_mode_changed events now subscribe to it. Trace/event names and wire shapes unchanged.
• `ApplicationTracker` loses its `orientationchange` stage (type change; the UiTracker owns it). `orientation_changed` is now de-duplicated and also fires on `screen.orientation` change.
• Retention: only env seams and the per-stage de-dup baseline survive the last unsubscribe; baselines are re-read when a stage gains its first listener or sources are rebound. Rebind re-attaches only on source changes (fresh function closures never reset a running fps window).

Test plan

• `pnpm ci:local` (lint, typecheck, cycles, per-package coverage, harness tests)
• Mutator loop on each new entity (injected bugs caught, mutations rolled back)
• Launch-level integration test: browser launch goes through `uiTrackerFor` (swap to `new` fails it)
• e2e job in CI

🤖 Generated with Claude Code

[claude]

Claude finished @krassx's task in 1m 5s —— View job

---

Review of PR #82: UiTracker (TR2)

I found no P0–P2 issues in the diff against origin/main. I did not run the tests, typecheck or lint. The checkout had no node_modules, so this is a read-through review of the code and its call sites. I did not look at the 935-line ui-tracker.test.ts or the other test files.

What I read: ui-tracker.ts, the emitter.ts hooks, the application-tracker.ts change, and the trace-producers.ts and system-events.ts rewiring. I also read the launch.ts call sites that feed the shared per-window tracker.

Things I tried to break and could not:

• Launch ordering: the traces and the events source reach the same UiTracker, each with a different slice of the env. The document is only supplied by the traces, and rebind treats it as a source change, which re-attaches. The function seams (matchMedia, requestAnimationFrame, cancelAnimationFrame, now) are swapped in place. So creation order doesn't matter. An early events-source creation without document or requestAnimationFrame is repaired by the traces' later rebind. A running fps window is never reset by fresh closures.
• Source changes in rebind: #detachAll() runs before Object.assign, so the interval is cleared on the scheduler that armed it. #wanted is cleared, so baselines re-read silently instead of announcing a swap.
• Emitter hook: onListenersChanged fires after onActiveChange on every on, off, once and onAny mutation. #sync is idempotent, and every throwing path (#attach, hostRead, the guarded listeners) reports through onError rather than escaping into on().
• Frame-rate loop: #running is reset on a failed setup, the loop-generation guard blocks stale rAF and timer callbacks, and the stop paths are best-effort. I didn't find a double-loop or leaked-timer path.
• Behaviour changes: ApplicationTracker dropping orientationchange has no remaining consumers in the diff. The orientation, screen-size and fps traces and the two events now come from one tracker, and orientation_changed is de-duplicated as described in the PR body.

P3 nits (non-blocking, no change needed to merge):

• system-events.ts: orientation_changed drops params unless both type and angle are present. An engine exposing only one of them loses it, where the old code forwarded both unvalidated. This follows from the "omit, never invent" rule, so it's probably intended. Worth confirming that no viewer path relied on a partial params.
• ui-tracker.ts:523-525: when the orientation is unreadable, #onOrientation emits {} on every event with no de-duplication. It also leaves the previous baseline in place, so if the reading later returns to the same value as before the gap, it won't be announced. This only matters on engines with a flaky screen.orientation.
• ui-tracker.ts:26: the header comment line is very long and runs two sentences together. This is cosmetic.

Overall risk: Low.

Merge recommendation: Approve once the CI e2e job is green. The PR checklist still has that box unchecked, and I couldn't confirm it from here.

Most important items:

1. Confirm CI, especially the e2e job and per-package coverage, since I couldn't run them.
2. Optionally decide whether the partial type/angle orientation case should still emit its params.

Positives:

• Per-stage activation means listeners and timers exist only while a subscriber wants that stage.
• Failure isolation is thorough: hostile getters, a throwing onError, and a throwing clock are all covered.
• The rebind semantics are carefully reasoned, including the ordering that keeps the interval on the scheduler that armed it.
• The launch-level integration test checks that browser launch goes through uiTrackerFor.
• Removing three single-signal trackers and their duplicated listeners is a net simplification.
  · branch feat/ui-tracker

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 138bb47a-2221-4b76-b27a-06f90db441cb

📥 Commits

Reviewing files that changed from the base of the PR and between 2adb618 and 3cee38a.

📒 Files selected for processing: 27

• .changeset/browser-ui-tracker.md
• docs/PROGRESS.md
• packages/browser/src/launch.test.ts
• packages/browser/src/launch.ts
• packages/browser/src/system-events.test.ts
• packages/browser/src/system-events.ts
• packages/browser/src/trace-producers.screen-lag.test.ts
• packages/browser/src/trace-producers.sensors.test.ts
• packages/browser/src/trace-producers.ts
• packages/browser/src/trace-producers.visibility.test.ts
• packages/browser/src/trackers/fps-tracker.test.ts
• packages/browser/src/trackers/fps-tracker.ts
• packages/browser/src/trackers/orientation-tracker.test.ts
• packages/browser/src/trackers/orientation-tracker.ts
• packages/browser/src/trackers/screen-size-tracker.test.ts
• packages/browser/src/trackers/screen-size-tracker.ts
• packages/trackers/README.md
• packages/trackers/package.json
• packages/trackers/src/browser/application-tracker.test.ts
• packages/trackers/src/browser/application-tracker.ts
• packages/trackers/src/browser/index.ts
• packages/trackers/src/browser/ui-tracker.test.ts
• packages/trackers/src/browser/ui-tracker.ts
• packages/trackers/src/emitter.test.ts
• packages/trackers/src/emitter.ts
• packages/trackers/src/index.test.ts
• packages/webview/src/launch.test.ts

💤 Files with no reviewable changes: 6

• packages/browser/src/trackers/orientation-tracker.test.ts
• packages/browser/src/trackers/fps-tracker.test.ts
• packages/browser/src/trackers/orientation-tracker.ts
• packages/browser/src/trackers/screen-size-tracker.test.ts
• packages/browser/src/trackers/screen-size-tracker.ts
• packages/browser/src/trackers/fps-tracker.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Summary

Summary by CodeRabbit

• New Features
  • Added unified browser UI tracking for orientation, screen size, viewport, color scheme, reduced motion, display mode, and frame rate.
  • UI tracking is shared per browser window and supports reliable listener management and cleanup.
  • Added public browser tracker exports for integrating these UI signals.
• Bug Fixes
  • Prevented duplicate orientation events and ignored incomplete orientation data.
  • Improved handling of injected browser environments and isolated reporting or callback errors.
• Documentation
  • Updated browser tracker documentation and progress notes to reflect the consolidated UI tracking capabilities.

The change adds a shared UiTracker for browser UI state and frame-rate tracking. Browser trace producers and system events consume its stages. The browser package exports the tracker and its related types. ApplicationTracker no longer handles orientationchange, and the emitter base adds listener-count reporting and a listener-change hook.

Sequence Diagram(s)

sequenceDiagram
  participant BrowserTraceProducers
  participant BrowserSystemEventsSource
  participant uiTrackerFor
  participant UiTracker
  participant BrowserWindow
  BrowserTraceProducers->>UiTracker: create shared tracker from UI environment
  BrowserSystemEventsSource->>uiTrackerFor: request tracker for browser environment
  uiTrackerFor-->>BrowserSystemEventsSource: return shared UiTracker
  BrowserTraceProducers->>UiTracker: subscribe to orientation, screenSize, and frameRate
  BrowserSystemEventsSource->>UiTracker: subscribe to orientation and colorScheme
  BrowserWindow->>UiTracker: deliver UI source events
  UiTracker-->>BrowserTraceProducers: emit subscribed trace stages
  UiTracker-->>BrowserSystemEventsSource: emit orientation and colorScheme stages

Loading

Priority: ➖ Normal

Change: Feature

Merge Risk: ⚪ Minimal · up to 3cee3

Browser and WebView UI signals now come from one shared per-window tracker. Trace and event names stay the same, and orientation events are de-duplicated. No concrete defect was found in the reachable launch paths. The change appears ready to merge with normal CI, including the untested e2e job.

Security architecture risk: 🔵 Low · up to 3cee3

The inspected browser and WebView paths keep UI signals tied to their window, retain capture gating, and release subscriptions when consumers stop. No new security exposure was established. Behavior when callers replace browser-environment functions or relaunch with unavailable sources remains less certain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A faulty shared source can affect multiple UI-signal consumers for one window, rather than only one former tracker. The inspected registry keys sharing by window identity; evidence does not establish cross-window sharing.

Trust Boundaries and Controls

• observed — System-event delivery remains subject to the CaptureSystemEvents provider gate. Provider stop removes its source subscription, and source deactivation removes its tracker subscriptions; sharing the tracker does not itself send an event to capture.

Resilience and Maintainability Implications

• inferred — For exported or dynamically replaced environment functions, a current media-query read can use a newer function while an existing change listener remains bound to the earlier query list. The inspected production launch ordering does not establish that replacement after activation occurs there; its security effect remains unproven.

Hardening Proposals

• proposed — Define how callers explicitly clear an unavailable source on relaunch, and distinguish equivalent fresh closures from a genuine function-source replacement before deciding whether to reattach active bindings.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 17 files. (4 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description clearly explains the new UiTracker, its integration, behavior changes, and test plan.
Title check		The title clearly identifies the main change: adding UiTracker for browser UI tracking domains.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 17 files. (4 skipped: 4 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 21
• Waived: $5.25 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

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