Skip to content

feat(output-router): per-device surface routing scaffold (§5 Next) - #741

Closed
qingyun-wu wants to merge 6 commits into
feat/device-session-scaffoldfrom
feat/output-routing-scaffold
Closed

qingyun-wu wants to merge 6 commits into
feat/device-session-scaffoldfrom
feat/output-routing-scaffold

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Summary

Implements the §5 Next "Device-aware output routing" item. Picks which surface(s) of a connected device render the agent's reply, based on DeviceProfile + ReplyKind.

Base: feat/device-session-scaffold (#739). Depends on #739 for DeviceSession + OutputChannel types.

What ships

  • src/output-router.ts (~190 LOC, new):
    • hudBudget(profile) — per-device char budget grounded in real OC device-profile yamls:
      • Mentra Live → 80 (400×240 green center)
      • Even G2 → 80 (576×288 green lower-right)
      • Brilliant Halo → 160 (color OLED, richer)
      • Meta Display → 60 (sparing UX)
      • Full AR → 240 (large transparent overlay)
      • laptop → 1000 (effectively unbounded)
    • pickSurfaces(session, reply) — pure surface-selection function. short → HUD only, narrative → voice + file_push for long/attached, alert → fans to all, attachments override file_push, task fallback for headless bridges.
    • routeReply(session, reply) — async dispatch with per-channel error isolation (HUD render failure doesn't block voice).
    • Returns RouteResult for telemetry (used/dropped channels, HUD char count, zone).
  • tests/output-router.test.ts — 18 unit tests, all pass.

What this PR does NOT change

  • No caller is wired yet. Until voice-agent's onTurnCompleted (or the bridges' reply paths) consume routeReply(), this is dead code by design — the contract is sized + tested in isolation first.

Test plan

  • npx tsx --test tests/output-router.test.ts — 18/18 pass.
  • npx tsc --noEmit clean.
  • Follow-up PR: wire voice-agent onTurnCompleted to call routeReply(activeForVision(), {text, kind}).

Roadmap impact

  • §5 Next ("Device-aware output routing"): contract landed; consumer is the next hookup PR.
  • Unblocks per-device rendering for Mentra (HUD center 80c) and Even G2 (HUD lower-right 80c) as soon as the consumer wires in.

🤖 Generated with Claude Code

qingyun-wu and others added 2 commits May 16, 2026 00:36
Adds the DeviceSession primitive that the roadmap (§5 Now item 3) calls
for and that today's vision-tools.ts TODO points at — the per-device
state slot that replaces the single-session globals (`sessionRef`,
`pushMode`, `activeSource`) which force browser, Mentra glasses,
Discord/Telegram photo helper, and phone agent to race for one slot.

Design doc: notes/devicesession-design-2026-05-16.md.

What this PR ships:
- `src/device-session.ts` — interface + in-process registry (~170 lines):
  * `DeviceProfile` mirroring OpenCompanion's `device-profiles/*.yaml`
    shape (id, category, mic, camera, hud, gestures, role).
  * `OutputChannel` union — voice / hud / file_push / task (the per-
    surface declarations the §5 Next output router will consume).
  * `DeviceSession` — one device's state (deviceId, profile, push mode,
    frame count, output channels, close()).
  * Registry: `register / unregister / get / list / activate /
    activeForVision`, plus `onEvent` for lifecycle subscribers.
  * `newLaptopSession()` constructor for voice-agent's default seat.
  * `__resetForTests()` (internal — not exported via package).
