fix(webxr): apply target frame rate before CloudXR session creation

[asierarranz]

Summary

The web client negotiates the CloudXR stream before the WebXR session is actually running at the configured frame rate. On a Quest 3 this leaves three clocks fighting each other — 72 Hz configured, 90 Hz negotiated, ~120 Hz actually rendered — and the mismatch shows up as cadence judder and, under load, a queue of undelivered frames that grows until the video freezes. This PR makes the client apply and await the configured rate before CloudXR.createSession(), and advertise the rate the browser actually accepted.

Symptoms we debugged

Sessions started clean and degraded after 5–10 seconds: latency artifacts building up progressively, then a burst/pause rhythm (roughly half a second of motion, two seconds frozen), and eventually a full video freeze while the server kept rendering and encoding normally. Frame-by-frame inspection of the encoder traces showed the server side perfectly healthy — monotonic frame IDs, stable ~5.5 ms encode times, zero drops — while the client fell further and further behind: a classic producer-outrunning-consumer queue buildup.

Two independent problems fed that queue:

1. The frame-rate race (this PR). createXRStore() is called without a frameRate option, so @react-three/xr applies its 'high' default (updateTargetFrameRate(120) on Quest 3) in parallel with xr.setSession(). The sessionstart handler creates the CloudXR session immediately, so the stream is negotiated while the session is still transitioning rates. The client's own store.setFrameRate(72) runs later, after enterVR()/enterAR() resolves, and is fire-and-forget — too late to affect negotiation. The server ends up pacing a 90 Hz stream for a headset consuming at 72.

2. Bitrate above what the last hop can carry (not in this PR, see below). The stock Quest 3 profile requests 150 Mbps — above the 100 Mbps ceiling StreamSDK 6.2 itself warns about — and a wireless headset link under that load collapsed into recovery: 3.93% packet loss (p95) and ~22,600 retransmission requests per session. Capping the stream at 25 Mbps on the same setup dropped that to 0.06% loss and 73 retransmissions, and the progressive freeze disappeared. We treat the cap as deployment guidance, but the 150 Mbps default and the silent <select> fallback that restores it deserve their own issue.

What this PR changes

• Create the XR store with frameRate: false — no automatic rate override racing the negotiation.
• In the sessionstart path, await a bounded applyTargetFrameRate(session, config.deviceFrameRate) helper before CloudXR.createSession():
  • validates the rate against session.supportedFrameRates;
  • bounded wait (2 s) so a hung updateTargetFrameRate() can never block CloudXR session creation — an unbounded wait reproducibly deadlocked the Quest at its loading screen;
  • briefly waits for frameratechange, because per the WebXR spec session.frameRate may still report the old value when the update promise resolves;
  • advertises the accepted target rate, with safe fallbacks when the request is rejected, unsupported, or the API is unavailable.
• Remove the late fire-and-forget store.setFrameRate(...) call.

What this PR does not claim

No bitrate, device-profile, UI or timestamp changes. It also does not fix transport-induced stalls on a saturated link — that is the bitrate topic above. Known limitation: with CloudXR JS 6.2.0 we still observe the signaled format requesting 90 Hz even when SessionOptions.deviceFrameRate=72 and the session is verified at 72 Hz before createSession (instrumented timeline available; being reported to the CloudXR team separately). The PR still removes the app-level race and makes the headset rate deterministic at negotiation time.

Testing

• 10 new unit tests for the helper: supported / unsupported / missing API / rejected / ordering / hang-timeout / late frameratechange / attribute-lag / no-attribute cases. Full suite: 23/23.
• Live on a Quest 3 (wired host, wireless headset): instrumented requestSession/updateTargetFrameRate beacons confirm the session reaches 72 Hz before negotiation; device pose interval steady at ~13.9 ms; no regression in session startup.

Related, kept intentionally separate: fix/wss-client-cache-headers (the hosted client is served without Cache-Control, so stale cached clients silently masked deployments) and perf/webclient-canvas-throttle (the metrics HUD re-uploads a 1024x500 canvas texture every XR frame even when its text is unchanged).

Summary by CodeRabbit

• New Features

  • Frame rate selection now follows the active XR session more closely, helping CloudXR use the rate actually accepted by the browser/device.
  • Added a new frame-rate handling flow that can validate, apply, and confirm the selected rate before streaming starts.

• Bug Fixes

  • Removed automatic frame-rate preference handling from connection startup so session negotiation is more reliable.
  • Added coverage for supported, unsupported, delayed, and fallback frame-rate scenarios to improve stability.

[github-actions]

📝 Docs preview is not auto-deployed for fork PRs.

