[robobun]

Problem

• A diagnostic gets the line, column and excerpt bounds of an earlier file of equal size and path length. bun install with two file: folders prints b/package.json:4:105 for a syntax error at 1:138. Two 5000-byte bunfig files print 29 token characters unmasked.
• LineColumnTracker::matches (src/ast/lib.rs:2259) knows a source by addresses and lengths. A reused buffer gives the next file the same address.
• No user reported it. A review of logger: search for the end of a line only as far as the excerpt of a diagnostic goes #44811 found it. It ships since 1.4.0.

Fix

• Each Source draws a SourceId at construction. The tracker keys on it. A clone keeps it only for borrowed bytes.
• No producer needs a change. The tracker drop that Fix use-after-free of BuildMessage.position.file: real lifetimes for Source, Path, ImportRecord and resolve results #46834 added in install/npm.rs is removed.
• Debug builds check a resumed cursor against a full scan, up to byte 4096.
• Verified: test/cli/install/bun-install.test.ts (4 new cases fail on main), unit tests under Miri. Self-reviewed: 21 concerns raised, 20 addressed. Rejected: a private tracker field (the bundler needs a public one).

Background

• A Log keeps one tracker, so N diagnostics in one source cost one scan (Compute diagnostic positions incrementally to fix quadratic hang in Bun.JSONC.parse #31273).
• Considered a tracker drop at each producer. Four are wrong on a release build. ASAN hides them.
• Question for a maintainer: is an id on Source acceptable? perf: cache thread_id::current() in a #[thread_local] slot #30971 uses a stamp, bun lint, bun fmt: the first run, oxlint's order, Prettier's Tailwind plugin, native JavaScript plugins, a faster Bun.Glob #45043 and Fix use-after-free of BuildMessage.position.file: real lifetimes for Source, Path, ImportRecord and resolve results #46834 a drop.

Downsides

• Source grows from 112 to 120 bytes. Each construction adds 6 instructions and no lock prefix.
• Bun.JSONC.parse('{}'): +0.24% instructions (control -0.05%). Release .text: +0 B, function bytes +115 B.
• A Source rewritten in place still resumes the scan. No producer does that.

Notes

Repro. A project with "a": "file:./a" and "b": "file:./b". Both package.json files are 260 bytes. a has a duplicate key on line 4. b is one line with a syntax error at column 138. The test case a syntax error after a warning writes these files. Run bun install with no bun.lock. main prints this for b:

4 | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",oops,"q":"yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
                                                                                                            ^
error: Expected string but found "oops"
    at /tmp/proj/b/package.json:4:105

The file b has one line. With this change the place is 1:138 and the gutter says 1. On main the excerpt is the text of b between the line start and the line end that the scan kept from a. A cursor is only resumed at an equal or later offset, so the second finding must be further into its file than the first.

When it shows. For file: folders: on a run that resolves the folders (no bun.lock, or bun update). A bun install with a bun.lock prints no located diagnostic for them. One byte of difference in the length of the file or of the path hides it.

Producers, on release builds (main 6f58f6b894 against this diff on that commit, mimalloc 92ef6587 in both, measured before #46834 merged):

Producer	main	this PR
file: folders, pooled read buffer (folder_resolver.rs:226), the 3 folder cases of the test	every file after the first is wrong	0 wrong
~/.bunfig.toml then ./bunfig.toml, 5000 and 20000 bytes, 5 runs each (bunfig/arguments.rs:100)	local file at 9:1, the file says 3:1, 10 of 10 runs	0 wrong
lifecycle scripts of installed packages, 3 manifests of 5000 bytes, 5 runs (Package/Scripts.rs:346)	10 of 15 wrong	0 of 15
chain of 100 local tarballs, 5000-byte manifests, 5 runs (processDependencyList.rs:252)	5 of 500 wrong, 1 in each run	0 of 500

The last three free the first text and allocate the next one. mimalloc gives the same address back for these sizes. ASAN's quarantine hides that, so the bunfig case of the test spawns bun with withoutAsanQuarantine. That case fails on a debug build of main and on a release build of main.

On main 8a0e49eedb, after #46834, the key and these producers are the same by reading, and the 4 cases of the test fail on a debug build of it.

The token. In the bunfig case, the stale line start is inside the value of token. The redactor masks a value when it sees its key, and the excerpt starts after the key. main prints 2 | 4e5f6g7h8i9j0k1l2m3n4o5p6q7r8", url = 5 }. This change prints the line with the value masked. The 120-byte excerpt window can cut a key off in the same way for one long line. That is #43316 and this PR does not change it.

Measurements.

• size_of (rustc -Zprint-type-sizes, main 8a0e49eedb): Source 112 to 120 B, Log 48 to 48 B, LineColumnTracker 256 to 248 B. Each type that holds a Source by value grows by 8 B (before Fix use-after-free of BuildMessage.position.file: real lifetimes for Source, Path, ImportRecord and resolve results #46834: Graph::InputFile 192 to 200 B, transpiler::ParseResult 1368 to 1376 B).
• The draw, from llvm-objdump of the release build: load of the thread-local, test, je, lea, store of the thread-local, store into the Source. The cold path calls SourceId::next_block, which has the only lock xadd for ids in the binary.
• A bun build of 2,001 modules drew 6,052 ids on 13 threads and took 13 blocks (read with gdb at exit).
• User-space instructions (perf_event_open, 10 interleaved rounds, a second copy of the PR binary as control, release builds of 6f58f6b894 with and without this diff):

Loop	main	this PR	delta	control
10M x Bun.JSONC.parse('{}')	36.150e9	36.237e9	+0.24%	-0.05%
1M x transformSync('1')	36.048e9	33.599e9	-6.80%	+0.02%
bun build of 2,001 modules	139.99e6	139.93e6	-0.04%	-0.20%

• I did not isolate the cause of the transformSync row. The diff adds only the draw on that path. Transpiler::parse_maybe_return_file_only_allow_shared_buffer is 2,256 B smaller in the PR build, so I think that ThinLTO inlines differently there. An earlier build of this branch gave the same -6.8%.
• Release binary (ThinLTO, llvm-size): .text 81,695,461 B in both (the section is aligned to 256 B). Function bytes, as the sum of unique symbol sizes from llvm-nm -S: +115 B. .tbss +8 B, .data +64 B, .bss +8 B.
• valgrind and bloaty are not installed on the machine that I used. The instruction counts are from a hardware counter and the byte counts from llvm-size and llvm-nm.

Designs that I weighed.

• A drop of the tracker in read_package_json_from_disk. It fixes one of the four producers, and a new producer must know the rule. ASAN does not reuse an address, so the debug lane does not catch a producer that forgets.
• A reset of the tracker where a parser starts. Each parser entry must call it, and a producer that logs without a new parse is not covered.
• A tracker that the consumer owns, as in esbuild. It removes the key. It changes every call site that logs with a source. Source has a lifetime since Fix use-after-free of BuildMessage.position.file: real lifetimes for Source, Path, ImportRecord and resolve results #46834, so this is possible as a later step.
• An id that is drawn at the first diagnostic. It needs an atomic in Source, which takes noalias from every &Source parameter.
• One AtomicU64::fetch_add for each Source, as perf: cache thread_id::current() in a #[thread_local] slot #30971 does for each thread pool. That is a lock xadd for each construction. The thread-local block has one for each thread.
• An id in the 3 padding bytes of Source. Source stays at 112 bytes. A 24-bit id repeats after 16,777,215 sources on one thread, and two threads draw the same ids.
• contents_is_recycled is not the marker. Only the runtime transpiler sets it, and none of the four producers does.

If an id on Source is not wanted, the other fix is a tracker drop in the four producers of the table. I can change this PR to that.

What the id does not cover. One Source value whose bytes are written again in place. I found no producer that does it. The two places that give a cached Source new contents keep the old buffer (stale_contents), so the address differs. The debug assertion catches this case for the first 4096 bytes.

#46834. It merged while this PR was open. It gave Source a lifetime, and it put a tracker drop in PackageManifest::parse (src/install/npm.rs) with the comment "It knows a source by its address". This PR removes that drop: the next manifest is a new Source with a new id. The branch has a merge of main for the one conflict, at pub struct Source.

Other open PRs. #35550, #44811 and #45554 rewrite the body of LineColumnTracker::error_position and leave matches as it is. The one that merges second has a small conflict in that function.

Self-review. One concern rejected: to make Log.line_column_tracker private. The bundler builds a Log with a struct literal, so every field has to stay public. The others are in this diff or in this text: the cold refill is out of line, the comments are one line each, the debug assertion, the bunfig case, and the measurements on release builds of main.

Audit. A Source that is built again for each diagnostic over one buffer would now restart the scan each time. An audit of every construction site of Source (init_*, to_source, struct literals, clones) found none. The closest is expr_from_blob in Macro.rs, which parses the whole blob again for each call anyway.

Suites. On the merge with main 8a0e49eedb: bun bd test test/cli/install/bun-install.test.ts gives 776 pass, 0 fail, and the 4 new cases fail when src/ is that of main. bun run rust:miri -p bun_ast: 26 pass. cargo clippy for bun_ast and bun_install. Before the merge, on this diff: bun run rust:clippy, cargo check --workspace --all-targets, rust:check-all for x86_64-pc-windows-msvc and aarch64-apple-darwin, and with the debug assertion on, no failure in: redacted-config-logs, config-precedence, bad-workspace, bun-install-patch, overrides, catalogs, frozen-lockfile-pruned, bun-workspaces, bun-lock, bun-update, test/cli/format/bunfig, test/bundler/transpiler/transpiler, jsonc, resolve-error, parse-error-column, test/bake/dev/{bundle,css,esm} and test/cli/hot/hot. Several of these need a longer timeout than 5 s on a debug build, with and without this change.

---

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-install.test.ts

[robobun]

Status: ready for review. This PR is #46277.

How I reproduced it (debug build and release build of main):

• bun install in a project with four file: folders and no bun.lock. Each package.json is 260 bytes and has a duplicate key. main prints b/package.json:4:105, c/package.json:4:130 and d/package.json:4:155. The files say 1:138, 6:128 and 1:188.
• A unit test in src/ast/lib.rs copies three files of one length into one buffer and logs to one Log. main reports [(4, 6), (4, 8), (4, 9)]. The files say [(4, 6), (1, 22), (9, 15)].

[robobun]

^{Updated 12:17 AM PT - Oct 27th, 2026}

✅ @robobun, your commit 7503d263e040ecc139a20b25b2c1c6c697473789 passed in Build #133456! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 46277

That installs a local version of the PR into your bun-46277 executable, so you can run:

bun-46277 --bun

[coderabbitai]

Walkthrough

The change adds SourceId to diagnostic scan tracking. Cloning and source reconstruction preserve or assign IDs based on contents ownership. LineColumnTracker uses source IDs and contents identity. Debug validation and tests cover scan state and diagnostic locations.

Changes

Diagnostic source identity

Layer / File(s)	Summary
Source IDs and propagation src/ast/lib.rs, src/bundler/bundle_v2.rs, src/bundler/linker_context/generateCodeForFileInChunkJS.rs, src/resolver/package_json.rs	Source gains an ID and explicit clone behavior. Reconstructed sources preserve IDs, and generated-file setup uses Source::clone().
Diagnostic scan tracking src/ast/lib.rs, src/install/npm.rs	LineColumnTracker matches sources by ID and contents identity. Debug builds compare short-offset scans with fresh scans. npm manifest parse failures no longer clear the tracker.
Diagnostic location tests src/ast/lib.rs, test/cli/install/bun-install.test.ts	Tests cover scan reuse, cloned and edited sources, recycled buffers, same-sized files, bunfig locations, and diagnostic token masking.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to b88ac

No material merge-blocking risk is established by the available evidence.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change accurately: the line and column scan in the logger is now keyed on a Source id instead of the address of its text. It matches the core change in src/ast/lib.rs (SourceId on …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description is complete and relevant. It explains the problem, fix, design trade-offs, known limitations, performance impact, reproduction steps, and verification results. It does not use the temp…

Full details: Title check

Explanation

The title names the change accurately: the line and column scan in the logger is now keyed on a Source id instead of the address of its text. It matches the core change in src/ast/lib.rs (SourceId on Source, LineColumnTracker matching by ID). It is specific and concise, and it is consistent with the PR objectives.

Full details: Description check

Explanation

The description is complete and relevant. It explains the problem, fix, design trade-offs, known limitations, performance impact, reproduction steps, and verification results. It does not use the template headings exactly, but it includes the required information through the Fix and Suites sections.

Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Merged main (8a0e49e) into this branch.

• One conflict, in src/ast/lib.rs, at pub struct Source. Main (98a4365, Fix use-after-free of BuildMessage.position.file: real lifetimes for Source, Path, ImportRecord and resolve results #46834) made it Source<'a> with path: Path<'a>. This PR removes #[derive(Clone)] because it adds a manual Clone that gives an owned copy a new SourceId. The resolution is Source<'a> without the derive.
• One more edit for the new type: the manual impl is now impl Clone for Source<'_>.
• The patch of the PR has the same size as before the merge (5 files, +256 -22).
• cargo check --workspace passes on the merged tree (Linux x64).
• Test, on a local debug build that contains this merge and the main merges of eight other open PRs: test/cli/install/bun-install.test.ts (776 pass, 0 fail).

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[claude]

I re-reviewed after b88ac25 and found no bugs; the earlier nit about the now-redundant tracker drop in src/install/npm.rs is addressed by that commit. A human look is still worthwhile because the PR adds a new field to bun_ast::Source and a process-wide id scheme, which the description itself asks a maintainer to sign off on.

What was reviewed:

• SourceId::new / next_block: ids from a per-thread block of 2^32 with one Relaxed fetch_add per block; checked that multiples of BLOCK are skipped so 0 stays "no source" and blocks cannot overlap across threads.
• Clone for Source: borrowed contents keep the id, owned contents mint a new one; the bundle_v2.rs call site now using .clone() only loses scan reuse, not correctness.
• LineColumnTracker::matches/reset_for and the debug-only rescan assertion in error_position; the path ptr/len checks were dropped, which is fine since the id already distinguishes sources.
• The four new install tests drain stderr and exit concurrently and assert exact line:column lists; the bunfig case uses withoutAsanQuarantine so the address reuse it depends on actually occurs.

Extended reasoning...

The change touches the shared Source struct in src/ast/lib.rs (new id: SourceId field, manual Clone, a #[thread_local] counter plus a global AtomicU64 for block allocation), rekeys LineColumnTracker on that id, updates the three struct-literal/clone sites in the bundler and resolver, removes the per-producer tracker reset in src/install/npm.rs, and adds unit tests plus four install tests. It touches no auth, crypto, or input-parsing surface; the only security-adjacent effect is that the bunfig token redaction now sees the correct line, which a test asserts. The latest commit addresses the one inline nit from the prior run and the hunt exited on dry_streak with no findings. Defer rather than approve because the PR introduces a new cross-cutting identity mechanism on a core type and the author explicitly requests a maintainer decision between this design and a tracker drop in each producer.

[coderabbitai]

🔇 Additional comments: 1

src/ast/lib.rs-2292-2292: 1

2292-2292: Trivial · Maintainability & Code Quality · Quick win

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Store the tracker ID as SourceId.

source_id: u64 removes the type distinction introduced by SourceId. Keep the field typed as SourceId, derive Default and equality for SourceId, and compare or assign the newtype directly. This keeps future byte offsets or counts from being assigned as source IDs. Based on learnings, “avoid using bare primitive types ... for domain identifiers” and use newtype structs instead.

Source: Learnings

---

Review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/4e81506c/stale-line-column-tracker
• File: .coderabbit.yaml
• Commit: b88ac25
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 5257c1e9-bdfa-4929-8943-f76aad632fcc

Commits

Reviewing files that changed from the base of the PR and between cb03774 and b88ac25.

Files selected for processing: 5

• src/ast/lib.rs
• src/bundler/bundle_v2.rs
• src/bundler/linker_context/generateCodeForFileInChunkJS.rs
• src/install/npm.rs
• src/resolver/package_json.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

[robobun]

On the CodeRabbit note about source_id: u64 in LineColumnTracker: I kept the integer.

• The tracker needs a value for the state before the first diagnostic. 0 is that value, because SourceId::new never returns it.
• src/bundler/ThreadPool.rs keys its per-thread worker cache in the same way: a u64 generation, where 0 never matches a live pool.
• A Default for SourceId would make an id that no Source has.