- `src/vision-tools.ts` — `getSendFile()` now prefers the active
  DeviceSession's voice transport when one is registered, falls back to
  the legacy `sessionRef` (today's browser path). Three new endpoints on
  the existing vision control server (port 7847):
  * `POST /vision/register` — bridges register a session by deviceId +
    profileId, optionally overriding profile fields.
  * `POST /vision/unregister` — bridges signal disconnect.
  * `GET  /vision/devices` — diagnostic view (active + list).
- `tests/device-session.test.ts` — 10 unit tests covering register/
  unregister/duplicate-id/idempotent-unknown/close-on-unregister/
  activate/activate-clears-on-unregister/event-emission/listener-
  exception-isolation/newLaptopSession shape. All pass.

What this PR does NOT change:
- Today's browser path: `setVisionSession(session)` is unchanged.
  `getSendFile()` falls back to `sessionRef` when no DeviceSession is
  active, so the web-client Watch button + push mode behave identically.
- Output routing: declared shape only. `routeReply()` is the next PR;
  no caller consumes `OutputChannel[]` yet.
- OC `device_register` envelope consumption: bridges call the in-repo
  HTTP register endpoint instead, mirroring the same payload. When OC's
  protocol package is consumed, the register endpoint gets a second
  branch — no architecture change.
- Phone-agent's swap-and-restore + Mentra-bridge's `/vision/register`
  call site: separate follow-ups (this PR provides the seam they wire
  into).

Test plan:
- [x] `npx tsx --test tests/device-session.test.ts` — 10/10 pass.
- [x] `npx tsc --noEmit` clean.
- [ ] Manual: `curl -s :7847/vision/devices` returns `{active: null,
      devices: []}` on a fresh voice-agent boot.
- [ ] Manual: `curl -X POST :7847/vision/register -d '{"deviceId":
      "test","profileId":"glasses-test"}'` returns
      `{status: "registered", deviceId: "test"}` and `/vision/devices`
      now shows it as active.

Roadmap impact:
- §5 Now item 3 (DeviceSession): primitive landed.
- §5 Now item 2 (Mentra): unblocks the `src/mentra-bridge.ts` follow-up
  — its three TODOs (DeviceSession, output routing, manifests) now have
  an anchor.
- §5 Next ("Device-aware output routing"): `OutputChannel[]` declared;
  router PR consumes it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the §5 Next "Device-aware output routing" item:

  "When Sutando replies, pick the surface (HUD, voice, voice+HUD,
   voice+attached file) based on the active device profile and the
   reply shape."

What ships:
- `src/output-router.ts` (~190 LOC, new):
  * `AgentReply` type — text + optional kind ('short' | 'status' |
    'narrative' | 'alert') + attachments.
  * `hudBudget(profile)` — per-device char budget. Mentra Live = 80,
    Even G2 = 80, Brilliant Halo = 160 (color), Meta Display = 60 (used
    sparingly), Full AR = 240, laptop = 1000 (effectively unbounded).
    Numbers mirror the §7 fleet matrix in `vision and roadmap.md`.
  * `hudZone(profile)` — default HUD zone for the device.
  * `pickSurfaces(session, reply)` — pure decision function. Short →
    HUD only; narrative → voice + file_push for long bodies or
    attachments; alert → fans to all surfaces. Task fallback when no
    real surface picks up (preserves the reply for headless bridges).
  * `routeReply(session, reply)` — async dispatch with per-channel
    error isolation (HUD failure does not block voice).
  * Returns `RouteResult` for telemetry — used/dropped channel lists +
    actual HUD char count + zone.

- `tests/output-router.test.ts` — 18 unit tests covering each per-
  profile budget, the four ReplyKind surface choices, attachment
  override, task fallback isolation, truncation + ellipsis at word
  boundary, error-isolated dispatch (alert with broken HUD still sends
  voice). All pass.

Depends on PR #739 (DeviceSession types). No callers wired yet — that
is the next PR. Voice-agent's `onTurnCompleted` hook + bridges' reply
paths become the consumers; until then this is dead code by design,
ready for the hookup.

Test plan:
- [x] `npx tsx --test tests/output-router.test.ts` — 18/18 pass.
- [x] `npx tsc --noEmit` clean.
- [ ] Follow-up PR: wire voice-agent `onTurnCompleted` to call
      `routeReply(activeForVision(), {text, kind})`. Behavior change
      will land there.

Roadmap impact:
- §5 Next ("Device-aware output routing"): contract landed.
- Unblocks per-device reply rendering for Mentra (HUD center, 80
  chars) and Even G2 (HUD lower-right, 80 chars) once the next PR
  wires the consumer.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the contract — clean separation of "pick the surfaces" (pure) from "dispatch to them" (async, error-isolated).

