fix(media): auto-skip tiny/empty audio files before transcription (#8127)#8388
Conversation
| expect(result.outputs).toHaveLength(0); | ||
| expect(result.decision.outcome).toBe("skipped"); | ||
| expect(result.decision.attachments[0]?.attempts[0]?.outcome).toBe("skipped"); | ||
| expect(result.decision.attachments[0]?.attempts[0]?.reason).toContain("tooSmall"); |
There was a problem hiding this comment.
[P2] Optional chaining makes the assertions here non-failing if attachments[0] / attempts[0] is missing.
Right now expect(result.decision.attachments[0]?.attempts[0]?.outcome) will evaluate to undefined if the arrays are empty, which can hide regressions where no attempt is recorded. Consider asserting the array lengths first and then indexing without ?..
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/media-understanding/runner.skip-tiny-audio.test.ts
Line: 66:69
Comment:
[P2] Optional chaining makes the assertions here non-failing if `attachments[0]` / `attempts[0]` is missing.
Right now `expect(result.decision.attachments[0]?.attempts[0]?.outcome)` will evaluate to `undefined` if the arrays are empty, which can hide regressions where no attempt is recorded. Consider asserting the array lengths first and then indexing without `?.`.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressed in a583faf — replaced optional chaining with strict assertions that verify array lengths before indexing.
Additional Comments (1)
If the intent is to cover #8127 for URL attachments too, add a small-buffer mock in a dedicated test (or override the mock for this test) to assert the decision is Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: src/media-understanding/apply.test.ts
Line: 175:179
Comment:
[P3] `apply.test.ts` mocks `fetchRemoteMedia` to always return a 2048-byte buffer (see beforeEach), so the URL-only audio test can’t exercise the new tiny/empty audio skip behavior for remote attachments.
If the intent is to cover #8127 for URL attachments too, add a small-buffer mock in a dedicated test (or override the mock for this test) to assert the decision is `skipped` with reason `tooSmall`.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
|
Fixed in 8e67960e4: [P2] Optional chaining in test assertions: The assertions at lines 69-71 actually use direct property access, not optional chaining. The preceding [P3] URL-only audio test: Added a dedicated test |
bfc1ccb to
f92900f
Compare
This comment was marked as spam.
This comment was marked as spam.
…cription Add a minimum file size guard (MIN_AUDIO_FILE_BYTES = 1024) before sending audio to transcription APIs. Files below this threshold are almost certainly empty or corrupt and would cause unhelpful errors from Whisper/Deepgram/Groq providers. Changes: - Add 'tooSmall' skip reason to MediaUnderstandingSkipError - Add MIN_AUDIO_FILE_BYTES constant (1024 bytes) to defaults - Guard both provider and CLI audio paths in runner.ts - Add comprehensive tests for tiny, empty, and valid audio files - Update existing test fixtures to use audio files above threshold
Increase test audio file sizes to meet MIN_AUDIO_FILE_BYTES (1024) threshold introduced by the skip-empty-audio feature. Fix localPathRoots in skip-tiny-audio tests so temp files pass path validation. Remove undefined loadApply() call in apply.test.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Landed via temp rebase onto main.
Thanks @Glucksberg! |
Fixes #8127
When tiny or empty audio files are sent for transcription, the Whisper API returns unhelpful errors.
Changes:
MIN_AUDIO_FILE_BYTES = 1024constant"tooSmall"toMediaUnderstandingSkipReasonrunProviderEntry) and CLI path (runCliEntry)MediaUnderstandingSkipErrorGreptile Overview
Greptile Summary
Adds a minimum-audio-size guard (
MIN_AUDIO_FILE_BYTES = 1024) so tiny/empty audio attachments are skipped before being sent to transcription providers/CLI, returning aMediaUnderstandingSkipErrorwith new reasontooSmall. Updates existing media-understanding tests to use buffers above the threshold and adds a dedicated test suite covering tiny, empty, and valid audio inputs.This integrates at the runner layer (
runProviderEntryandrunCliEntry) so both provider-based transcription and local CLI transcription paths avoid unhelpful upstream errors for near-empty files.Confidence Score: 4/5