Skip to content

refactor(xr-render): adopt native agent runtime and tools - #354

Merged
nvddr merged 8 commits into
mainfrom
agent/xr-render-dispatcher
Aug 13, 2026
Merged

refactor(xr-render): adopt native agent runtime and tools#354
nvddr merged 8 commits into
mainfrom
agent/xr-render-dispatcher

Conversation

@nvddr

@nvddr nvddr commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • migrate the xr-render worker to the shared AgentRuntime, VoiceAgent, and Relay-managed native tool-calling path
  • remove xr-ai-nat and xr-ai-pipecat from the sample worker while preserving its model-visible tool schemas and runtime behavior
  • organize the worker as the xr_render_demo_worker package, with the resident agent in agent.py and lifecycle, model loop, model I/O, tool composition, and spatial tools in focused modules
  • split shared tracking, video-memory, historical-vision, text-memory, and request types into focused xr-ai-tools modules
  • retire the six in-tree OXR, render, transcript, vector, video, and VLM MCP compatibility server packages and their dedicated tests
  • keep the generic xr-ai-nat MCP publisher available for applications that explicitly need an outward MCP compatibility surface

Behavior preserved

The render tool loop, tool names and schemas, quick acknowledgement, progress and panel messages, conversation history, vision calls, participant routing, XR lifecycle, text input, interruption, and TTS behavior remain unchanged. Typed capability services remain the process boundaries; native agents call their Relay-managed tools directly.

Impact

The branch removes 4,959 lines while adding 621 lines, for a net reduction of 4,338 lines. The sample no longer starts or depends on any MCP server or NAT agent compatibility layer.

Validation

  • full non-GPU suite: 804 passed, 10 deselected
  • focused render, native-tool, and typed-service suite: 59 passed
  • strict Sphinx documentation build passed
  • Ruff 0.15.16 passed
  • worker package import smoke test passed
  • SPDX checks passed

Depends on #351.

@nvddr
nvddr deployed to github-pages August 12, 2026 16:19 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from 933c6e5 to 1f829a1 Compare August 12, 2026 16:35
@nvddr
nvddr deployed to github-pages August 12, 2026 16:36 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@nvddr
nvddr marked this pull request as ready for review August 12, 2026 16:46
@nvddr
nvddr force-pushed the agent/voice-output-mailbox branch from 960d0a1 to 99edbad Compare August 12, 2026 17:21
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from 1f829a1 to 740af31 Compare August 12, 2026 17:23
@nvddr

nvddr commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Restacked on simplified PR 351 in 740af31. XR render uses the same single VoiceAgent boundary and no removed compatibility API. The combined XR-render/runtime/voice/simple-VLM suite passes 143 tests; targeted Pyright, pre-commit, the worker wheel build, and lock resolution are clean. @wenxind-nvidia @blongs-nv @yanziz-nvidia, please review the updated head.

@nvddr
nvddr deployed to github-pages August 12, 2026 17:25 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@nvddr
nvddr force-pushed the agent/voice-output-mailbox branch from 99edbad to 13b98ea Compare August 12, 2026 17:58
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from 740af31 to 74729aa Compare August 12, 2026 18:02
@nvddr

nvddr commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

Restacked at 74729aa. XR render now injects its sample-owned xr-render.user-query topic into VoiceAgent; lifecycle messages use the separate xr-render.notice topic. The combined RenderQuery(kind=...) model and unused fresh_match plumbing are gone. The 143-test combined suite, focused Pyright, pre-commit, worker build, and lock check pass. @wenxind-nvidia @blongs-nv @yanziz-nvidia, please review the updated head.

@nvddr
nvddr deployed to github-pages August 12, 2026 18:04 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@blongs-nv

Copy link
Copy Markdown
Contributor

Reviewed at 74729aadddeca4f6159125a88f909bab883551d9 by a bot.

