Skip to content

fix(discovery): audit fixes for the realtime streaming flow - #36

Open
jhosepmyr wants to merge 3 commits into
developfrom
bugfix/discovery-streaming-audit
Open

fix(discovery): audit fixes for the realtime streaming flow#36
jhosepmyr wants to merge 3 commits into
developfrom
bugfix/discovery-streaming-audit

Conversation

@jhosepmyr

Copy link
Copy Markdown
Contributor

Description

Fixes from a full end-to-end audit of the discovery realtime/streaming flow (backend + frontend) ahead of the live demo. Two defects surfaced, both frontend:

  1. Trailing audio dropped on stopstopStreaming() closed the STT websocket and then discarded the PCM accumulator, losing the sub-chunk tail (< 2048 samples ≈ 128ms) — often the end of the speaker's final sentence. It now flushes the remaining samples before closing.
  2. Realtime schema drift was invisible — the SessionEventType union carried a phantom SESSION_RESET (the backend never emits it) and was missing SESSION_CREATED (which it does emit on the project topic); and applyRealtime silently dropped any unrecognized message type, so a backend that ships a new event first looks like a dead feature. The union now mirrors the backend enum exactly and unknown types log a console warning.

Audit context (no code defects found in these areas): backend message wire-types, suggestion generation cadence/grounding, duplicate thresholds (0.50 recall → LLM judge → 0.84 auto-dedup → 0.85 import gate), notify-after-commit ordering, RBAC gates on REST + STT websocket, and the recently merged watermark/audio-pause fix — all verified sound.

Feature module / area: discovery / core (audio)

Related issue / US:


Type of Change

  • fix — bug fix
  • test — tests only

Checklist

  • The PR targets develop (not main)
  • Branch name follows feature/*, bugfix/*, or hotfix/*
  • Commits follow Conventional Commits
  • bun run lint passes locally (ESLint + angular-eslint)
  • bun run test passes locally (Vitest) — 271 passed (+1 new)
  • bun run build passes locally (no type errors, no budget exceeded)
  • New components use ChangeDetectionStrategy.OnPush and Angular signals — N/A (no new components)
  • No localStorage/sessionStorage access for JWT tokens (use the auth store)
  • No bypassSecurityTrust* calls without explicit review
  • No secrets, credentials, or .env content committed
  • CHANGELOG.md updated under [Unreleased] — omitted: two small internal fixes; can add if required

How to Test

  1. Record a session and stop right after finishing a sentence — the final words now appear in the transcript (previously the last ~128ms could be missing).
  2. In DevTools console, inject an unknown realtime type via the STOMP topic (or just review the new unit test) — a [discovery] Unhandled realtime message type: warning appears instead of silence.
  3. bun run test — includes the new store spec for the unknown-type warning.

Notes (optional)

Backend audit found no code defects to fix; the two known critical issues (wire-type discriminator race, suggestion-watermark clobbering live transcription) were already fixed on develop (844fda3, 587344a). Demo-setup reminders: the presenting user needs a role with SESSION_RUN/SESSION_DECIDE (base READ floor only grants viewing), and LLM/embedding provider availability should be checked before going live (degradation is silent by design).

… socket

stopStreaming() closed the websocket and then discarded the PCM accumulator,
dropping the sub-chunk tail (< 2048 samples, ~128ms) — often the end of the
speaker's final sentence — so the last words could be missing from the
transcript. Send the remaining samples before the close.
… enum

The SessionEventType union carried a phantom SESSION_RESET the backend never
emits and was missing SESSION_CREATED, which it does (project topic, DRAFT).
applyRealtime also dropped any unrecognized type silently, so a backend that
ships a new event first would look like a dead feature. Align the union with
the backend enum and log a console warning for unhandled types.
@jhosepmyr
jhosepmyr requested a review from Eric396 as a code owner July 10, 2026 18:30
…CIDE

The discovery chat gated recording AND suggestion accept/dismiss on
canManage = org OWNER only — so an org admin, or a member whose project role
grants SESSION_RUN/SESSION_DECIDE, could neither record nor decide, and a
read-only viewer clicking accept hit a spurious 403 toast. Drive canRecord
from SESSION_RUN and canDecide from SESSION_DECIDE (owner/admin bypass via
PermissionsStore.has()); the SESSION_READ route guard has already loaded the
project permissions before the page renders.
@jhosepmyr
jhosepmyr force-pushed the bugfix/discovery-streaming-audit branch from 0577137 to 0015db2 Compare July 10, 2026 19:43
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.

1 participant