A maintainer with write access to NVIDIA/IsaacTeleop can deploy a preview by
commenting /preview-docs on this PR. Once deployed, the preview
will live at:

https://nvidia.github.io/IsaacTeleop/preview/pr-726/

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0e820d9a-66c6-4fe8-ba97-ae40f8f0ca45

📥 Commits

Reviewing files that changed from the base of the PR and between cb8aa84 and 3c9fd64.

📒 Files selected for processing (4)

• deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsx
• deps/cloudxr/webxr_client/src/App.tsx
• deps/cloudxr/webxr_client/src/config/frameRate.test.ts
• deps/cloudxr/webxr_client/src/config/frameRate.ts

---

📝 Walkthrough

Walkthrough

This PR introduces a new frame-rate negotiation module (frameRate.ts) that validates and applies a target WebXR frame rate against session capabilities, with timeout/fallback handling and optional confirmation via frameratechange events. CloudXRComponent now computes an effectiveDeviceFrameRate using this module on session start and uses it for CloudXR session options instead of the static configured rate. App.tsx disables the XR store's automatic frame-rate preference and removes its previous store.setFrameRate logic from doConnect. A new Jest test suite covers the negotiation module's behaviors.

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

Sequence Diagram(s)

sequenceDiagram
  participant App
  participant CloudXRComponent
  participant applyTargetFrameRate
  participant XRSession
  participant CloudXR

  App->>CloudXRComponent: XR store created with frameRate: false
  CloudXRComponent->>XRSession: get current XRSession on sessionstart
  CloudXRComponent->>applyTargetFrameRate: applyTargetFrameRate(session, config.deviceFrameRate, logger)
  applyTargetFrameRate->>XRSession: updateTargetFrameRate(target)
  XRSession-->>applyTargetFrameRate: outcome (updated/timeout/rejected)
  applyTargetFrameRate->>XRSession: wait for frameratechange (bounded by grace period)
  XRSession-->>applyTargetFrameRate: reported frameRate
  applyTargetFrameRate-->>CloudXRComponent: effectiveDeviceFrameRate
  CloudXRComponent->>CloudXR: SessionOptions with effectiveDeviceFrameRate

Loading

Related Issues: None specified

Related PRs: None specified

Suggested labels: webxr, cloudxr, frame-rate

Suggested reviewers: None specified

Poem:
A rabbit hopped through XR skies,
Chasing frame rates with careful eyes,
No more racing the store's own guess,
Now negotiated with graceful finesse,
Ticking timeouts, awaiting the sign,
Frames align, and the stream runs fine. 🐇✨

🚥 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 summarizes the main change: applying the target WebXR frame rate before creating the CloudXR session.
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

---

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

[gareth-morgan-nv]

This is a great! Thanks for tracking this down. Look at the agent review thats coming for some minor things, but this looks good to merge. There should be more comments though, particularly JSdoc comments on exported functions. And you should use the actual WebXR types (even though that will mean some ignore-error tags in the tests).

[gareth-morgan-nv]

Should use actual WebXR types though that will mean adding some @ts-expect-error tags in the tests, thats better than the having this "homemade" WebXR spec here.

[gareth-morgan-nv]

claude-review-bot (claude-opus-4-8)

Summary

Solid, well-motivated fix that removes a real frame-rate negotiation race; the applyTargetFrameRate helper is cleanly designed for testability and correctness looks sound (no logic bug; timeout/rejection handling is careful). Findings are the WebXR-type cleanup, one promise-idiom refactor, a handful of readability improvements, and doc/comment polish. Nothing blocking.

Legend: 🚫 Blocker · 💡 Suggestion · 🔍 Nit

	Finding
