[robobun]

Problem

• assert.partialDeepStrictEqual is quadratic on Sets and Maps whatever they contain. Release build, two equal collections: a Set of 100k integers takes 4.8 s (node: ~30 ms), a Map of 10k string keys 275 ms (node: ~10 ms); every doubling costs 4x.
• setSubset (src/jsc/modules/NodeUtilTypesModule.cpp) has no hash path at all: for each expected member it walks every actual member from index 0, skipping the ones already claimed, until compareBranch matches. Primitives go through that walk too, and in the common same-order case the skips alone are n*n/2.
• mapSubset does look keys up, but records each claimed key in a MarkedArgumentBuffer and rescans that list with sameValue (identityKeyUsed) once per expected entry, so a Map with n primitive keys does n*n/2 string comparisons; its object-key path also rescans actual from index 0 and calls identityKeyUsed per candidate.
• A smaller correctness bug in the same code: a Set member that both sides hold by identity could be claimed by some structural subset of it that came earlier in expected, after which the member itself found nothing, so partialDeepStrictEqual(new Set([x, w, 1]), new Set([{ a: 1 }, x])) threw (with x = { a: 1, b: 2 }, w = { a: 1, c: 3 }). Node passes it.

Fix

• Both functions now work in two passes, the shape of node's setEquiv / mapEquiv (lib/internal/util/comparisons.js, v26.3.0 L628-L921):
  • Pass over expected: everything a hash lookup can settle is settled by one lookup. Set: a member actual holds (JSSet::has) is done; a special member (node's typeof test, the file's isSpecialValue: primitives, null, functions) it does not hold fails the comparison, since nothing else can match it. Map: every key is looked up (JSMap::get, plus has when the stored value is undefined) and its value compared; a special key that fails this fails the comparison, an object key that fails it is queued, and an object key that passes reserves its actual entry. Only the queued objects go on to pass two.
  • Pass over actual, once: each object member / object-keyed entry that is not reserved claims one queued entry (PendingEntries::claimWith, new header src/jsc/bindings/PendingEntries.h) in node's probe order: the front of the open window while that keeps hitting, the back while that keeps hitting, otherwise a scan of the rest; a claimed entry is swap-removed; the pass stops as soon as the window is empty or fewer actual entries remain than are open. A stray Set member (special, not held by expected) records a miss, which is all node's probes of it leave behind; a stray Map key is passed over, as in node.
• Result: primitive and identity content costs one or two hash lookups per entry, and a structural residue costs one or two comparisons per actual entry when the sides were built in the same or the opposite order. Residues in unrelated orders still scan the window, as node's do.
• Where the results come from:
  • The lookup paths give the answers the old code gave: compareBranch of anything against a special value is true only for SameValueZero pairs (what has/get test; Sets and Map keys normalize -0), a special Map key already had to be held with a matching value, and object keys both Maps hold were already settled by lookup on main and in every release.
  • Structural matching is not one-to-one (an expected { a: 1 } fits every actual object with a: 1), so every implementation here is a greedy pairing and the question is only whose order to use. For everything except Map keys both sides hold it is node's, down to which entry is claimed when several fit: on 4,000 randomized contended instances without shared keys (strays included) plus the hand-written cases, this branch's Set and Map results are identical to node v26.3.0's, and so are its Set results on 4,000 more instances with shared members. (Node's separate shortcut for Sets with fewer than three actual members is not mirrored; it contradicts node's general path.)
  • Map keys both sides hold are the one deliberate divergence, kept from main: node queues them like any other object key (mapEquiv L882-L891), so the result for an identity-keyed Map depends on insertion order and, once the orders differ, the pairing is quadratic; an earlier revision of this PR did the same and turned a reordered 10k-key identity Map from milliseconds into seconds while flipping it from passing (every release so far) to throwing. Settling such keys first is what node does for Set members and what Bun always did for Maps. On the 4,000 shared-key instances the Map results differ from node's in 22, all of them Maps node rejects because of the order its walk happened to take; the code comments and the two table rows marked "node: ..." in the test spell the divergence out.