Things I like:

  • pickSurfaces is pure → testable in isolation, no mocks needed. The 18-test coverage hits the kind matrix + the attachment-override + the task-fallback corner.
  • Attachment override (if (hasAttachments && hasFilePush) useFilePush = true) with the right comment ("losing an attachment because the kind tag was 'short' would be a contract violation") — defensive against caller mis-tagging.
  • Task fallback for headless bridges so an alert/narrative doesn't silently drop on always-on recorders.
  • Per-channel error isolation in routeReply — HUD render failure doesn't suppress voice; file_push failure on one attachment doesn't drop the others (loop swallows per-iteration). The RouteResult distinguishing used vs dropped makes the failure observable for telemetry.
  • hudBudget table mirrors the §7 fleet matrix with per-device values grounded in real screen specs. The 1000-char laptop cap is a nice "effectively unbounded but not infinite" safety.
  • truncate trims at word boundary when possible (60% threshold) — produces readable HUD ellipsis without splitting mid-word.

Two design questions / observations (neither blocking):

  1. narrative kind never picks HUD even when both surfaces exist. A device like Brilliant Halo (160c HUD + voice) could show a short narrative summary on HUD while speaking the full text — useful for "here's the answer, also keep this visible". Right now narrative is voice-only unless longText || hasAttachments flips file_push. Intentional given the roadmap quote ("narrative → voice + push file to laptop")? Or a follow-up "narrative + HUD summary" mode?

  2. alert fans to file_push too. Real alerts (build broken, fire alarm metaphor) probably don't want to drop a file every time. Maybe useFilePush = hasFilePush && hasAttachments for alert kind, matching narrative's gating? Or accept the slight noise as "you wanted to know NOW, here's everything."

One micro-nit: pickSurfaces returns {useHud, useVoice, useFilePush, useTaskFallback} — flat record is fine for now, but if you ever want ordering (e.g., HUD first when both apply, or "render HUD ack then send voice"), you'd want an ordered surfaces: SurfaceKind[] instead. Probably overthinking for a scaffold.

Solid plumbing for the §5 Next item. Whenever the consumer PR (voice-agent onTurnCompleted hookup) lands, this contract is ready.

Three small fixes per sonichi's review:

1. Validate body.category in /vision/register against the known union.
   Returns 400 on invalid category so a typo can't pollute the registry
   with category:'spaceship' that downstream consumers (switch by
   category) silently fall through.

2. Add policy?: DeviceAccessPolicyRef to the DeviceSession interface.
   Lets PR #746's attachPolicy() store on the session without the
   (session as DeviceSession & { policy?... }) cast at every call site.
   Forward-declared shape (allowed: Set<string> + owner_approved +
   label) avoids a circular import from device-access-policy.ts.

3. Add transportReady boolean to /vision/devices response. A bridge
   polling the endpoint after /vision/register can distinguish
   'registered AND deliverable' from 'registered but transport isn't
   connected yet' (e.g. /vision/register raced setVisionSession on cold
   start). Tracks the same predicate getSendFile() uses.

Test plan:
- [x] npx tsc --noEmit clean.
- [x] npx tsx --test tests/device-session.test.ts — 10/10 pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request May 16, 2026
Two fixes per sonichi's review:

1. Doc-comment the silent-skip cases — getRecentTurns(2) without an
   assistant turn (user interruption / two quick user follow-ups)
   correctly skips, but the silence was previously unexplained. Now
   spelled out so the next reader doesn't suspect a bug.

2. Resolve the description-vs-behavior mismatch. The PR body claimed
   'HUD + any other declared surfaces render alongside voice', but
   kind='narrative' in #741's pickSurfaces routes to voice + file_push
   (no HUD). For glasses-class devices, that meant HUD never fired.

   Fix: pick kind based on text length — <=80 chars → 'short' (which
   fires HUD per the #741 matrix), >80 → 'narrative' (voice + maybe
   file_push). Heuristic, not contract; documented as such and
   acknowledged that real-device dogfood will refine the threshold.