💡	src/config/frameRate.ts:18-25 — drop hand-rolled FrameRateSession; type against the real XRSession (members are in @types/webxr via @types/three, so not a "missing typings" exception). Removes the as XRSession & FrameRateSession cast at the call site.
💡	src/config/frameRate.ts:59-73 — raceUpdateAgainstTimeout wraps an already-a-promise in new Promise (promise-constructor antipattern) with manual resolve/clearTimeout in three branches. Rewrite with Promise.race + outcome mapping in .then + .finally(clearTimeout).
💡	src/config/frameRate.ts:54-106 — the non-obvious control flow (raceUpdateAgainstTimeout's never-reject race + tagged union; waitForReportedFrameRate's event-vs-timeout + done/finish guard) needs richer explanatory comments so a reader unfamiliar with the file can follow without tracing it. Explain why the timeout resolves instead of rejects, and why the timer is cleared.
💡	src/config/frameRate.ts:121 — name the fallback once (const fallbackFrameRate = currentFrameRate ?? targetFrameRate;) instead of repeating currentFrameRate ?? targetFrameRate at six early returns; gives one place for the "honest rate to advertise" comment.
💡	src/config/frameRate.ts:168,175 — the duplicated session.frameRate !== undefined && session.frameRate !== targetFrameRate (recheck after the settle-await) reads as copy-paste; extract a named reportsStaleRate() predicate so the check and recheck are self-documenting.
💡	src/config/frameRate.ts:108-183 — make the two phases explicit: "apply the rate (bounded)" vs "confirm reported rate & choose what to advertise". Section comments, or extract the settle/advertise tail into confirmAppliedRate(...).
💡	src/config/frameRate.ts:119-183 — body doesn't follow a step-comment + blank-line convention (one // comment immediately above each logical step, one blank line before it).
💡	src/config/frameRate.test.ts — after the type switch, annotate partial fakes with // @ts-expect-error <reason> (the correct tag; fails loudly if it becomes unnecessary), not as unknown as.
💡	src/config/frameRate.test.ts — Jest unit test mocks a WebXR XRSession; project convention pushes mocked-Web-API coverage toward Playwright. Defensible since the helper is pure logic over an injected contract — confirm against the test-boundary guide and note the rationale in the PR.
🔍	src/config/frameRate.ts:42,46,59,108 — named functions isFinitePositive, containsFrameRate, raceUpdateAgainstTimeout lack JSDoc; applyTargetFrameRate lacks @param/@returns.
🔍	src/config/frameRate.ts:19,27 — FrameRateSession/FrameRateLogger don't document each public property; FrameRateLogger has no summary.
🔍	helpers/react/CloudXRComponent.tsx:154 — (webXRManager as any).getSession uses as any; three's WebXRManager types getSession(): XRSession | null, so it's avoidable. Pre-existing, relocated by this diff.
🔍	src/config/frameRate.ts:81-106 — waitForReportedFrameRate's new Promise is correct (bridges event + timeout, owns listener lifecycle); keep it, just add @param/@returns.

Actionables (for bots — copy-paste-ready for AI)

Agent-generated suggestions, not human-vetted obligations. Skip anything that's wrong, already addressed, or not worth the churn.

• src/config/frameRate.ts:18 — replace FrameRateSession with XRSession in the helper signature and CloudXRComponent.tsx; delete the interface (or derive it via Pick<XRSession, …> and document each member).
• src/config/frameRate.ts:59 — rewrite raceUpdateAgainstTimeout with Promise.race:

  function raceUpdateAgainstTimeout(update: Promise<void>, timeoutMs: number): Promise<UpdateOutcome> {
    let timer: ReturnType<typeof setTimeout>;
    const timeout = new Promise<UpdateOutcome>(resolve => {
      timer = setTimeout(() => resolve({ kind: 'timeout' }), timeoutMs);
    });
    const settled = update.then(
      (): UpdateOutcome => ({ kind: 'updated' }),
      (error: unknown): UpdateOutcome => ({ kind: 'rejected', error }),
    );
    return Promise.race([settled, timeout]).finally(() => clearTimeout(timer));
  }

• src/config/frameRate.ts:54-106 — add explanatory comments to the complex helpers: on raceUpdateAgainstTimeout, why the timeout branch resolves (not rejects) and why the timer is cleared so a hung updateTargetFrameRate() can't block CloudXR; on waitForReportedFrameRate, why the WebXR spec lets session.frameRate lag until frameratechange and what the done/finish guard prevents (double-resolve / listener leak).
• src/config/frameRate.ts:121 — introduce const fallbackFrameRate = currentFrameRate ?? targetFrameRate; and reuse it at each early return.
• src/config/frameRate.ts:168 — add const reportsStaleRate = () => session.frameRate !== undefined && session.frameRate !== targetFrameRate; and use it for both the settle-wait guard and the post-wait recheck.
• src/config/frameRate.ts:108-183 — make the two phases explicit (section comments or extract confirmAppliedRate(...)), and add a // step comment above each guard/early return, the try, each outcome branch, and the settle-wait.
• helpers/react/CloudXRComponent.tsx:154 — type webXRManager as three's WebXRManager, call .getSession() without as any, and drop the as XRSession & FrameRateSession cast at line 161.
• src/config/frameRate.test.ts — annotate each fake with // @ts-expect-error partial XRSession fake — only frame-rate members are exercised.
• src/config/frameRate.ts:42/46/59/81/108 — add JSDoc (@param/@returns) to the named functions; add a summary + per-member docs to FrameRateLogger (and FrameRateSession if kept).