Blockers

  1. The LLM warmup now runs before any health probe, which defeats it on exactly the cold-start path it exists for.

    agent-samples/xr-render-demo/worker/xr_render_demo_worker.py:113-122
    On the base branch, wait_for_services({LLM, agent-LLM, STT, TTS, VLM}) completed before the llm.chat("Add a small cube.", ...) warmup. In this PR the warmup is the first statement inside the WorkflowBuilder block, while the probes only run in VoiceSession.__aenter__, which is reached from VoiceAgent.start inside async with runtime: at line 195, several statements later. Nothing else in main() awaits LLM health before that chat call. On a cold boot with vLLM still loading (the only case where health_check: true makes the warmup run at all), the call either fails fast into the except Exception warning or burns its 120 s timeout, and the engine is still cold for the first real turn: the kernel-autotune-exceeds-quick-ack-timeout failure the deleted comment documented. Await the LLM/agent-LLM probes (or an explicit wait_for_services) before the warmup, or move the warmup to run after session readiness. While fixing this, also move or restore the comments: the "VLM readiness must settle GPU memory before LOVR creates its Vulkan device" line at 113 described the deleted probe block and now sits over the unrelated warmup, whose own cold-vLLM rationale was dropped.

  2. Interruption and participant departure now block the voice pipeline on a synchronous cross-agent cancel round-trip.

    agent-samples/xr-render-demo/worker/xr_render_demo_worker.py:166-182
    The participant_left and interrupted callbacks are awaited from the voice IO processor's process_frame, i.e. on the pipeline task that processes all audio for all participants. Each does an awaited runtime.call into the render agent's mailbox, where the cancel handler awaits _cancel, which awaits the turn task's full unwind including its finally terminator publish. The old code's interruption path was a non-awaited task.cancel(). So every barge-in and every departure now stalls the whole pipeline until the turn (possibly mid LLM or tool call) finishes unwinding, and the call can also queue behind whatever the render mailbox is already processing, since deliveries are sequential per agent. Under output backpressure (voice mailbox and response stream both bounded at 32 while TTS drains in real time) this widens from a stall toward a hang: the terminator publish at dispatch.py:160-165 can block on a full mailbox that only the stalled pipeline would drain. Make both callbacks fire-and-forget (asyncio.create_task or a runtime-owned task) and keep the awaited cancel only inside _start_turn, where it buys terminator-before-next-stream ordering.

  3. This PR makes two shipped docs false, and the repo's documentation rule requires them updated in the same change.

    README.md:322 still says "The Pipecat pipeline runs quick-acks and a Nemotron-30B agentic tool-calling loop" for this sample, and docs/source/components/agent-sdk.md:22-24 describes xr-ai-pipecat as "retained for unmigrated consumers such as xr-render-demo", where this PR is precisely the migration and no in-tree sample uses that surface afterwards. The documentation rule at AGENTS.md:234 says README and relevant docs are updated in the same task as the code change, and the PR did update five other docs, so these two are misses rather than scope calls. Also update the follow-ups item at AGENTS.md:215, which still refers to RenderSceneProcessor, a class this PR renames, leaving the reference dangling.

  4. The notice-to-voice seam this PR introduces is never exercised end-to-end, and the interrupt flag is asserted nowhere.

    tests/test_xr_render_dispatch.py:155-176
    The old wire test proved a launch-failure notice reached the TTS-facing sink through the assembled pipeline. Its replacements cover the two ends but not the middle: the wire test iterates handle_notice directly, and the new dispatch test registers only a _QueryRecorder and asserts the RenderNotice was published. Nothing registers a RenderAgent as a consumer of RENDER_NOTICE_TOPIC, so answer_notice_start_turn → the is_notice branch → VoiceOutput(..., interrupt=interrupt_output and first) at dispatch.py:130-158 is exactly the new code and exactly what no test runs; a notice subscribed to the wrong topic or an interrupt that never gets set would pass the whole suite. Add one test that registers RenderAgent plus the existing _VoiceRecorder, publishes a RenderNotice with interrupt_output=True on RENDER_NOTICE_TOPIC, and asserts the first VoiceOutput carries the notice text with interrupt true, followed by the empty terminator.