• Tests, all in test/js/node/assert/deep-equal.test.ts, describe("assert.partialDeepStrictEqual on Sets and Maps") (every expectation checked against node except the two rows that say so): a table of Set/Map pairing cases including the shared-key shapes and the reservation, the stray-member cases (the same four objects pair off differently as Set members and as Map keys, with a primitive and with a function as the stray), a permuted-order pairing test, self-referential members, exceptions propagating from each comparison site, getter counts (same-order and reversed-order residues are linear, and contents both sides hold are never read at all), a GC test in which a getter empties the expected collection entry by entry, collects, and reuses the freed cells (without the gcBuffer appends that root the pending copies it fails for Sets and trips UBSan for Maps), and a spawned child comparing a 100k-integer Set and a 12k-string-key Map, which on main is still running when its 60 s timeout kills it and finishes in a few seconds here. Against main 6 of the new tests fail (identity case, the two Map stray cases, both reversed getter counts, the spawned child); with the fix the file passes (356 tests).
• Also run: assert.test.cjs, regression/issue/24338, regression/issue/28522, assert-typedarray-deepequal.test.ts, the vendored test-assert-class.js, test-assert-typedarray-deepequal.js, test-assert-deep-with-error.js, test-util-isDeepStrictEqual.js (all pass), the new tests and the corpus under BUN_JSC_validateExceptionChecks=1 (clean), and node's upstream test/parallel/test-assert-partial-deep-equal.js (140 cases, not vendored), which passes except its File/Blob prototype case, a pre-existing unrelated difference (Object.getPrototypeOf(File.prototype) is not Blob.prototype in Bun).
• Timings, comparison only, debug build (bun bd), main -> this branch: Set of 20k integers 2114 ms -> 63 ms; Map of 4k string keys 9456 ms -> 34 ms; Set of 2k objects with expected reversed 106.7 s -> 147 ms; Map of 2k object keys with expected reversed 100.1 s -> 104 ms; same-order 2k objects unchanged (~120 ms). Release build: Set of 100k integers 4.8 s -> 34 ms; Map of 10k string keys 275 ms -> 11 ms. Identity-keyed Map with expected shuffled, this branch: 10k keys 60 ms, 20k 113 ms, and with one structural entry added (so the reservation set is built and consulted) 109 ms / 240 ms, measured in a run where the 20k-integer Set took ~115 ms; main is linear here too.
• Sibling PR deepEquals: pair Set and Map entries one-to-one on node's strict entry points #38510 does the full-equality counterpart in bindings.cpp and currently carries its own copy of the window (pairOffEntries). PendingEntries is in a header, returns Missed / Claimed / Exhausted and leaves an exhausted window at zero open entries, so that engine can drive it too (strict: a miss is final; matcher mode: fall back, then sweep what is still open); whichever lands second can drop its copy.

Background

• Partial mode (assert.partialDeepStrictEqual(actual, expected)): expected must be contained in actual. For Sets and Maps every expected member/entry needs a distinct counterpart in actual, and an object member matches with the same partial semantics, so Set([{ a: 1, b: 2 }]) contains Set([{ a: 1 }]). compareBranch in this file is that comparison; it short-circuits on SameValueZero, so a member compared with itself always matches.
• JSSet::has / JSMap::get use the collection's own hash table: SameValueZero, i.e. identity for objects and value equality for primitives (-0 is stored as +0). That settles primitives and identical objects in O(1); structurally equal but distinct objects can only be found by comparing.
• Node's window (partialObjectSetEquiv / partialObjectMapEquiv): the leftover expected objects sit in an array with an open range and a direction bit. Since a matching actual entry usually arrives at one end of the range, node tries the end that hit last and falls back to a scan; a scan hit is replaced with the range's last entry and the range shrinks, so claimed entries are never compared again. The direction bit is state: an entry that misses everything flips it back to the front, which is why a stray member between the objects can change which pairing the walk finds, and why the Set walk (node probes strays) and the Map walk (node skips non-object keys) treat strays differently. PendingEntries is that array, range and bit.
• gcBuffer is the MarkedArgumentBuffer the whole comparison shares to keep every value it has copied out of a collection alive across the user code (getters) comparisons can run. The queued entries and the settled keys are appended to it; the window itself is a plain WTF::Vector and the reservation set holds raw cell pointers, neither of which the GC sees, which is what the GC test checks.

