Refactor (packages/opencode/src/util/media.ts): Function with many returns (count = 7): sniffAttachmentMime

[alexjack58]

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue:

Closes #166 (#166)

Full path to the refactored file:

packages/opencode/src/util/media.ts

What do you think this file does?

It is a small helper module for classifying attachments by MIME type. It exposes predicates (isPdfAttachment, isMedia, isImageAttachment) plus sniffAttachmentMime, which inspects the leading bytes of a file to detect PNG/JPEG/GIF/BMP/PDF/WebP and falls back to an extension-derived MIME type. The read tool uses it to decide whether a file should be returned to the model as an image/PDF or as text.

What is the scope of your refactoring within that file?

Only the sniffAttachmentMime function (and the private startsWith helper it used). The three exported predicate functions are untouched.

Which Qlty‑reported issue did you address?

Function with many returns (count = 7): sniffAttachmentMime at packages/opencode/src/util/media.ts:15. After the change Qlty reports no smells for the file (count 7 → 1 return).

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?

The function was a chain of six if (startsWith(...)) return "<mime>" guards followed by a fallback return, with the magic-byte constants embedded inline in the control flow. Adding or reordering a format meant editing branching logic rather than data, and the WebP case had its own ad-hoc subarray(8) handling that made it look structurally different from the others even though it is just a second byte sequence at an offset.

What changes did you make to resolve the issue?

I moved the format knowledge into a declarative SIGNATURES table where each entry is a MIME type plus one or more { offset, bytes } magic sequences (WebP is expressed as two sequences at offsets 0 and 8). Two tiny helpers (matchesMagic, matchesSignature) test a buffer against an entry, and sniffAttachmentMime is now a single expression: SIGNATURES.find(...)?.mime ?? fallback.

How do your changes improve maintainability? Did you consider alternatives?

The function now has one return, one place to add a new format (a table row), and no special-casing for WebP. Match order is preserved so behavior is identical, and out-of-range indexing still fails closed for short buffers. I considered keeping the if chain but extracting per-format helper functions; that would have reduced complexity less and still left the returns count high, so the table-driven approach was the better fit for this smell.

3. Validation

How did you validate that the change is correct?

I added packages/opencode/test/util/media.test.ts (13 tests) that execute the refactored code directly. It covers every signature in the table (PNG, JPEG, GIF87a/89a, BMP, PDF, WebP), the WebP two-marker rule (RIFF+WEBP matches, RIFF+WAVE and a truncated RIFF header do not), empty and truncated inputs, unknown content falling back to the caller-supplied MIME, trailing bytes after a signature, and the three predicate helpers. Coverage for src/util/media.ts is 100% functions / 100% lines from this test alone. These tests are sufficient because the function is pure and its entire behavior is "which signature matches, or fallback"; each table row and each failure mode is exercised. bun lint, bun run typecheck (opencode package), and the full packages/opencode bun test suite also pass locally. The opencode package's tests already run in CI via the opencode#test turbo task, so the new test file executes on this PR without any workflow changes.

Attach a screenshot of the test coverage showing the lines were executed by the tests.

Attach a screenshot showing the tests that cover the change passing during CI

All three checks on this PR are green (unit, typecheck, e2e smoke). The unit job runs the opencode#test turbo task, which executes bun test for the whole packages/opencode package including the new test/util/media.test.ts (3294 tests, 0 failures). Note the package script uses --only-failures, so passing files are not listed individually in the log.

Local bun lint and bun test for reference. Lint is clean on both changed files; the repo-wide bun lint reports 2 errors that are pre-existing on upstream main in files this PR does not touch (packages/web/src/types/lang-map.d.ts, packages/session-ui/src/v2/components/prompt-input/index.tsx).

Local run of the new test file:

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.

Before:

After: