console.table: invoke each cell getter once

[robobun]

What does this PR do?

Makes console.table and Bun.inspect.table read and format each cell exactly once, matching Node.

Previously console.table invoked an enumerable getter on a row object twice per cell, and the rendered table showed the value of the second call. Node calls it once.

let calls = 0;
const row = {};
Object.defineProperty(row, "x", { get() { return ++calls; }, enumerable: true });
console.table([row]);
console.log("getter invocations =", calls);

Bun renders 2 in the cell and prints getter invocations = 2. Node renders 1 and prints 1. The output both doubles the side effect and shows a value the object never observably returned on first read.

The same defect exists one level down. String-ifying a cell runs user code too, and the formatter ran three times per cell (two width computations and a render):

let calls = 0;
Bun.inspect.table([{ x: { [Bun.inspect.custom]() { return "call " + ++calls; } } }]);

renders call 3.

Cause. TablePrinter::print_table in src/jsc/ConsoleObject.rs made two full passes over the rows: a first pass that read every cell to compute column widths, then a second pass that read every cell again to render it. Each read goes through getOwn / JSPropertyIterator, both of which invoke getters, so every getter fired twice and the second call won. The second pass then formatted the value it read twice more, once to re-measure its width and once to write it, so a custom inspect hook ran three times.

For iterable inputs the two passes also ran the JS iteration protocol twice, so a one-shot iterable was exhausted by the width pass and console.table(someGenerator()) rendered a header-only table with no rows.

Fix. Read and format each cell exactly once, in the width pass. format_cell appends the rendered bytes to a single scratch Vec<u8> shared by the whole table and records { offset, len, width } on the row. The render pass writes those byte ranges back out and makes no JS calls at all.