Earlier revisions

• The first push skipped stray Set members without recording the miss and classified Map keys with isObject(), which admits functions; review caught both (new Set([{c:3,w:1}, 7, {a:1,b:2}, {b:2,y:1}]) against new Set([{a:1}, {b:2}, {c:3}]) passes in node and on main but threw there). Fixed in 406614f with the stray-member cases.
• Through d3000dd the Map path queued keys both sides hold, for exact node parity. Self-review measured what that costs (a reordered identity-keyed Map of 10k entries: milliseconds on main and 1.3.14, seconds there) and pointed out the behaviour flip it caused; a81f038 restored the identity settle described above, added the reservation, moved the window into the header and added the GC test.

---

[review] gate passed · iteration 1 · 3 files touched

fails on main (without fix)

ASAN without fix: 7 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/assert/deep-equal.test.ts
bun test v1.4.0 (235f97c1e)

test/js/node/assert/deep-equal.test.ts:
(pass) assert.deepStrictEqual > rejects 0 and -0 [63.27ms]
(pass) assert.deepStrictEqual > accepts NaN and NaN [4.63ms]
(pass) assert.deepStrictEqual > rejects [0] and [-0] [69.95ms]
(pass) assert.deepStrictEqual > rejects '1' and 1 [13.15ms]
(pass) assert.deepStrictEqual > rejects ['1'] and [1] [8.08ms]
(pass) assert.deepStrictEqual > rejects '+00000000' and false [4.57ms]
(pass) assert.deepStrictEqual > rejects '' and false [4.68ms]
(pass) assert.deepStrictEqual > rejects null and undefined [4.32ms]
(pass) assert.deepStrictEqual > rejects { a: -0 } and { a: 0 } [12.40ms]
(pass) assert.deepStrictEqual > rejects 1n and 1 [5.79ms]
(pass) assert.deepStrictEqual > rejects new String('a') and 'a' [10.05ms]
(pass) assert.deepStrictEqual > accepts two boxed equal strings [5.58ms]
(pass) assert.deepStrictEqual > accepts two boxed equal numbers [4.89ms]
(pass) assert.deepStrictEqual > rejects boxed -0 and boxed 0 [10.00ms]
(pass) assert.
... (truncated)

release without fix: 6 FAILED
bun test v1.4.0-canary.1 (b7a043103)

test/js/node/assert/deep-equal.test.ts:
(pass) assert.deepStrictEqual > rejects 0 and -0 [1.15ms]
(pass) assert.deepStrictEqual > accepts NaN and NaN [0.07ms]
(pass) assert.deepStrictEqual > rejects [0] and [-0] [1.27ms]
(pass) assert.deepStrictEqual > rejects '1' and 1 [0.25ms]
(pass) assert.deepStrictEqual > rejects ['1'] and [1] [0.10ms]
(pass) assert.deepStrictEqual > rejects '+00000000' and false [0.05ms]
(pass) assert.deepStrictEqual > rejects '' and false [0.03ms]
(pass) assert.deepStrictEqual > rejects null and undefined [0.04ms]
(pass) assert.deepStrictEqual > rejects { a: -0 } and { a: 0 } [0.18ms]
(pass) assert.deepStrictEqual > rejects 1n and 1 [0.07ms]
(pass) assert.deepStrictEqual > rejects new String('a') and 'a' [0.16ms]
(pass) assert.deepStrictEqual > accepts two boxed equal strings [0.08ms]
(pass) assert.deepStrictEqual > accepts two boxed equal numbers [0.08ms]
(pass) assert.deepStrictEqual > rejects boxed -0 and boxed 0 [0.06ms]
(pass) assert.deepStrictEqual > accepts two boxed booleans [0.05ms]
(pass) assert.deepStrictEqual > accepts two boxed symbols [0.05ms]
(pass) assert.deepStrictEqual > accepts two boxe
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/assert/deep-equal.test.ts
bun test v1.4.0 (235f97c1e)

