Blob: drop is_bun_file, a duplicate of needs_to_read_file

[robobun]

Problem

• mordant's reimplemented_helper finding baselined for src/jsc/webcore_types.rs: Blob::is_bun_file and Blob::needs_to_read_file had the same signature and the same body (the store is Some and its data is Store::File), so a change to one would have silently missed the other.
• The duplication is inherited, not a porting mistake: Blob.zig already had isBunFile and needsToReadFile with the same body, and neither side has changed since the port (Rewrite Bun in Rust #30412). Nothing relies on them differing.

Fix

• Delete is_bun_file and keep needs_to_read_file, which is what the ~40 other call sites (and the blob_needs_to_read_file hook that bun_sql_jsc goes through) already use.
• Point the two is_bun_file callers at it: Response.rs (getCompleteWebRequestOrResponseBodyValueAsArrayBuffer returns undefined for a file-backed body) and CryptoHasher.rs (the synchronous hashers reject Bun.file() input). Same predicate, so no behavior change.
• Remove the reimplemented_helper:src/jsc/webcore_types.rs entry from mordant-baseline.toml, and the two stale comments in webcore/Blob.rs that named the deleted method. The entry is removed by hand rather than by regenerating the file: a full bun run rust:mordant:baseline on Linux also drops two unrelated entries (always_unwrapped_option in PackageInstall.rs, narrowed_two_ways in node_crypto_binding.rs) that this PR has no business touching.
• No new test: there is no observable behavior to pin (the two predicates were byte-for-byte the same), so no test can distinguish before from after. The Bun.file() rejection at the CryptoHasher call site is already covered by bun-cryptohasher.test.ts ("Bun.file in CryptoHasher is not supported yet"), and the mordant run below is the check for the finding itself. Same shape as bun:ffi: remove ABIType::param_typename, a copy of ABIType::typename #39124, yaml: map StringifyError to JsError in one place #39116 and sql: set a cell's column index and kind in one place #39135 from this batch.
• Verified:
  • bun run rust:mordant (full workspace) on this branch: clean, no target/mordant/over-baseline.txt. With main's webcore_types.rs restored on top of the trimmed baseline it reports reimplemented_helper ... over the mordant baseline (0 recorded for src/jsc/webcore_types.rs) and 1 finding(s) over the baseline in bun_jsc, so the entry was this site and the lint no longer fires.
  • The mordant workflow also runs on this PR since the baseline changed; with the entry removed it fails if the finding is still reported.
  • bun bd test test/js/bun/util/bun-cryptohasher.test.ts (covers the CryptoHasher call site: Bun.file() input still throws): 402 pass.
  • bun bd test on test/js/web/fetch/{blob,blob-write,body,blob-file-name-ownership,response}.test.ts, test/js/web/structured-clone-blob-file.test.ts and test/js/bun/io/bun-write.test.js (--timeout 60000, since several of these spawn a debug+ASAN child and exceed the 5s default): everything passes except blob.test.ts "Bun.file(path).slice(start, end) streams only the slice", which fails identically on a clean checkout of main (88a6398) and is being handled separately; it is a FileReader streaming bug unrelated to this predicate.
• Two open PRs add is_bun_file() calls in Blob.rs (Move name and lastModified from Blob.prototype to File.prototype #32434, Bun.file: make exists()/size/lastModified reflect the current filesystem state #33659); whichever lands after this one needs those calls renamed to needs_to_read_file(). That is a compile error, not a silent change.

Background

• Blob (src/jsc/webcore_types.rs) is a view (offset + size) onto a refcounted Store, whose data is one of Bytes (in memory), File (a path or fd; what Bun.file() creates) or S3. needs_to_read_file() asks whether the blob is File-backed, i.e. whether its bytes have to be read off disk before anything in memory can look at them; synchronous consumers use it to bail out.
• mordant is the advisory Rust lint pack the rust-lints workflow runs (bun run rust:mordant). mordant-baseline.toml holds the per-(lint, file) counts of findings that predate the job, so CI only fails when a count goes up; once a baselined finding is fixed its entry is deleted.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa7233f7-d0b0-44d8-83a1-eb73a14997a7

📥 Commits

Reviewing files that changed from the base of the PR and between aeca7bc and 235e162.

📒 Files selected for processing (5)

• mordant-baseline.toml
• src/jsc/webcore_types.rs
• src/runtime/crypto/CryptoHasher.rs
• src/runtime/webcore/Blob.rs
• src/runtime/webcore/Response.rs

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 2:05 PM PT - Aug 15th, 2026}

@alii, your commit 235e162 is building: #98644

[robobun]

Status: ready to merge from my side.

• Scope: one mordant reimplemented_helper site (Blob::is_bun_file == Blob::needs_to_read_file in src/jsc/webcore_types.rs), requested by @alii. Behavior-preserving: the two call sites now use the identical predicate that the rest of the tree already uses, and the baseline entry for the site is removed. Rebased once (a880009) after transpiler cache: convert the cache file length to i64 once in save #39132 removed the neighbouring baseline line.
• CI on a880009: the Rust lints workflow is green, including the mordant job that checks this baseline change, plus clippy, miri, Lint and source-lints. The Buildkite build (#98447) did not fail on anything: it was cancelled in the same sweep as the other builds from this batch (#98446, #98497, #98499) with only the two darwin aarch64 test jobs still outstanding and no failed jobs, so I am not re-running it for a five-file rename.
• Verified locally: bun run rust:mordant is clean on this branch and reports the site as 1 over baseline when main's webcore_types.rs is restored; bun-cryptohasher.test.ts (402 pass) plus the blob/body/response/Bun.write files listed in the description pass, apart from one blob.test.ts case that fails identically on main and is tracked separately.

[claude]

LGTM — mechanical de-duplication of two byte-identical predicates.

What was reviewed:

• Confirmed the deleted is_bun_file body was identical to needs_to_read_file (both matches!(… Store::File(_))), so the two retargeted callers cannot change behavior.
• Grepped src/ for is_bun_file/isBunFile — only the local is_bun_file_blob helper in CryptoHasher.rs remains (now calling needs_to_read_file); no stragglers.
• The mordant-baseline.toml edit removes exactly the one reimplemented_helper:src/jsc/webcore_types.rs entry; the Blob.rs changes are comment-only.

Extended reasoning...

Overview

This PR deletes Blob::is_bun_file() from src/jsc/webcore_types.rs, whose body was byte-identical to Blob::needs_to_read_file() (both check store.data is Store::File). The two callers of the deleted method — Response.rs (getCompleteWebRequestOrResponseBodyValueAsArrayBuffer) and CryptoHasher.rs (is_bun_file_blob) — are pointed at the surviving needs_to_read_file(). The corresponding reimplemented_helper entry is removed from mordant-baseline.toml, and two stale comments in webcore/Blob.rs that named the deleted method are cleaned up.

Security risks

None. This is a pure rename at two call sites plus a method deletion; no input handling, permissions, or crypto logic changes. The CryptoHasher touch only changes which identically-bodied predicate gates the existing "Bun.file() not supported here" throw.

Level of scrutiny

Low. The two method bodies were provably identical (visible in the diff), so retargeting callers cannot change runtime behavior. A repo-wide grep confirms no remaining is_bun_file references in src/, so any missed caller would be a compile error, not a silent behavior change. The baseline edit is a single-line removal of the entry this PR resolves.

Other factors

The PR description documents thorough verification: the mordant lint is confirmed clean and confirmed to re-fire with the old code on the trimmed baseline; the CryptoHasher and Blob/Response test suites pass on the debug build. The one noted pre-existing failure (blob.test.ts slice streaming) reproduces on clean main and is unrelated. The author also flagged two in-flight PRs that add is_bun_file() calls — those will fail to compile rather than silently diverge, which is the desired outcome.

[claude]

LGTM — mechanical dedup of two byte-identical predicates.

What was reviewed:

• Confirmed is_bun_file and needs_to_read_file had identical bodies; the two rewired call sites (Response.rs, CryptoHasher.rs) now use the same predicate the rest of the tree already uses — no behavior change.
• Grepped for remaining is_bun_file references: only the local is_bun_file_blob helper in CryptoHasher.rs remains (its body was updated), so no stray callers.
• Baseline entry removal and comment cleanups match the deleted method.

Extended reasoning...

Overview

This PR removes Blob::is_bun_file() from src/jsc/webcore_types.rs, which was byte-for-byte identical to Blob::needs_to_read_file() (both check whether the store is Some with Store::File data). The two call sites of the deleted method — Response.rs (getCompleteWebRequestOrResponseBodyValueAsArrayBuffer) and CryptoHasher.rs (is_bun_file_blob helper) — are pointed at the surviving predicate. The corresponding reimplemented_helper mordant baseline entry is removed, along with two stale comments in webcore/Blob.rs that named the deleted method.

Security risks

None. This is a pure dedup of two identical predicates on an internal Rust type; no user-facing behavior, input handling, or trust boundaries are touched.

Level of scrutiny

Low. The deleted function and the surviving one had the same signature and the same body, so replacing one call with the other is provably behavior-preserving. This matches the shape of several recently-merged PRs from the same mordant-cleanup batch (#39124, #39140, #39141, #39123). REVIEW.md explicitly asks for this: "If your fix makes two functions byte-identical, delete one."

Other factors

• I grepped the tree for is_bun_file / isBunFile and confirmed the only remaining hits are the local is_bun_file_blob helper in CryptoHasher.rs (whose body now calls needs_to_read_file()); any missed caller would be a compile error, not a silent change.
• The PR description documents that bun run rust:mordant is clean on this branch and fails when main's webcore_types.rs is restored on top of the trimmed baseline, confirming the removed entry was this exact site.
• No new test is warranted since there is no observable behavior to distinguish before from after; existing coverage (bun-cryptohasher.test.ts) already pins the Bun.file() rejection at the CryptoHasher call site.
• No prior reviews or outstanding comments on this PR.

[claude]

LGTM — mechanical dedup of two byte-identical predicates.

What was reviewed:

• Confirmed is_bun_file and needs_to_read_file had identical bodies; the two callers (Response.rs, CryptoHasher.rs) now call the survivor with no behavior change.
• Grepped src/ for remaining is_bun_file references — none (the local is_bun_file_blob helper in CryptoHasher is a distinct name).
• The mordant-baseline.toml entry removal is enforced by the mordant CI job, so a resurfaced finding would fail the workflow.

Extended reasoning...

Overview

Deletes Blob::is_bun_file() from src/jsc/webcore_types.rs — its body was byte-for-byte identical to Blob::needs_to_read_file() (both check matches!(self.store..., Some(s) if matches!(s.data, Store::File(_)))). Points the two remaining callers at needs_to_read_file(), removes stale comments in webcore/Blob.rs that named the deleted method, and drops the corresponding reimplemented_helper entry from mordant-baseline.toml.

Security risks

None. This is a pure rename/dedup of a data-only predicate with no I/O, allocation, or user-input handling.

Level of scrutiny

Low. The two predicates were provably identical, so substituting one for the other cannot change behavior. Any missed caller would be a compile error, not a silent regression. The baseline-entry removal is CI-enforced (the mordant workflow re-runs when the baseline file changes and fails if the finding still fires). REVIEW.md explicitly calls for this shape: "If your fix makes two functions byte-identical, delete one."

Other factors

• The CryptoHasher call site's observable behavior (rejecting Bun.file() input) is already covered by bun-cryptohasher.test.ts.
• No new test is expected: the change has no observable delta to pin, and REVIEW.md's "every behavioral change ships an automated test" does not apply to a non-behavioral dedup.
• The PR description flags two open PRs that add is_bun_file() calls; those will surface as compile errors when rebased, not silent breakage.
• No prior review from me on this PR; no outstanding human reviewer comments.