-
Notifications
You must be signed in to change notification settings - Fork 78
feat(voice): instrument judge STT pre-pass #880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| /** Judge pre-pass STT span coverage for issue #785. */ | ||
|
|
||
| import { trace, SpanStatusCode } from "@opentelemetry/api"; | ||
| import { InMemorySpanExporter, SimpleSpanProcessor } from "@opentelemetry/sdk-trace-base"; | ||
| import { NodeTracerProvider } from "@opentelemetry/sdk-trace-node"; | ||
| import type { ModelMessage } from "ai"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { AudioChunk } from "../audio-chunk"; | ||
| import { prepareJudgeInput } from "../judge-stt"; | ||
| import { createAudioMessage } from "../messages"; | ||
| import type { STTProvider } from "../stt"; | ||
|
|
||
| function tone(marker: number): AudioChunk { | ||
| const data = new Uint8Array(4800); | ||
| data.fill(marker); | ||
| return new AudioChunk({ data }); | ||
| } | ||
|
|
||
| function textPart(message: ModelMessage): string | undefined { | ||
| const content = (message as { content?: unknown }).content; | ||
| if (!Array.isArray(content)) return undefined; | ||
| const part = content.find( | ||
| (item) => | ||
| item !== null && | ||
| typeof item === "object" && | ||
| (item as { type?: unknown }).type === "text", | ||
| ) as { text?: string } | undefined; | ||
| return part?.text; | ||
| } | ||
|
|
||
| describe("judge pre-pass voice.stt.transcribe spans (#785)", () => { | ||
| let exporter: InMemorySpanExporter; | ||
| let provider: NodeTracerProvider; | ||
|
|
||
| beforeEach(() => { | ||
| exporter = new InMemorySpanExporter(); | ||
| provider = new NodeTracerProvider({ | ||
| spanProcessors: [new SimpleSpanProcessor(exporter)], | ||
| }); | ||
| trace.setGlobalTracerProvider(provider); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| await provider.shutdown(); | ||
| trace.disable(); | ||
| }); | ||
|
|
||
| it("emits a judge-scoped span with success attributes", async () => { | ||
| const stt: STTProvider = { | ||
| transcribe: vi.fn(async () => "account restored"), | ||
| }; | ||
| await prepareJudgeInput({ | ||
| messages: [createAudioMessage(tone(1), "assistant") as ModelMessage], | ||
| stt, | ||
| }); | ||
|
|
||
| const spans = exporter | ||
| .getFinishedSpans() | ||
| .filter((span) => span.name === "voice.stt.transcribe"); | ||
| expect(spans).toHaveLength(1); | ||
| expect(spans[0]!.attributes["voice.stt.scope"]).toBe("judge"); | ||
| expect(spans[0]!.attributes["voice.stt.speaker"]).toBe("assistant"); | ||
| expect(spans[0]!.attributes["voice.stt.audio_bytes"]).toBe(4800); | ||
| expect(spans[0]!.attributes["voice.stt.transcript_chars"]).toBe( | ||
| "account restored".length, | ||
| ); | ||
| expect(spans[0]!.attributes["langwatch.span.type"]).toBe("span"); | ||
| }); | ||
|
|
||
| it("keeps a successful sibling when one message fails and exports only a sanitized error", async () => { | ||
| const rawError = "401 invalid key sk-secret body={provider response}"; | ||
| const stt: STTProvider = { | ||
| async transcribe(audio) { | ||
| if (audio.data[0] === 1) throw new Error(rawError); | ||
| return "successful sibling"; | ||
| }, | ||
| }; | ||
| const warn = vi.fn(); | ||
| const messages = [ | ||
| createAudioMessage(tone(1), "user") as ModelMessage, | ||
| createAudioMessage(tone(2), "assistant") as ModelMessage, | ||
| ]; | ||
|
|
||
| const prepared = await prepareJudgeInput({ messages, stt, logWarn: warn }); | ||
|
|
||
| expect(textPart(prepared.messages[0]!)).toBeUndefined(); | ||
| expect(textPart(prepared.messages[1]!)).toBe("successful sibling"); | ||
| const spans = exporter | ||
| .getFinishedSpans() | ||
| .filter((span) => span.name === "voice.stt.transcribe"); | ||
| expect(spans).toHaveLength(2); | ||
| const failed = spans.find((span) => span.status.code === SpanStatusCode.ERROR)!; | ||
| const succeeded = spans.find((span) => span.status.code !== SpanStatusCode.ERROR)!; | ||
| expect(failed.attributes["voice.stt.scope"]).toBe("judge"); | ||
| expect(failed.attributes["voice.stt.speaker"]).toBe("user"); | ||
| expect(failed.attributes["voice.stt.transcript_chars"]).toBeUndefined(); | ||
| expect(succeeded.attributes["voice.stt.transcript_chars"]).toBe( | ||
| "successful sibling".length, | ||
| ); | ||
| const recorded = JSON.stringify(failed.events); | ||
| expect(recorded).toContain("STT provider failed"); | ||
| expect(recorded).not.toContain("sk-secret"); | ||
| expect(recorded).not.toContain("401"); | ||
| expect(recorded).not.toContain("provider response"); | ||
| expect(warn).toHaveBeenCalledOnce(); | ||
| expect(warn.mock.calls[0]![0]).toContain("STT provider failed: Error"); | ||
| expect(warn.mock.calls[0]![0]).not.toContain(rawError); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| from .audio_chunk import AudioChunk | ||
| from .recording import AudioSegment, VoiceRecording | ||
| from .stt import STTProvider, get_stt_provider | ||
| from ._telemetry import voice_span | ||
|
|
||
| logger = logging.getLogger("scenario.voice") | ||
|
|
||
|
|
@@ -31,6 +32,7 @@ async def transcribe_segments( | |
| recording: VoiceRecording, | ||
| provider: Optional[STTProvider] = None, | ||
| only_missing: bool = True, | ||
| telemetry_scope: Optional[str] = None, | ||
| ) -> None: | ||
| """ | ||
| Run STT over recording.segments, mutating .transcript in place. | ||
|
|
@@ -42,6 +44,9 @@ async def transcribe_segments( | |
| only_missing: If True (default), skip segments whose transcript is | ||
| already set. If False, re-transcribe everything (e.g. to overwrite | ||
| adapter-side STT with a different provider). | ||
| telemetry_scope: When set, emit a ``voice.stt.transcribe`` span for | ||
| each provider call using this scope. Public callers are | ||
| uninstrumented by default; the judge path opts in with ``judge``. | ||
|
|
||
| Concurrency: transcribes segments concurrently with asyncio.gather. Each | ||
| segment's STT call is independent. Empty-data segments are skipped. | ||
|
|
@@ -55,12 +60,13 @@ async def transcribe_segments( | |
| if p is None: | ||
| return # already warned | ||
| targets = [ | ||
| s for s in recording.segments | ||
| s | ||
| for s in recording.segments | ||
| if s.audio and (not only_missing or s.transcript is None) | ||
| ] | ||
| if not targets: | ||
| return | ||
| await asyncio.gather(*(_transcribe_one(p, s) for s in targets)) | ||
| await asyncio.gather(*(_transcribe_one(p, s, telemetry_scope) for s in targets)) | ||
|
|
||
|
|
||
| def _try_get_provider() -> Optional[STTProvider]: | ||
|
|
@@ -76,10 +82,41 @@ def _try_get_provider() -> Optional[STTProvider]: | |
| return None | ||
|
|
||
|
|
||
| async def _transcribe_one(provider: STTProvider, segment: AudioSegment) -> None: | ||
| async def _transcribe_one( | ||
| provider: STTProvider, | ||
| segment: AudioSegment, | ||
| telemetry_scope: Optional[str], | ||
| ) -> None: | ||
| try: | ||
| text = await provider.transcribe(AudioChunk(data=segment.audio)) | ||
| segment.transcript = text or None | ||
| if telemetry_scope is None: | ||
| text = await provider.transcribe(AudioChunk(data=segment.audio)) | ||
| segment.transcript = text or None | ||
| return | ||
|
|
||
| with voice_span( | ||
| "voice.stt.transcribe", | ||
| { | ||
| "voice.stt.scope": telemetry_scope, | ||
| "voice.stt.speaker": segment.speaker, | ||
| "voice.stt.audio_bytes": len(segment.audio), | ||
| }, | ||
| ) as stt_span: | ||
| try: | ||
| text = await provider.transcribe(AudioChunk(data=segment.audio)) | ||
| except Exception as exc: | ||
| # Provider SDK errors can include response bodies and key fragments. | ||
| # Keep the raw detail local and let telemetry record only a minimal | ||
| # provider-agnostic exception, matching the #783 STT guard. | ||
| logger.debug( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 — Do not log the raw provider exception in the sanitized path. |
||
| "scenario.voice.transcribe: STT provider error detail", | ||
| exc_info=True, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [review-clerk, Fix] This This concurs with the still-unresolved |
||
| ) | ||
| raise RuntimeError( | ||
| f"STT provider failed: {type(exc).__name__}" | ||
| ) from None | ||
| segment.transcript = text or None | ||
| if text: | ||
| stt_span.set_attribute("voice.stt.transcript_chars", len(text)) | ||
| except Exception as e: | ||
| logger.warning( | ||
| "scenario.voice.transcribe: STT failed for %s segment at %.2fs: %s", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[review-clerk, Fix/New AC] Missing test: nothing here proves a direct (non-judge)
transcribeAudioMessagescall stays uninstrumented — i.e. emits zerovoice.stt.transcribespans whentelemetryScopeis omitted.The Python side has this exact test (
test_direct_transcription_is_uninstrumented_by_defaultinpython/tests/voice/test_judge_stt_telemetry.py), and the PR description explicitly claims parity: "Direct TypeScript and Python transcription callers remain unchanged and uninstrumented." Right now only the Python half of that claim is evidenced. Add a JS sibling test callingtranscribeAudioMessages(or the underlying non-judge path) withouttelemetryScopeand assertingexporter.getFinishedSpans()has novoice.stt.transcribespan.