test/js/node/assert/deep-equal.test.ts:
(pass) assert.deepStrictEqual > rejects 0 and -0 [63.55ms]
(pass) assert.deepStrictEqual > accepts NaN and NaN [4.52ms]
(pass) assert.deepStrictEqual > rejects [0] and [-0] [71.30ms]
(pass) assert.deepStrictEqual > rejects '1' and 1 [12.32ms]
(pass) assert.deepStrictEqual > rejects ['1'] and [1] [8.29ms]
(pass) assert.deepStrictEqual > rejects '+00000000' and false [4.76ms]
(pass) assert.deepStrictEqual > rejects '' and false [4.19ms]
(pass) assert.deepStrictEqual > rejects null and undefined [4.72ms]
(pass) assert.deepStrictEqual > rejects { a: -0 } and { a: 0 } [11.83ms]
(pass) assert.deepStrictEqual > rejects 1n and 1 [5.74ms]
(pass) assert.deepStrictEqual > rejects new String('a') and 'a' [10.24ms]
(pass) assert.deepStrictEqual > accepts two boxed equal strings [5.09ms]
(pass) assert.deepStrictEqual > accepts two boxed equal numbers [4.87ms]
(pass) assert.deepStrictEqual > rejects boxed -0 and boxed 0 [10.05ms]
(pass) assert.
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     235f97c1ed
  features     baseline

22 deps, 123 codegen, 1176 objects in 806ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] gen ErrorCode+*.h
[2/1238] install /workspace/bun
bun install v1.4.0-canary.1 (b7a043103)

Checked 107 installs across 153 packages (no changes) [11.00ms]
[3/1238] gen bindgenv2
[4/1238] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[5/1238] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[6/1238] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (b7a043103)

Checked 1 install across 2 packages (no changes) [1.00ms]
[7/1238] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[8/1238] fetch tinycc
[tinycc] up to date
[9/1237] fetch zlib
[zlib] up to date
[10/1237] gen ProcessBindingBuffer.lut.h
Generating /workspace/
... (truncated)

diff hotspot

src/jsc/bindings/PendingEntries.h       |  74 +++++++
 src/jsc/modules/NodeUtilTypesModule.cpp | 246 ++++++++++-----------
 test/js/node/assert/deep-equal.test.ts  | 364 ++++++++++++++++++++++++++++++++
 3 files changed, 546 insertions(+), 138 deletions(-)

gate history · 1 passed · 0 rejected · iteration 1

evidence per changed file

file                                     reads  edits  tests
src/jsc/bindings/PendingEntries.h            0      2      0
src/jsc/modules/NodeUtilTypesModule.cpp     12     26      0
test/js/node/assert/deep-equal.test.ts       6     17      0

[robobun]

^{Updated 6:05 PM PT - Aug 14th, 2026}

❌ @robobun, your commit 235f97c has 5 failures in Build #96714 (All Failures):

• vendor/elysia/test/sucrose/integration.test.ts - code 1 on 🐧 13 x64-asan
• vendor/elysia/test/response/stream.test.ts - code 1 on 🐧 13 x64-asan
• test/js/node/vm/sourcetextmodule-leak.test.ts - code 1 on 🐧 13 x64-asan
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 2019 x64 - test-bun
• Failed to create agent for 🪟 2019 x64 - test-bun
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 11 aarch64 - test-bun
• Failed to create agent for 🪟 x64 - verify-baseline
• Failed to create agent for 🪟 2019 x64 - test-bun
• test/js/web/timers/setInterval.test.js - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 38554

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

bun-38554 --bun

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 seconds

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: a9c9761e-6b1c-4ed5-9811-82f76aead995