Per cell this costs the offset/len/width record plus the cell's rendered bytes, held once in one contiguous buffer that is a subset of the output string the call is already building (output = cell text + borders + padding). There is no per-cell heap allocation (an earlier revision of this PR cached each cell's bytes in its own Vec, which roughly doubled peak RSS for the call), and no value is formatted twice (the revision after that rooted each cell's JSValue in a MarkedArgumentBuffer and re-formatted it in the render pass). It is also the fastest of the three, since the old code formatted every cell three times. The MarkedArgumentBuffer files are back to main; nothing here uses it anymore.

The iteration protocol now runs once, so one-shot iterables are no longer exhausted by the width pass. Formatter errors (a throwing [Bun.inspect.custom]) propagate from the width pass with ? rather than being swallowed by get_width_for_value and leaking a pending exception into the next host call.

How did you verify your code works?

Eleven tests in test/js/bun/console/console-table.test.ts under console.table reads each cell once:

• enumerable getter on an array row
• enumerable getter with an explicit properties list
• a getter on a plain-object row key (the outer tabularData getter)
• a getter on a primitive routed to the trailing Values column
• a custom inspect on a cell value is invoked exactly once
• a generator input (one-shot iterable)
• cell values survive a full GC between the width and render passes
• a row whose key order differs from the column order
• a Proxy row renders the [[Get]] value the width pass saw
• a throwing custom inspect in a cell still propagates
• the spawned console.table repro above

Eight of the eleven fail on the unfixed build: the four getter-count tests (the getter fires twice and the cell renders 2), the custom inspect test (the hook fires three times and the cell renders the third result), the Proxy test, the generator test, and the spawned repro. The remaining three guard the new mechanism rather than the original bug: exception propagation out of format_cell, a getter that triggers a full GC in the middle of the width pass, and the positional cell-slot mapping.

All 45 existing snapshots across console-table.test.ts and bun-inspect-table.test.ts pass with byte-identical output, with and without ANSI colors.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cac0b488-d851-4269-8619-1dd349ecbced

📥 Commits

Reviewing files that changed from the base of the PR and between 1b2dbeb and 0a36c5e.

📒 Files selected for processing (1)

• src/jsc/ConsoleObject.rs

---

Walkthrough

TablePrinter now formats each cell once into a shared cell_text buffer during collection, stores cached widths and byte ranges in CollectedRow, and renders from those cached values. The render pass no longer re-invokes getters. Tests cover getters, generators, proxies, GC timing, custom inspect hooks, and errors.

Changes

TablePrinter single-pass row caching

Layer / File(s)	Summary
Cached data structures and format_cell helper src/jsc/ConsoleObject.rs	RowKey gains a cached UTF-8 slice and visible width. New CellRef and CollectedRow types store per-cell byte ranges and widths into a shared scratch buffer. format_cell formats a JSValue once and returns a CellRef. Map-specific collect_row path caches key/value cells and updates column widths.
collect_row: per-row collection logic src/jsc/ConsoleObject.rs	For object rows with a properties list, caches each value into a cell or inserts a None placeholder. On-demand property iteration computes column indices, resizes the cell vector, and stores cached cells. Primitive rows are routed into values_cell.
print_row: cached rendering src/jsc/ConsoleObject.rs	Rewrites print_row to read from CollectedRow and the shared cell_text buffer: right-aligns the index column from RowKey::width(), selects values_cell or indexed cells per column, and pads with cached widths.
print_table: collection and render passes src/jsc/ConsoleObject.rs	Width/collection phase now builds cell_text: Vec<u8> and rows: Vec<CollectedRow>, stops on the first error, and handles non-iterable input via collect_row. The render pass replays collected rows through print_row.
console.table single-evaluation tests test/js/bun/console/console-table.test.ts	New suite verifies getters are called once (with/without properties list), generator rows appear once, primitives route to the Values column, Bun.inspect.custom hooks fire once, throwing hooks propagate, GC between passes preserves cells, row key ordering doesn't truncate cells, Proxy reads match the width-pass value, and a subprocess test confirms one getter call and clean exit.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main behavior change and is concise.
Description check	✅ Passed	The PR description includes both required sections and gives a detailed change summary plus verification steps.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

[robobun]

^{Updated 8:42 PM PT - Jun 28th, 2026}

✅ @robobun, your commit 0a36c5e5f9a2d30fa135cc0753280be1ebd24619 passed in Build #66567! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32924

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

bun-32924 --bun

[github-actions]

Found 1 issue this PR may fix:

1. Node compatibility: console.table() renders ANSI escape sequences instead of escaping them #32223 - The ANSI color propagation fix in this PR addresses console.table rendering raw ANSI escape sequences instead of escaping them

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #32223

🤖 Generated with Claude Code

[claude]

I didn't find any bugs, but this is a ~200-line refactor of the native TablePrinter (restructuring the two-pass loop into cached FormattedRows, dropping the explicit visited-map pool release, and buffering all cell bytes in memory), so it's worth a human look — especially with build-rust CI still red on the last reported commit.

Extended reasoning...

Overview

This PR refactors TablePrinter in src/jsc/ConsoleObject.rs to collapse the two-pass (width-measure → render) loop into a single pass that formats each cell once, caches the rendered bytes + visible width in new FormattedCell/FormattedRow structs, and then renders from the cache. It removes VisibleCharacterCounter, get_width_for_value, and the entire second iteration block, and rewrites print_row to consume pre-formatted cells instead of re-reading JSValues. Five new tests cover array rows, explicit properties, plain-object tabular data, generators, and a spawned console.table repro.

Security risks

None identified. This is output-formatting logic for console.table/Bun.inspect.table; no auth, crypto, filesystem, or network surface is touched. The unsafe block touched is the existing callback_ctx pattern, unchanged in semantics.

Level of scrutiny

Medium-high. While the user-facing behavior change is well-motivated and well-tested, the implementation is a non-trivial restructuring of native Rust that interfaces with JSC: it changes how the per-cell Formatter clone's map_node pool entry is released (now relying purely on Drop rather than the explicit release block), changes column lookup from &mut Column to index-based with a resize_with slot fill, and trades streamed rendering for buffering every cell's bytes in a Vec<FormattedRow>. These are the kinds of resource-management and memory-tradeoff details a maintainer should sign off on.

Other factors

• CI (robobun) reports build-rust failures across all platforms for commit d7f0d91; an autofix commit (edf83d1) followed, but no green build is visible in the timeline yet.
• The removed explicit pool-release block carried a comment explaining capacity-based deinit vs clear; the new code relies on Formatter::Drop doing the equivalent — worth confirming that's still the intended lifecycle.
• Buffering all formatted cells changes peak memory for very large tables; probably fine for console.table, but it's a deliberate tradeoff.
• Existing snapshot tests (21 console-table + 35 bun-inspect-table) reportedly pass unchanged, which gives reasonable confidence in output parity.

[robobun]

Thanks for the careful read. Taking the concerns in turn.

build-rust red on d7f0d91: those were not compile failures. Each of those job logs ends with # Received cancellation signal, interrupting: BuildKite cancelled the in-flight builds when the autofix.ci commit landed a few minutes later. That commit (edf83d1) is formatting only (one rustfmt line wrap, two prettier wraps). Its build, #65802, has cargo clippy, Format, and every build-cpp lane green, with build-rust still running.

visited-map pool release: the deleted explicit block was already documented in place as redundant:

Formatter::Drop does the same release, so a plain scope is sufficient.

Formatter::Drop does everything the block did and also returns the working map's allocation to the pooled node (the block only cleared the node). More to the point, the pre-existing get_width_for_value, which ran on every cell and had no explicit block, already relied on Drop alone; format_cell is the same shape. Drop was already the path exercised on every cell.

error propagation: related to the above, format_cell discards the JsResult from format() with let _ =, the same as the old get_width_for_value, while the old print_row checked it with result?. I verified this is not a regression: a cell whose custom inspect throws still propagates in every arrangement, including when the throwing cell is the last cell of the last row, because the pending VM exception surfaces through the exception scope of whatever touches JSC next.

const bad = { [Symbol.for('nodejs.util.inspect.custom')]() { throw new Error('boom'); } };
console.table([{ a: bad }]);       // throws before and after
console.table([{ a: bad, b: 1 }]); // throws before and after

memory: agreed it is a deliberate tradeoff. console.table already buffered the entire rendered table into one Vec<u8> before writing anything, so cells were never streamed to the terminal; this adds the per-cell intermediate copies on top of that, which is also what Node does (it util.inspects every cell into a string up front).

One behavior delta I found while auditing that is not in the PR description: when a row object has a property the iterator yields but a string-named getOwn cannot find (observed with a symbol-keyed method), the old code showed the column with a blank cell because the width pass and the render pass disagreed about the value. The new code renders the value the iterator returned, since there is only one read. No existing snapshot covers it and I left it as is; the one-read invariant is the point of the PR.

Re-ran both console table suites against the debug build: 61 pass, 0 fail, 45 snapshots unchanged.

[claude]

I didn't find any bugs and the single-pass refactor looks sound, but this is a ~200-line restructure of the native TablePrinter (JSC iteration, formatter pool/Drop semantics, error-swallowing in format_cell) — worth a human pass before merging.

Extended reasoning...

Overview

This PR refactors TablePrinter in src/jsc/ConsoleObject.rs to collapse the two-pass column-sizing/render loop into a single pass that caches each cell's formatted bytes and visible width in new FormattedCell/FormattedRow structs. It removes VisibleCharacterCounter and get_width_for_value, renames update_columns_for_row → collect_row, rewrites print_row to consume cached bytes instead of re-reading JS properties, and deletes the entire second iteration block in print_table (including its duplicate extern "C" callback and JSPropertyIterator walk). Five new tests cover getters on array rows, the explicit properties arg, plain-object tabular data, generators, and a spawned console.table repro; the 56 existing snapshot tests are reported unchanged.

Security risks

None identified. console.table is pure output formatting with no auth, filesystem, network, or privilege implications. The one unsafe block touched (the bun_ptr::callback_ctx cast in the iterable callback) is structurally identical to before — and the PR actually deletes the second copy of it, reducing unsafe surface.

Level of scrutiny

Medium. This is not a config tweak or mechanical rename — it's a substantive control-flow restructure of native Rust that interacts with JSC (getOwn, get_index, JSPropertyIterator, for_each_with_context). Several non-obvious correctness questions were already raised and answered in-thread: the removed explicit visited-map pool release now relies on Formatter::Drop (which the old get_width_for_value already did per cell), format_cell discards the format() JsResult with let _ = and relies on the pending VM exception surfacing later, and the per-cell Vec<u8> buffering is a deliberate memory tradeoff matching Node. These answers look right to me, but they're exactly the kind of subtle invariants a maintainer familiar with the formatter pool and JSC exception scopes should sign off on.

Other factors

The diff is net-negative (~170 deleted vs ~110 added) and removes duplicated iteration logic, which is a structural win. Test coverage is solid — new tests assert both the rendered output and the getter call count, and the generator test covers the one-shot-iterable fix. The author's follow-up comment also notes a minor behavior delta for symbol-keyed properties (previously blank, now rendered) that has no snapshot coverage. Given the scope and the reliance on Drop/exception-scope invariants, I'm deferring rather than auto-approving.

[robobun]

The error-swallowing in format_cell was a real find. The old code propagated a throwing custom inspect from the render pass's result?; with that pass merged away, the let _ dropped it and left the exception pending across the next host call. Pushed a fix: format_cell now returns the formatter's JsResult with ?, plus a test that a throwing [Bun.inspect.custom] in a cell still propagates (that one passes on the released build too, since the bug it guards against was only introduced by this PR's earlier revision).

Also added a test for a getter on a primitive routed to the trailing Values column, a code path none of the existing tests hit.

Now seven tests in the new describe block: six fail on the unfixed build, the exception one guards the new line.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/ConsoleObject.rs`:
- Around line 937-940: The comment in ConsoleObject::format-like row rendering
logic is too long and must be reduced to 3 lines or fewer while preserving the
key invariant. Shorten the existing note near the single-pass row formatting and
width computation so it still states that each cell is read and formatted once,
widths are computed during that pass, and cached rendered cells prevent
re-reading getters or exhausting one-shot iterables. Keep it concise and place
it on the same code path where the row rendering loop and column-width
computation are described.

In `@test/js/bun/console/console-table.test.ts`:
- Around line 308-309: The spawned regression check currently only asserts
stdout and exitCode, so unexpected stderr output can slip through; update the
test around proc.stdout.text, proc.stderr.text, and proc.exited to assert the
combined { stdout, stderr, exitCode } object instead. Keep the existing repro
behavior, but include stderr in the expected result so console-path regressions
that emit diagnostics are caught.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ffbb22fd-2089-4211-a9fd-c526f4a2aba6

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9331d and 07b50f0.

📒 Files selected for processing (2)

• src/jsc/ConsoleObject.rs
• test/js/bun/console/console-table.test.ts

[claude]

I didn't find any bugs, but this is a non-trivial restructuring of the native TablePrinter (JSC interop, formatter Drop/pool semantics, cell caching) — worth a human pass before merging.

Extended reasoning...

Overview

This PR collapses console.table's two-pass implementation (width computation, then rendering) into a single pass that formats each cell once and caches the rendered bytes plus visible width in new FormattedCell/FormattedRow structs. It touches src/jsc/ConsoleObject.rs (~250 line diff: removes VisibleCharacterCounter, get_width_for_value, the second iteration block, and the explicit visited-map pool release in print_row; rewrites collect_row and print_row) and adds 7 tests to test/js/bun/console/console-table.test.ts.

Security risks

None identified. This is output-formatting code; no auth, crypto, permissions, or untrusted input parsing beyond what console.table already did. The unsafe blocks touched are the pre-existing callback_ctx casts for the extern "C" iteration callback, with the same shape and safety invariant as before.

Level of scrutiny

Medium. The fix is conceptually straightforward (read once, cache, render) and matches Node's approach, but the implementation is a substantive refactor of Rust code at the JSC FFI boundary: it changes how errors propagate from format() (now ? in format_cell with a regression test added in 07b50f0), removes an explicit pool-release block in favor of Formatter::Drop, restructures column-index bookkeeping, and changes the lifetime of formatted data (now buffered per-cell in Vec<u8> before the final write). These are all reasoned through in the PR thread, but they're the kind of changes a maintainer familiar with the formatter's pooling and exception-scope conventions should sign off on.

Other factors

The bug hunting system found nothing. Test coverage is good — new tests exercise array rows, explicit properties, plain-object tabular data, generators (one-shot iterables), the Values column, throwing custom inspect, and a spawned console.table end-to-end check; the existing 21 snapshot tests and 35 bun-inspect-table tests reportedly pass unchanged. The author's follow-up comment addresses build status, pool release, error propagation, and memory tradeoffs in detail. Still, this isn't a mechanical change, so I'm deferring rather than auto-approving.

[Jarred-Sumner]

This roughly doubles peak memory for the call: it holds every rendered cell's bytes (FormattedCell.text: Vec<u8>) for the entire table at once, on top of the final output buffer.

Isolated Bun.inspect.table() call, max RSS delta over a baseline that builds the same rows but skips the call (median of 5, macOS arm64):

rows	main	this PR
4,096	2.9 MB	4.7 MB
100,000	67.9 MB	116.6 MB

Instead of buffering the rendered text, put each cell's JSValue into a MarkedArgumentBuffer during the width pass and format from those in the render pass. That still reads each property / invokes each getter exactly once (the actual bug), is GC-safe to hold across the loop, and costs a pointer per cell instead of the rendered text.

[robobun]

Reworked as described in 10fdae5.

The width pass now reads each property exactly once, appends the cell's JSValue to a MarkedArgumentBuffer scoped across the whole print_table call, and records its visible width in a CellRef { value: JSValue, width: u32 } stored inline in the row. The render pass formats those rooted values directly. The rendered-text cache is gone; per cell the only overhead is the 16-byte CellRef plus the buffer slot, with no per-cell heap allocation.

I have not re-run your RSS benchmark on a release build, so treat the memory claim as structural rather than measured. All 45 existing snapshots across the two table suites are byte-identical, and the getter tests still pass (six fail on the unfixed build).

[robobun]

CI note on 10fdae5 (build 66229): the two red jobs are both darwin 26 aarch64 - test-bun, the original run and its automatic retry. Both died during setup with

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. Refusing to continue with a partial download (would silently fall back to the wrong binary).

so neither ran a single test. darwin-aarch64-build-bun itself passed, and the other lanes that consume the same artifact are queued normally, so this is an agent/network timeout on that one macOS 26 machine, not something in the diff. Everything that has finished so far is green, including debian-13-x64-asan-test-bun, which runs the new console.table reads each cell once tests under ASAN. The rest of the matrix is still running.

That lane needs a manual retry from the Buildkite UI (it already used its automatic one); there is nothing to change in the PR for it.

[robobun]

Pushed 11cf6d9, which closes a gap in the MarkedArgumentBuffer rework rather than changing its shape.

JSC's MarkedArgumentBuffer uses the RecordOverflow policy: if the spill-to-heap allocation fails, append() is a silent no-op and the value is never rooted. The cell's JSValue then sits in the CellRef Vec on the Rust heap, which the GC does not scan, so an allocation failure during the width pass would turn into a use-after-free in the render pass instead of an OOM. The fix is one line in the C++ shim: appendWithCrashOnOverflow instead of append. That is JSC's own API for callers that cannot tolerate a silent drop, and it matches the project's rule that allocation failure must be loud. It upgrades every Rust caller of the binding at once, and for all of them an un-rooted value was already unrecoverable.

All 81 console tests pass locally against the new build, all 45 snapshots byte-identical, and six of the seven getter tests still fail on the unfixed released build.

On build 66229 (the run before this push): every red lane was infrastructure, none touched this diff.

• alpine-3.23-x64-test-bun: the mysql_native_password Docker service never passed its healthcheck within 1m, so sql-mysql.auth.test.ts could not start. Unrelated to console.table.
• darwin-26-aarch64-test-bun: buildkite-agent artifact download timed out during setup; zero tests ran. Its automatic retry hit the same thing.
• darwin-14-aarch64-test-bun and linux-x64-android-build-cpp: both "Expired", no agent picked them up.

The lane that actually exercises the new tests under ASAN, debian-13-x64-asan-test-bun, was green. This push starts a fresh build, which also re-rolls those lanes.

[robobun]

Addressed both review comments in b035424.

• RowKey::str now uses name.to_utf8_bytes() instead of format!("{name}").into_bytes(). Both paths call to_utf8_without_ref() so the bytes are identical; the helper just skips the std::fmt hop and the intermediate std::string::String.
• The appendWithCrashOnOverflow comment in MarkedArgumentBufferBinding.cpp is now 3 lines.

No behavior change (one is comment text, the other is a byte-identical helper swap), cargo check -p bun_jsc is clean, and both threads are resolved.

[robobun]

CI note on b035424 (build 66422): 280 jobs, 1 real failure.

The one red lane is alpine 3.23 x64-baseline - test-bun on test/js/sql/sql-mysql.auth.test.ts. Its job log shows the mysql_native_password docker service never passed its health check on that agent ("application not healthy after 1m0s"), so the SQL auth tests could not connect. This PR only touches console.table formatting in ConsoleObject.rs and its test file, so that failure is test infrastructure, not this change.

The two darwin 14 lanes showed as "Expired" (no agent picked them up before the timeout); their retries were already queued.

Re-rolled CI with an empty commit. All five review threads on this PR are resolved.

[robobun]

CI status on 0cdfd2f (build 66440): the diff is green on every lane that ran. The build is only red because the two macOS 14 test lanes never got an agent.

What ran and passed:

• All linux, alpine, freebsd, android and windows build and test lanes.
• debian-13-x64-asan-test-bun, which matters here: the rework roots cell JSValues in a MarkedArgumentBuffer across the two passes, and the ASan lane is where a missed root would surface.
• darwin-26-aarch64-test-bun, so the change is exercised on macOS. Its first attempt failed on agent setup and the automatic retry passed.
• The only test annotation is a known-flaky bun-install.test.ts retry on windows 11 aarch64, unrelated to this change and green after one retry.

What is red:

• darwin-14-aarch64-test-bun and darwin-14-x64-test-bun report "Expired". The Buildkite API shows all four of those job instances (each lane plus its retry slot) still in the scheduled state: no macOS 14 agent ever picked them up, and the check window timed out. There is nothing in the diff for those lanes to run differently from the macOS 26 lane that did pass.

This branch has already been retriggered twice and each rebuild hit a different piece of infrastructure (an artifact download timeout, a MySQL health check, now darwin-14 agent availability), so I am not pushing another empty commit. The change and its tests are ready; this needs a maintainer to either merge over the expired lanes or re-run the build once darwin-14 agents are back.

[robobun]

Pushed 31564bd: three regression tests, no source change. A coverage review of the diff found three things the suite could not detect.

The roots.append is load-bearing but was untested. The width pass caches each cell's JSValue in a Rust Vec<CollectedRow>, which JSC's conservative stack scan never visits, so roots.append(value) in capture_cell is the only thing keeping those cells alive across the user code (getters, custom inspect) that runs between the two passes. Every existing test's cell was either an int32 immediate or still reachable through the live tabularData argument, so deleting that line left the whole file green.

The new test uses 64 rows whose getters each return a fresh object and then force a full collection. 64 is deliberate: it spills the MarkedArgumentBuffer past its 8-slot inline (stack-scanned) storage into the heap-registered regime, so both of JSC's rooting paths are exercised. Verified by mutation: with roots.append(value) replaced by a no-op and the binary rebuilt, all 64 of 64 cells are collected before the render pass and the test fails (missing = [0..63]); restored, it passes. Nothing else in the suite catches that deletion.

Out-of-order row keys. Cells are now stored positionally by column index (row.cells[col_idx - 1]). The branch that handles a row revisiting an already-discovered column after creating a later one was never exercised, and two plausible one-line rewrites of it silently drop a cell while passing every existing case. [{ a: 1 }, { b: 2, a: 3 }] pins it with an exact expected table.

Proxy rows. Reading each cell once also means the column is sized from the same value that gets rendered: the [[Get]] result, matching Node. The old render pass re-read through [[GetOwnProperty]], so on 1.4.0 a one-trap Proxy renders the raw target value into a column sized for the trap value, which also breaks the box:

$ bun -p 'Bun.inspect.table([new Proxy({x:"FROM_TARGET"},{get:()=>"FROM_GET"})])'
┌───┬──────────┐
│   │ x        │
├───┼──────────┤
│ 0 │ FROM_TARGET │
└───┴──────────┘

With this branch it renders FROM_GET in a consistent 8-wide column, matching node. This test fails on the unfixed build; the GC and slot-mapping tests pass on it by design, since they guard the new mechanism rather than the original bug.

Full file: 31 pass / 0 fail on this branch, 24 pass / 7 fail on unmodified bun 1.4.0.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/ConsoleObject.rs`:
- Around line 998-1035: The iterable collection path in
ConsoleObject::collect_tabular_data is collapsing every collect_row failure into
a generic JsError::Thrown, unlike the non-iterable path that preserves the typed
error. Update the Ctx/callback flow to carry the actual JsError returned by
collect_row through the for_each_with_context callback, store that error in the
context instead of a bool flag, and return that preserved error after iteration
rather than always throwing a generic error.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fa97e4b8-ca84-4f3f-8030-889fc16a0d9f

📥 Commits

Reviewing files that changed from the base of the PR and between 07b50f0 and 31564bd.

📒 Files selected for processing (4)

• src/jsc/ConsoleObject.rs
• src/jsc/MarkedArgumentBuffer.rs
• src/jsc/bindings/MarkedArgumentBufferBinding.cpp
• test/js/bun/console/console-table.test.ts

[robobun]

CI status on df3d47d (build 66485). All review threads are now resolved and the only red is infrastructure.

Review: the last open thread, on the Verification section of the description, is addressed. I re-ran the new describe block against stock bun 1.4.0 rather than restating numbers: 7 of the 10 tests fail on the unfixed build, and 3 pass on both by design because they guard the new mechanism (exception propagation, the MarkedArgumentBuffer rooting, and the cell-slot mapping) rather than the original bug. The description now states that split. df3d47d itself was the response to the coderabbit thread about ctx.err: bool discarding the typed JsError; that thread is resolved inline.

CI, and why I am not pushing a retrigger: darwin-26-aarch64-test-bun failed both attempts with

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

Zero tests ran on that lane; the runner never obtained the binary. The only two annotated test failures on the build are pre-existing flaky install tests on unrelated lanes (update_interactive_install.test.ts on Windows aarch64 and complex-workspace.test.ts on debian x64-asan), both auto-retried and both tagged flaky by the reporter. Neither is in a subsystem this diff touches.

A ci: retrigger (0cdfd2f) has already been spent on this PR, so I am not pushing another one to chase an artifact-download timeout. The diff is complete and green on every lane that actually ran its tests; this needs a maintainer rather than another CI roll.

[Jarred-Sumner]

Use ZigStringSlice since that becomes UTF8 and usually avoids the extra clone

[robobun]

Done in 3729432.

RowKey::str now stores the name as a ZigStringSlice via to_utf8() instead of an owned Vec<u8> from to_utf8_bytes(). For an all-ASCII property name, the common case, that is a ref() on the existing WTFStringImpl plus a borrowed pointer into its Latin-1 buffer: no allocation, no copy. Non-ASCII or 16-bit names still transcode into an Owned slice. Both variants own their backing (the slice's Drop derefs the ref or frees the buffer), so holding it in the rows vec past the property iterator is safe.

The rendered bytes are identical either way. All 45 table snapshots are unchanged and the full console suite still passes (83/83).

[robobun]

CI triage on 3729432 (build 66504): three failed jobs, zero failed tests.

• alpine 3.23 x64 - test-bun and alpine 3.23 x64-baseline - test-bun: the mysql_native_password docker service never passed its health check ("application not healthy after 1m0s") before test/js/sql/sql-mysql.auth.test.ts ran. 21 of the 22 alpine 3.23 x64 jobs in the same build passed; only the shard that needed the SQL service hit the stuck agent. Neither job had an automatic retry left, so nothing but a push could re-roll them.
• darwin 26 aarch64 - test-bun: the first attempt died in bun install, where puppeteer's postinstall found a corrupted chrome-headless-shell cache on the agent. Its automatic retry passed, so the check itself is green.

None of the three ran a failing test: grepping each log for (fail) returns zero lines. Every lane that did run tests is green, including debian-13-x64-asan-test-bun, which runs the new console.table reads each cell once block under ASAN.

Re-rolled with f9f81eb. The diff has not changed since 3729432, which addressed the ZigStringSlice review. All seven review threads are resolved.

[robobun]

CI triage on f9f81eb (build 66517), the re-roll of build 66504.

One failed job, alpine 3.23 x64-baseline - test-bun, and it is the same infrastructure failure the previous build hit: the mysql_native_password docker service on the agent never passed its health check ("application not healthy after 1m0s"), so test/js/sql/sql-mysql.auth.test.ts could not start. No test in the 237-file shard failed for any other reason, and that file has now failed on the same docker service across builds 66504 and 66517. This PR touches src/jsc/ConsoleObject.rs, src/jsc/MarkedArgumentBuffer.rs (plus its C++ binding), and the console.table tests. Nothing near SQL or docker.

Every other lane is green, including debian-13-x64-asan-test-bun, which runs the new console.table reads each cell once block under ASAN. All seven review threads are resolved.

I am not going to keep pushing empty retrigger commits for a recurring agent-side docker timeout. The diff is ready as of 3729432; this is a maintainer call from here.

[Jarred-Sumner]

not correct. we shouldn't be formatting the JSValue twice. We format the string-ified JSValue only.

[robobun]

Done in 1b2dbeb. format_cell string-ifies the value once, during the width pass, appending the rendered bytes to a single scratch Vec<u8> shared by the whole table, and the row records { offset, len, width }. The render pass writes those byte ranges back out and makes no JS calls.

You are right that this was not just redundant work. String-ifying runs user code, so the double format had the same observable bug as the double read, one level down:

let calls = 0;
Bun.inspect.table([{ x: { [Bun.inspect.custom]() { return "call " + ++calls; } } }]);

main invokes the hook three times per cell (two width computations plus the render) and the table shows call 3; the revision you reviewed invoked it twice and showed call 2. Now it is invoked once and the table shows call 1. That is the new a custom inspect on a cell value is invoked exactly once test; I verified the 3 / 2 / 1 counts by building each of the three revisions and running the same test against them.

On memory, relative to the first revision you measured: the per-cell heap Vec<u8> is gone. The per-cell cost is the 24-byte offset/len/width record, and the one buffer this adds over the revision you last looked at is the contiguous cell text, which is a subset of the output string the call is already building. Holding the rendered cell text once is the floor for any single format, because every column's width depends on every row. It is also the fastest of the three, since main formatted every cell three times; on a 50k-row, 5-column table the same debug build goes from 12.3s on main to 9.3s here.

Since nothing uses MarkedArgumentBuffer anymore I reverted those two files to main, so the diff is ConsoleObject.rs plus the tests.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/ConsoleObject.rs`:
- Around line 680-687: The `CellRef::text` path and the related cell-width
handling are narrowing a user-controlled byte length into `u32`, which can panic
on large `console.table` output. Update `CellRef` and the surrounding formatting
logic so byte lengths stay as `usize` end-to-end, and remove any `expect("int
cast")`/fallible narrowing in the code that builds `CellRef` or computes text
slices. Make the `CellRef::text` indexing use the existing `usize`
offsets/lengths directly, and apply the same fix anywhere else in the cell
formatting flow that currently converts `text.len()` or similar external sizes
into smaller integer types.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1fad7a99-70d3-48e4-9be9-9eb03914392d

📥 Commits

Reviewing files that changed from the base of the PR and between 3729432 and 1b2dbeb.

📒 Files selected for processing (2)

• src/jsc/ConsoleObject.rs
• test/js/bun/console/console-table.test.ts

[Jarred-Sumner]

Measured peak RSS and time for Bun.inspect.table on this PR (local release build) vs bun 1.4.0. One cold process per data point, /usr/bin/time -l for max RSS, Bun.nanoseconds() around the single Bun.inspect.table(rows) call. Rows are {id, name, email, age, active, score}. Output string length is byte-identical between the two builds at every size. Numbers are medians of 3 runs (RSS within ~0.1%, time within ~1% across runs).

Time for the Bun.inspect.table() call

rows	bun 1.4.0	this PR	speedup
100	0.139 ms	0.115 ms	1.21×
1,000	1.00 ms	0.69 ms	1.45×
10,000	9.50 ms	6.30 ms	1.51×
100,000	98.1 ms	64.2 ms	1.53×
1,000,000	975.0 ms	654.2 ms	1.49×

Process max RSS

rows	output bytes	bun 1.4.0	this PR	delta
1,000	72 KB	24.5 MB	25.0 MB	+2%
10,000	770 KB	31.3 MB	36.6 MB	+17%
100,000	8.2 MB	109.3 MB	154.5 MB	+41%
500,000	43 MB	440.9 MB	648.1 MB	+47%
1,000,000	87 MB	854.4 MB	1,274.0 MB	+49%

Isolating RSS growth attributable to the call itself (process.memoryUsage.rss() after − before; baselines match between builds):

rows	bun 1.4.0	this PR	ratio
10,000	6.1 MB	11.3 MB	1.86×
100,000	64.9 MB	110.1 MB	1.70×
1,000,000	636.1 MB	1,056.3 MB	1.66×

So the correctness fix comes with a consistent ~1.5× speedup from ~1k rows up, but peak memory for the call is ~1.66× main, converging to about +49% total process RSS at 1M rows.

The PR description says the scratch buffer holds the cell bytes once as "a subset of the output string the call is already building", which predicts roughly the cost of one extra copy of the cell text — at 1M rows that's well under the ~420 MB observed. The measured growth is closer to the "roughly doubled peak RSS" the description attributes to the earlier per-cell-Vec revision, so it's worth checking whether the scratch Vec<u8> is over-reserving (e.g. growing by doubling with no shrink, or a worst-case capacity estimate), or whether the per-cell {offset, len, width} records are larger than they look — at 6M cells even 32 bytes/record is ~190 MB.

Harness

table-rss.mjs:

const count = parseInt(process.argv[2], 10);
const rows = new Array(count);
for (let i = 0; i < count; i++) {
  rows[i] = {
    id: i,
    name: `user-${i}`,
    email: `user-${i}@example.com`,
    age: 18 + (i % 60),
    active: i % 2 === 0,
    score: i * 1.5,
  };
}
const baseline = process.memoryUsage.rss();
const t0 = Bun.nanoseconds();
const str = Bun.inspect.table(rows);
const t1 = Bun.nanoseconds();
const after = process.memoryUsage.rss();
console.log(JSON.stringify({ count, len: str.length, baseline, after, ms: (t1 - t0) / 1e6 }));

Driver (macOS):

for bin in bun ./bun-pr; do
  for n in 1 100 1000 10000 100000 500000 1000000; do
    for i in 1 2 3; do
      /usr/bin/time -l "$bin" table-rss.mjs "$n"
    done
  done
done