Suggestions

  1. Register voice before xr-render so shutdown (which stops agents in reverse registration order) cancels render turns before the voice agent tears down the transport and closes the LLM/VLM clients they unwind against. Today xr_render_demo_worker.py:159-192 registers them the other way, so on any shutdown with an active turn the turn's chunk publishes raise RuntimeClosedError (only the terminator publish is suppressed) and _discard logs "render agent failed" as an ERROR. Whichever order you land on, suppress RuntimeClosedError consistently for the per-chunk publish at dispatch.py:150-158 or treat a closed runtime as a clean end of turn.

  2. In _run_turn's finally, await response.aclose() (with a suppress, and guarding for response being unbound if handle_query raised) before publishing the terminator. The SDK's IO processor deliberately closes response generators on cancellation, but dispatch.py:159-165 drops the generator, so a cancel that lands while the task is suspended in ctx.publish (widened under TTS backpressure) defers the scene generator's finalization to GC. Its teardown at processors.py:266-271 can then set the participant status to "idle" after the superseding turn already set "processing", and the CancelledError branch that flushes return audio is skipped under GeneratorExit.

  3. Guard agent.py:91 (_notify_launch_failed) against RuntimeClosedError: it is reachable from a hub on_data callback after runtime shutdown has begun, and the exception would propagate into the endpoint's callback dispatch. (The SDK already wraps the participant_left/interrupted callbacks in its own exception guard, so those need nothing.)

  4. interrupt_on_supersede=True at xr_render_demo_worker.py:190 is new behavior: the old sample never flushed the previous turn's draining TTS when a new query arrived for the same participant, and now it does. Likely an improvement, but it contradicts the PR's "behavior preserved" claim; note it in the description or drop the flag.

  5. Add tests for the addressed-message surface the worker wiring depends on: nothing dispatches CancelRender or CancelAllRender through the runtime, so a broken @handler registration would fail silently in participant_left / interrupted. tests/test_simple_vlm_example_worker.py:589 (test_voice_agent_cancels_participant_background_work_on_call) has the copyable pattern; mirror it for CancelRender, and add a second participant for CancelAllRender. Also give test_render_agent_supersedes_a_participant_turn output assertions: register a _VoiceRecorder and assert the superseded turn's terminator arrives with the first turn's response_id before the second turn's chunks. Today it asserts nothing about output, so it still passes if the finally terminator is deleted, and it is the test that would prove the terminator actually escapes the cancelled task.

  6. Restore the two load-bearing comments this refactor dropped in xr_render_demo_worker.py: text_topic="" at line 108 overrides a default of "agent.response" because the scene agent sends its own panel message (without the note, the empty string reads as an accident), and the bare RenderDemoAgent(...) construction at lines 160-165 now reads as dead code without the line explaining the endpoint retains its bound callbacks. Also update the _CaptureTransport docstring at tests/test_xr_render_demo_wire.py:393, which still names the deleted XRMediaHubTransport.