📥 Commits

Reviewing files that changed from the base of the PR and between eabb96d and 235f97c.

📒 Files selected for processing (3)

• src/jsc/bindings/PendingEntries.h
• src/jsc/modules/NodeUtilTypesModule.cpp
• test/js/node/assert/deep-equal.test.ts

---

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

[robobun]

Status: ready for review (a81f038; 235f97c is an empty commit to re-run CI after the first build for a81f038 expired without an agent).

Reproduced on the pre-change build with the snippet from the report (partialDeepStrictEqual of two equal 100k-integer Sets: 4.8 s, 4x per doubling; Map of 10k string keys: 275 ms). With this branch the same calls take 34 ms and 11 ms. test/js/node/assert/deep-equal.test.ts fails 6 of its new tests against main (identity pairing, the two Map stray-key cases, both reversed-order getter counts, the spawned large-primitive child) and passes with the fix, on every CI lane including x64-asan.

CI on the re-run (build 96714): 165 jobs green. What is red does not involve this change: the Windows lanes could not get an agent, and one x64-asan shard has timeouts / RSS failures in setInterval.test.js, vm/sourcetextmodule-leak.test.ts and two vendored elysia files (timers, vm and elysia; none of them call into assert), reported separately. The one open design question is summarized in the comment above for whoever reviews.

[claude]

The stray-member/function-key divergence I flagged is fixed in 406614f — the reproducer and its mirror are now in the test table, and the isSpecialValue gate is applied consistently on both passes for both containers. No further issues found this pass.

Given this is a ~200-line algorithmic rewrite in JSC-binding C++ (raw JSValues held in a WTF::Vector alongside the gcBuffer roots, exception propagation through the claimWith lambda, node-compat greedy pairing whose result depends on probe order), a human look is still worthwhile before merge.

What was reviewed

• PendingExpected::claimWith probe order vs node's direction bit — front/back/scan sequence, swap-remove, and the single-entry and post-back-miss boundary cases.
• GC rooting: every queued.append is preceded by a gcBuffer.append of the same values, so the WTF::Vector<JSValue> window only ever holds already-rooted copies; the swap-remove reorders within that.
• Exception paths: RETURN_IF_EXCEPTION after each matches() in claimWith and after each claimWith/has/get at the call sites; the throwing-getter tests exercise all four comparison sites.
• The unvisited < openCount() early-exit reads size() fresh each iteration, so a getter that mutates the collection mid-comparison can't underflow unvisited.

Extended reasoning...

Overview

Rewrites setSubset and mapSubset in src/jsc/modules/NodeUtilTypesModule.cpp to mirror node's setEquiv/mapEquiv: a first pass over expected settles primitives/identity by hash lookup and queues the leftover objects into a PendingExpected window; a second pass over actual claims from that window using node's front/back/scan probe order with swap-remove. Adds a new PendingExpected<Entry> template and moves isSpecialValue up-file. Roughly 200 net lines of new C++ plus ~270 lines of new tests in test/js/node/assert/deep-equal.test.ts.

Security risks

None. This is assert.partialDeepStrictEqual, a test-utility comparison. No untrusted-input parsing, no I/O, no auth. The user-code re-entry surface (getters during compareBranch) already existed and is exercised by the new exception-propagation tests.

Level of scrutiny

High. It is a full algorithm replacement in C++ with JSC GC and exception-scope obligations, and correctness here means "byte-identical to node's greedy pairing", which is order-sensitive: my earlier review found a real regression (isObject() gate skipping stray Set members without resetting probeFront, plus function-key misclassification for Maps), now fixed and covered by the stray-member test block. The author's randomized validation (8,694 instances byte-identical to node, plus 194 stray-sensitive instances that went 112→0 divergences after the fix) is strong evidence, but the probe-order logic in claimWith is subtle enough that a maintainer familiar with node's comparisons.js should confirm the loop bounds independently.

Other factors