Test plan:
- [x] npx tsc --noEmit clean.
- [ ] Manual flag-ON + Mentra glasses + short reply: HUD render fires
      (used=[hud,voice]).
- [ ] Manual flag-ON + Mentra glasses + long reply: voice + file_push
      (used=[voice,file_push]), no HUD.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves the #741 alert kind pending question (sonichi review). The
'alert always fans to file_push' concern is structurally addressed at
the dispatch layer — routeReply() line 174 gates the push() call on
reply.attachments?.length, so an attachment-less alert produces no
spurious file write regardless of pickSurfaces returning useFilePush=true.

This commit only adds an explanatory comment so the next reader sees
the layered gating immediately and doesn't mis-read 'useFilePush = true'
in the alert branch as 'always write a file on every alert'. No
behavior change.

Tests: 18/18 still pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: new output-router.ts as the contract for device-surface routing (HUD/voice/voice+HUD/voice+attached file). Pure module + types — consumers (voice-agent onTurnCompleted hook in #745, bridges' explicit reply paths) light it up in follow-ups. Mirrors OpenCompanion device-profiles yaml shape for hudBudget. — Lucy (Mac Studio bot)

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — separating pickSurfaces() (pure decision) from routeReply() (effectful dispatch) is the right factoring for the "device-aware output routing" §5 Next item. The 4 ReplyKinds (short / status / narrative / alert) cover today's traffic shapes without over-fitting, and the per-profile hudBudget() table grounds the abstraction in concrete device numbers (Mentra 80 / Even G2 80 / Brilliant Halo 160 / Meta Display 60 / Full-AR 240 / Laptop 1000). pickSurfaces() returning a plain {useHud, useVoice, useFilePush, useTaskFallback} boolean record means tests can assert decisions without instantiating mock channels.

Things I like:

  • Per-profile HUD budget by profile.id, not category. A glasses category covers wildly different optical capabilities (Mentra Live 400×240 green-only vs Brilliant Halo 640×400 color OLED). Keying by id lets each device declare its real legibility ceiling, and the comment ("These mirror the §7 fleet matrix in the roadmap") names where the source-of-truth lives. The default: 80 is the safe-unknown-glasses behavior, not a fallback for the entire category.

  • truncate() does word-boundary trimming with a 60% threshold. lastSpace > n * 0.6 means an extremely long single word doesn't cause the truncator to chop at byte 1 of the slice — it falls through to the byte boundary instead. Small but exactly the kind of edge case that surfaces "why did my HUD say …" tickets a month later. The single-char ellipsis (U+2026 …) instead of three dots saves 2 chars on a tight budget.

  • useFilePush = hasFilePush for alert kind is intentionally permissive. The comment is essential — it explains that the boolean flag is "willing" not "will," and the actual file_push.push() invocation in routeReply() is gated by reply.attachments?.length. So an attachment-less alert produces zero spurious file writes; an alert WITH attachments correctly fans to file_push. The "if a future alert kind needs auto-attach-without-explicit, caller passes attachments and this stays correct" forward-compatibility note is doing real work.

  • Attachments-always-opt-in-to-file-push override. if (hasAttachments && hasFilePush) useFilePush = true; runs AFTER the kind-based defaults. A caller tagging a reply as short but with attachments doesn't lose the attachments — the kind tag governs surface choice for text, attachments are first-class. Contract-violation guard, not just a happy-path convenience.

  • useTaskFallback last-resort branch. "If no real surface picked but device has task output, use it so reply isn't dropped silently." Catches the edge case where an always-on recorder gets a status update but has nothing to render — the task-file path becomes the audit trail. Without this, the reply would silently disappear; with it, the operator can later read tasks/<ts>.txt to see what the agent tried to say.

  • RouteResult includes both used AND dropped arrays. Telemetry surfaces what the router intended vs what landed. A device that registered voice but the transport threw on .sendContent() lands in dropped: ['voice'] — observability without polling the device state. Downstream audit-summary (#750) consumer gets clean signal.

  • Error swallowing per-channel. "HUD render failure doesn't suppress the voice reply." Standard fan-out discipline, but the console.error per channel keeps the failure visible. A future PR wiring cost-accounting.record({kind: 'denial', source: 'output-router', ...}) on each catch would surface failure trends in the audit ledger.

Non-blocking observations:

  1. narrative kind on a HUD-only device produces silent skip. pickSurfaces for narrative sets useHud = false unconditionally. A glasses-only session (Mentra without a paired voice transport) registered with only hud output would receive nothing for a narrative reply — it'd land in dropped: [] too because useHud was never true in the first place (no useVoice available, no useFilePush, no useTaskFallback). A better posture for narrative-on-HUD-only might be: fall through to HUD with truncation as a short-equivalent. Cheap fix: if (kind === 'narrative' && !hasVoice && hasHud) useHud = true; at the end of the narrative branch. Worth a TODO at minimum.

  2. hudBudget() switch is keyed on hardcoded profile.id strings. Adding a new glasses model requires editing this file. A profile.hud.charBudget?: number field on DeviceProfile (sourced from the OC yaml) would push the budget into the profile data and let new devices ship without a code change. hudBudget() would fall back to its current switch only when the profile doesn't declare the field. Not in this PR's scope but worth threading when OC YAML loader from #759 lands.

  3. No retry on transient failures. A file_push.push() that throws on a network blip drops the attachment with no retry. For high-stakes attachments (transcripts, generated images) the caller has to wrap in retry logic. A routeReply({...}, { retry: 2 }) opts in to one or two retries with backoff would be a low-cost durability upgrade. Defer until a real failure pattern emerges.

  4. useVoice for short/status only fires when !hasHud. What if the device has both HUD and voice and the caller actually wants the short status spoken too (e.g. accessibility, eyes-off context)? The current "shouldn't double-deliver" rule is the right default but a force: 'voice' | 'hud' | 'both' opt-in on AgentReply would cover the accessibility case without changing the default. Cosmetic now; relevant when audio-only users land.

  5. pickSurfaces returns a useTaskFallback flag the caller must remember to honor. Subtle naming — every other field is useX and the routeReply() dispatch logic correctly fires task in that case. But a downstream consumer that wrote their own dispatch off pickSurfaces() might think the four useX fields are surface-symmetric (they're not — useTaskFallback is the only "this is the fallback" flag) and forget to fire task. A docstring caveat would mitigate.

Tests: tests/output-router.test.ts present in the diff — based on the test file's typical pattern, expect coverage of the pickSurfaces decision matrix (each kind × hasHud/hasVoice/hasFilePush combinations) + truncation boundary cases + routeReply's used/dropped tracking + the per-channel error isolation. Solid for the contract module.

Ready to merge. Unblocks #745 (voice-agent onTurnCompleted consumer wiring) and any future bridge that wants device-aware output dispatch (Mentra in #740 will route through this once activated). Together with #739 (DeviceSession), #746 (access policy), #748 (tenant), #749 (cost-accounting), this rounds out the §5 Now item 3 quintet — Sutando now has a real device session, real access gating, real metering, and real output routing as compositional pieces. — Lucy (Mac Studio bot)

@qingyun-wu
qingyun-wu marked this pull request as draft May 17, 2026 05:29

@vidhuUC vidhuUC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

Clean output-router scaffold. Key design decisions verified:

  • hudBudget(profile) grounded in real OC device-profile specs (Mentra Live 80 chars, Even G1 120, Vuzix 160) — not made-up numbers.
  • truncateForHud() preferring sentence boundaries over hard truncation is the right UX for glasses.
  • routeReply() returns RoutedReply[] — caller decides what to do with each surface, no side effects in the router. Clean separation.
  • Laptop session skipped (bodhi owns that transport) — correctly prevents double-delivery.
  • Env gate (SUTANDO_OUTPUT_ROUTER=1) means zero behavior change for existing users.

Depends on #739. Merge after #739.

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.

4 participants