This is a clean, well-shaped migration: the worker genuinely sheds every Pipecat import, the sample-namespaced xr-render.user-query / xr-render.notice topics are the right shape, the supersede design (await the prior turn's teardown, publish the terminator from its finally, then start the next turn) is careful, and the docs and changelog that were updated are accurate and thorough. The blockers above should be resolved before merge.

@nvddr
nvddr force-pushed the agent/voice-output-mailbox branch from 1cc0040 to 2c96462 Compare August 12, 2026 18:26
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from 74729aa to f1184b0 Compare August 12, 2026 18:29
@nvddr
nvddr deployed to github-pages August 12, 2026 18:30 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@wenxind-nvidia

Copy link
Copy Markdown
Collaborator

Reviewed at f1184b0b666a4fc7685f24ac03756cf6378c492d.

Blockers

  1. LLM warmup runs before service readiness. xr_render_demo_worker.py calls llm.chat() before VoiceSession.__aenter__() runs the health probes during runtime startup. On a cold local startup, the warmup can therefore fail or consume its timeout without warming the model, recreating the first-turn timeout it exists to prevent. Restore readiness before warmup.

  2. Normal interruption can produce an orphan stream terminator. RenderAgent._run_turn() always publishes a terminator after an opened turn is cancelled. The voice IO processor closes the response stream before invoking the interruption callback, and VoiceAgent.output() rejects a terminator whose stream has already been evicted. I reproduced the resulting ValueError: voice stream terminator has no open response by closing the active voice stream and dispatching CancelRender. Distinguish consumer-aborted cancellation from render supersession so only the latter publishes a terminator.

  3. Interruption blocks the shared voice pipeline on render cleanup. The callbacks in xr_render_demo_worker.py await runtime.call(), and its handler waits for the complete render task unwind. That cleanup includes audio flushing, status IPC, generator cleanup, and potentially a mailbox publish. Because the voice IO processor awaits these callbacks, one slow cancellation stalls media processing for every participant. Dispatch cancellation asynchronously from these callbacks.

  4. Required documentation remains stale. README.md still says the Pipecat pipeline runs the agentic loop; docs/source/components/agent-sdk.md names xr-render-demo as an unmigrated Pipecat consumer; and AGENTS.md references the removed RenderSceneProcessor. The repository documentation rule requires these updates in the same change.

Suggestions

  1. Replace CancelRender and CancelAllRender with CancelRender(all: bool = False), matching the simpler established CancelVision pattern. This removes one message model and one handler.

  2. Remove the pid or self._transport.target_participant fallback in processors.py. Runtime turns already require a participant, so the fallback is unreachable during valid operation and can route output to the wrong client if that contract is violated.

  3. Explicitly close the scene response generator when _run_turn() exits. Cancellation while suspended in ctx.publish() otherwise leaves generator cleanup and its idle status update to asynchronous finalization.

  4. Register voice before xr-render in xr_render_demo_worker.py, so reverse-order shutdown cancels render work before VoiceAgent closes the LLM/VLM clients it may still be using.

  5. The separate RenderNotice topic and is_notice branch serve one fixed launch-failure string. Consider cancelling the active turn, sending its panel message, and publishing a finite VoiceOutput directly; that removes a model, topic, subscription, branch, and test surface.

  6. interrupt_on_supersede=True adds behavior during what is described as a behavior-preserving refactor. Remove it to keep the PR focused, or document and test the intentional behavior change.

Validation: 70 focused tests passed and Ruff passed. The additional interruption probe exposed blocker 2.

@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from f1184b0 to 6191f3b Compare August 12, 2026 20:15
@nvddr
nvddr deployed to github-pages August 12, 2026 20:15 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 12, 2026
@nvddr
nvddr force-pushed the agent/voice-output-mailbox branch 2 times, most recently from 5a77c2b to baeed89 Compare August 12, 2026 21:05
@nvddr
nvddr deployed to github-pages August 13, 2026 02:44 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 13, 2026
@nvddr

nvddr commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

@blongs-nv @wenxind-nvidia — review disposition for d345821, restacked onto PR #351 at 7e91705.

Blockers

  • LLM readiness/warmup ordering. Warmup is now part of the LLM readiness probe: health must pass first, and readiness remains false until the optional warmup succeeds. The probe has cold, failed-warmup, and success coverage.
  • Shared voice-pipeline blocking. Participant-left and interruption callbacks now return immediately and schedule publications on VoiceAgent-owned tasks. Those tasks are cancelled and awaited during shutdown. Replacement-query ordering waits for old response cleanup only on the participant input task, outside the shared media processor.
  • Orphan stream terminators. Render cancellation now distinguishes producer supersession from consumer abort. Supersession finishes the old stream before replacement chunks; interruption/departure closes the scene generator without publishing to an evicted voice stream. The regression consumer explicitly rejects orphan terminators.
  • Stale documentation. The root README, AGENTS.md, Agent SDK docs, quickstart, xr-render guide/mirror, voice README, and changelog now describe the native runtime/tool topology and lifecycle behavior.
  • Notice-to-voice coverage. The launch-failure test now runs the complete lifecycle notice → RenderAgentvoice.output path and asserts first-chunk interrupt=True plus the matching terminator.

Suggestions

  • Voice is registered before render; active render turns stop before render-owned model clients close; per-chunk RuntimeClosedError is a clean exit.
  • Scene response generators are explicitly closed on every exit path.
  • Launch-failure publication ignores runtime closure during shutdown.
  • The behavior-changing interrupt_on_supersede=True flag was removed.
  • Participant and global interruption, departure cleanup, producer-supersession terminator ordering, and generator finalization all have runtime-level tests.
  • The participant fallback to target_participant was removed.
  • The load-bearing text_topic="" and retained-lifecycle comments were restored; the stale transport test name was corrected.
  • The earlier CancelRender / CancelAllRender suggestion is superseded by the current typed topic subscription design; those message types no longer exist.
  • [~] RenderNotice retained intentionally. Lifecycle producers publish a typed notice while RenderAgent remains the single owner of panel/voice turn ordering and interruption semantics. The new end-to-end test covers that seam.

Validation on the final rebased tree:

  • uvx ruff check .
  • 825 passed, 10 deselected for the full non-GPU suite
  • 167 passed in the expanded voice/render/simple-VLM suite
  • strict Sphinx build
  • SPDX validation for all modified tracked files

CI has been triggered for the pushed head and is still running.

@blongs-nv

Copy link
Copy Markdown
Contributor

Reviewed at d345821a1d0364e33fad164c30a5bc2376430f31 by a bot.

Everything from my previous review is resolved, and resolved well: the warmup lives inside the LLM readiness probe, cancellation is topic-based and fire-and-forget, the docs were rewritten, the notice path has a real end-to-end test, generators are closed on every exit path, and the shutdown races are guarded and tested. The scene-loop port is byte-identical to the base where it claims to be. The PR has also roughly quadrupled in scope since then (worker package reorg, the xr-ai-tools split, six MCP servers retired), so the findings below are about the new surface, not the previously reviewed migration.

Blockers

  1. The client and server halves of the service RPC protocol now run different copies of the code, and no test exercises that pairing.

    agent-sdk/xr-ai-tools/xr_ai_tools/rpc.py:19 is a near-verbatim copy of xr_ai_nat/functions/_service/rpc.py. The duplication is fine as a transitional state while xr-ai-nat winds down; the problem is what production runs during the window. The worker's new clients (tracking.py, video_memory.py) speak the tools copy, the services they connect to still bind the nat copy (openxr_service/__main__.py:16, video_memory_service/__main__.py:15), and the tests cover only nat↔nat and tools↔tools, so a change to either copy passes CI and fails only in a live stack. Either migrate the two services onto xr_ai_tools.rpc now (the migration the nat wind-down implies anyway), or add one tools-client↔nat-server test as the interim gate. Either way, port the correlation and remote-error-code tests from tests/test_xr_tracking_functions.py:39-57 to the tools copy, which is the one that survives.

  2. Five scene-engine regression tests guarding named, previously-fixed bugs were deleted along with an MCP suite they never depended on.

    The deleted tests/test_local_render_mcp.py (base version, lines 382-601) contained test_close_cancels_lovr_watch_task, test_lovr_respawn_closes_previous_launch_context (issue render-mcp leaks ManagedProcess contexts on LOVR respawn (parked in app-lifetime AsyncExitStack) #196), test_resync_delivers_after_late_peer_connect (render-mcp scene resync after LOVR respawn silently dropped (PUSH+NOBLOCK with no connected peer) #198), test_live_forward_fast_drops_during_resync_window, and test_resync_is_bounded_when_lovr_never_connects. These import xr_render_scene.engine and SceneService directly, not the MCP wrapper, and engine.py ships unchanged at this head with essentially no remaining coverage. Re-home the five tests near verbatim (e.g. tests/test_scene_engine.py); they need no MCP.

  3. The worker's execution path moved onto new xr-ai-tools implementations while their behavioral tests stayed behind on the nat originals the worker no longer runs.

    CI stays green on dead code, and since the nat suites disappear with xr-ai-nat, this is the moment the behavioral guarantees either move to the surviving copies or vanish. The consequential gaps: the spatial math (spatial.py plus spatial_tools.py) is a fresh reimplementation with no numeric test — the only new-path assertion is a schema-shape check that passes with every formula wrong — and TextMemoryTool._path reimplements the name disambiguation and symlink path-escape guard whose tests now cover only the nat copy. Add a table-driven test over the five spatial functions with a known SpatialFrame (including the degenerate forward≈0 fallback), and port the two text-memory tests against TextMemoryTool.

Suggestions

  1. In agent.py:214-236, set opened = True before the first chunk's publish rather than after. A supersede cancel landing mid-publish can deliver the chunk but skip the terminator, leaving an unterminated stream that silently discards the participant's later responses; every input path today self-heals through the voice IO supersede-cancel, but a non-interrupting RENDER_NOTICE_TOPIC publisher (which the changelog invites) would not. One wrinkle: the fix can produce an orphan empty terminator, which reaches VoiceAgent.output's ValueError wrapped in a BaseExceptionGroup that suppress(RuntimeClosedError) will not catch — widen that suppression accordingly.

  2. agent.py:265-268: _discard never retrieves the task's exception, and with bare asyncio.create_task there is no longer a runtime backstop, so a turn that dies outside the loop's own except Exception surfaces only at garbage collection. Log task.exception() there.

  3. Add a _stopped guard to RenderAgent._start_turn (agent.py:92): a launch-failure notice published while the LOVR poll is still running can spawn a turn after stop()'s snapshot, and that task survives render.stop() into main's finally, where the model clients are closed underneath it.

  4. __main__.py:179-185: the shutdown asyncio.gather needs return_exceptions=True, or one failing close abandons the rest mid-flight.

  5. rpc.py:83-92: self._socket is re-read after acquiring _send_lock, but the receive loop's failure path sets it to None, so a concurrent call after a receiver death raises AttributeError instead of the typed RPCError. Bind it to a local before the lock.

  6. Two behavior changes contradict "behavior preserved" and deserve a changelog line: get_frame_from_time is no longer model-visible (tools.py:62), and launch-failure notices now set interrupt_output=True (lifecycle.py:101), cutting off speech already draining through TTS.

  7. scene_loop.py:889: the live-perception failure gate string-matches LiveVisionTool's prose, and the hub's "Frame data unavailable — please retry." message slips through to the model as a genuine VLM answer. A typed marker on VisionResponse would be sturdier.

  8. The new spatial request models declare extra="forbid" (spatial_tools.py:28) where the NAT-derived models ignored extras, and distance lost its lower bound (negative values are now a runtime ValueError with no test — the deleted test_oxr_mcp case was the guard). Add the validation test and confirm the schema change is intended.

  9. lifecycle.py:53: the XR launch success path has no test — session-start handling, the _xr_started re-ack, start_xr error → notice, and the _wait_lovr spawn-error/timeout branches are all cheap to drive with a fake ToolSet and capture transport.

  10. Tool plumbing is advertised but not wired: owned_tools (__main__.py:154) feeds Agent.tools, which nothing reads, and lifecycle.py:126 JSON round-trips a synthetic ToolCall to reach a typed client it already holds. Call the typed tool directly, and either make the runtime consume agent.tools or stop passing them.

  11. Turn state is still process-global under the new per-participant surface: _history, _recent_moves, and _pre_move_positions (scene_loop.py:210-217) are shared across participants, and _pre_move_positions is reassigned mid-turn at line 550, so concurrent participants corrupt each other's move log. Inherited from the base, but RenderAgent now runs participant-keyed concurrent turns above it: key the state by participant or assert single-session loudly.

  12. Package hygiene: the brand-new __main__.py inherits an I001 suppression (ruff.toml:35) masking a genuinely unsorted import block; __main__.py and eval/eval.py import module-private _PERCEPTION_* names from scene_loop (two external consumers — drop the underscores); the worker's pyproject.toml:39 uses only-include where every other package uses packages; and the comment at scene_loop.py:55-57 still describes the retired xr_vision_tools NAT group.

The migration itself is in excellent shape: every finding from the previous round was addressed properly, the MCP retirement left no dangling references anywhere, and the scene-loop port preserves base behavior byte-for-byte. The blockers share one theme — the code migrated ahead of its tests — and should be resolved before merge.

Base automatically changed from agent/voice-output-mailbox to main August 13, 2026 06:02
nvddr added 6 commits August 12, 2026 23:07
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from d345821 to 7ce1548 Compare August 13, 2026 06:46
@nvddr

nvddr commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

@blongs-nv — review disposition for c88c485, rebased onto merged PR #351 / current main (b641bf5).

Blockers

  • Single native RPC implementation. OpenXR, video-memory, and RAG services now use xr_ai_tools.rpc; xr-ai-nat has been removed completely. Native client/server integration tests cover correlation and typed remote error codes.
  • Scene-engine regressions restored. The five non-MCP tests were re-homed in tests/test_scene_engine.py: watch cancellation, respawn context cleanup, late-peer resync, live-forward dropping during resync, and bounded no-LOVR resync.
  • Behavioral tests moved to the surviving tools. Spatial math has table-driven numeric and degenerate-forward coverage. Native text memory covers identity collisions and symlink/path escape. Vision, tracking, video-memory, and RAG tests exercise the native implementations and service boundary.

Suggestions

  • Render marks a stream opened before its first publish and treats runtime closure / the exact consumer-evicted orphan terminator (including exception groups) as clean shutdown behavior.
  • Background task completion retrieves and logs unexpected exceptions; _start_turn is guarded after stop.
  • Shutdown closes all resources with return_exceptions=True; RPC binds the socket locally before entering the send lock.
  • Behavior preservation restored: get_frame_from_time remains model-visible and launch-failure notices do not interrupt existing speech.
  • Vision availability is typed (VisionResponse.available) rather than inferred from prose.
  • Spatial request compatibility is preserved, including ignored extras and nonnegative distance validation; regression coverage was added.
  • XR lifecycle tests cover start/re-ack, start failure, and LOVR wait behavior.
  • Lifecycle calls typed tools directly, and unused Agent.tools plumbing was removed from the render agent.
  • Scene history, move history, and pre-move positions are participant-scoped, with isolation coverage.
  • Package hygiene items were addressed: public perception constants, sorted imports/no suppression, Hatch packages, and current native-tool comments.
  • NAT’s remaining RAG and text-memory capabilities were migrated to native typed tools; NAT dependencies, package, compatibility tests, and migration-only docs were retired. Dependency and architecture docs were updated in the same change.

Validation on the pushed rebased head:

  • 829 passed, 10 deselected in the full non-GPU suite
  • uvx ruff check .
  • SPDX validation for all 45 modified/added source files
  • git diff --check

The final change is a net reduction: 1,409 insertions and 3,848 deletions, including the restored/new regression coverage.

@nvddr
nvddr deployed to github-pages August 13, 2026 06:47 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 13, 2026
Comment thread agent-sdk/xr-ai-tools/xr_ai_tools/rpc.py Fixed
Comment thread agent-samples/xr-render-demo/worker/xr_render_demo_worker/agent.py Fixed
Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
@nvddr
nvddr force-pushed the agent/xr-render-dispatcher branch from 7ce1548 to c88c485 Compare August 13, 2026 06:49
@nvddr
nvddr deployed to github-pages August 13, 2026 06:49 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 13, 2026
@blongs-nv

Copy link
Copy Markdown
Contributor

Reviewed at c88c485fcd5370d94f9765aac5f1d5c336b06d67 by a bot.

All blockers and suggestions from my previous review are resolved, verified in the code: services share the single xr_ai_tools.rpc implementation (with xr-ai-nat retired outright), the five engine regression tests are re-homed in tests/test_scene_engine.py, the spatial and text-memory tests moved to the surviving copies with numeric coverage, and the NAT retirement leaves no dangling references. One new defect came in with the fixes.

Blockers

  1. The eval harness no longer imports.

    eval/eval.py:40-44 still imports _LIVE_PERCEPTION_TOOL, _PAST_PERCEPTION_TOOL, and _PERCEPTION_TOOL_DEFS, but the rename to public names (scene_loop.py:57-107) missed the eval, so it dies with ImportError at startup — invisible to pytest since the eval isn't in the suite. Update the imports and their use sites (e.g. lines 1413-1415).

Suggestions

  1. agent.py:61-68: _expected_stream_close matches the consumer-evicted terminator by exact string equality against the message raised at _runtime.py:284; a rewording turns every expected close into a spurious error log. Export a typed error or shared constant from xr-ai-voice.

  2. video_memory_service/service.py:58-62: bare request validation surfaces malformed requests as internal_error, while the sibling rag and openxr services map the same case to invalid_request. Align it.

  3. Restore two assertions lost in the test migration: every model-visible spatial schema property carries a description, and live vision degrades gracefully without the video-memory service (test_look_at_current_frame_builds_and_runs_without_video_memory).

  4. Follow-ups: the per-participant state keyspace (scene_loop.py:209-222) is pruned only by departure events, so a missed event grows it unbounded; and client-samples/web/App/app.js:43 cites a glasses-agent-nat sample that no longer exists.

The fixes this round are thorough and well-tested, and the RPC unification went further than asked. The blocker is a three-line import fix; with that resolved this is ready to merge.

Signed-off-by: Devdeep Ray <devdeepr@nvidia.com>
@nvddr

nvddr commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

@blongs-nv — final review disposition for 62c7428.

Final blocker

  • Eval harness imports. The eval now imports and uses the public LIVE_PERCEPTION_TOOL, PAST_PERCEPTION_TOOL, and PERCEPTION_TOOL_DEFS names. A smoke regression executes the eval module so this cannot silently drift again.

Final suggestions and follow-ups

  • Typed stream closure. xr-ai-voice exports VoiceStreamClosedError; render recognizes that type through nested ExceptionGroup values and no longer matches exception text.
  • Video RPC validation. Pydantic request failures now map to RPCError(code="invalid_request"), consistent with the sibling typed services.
  • Restored assertions. Every model-visible spatial property must have a description, and finite live vision is explicitly exercised without video-memory.
  • Bounded participant state. Scene state uses a 1,024-participant LRU keyspace and evicts history, move history, and pre-move snapshots as one unit if a departure event is missed.
  • Stale web reference. The removed glasses-agent-nat example is replaced with xr-render-demo.

Additional coverage restored after the migration audit

  • Native vector-tool execution covers world offsets, scaling, signed along-direction movement, and coincident-point errors.
  • Text memory covers blank rejection and exact JSONL persistence.
  • Video memory covers malformed/disabled RPC behavior, ready-file startup, native client/server decode-to-PNG, and a GPU-gated real H.264 encode/decode/export round trip.
  • Live vision covers real hub frame acquisition, VLM failure/idle recovery, and private <think> removal for live and historical finite answers.
  • The three GitHub Code Quality findings are fixed and their inline threads are resolved.

Validation on the pushed head:

  • 840 passed, 10 deselected in the full non-GPU suite
  • ruff 0.15.16 check .
  • SPDX validation on all 21 changed files
  • git diff --check

The formatter-only churn was removed before commit; this follow-up is 398 insertions and 96 deletions, mostly restored regression coverage.

@nvddr
nvddr deployed to github-pages August 13, 2026 17:57 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 13, 2026
@nvddr
nvddr merged commit 01b605b into main Aug 13, 2026
14 checks passed
@nvddr
nvddr deleted the agent/xr-render-dispatcher branch August 13, 2026 19:47
@nvddr
nvddr deployed to github-pages August 13, 2026 19:47 — with GitHub Actions Active
github-actions Bot added a commit that referenced this pull request Aug 13, 2026
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.

3 participants