• All prior review threads are resolved: the bug I raised is fixed with tests; the comment-cop long-comment flags were trimmed in d3000dd.
• Test coverage is thorough: table-driven Set/Map cases, the stray-member asymmetry (primitive and function, Set vs Map), permuted ordering, self-referential members, exception propagation from all four comparison sites, getter-count bounds for same/reversed order, and a spawned child that times out on the old quadratic path.
• GC discipline looks correct (every queued JSValue is appended to gcBuffer before being copied into the plain WTF::Vector), and exception checks follow every VM-entering call, but a run under BUN_JSC_validateExceptionChecks=1 would be worth confirming given the RETURN_IF_EXCEPTION inside the claimWith lambda — the removed code had explicitly avoided the macro inside a lambda.
• Not simple/mechanical, so not auto-approvable per the guidelines even with no findings.

[robobun]

Ran the new test block and the randomized comparison script under BUN_JSC_validateExceptionChecks=1 (with BUN_JSC_dumpSimulatedThrows=1) on the debug build: no unchecked-exception reports, results unchanged (still identical to node). Also confirmed a getter throwing from inside the back probe, from inside the scan, and from the value comparison under an object key propagates as that error under validation; the only paths that end in an AssertionError instead are the ones where the early exit (fewer actual entries left than open window slots) fires before the throwing member is reached, which is node's behavior too.

[claude]

The stray-member fix in 406614f looks right — recordMiss() correctly models Node's post-miss direction = 1 state without running the doomed comparisons, and both passes now use isSpecialValue so function keys go through the lookup path. No new issues found this round.

A human should still weigh in on a81f038's deliberate divergence from Node for Map keys both sides hold by identity (the two "(node: rejected)" / "(node: accepted)" test cases): settling them by identity is what Bun already did and avoids Node's order-dependent quadratic behavior, but it's a compat call.

What was reviewed

• Verified claimWith's probe/swap-remove matches Node's partialObject{Set,Map}Equiv window, including the openCount() == 1 early return and scanFrom bounds.
• Checked GC rooting: every queued JSValue/PendingMapEntry field and every settledKeys/reserved cell is appended to gcBuffer before landing in the plain WTF::Vector; iterators and per-iteration values are stack-visible.
• Traced exception propagation through the matches lambda → claimWith's per-call RETURN_IF_EXCEPTION → caller check; the size > visited guard prevents underflow when a getter shrinks the collection mid-walk.

Extended reasoning...

Overview

Rewrites setSubset and mapSubset in src/jsc/modules/NodeUtilTypesModule.cpp to eliminate quadratic behavior in assert.partialDeepStrictEqual on Sets and Maps, adds a new PendingEntries.h header implementing Node's front/back-probe-then-scan pairing window, and adds ~370 lines of tests to test/js/node/assert/deep-equal.test.ts. The old code rescanned all actual entries per expected entry (n²); the new code settles primitive/identity content by hash lookup and pairs off the object residue with Node's greedy window walk.

Security risks

None. This is a pure comparison function reachable only via assert.partialDeepStrictEqual / util internals. No untrusted-input parsing, no I/O, no auth surface. The only user-code reentry is via getters during compareBranch, which the existing code already exercised.

Level of scrutiny

High. This is native C++ touching JSC iteration, GC rooting (JSValues copied into plain WTF::Vectors that are not GC roots — kept alive via the shared MarkedArgumentBuffer), and exception propagation through lambdas. The algorithm itself is subtle enough that my first-pass review found a real regression (the isObject() gate broke Node's probe-direction reset for stray Set members). That was fixed and covered with tests plus 8.6k randomized instances cross-checked against Node, but the class of bug — greedy pairing where an off-by-one in probe order changes the answer — is easy to reintroduce.

Other factors

• The a81f038 commit deliberately keeps Bun's existing behavior for Map keys both sides hold by identity, rather than adopting Node's queue-them-too approach. The PR documents this as two test cases that intentionally disagree with Node ("node: rejected", "node: accepted"). The rationale (Node's path is order-dependent and quadratic when reordered; Bun's identity settlement matches what Node itself does for Sets) is sound, but a maintainer should confirm the compat tradeoff is acceptable — this is exactly the kind of "Never change a Bun-native default to fix Node compatibility" boundary REVIEW.md calls out.
• Test coverage is thorough: table of pairing cases, the stray-member asymmetry (Set probes vs Map skips, primitive and function), permuted orders exercising swap-remove, self-reference, exception propagation from every comparison site, a GC-safety test that empties the expected collection mid-walk, getter-count bounds proving linearity, and a spawned 100k-int / 12k-key perf regression guard. The author ran under BUN_JSC_validateExceptionChecks=1 clean.
• All prior review threads (my bug report, comment-cop) are resolved.

[robobun]

For whoever reviews the one compat call in here (Map keys that both Maps hold by identity), the two options in short:

• As pushed: settle them by lookup first (what main and every release did, and what node does for Set members). Identity-keyed Maps stay order-independent and linear. Differs from node only on shapes where node's walk happens to hand a shared key's entry to some other key first; the test rows marked "(node: rejected)" / "(node: accepted)" are the two directions, and on 4,000 randomized shared-key instances the difference showed up 22 times, every one a Map node rejects and Bun accepts.
• Exact node parity instead: queue those keys like the others. That is the d3000dd revision: a reordered identity-keyed Map of 10k entries goes from milliseconds to seconds and from passing to throwing, as it does in node. If that is preferred it is a small change (drop the settle branch and the reservation set in mapSubset; the two rows flip).

Everything else in the PR is node's behaviour either way.

[claude]

I re-reviewed after a81f038 and didn't find any bugs — the stray-member and function-key issues from my earlier comment are fixed and covered by the new test rows. Deferring to a human for the compat call the author flagged (settle identity-held Map keys first vs. exact node parity), and because this is ~200 lines of new C++ pairing logic with GC-visible state plus a new shared header.

Extended reasoning...

Overview

Rewrites setSubset and mapSubset in src/jsc/modules/NodeUtilTypesModule.cpp from an O(n²) rescan into node's two-pass shape (hash-lookup settle, then front/back-probe window over the residue), adds a new templated header src/jsc/bindings/PendingEntries.h implementing that window, and adds ~360 lines of tests to test/js/node/assert/deep-equal.test.ts.

What was checked this run

• GC rooting: every JSValue placed in queued / settledKeys / reserved is first appended to the shared gcBuffer, so the raw WTF::Vector<JSValue> and HashSet<JSCell*> never hold the only reference; the new GC test exercises exactly this.
• Exception propagation: RETURN_IF_EXCEPTION after every has/get/compareBranch, and claimWith re-checks scope after each matches() call before touching the result; the PR reports BUN_JSC_validateExceptionChecks=1 runs clean.
• The earlier stray-member / isObject()-vs-isSpecialValue bug I flagged is fixed: setSubset now calls pending.recordMiss() for a stray special member and mapSubset gates both passes on isSpecialValue, with the four-way stray tests pinning both directions.
• The unvisited < openCount() early-exit uses a clamped subtraction (size > visited ? size - visited : 0) so a getter that shrinks actual mid-walk cannot underflow.

Security risks

None — this is a pure comparison algorithm on user-supplied JS values with no I/O, parsing, or privilege boundaries.

Level of scrutiny

High. This is native JSC-facing C++ that holds raw cell pointers across calls that can run user JS (getters), introduces a new header intended to be reused by sibling PR #38510, and deliberately diverges from Node.js on one axis (identity-held Map keys). The author explicitly asked a reviewer to make that compat call.

Other factors

• Test coverage is thorough (table cases, stray-member asymmetry, permuted order, cycles, exception propagation, getter-count bounds, GC stress, and a spawned perf regression guard), and the description reports the full assert suite plus node's upstream test-assert-partial-deep-equal.js passing.
• The one open design question (last robobun comment) is a maintainer decision, not a code defect, so it should not be resolved by auto-approval.