isolated install: name the entry-hash DFS indices after the table they index

[robobun]

Problem

• mordant index_of_other_kind in src/install/isolated_install.rs: "states is indexed by dep_idx here. Elsewhere in this function it is indexed by _root_id (1 place), and dep_id (the same kind as dep_idx) indexes dependencies. This looks like the wrong table" (install_isolated_packages, the match states[dep_idx] in the entry-hash DFS).
• states[dep_idx] is correct. states has one slot per store entry, and that dep_idx is dep.entry_id.get(), the store entry index of the dependency being visited. The lockfile DependencyID (dep.dep_id) is the one used to index dependencies on the line above.
• The names are what is wrong. Within a few lines dep_idx meant two different things (StackFrame.dep_idx is the cursor into the entry's dependency list; the local is a store entry index), and earlier in the same file (build_store's workspace loop) dep_idx is a DependencyID. The root loop variable _root_id is a store entry index too, named as if unused.

Fix

• Rename the three store entry indices in the DFS to entry_idx, root_entry_idx and dep_entry_idx, and the per-frame cursor StackFrame.dep_idx to next_dep. Every index into states / entry_hashes / entry_node_ids / entry_dependencies now says entry; nothing index-shaped is called dep_* unless it is a DependencyID. No behavior change: renames only.
• Not adding an index newtype for the tables: the ids are already newtyped (store::entry::Id, a NewId<Entry> distinct from NewId<Node>), and the per-entry tables are MultiArrayList column slices indexed with id.get() as usize at roughly 120 sites across this crate. Wrapping this one local scratch table would not stop a crossing on any of its siblings; making the columns typed is a crate-wide change, not this one.
• Drops index_of_other_kind:src/install/isolated_install.rs from mordant-baseline.toml.
• No test added: renames cannot change what any test observes, so nothing under test/ can fail before and pass after. The baseline line is the before/after check instead: with it removed, the mordant CI job fails on the old names and passes on the new ones (shown below).
• Verified:
  • cargo dylint --all -p bun_install --no-deps (the rust:mordant command scoped to this crate) with the baseline line removed: reports the finding at states[dep_idx] without the rename, nothing with it (output below).
  • MORDANT_BASELINE_WRITE=1 on this crate regenerates the [bun_install] section without this line. It also drops always_unwrapped_option:src/install/PackageInstall.rs, which is already stale on main (it disappears with or without this change), so that line is left for a later regenerate rather than folded in here.
  • cargo check -p bun_install, cargo fmt -p bun_install -- --check.

Background

• The isolated linker builds a store: one entry per (package, peer set) that gets its own directory under node_modules/.bun, with per-entry columns (entry_hashes, entry_node_ids, entry_dependencies) indexed by store::entry::Id. Each entry's dependency list holds DependenciesItem { entry_id, dep_id }: the store entry the symlink points at, plus the lockfile DependencyID that names the alias.
• With the global virtual store enabled, install_isolated_packages runs an iterative DFS over entries to compute entry_hashes, a hash of each entry's resolved dependency closure used to share store directories across projects. states is that DFS's per-entry visit state; StackFrame is one entry being visited, and its cursor says how far through the entry's dependency list it is.
• mordant's index_of_other_kind reads index kinds off names only (the prefix before _idx / _id). It flagged states because it was indexed under two unrelated kinds (_root_id, dep_idx) while dep_id indexed a table of its own kind; after the rename every index into states carries the entry kind.

mordant output with the baseline line removed

Without the rename:

warning: `states` is indexed by `dep_idx` here. Elsewhere in this function it is indexed by `_root_id` (1 place), and `dep_id` (the same kind as `dep_idx`) indexes `dependencies`. This looks like the wrong table
    --> src/install/isolated_install.rs:1337:31
     |
1337 |                         match states[dep_idx] {
     |                               ^^^^^^^^^^^^^^^
     |
note: `states` indexed the usual way
    --> src/install/isolated_install.rs:1200:20
     |
1200 |                 if states[_root_id] != State::Unvisited {
     |                    ^^^^^^^^^^^^^^^^
     = note: `index_of_other_kind` over the mordant baseline (0 recorded for src/install/isolated_install.rs)

warning: mordant: 1 finding(s) over the baseline in bun_install

With the rename: no warnings, target/mordant/over-baseline.txt not written.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 29 minutes

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b749e791-8e8e-46a1-b5e1-d3f6c14f1c90

📥 Commits

Reviewing files that changed from the base of the PR and between 88a6398 and 705836d.

📒 Files selected for processing (2)

• mordant-baseline.toml
• src/install/isolated_install.rs

---

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

[claude]

LGTM — pure variable renames, no behavior change.

What was reviewed:

• Verified each rename maps 1:1 (_root_id→root_entry_idx, idx→entry_idx, local dep_idx→dep_entry_idx, StackFrame.dep_idx→next_dep) with no logic drift.
• Confirmed StackFrame is only referenced inside this DFS, so the field rename has no other consumers.
• The remaining dep_idx at isolated_install.rs:367-380 is a genuine DependencyID in build_store, correctly left alone.
• Baseline removal matches the lint being resolved.

Extended reasoning...

Overview

This PR renames four local variables / one struct field in the entry-hash DFS of install_isolated_packages (src/install/isolated_install.rs) so that every index into the per-store-entry tables (states, entry_hashes, entry_node_ids, entry_dependencies) is named with an entry prefix, and the per-frame dependency-list cursor is renamed from dep_idx to next_dep. It also drops the corresponding index_of_other_kind suppression from mordant-baseline.toml. No control flow, expressions, or types change — I walked each hunk and every renamed identifier substitutes into the identical position with the identical value.

Security risks

None. This is a rename-only change in the isolated-install linker's hash-computation DFS; no inputs, boundaries, or error paths are touched.

Level of scrutiny

Low. The diff is mechanical and self-contained. StackFrame is a private struct defined and consumed only within this file (grep confirms three reference sites, all inside the diff), so the field rename cannot break anything elsewhere. The other dep_idx occurrences in the file (lines ~367-380) are in build_store where the variable is explicitly a DependencyID — leaving those unchanged is consistent with the PR's stated rule that dep_* names are reserved for DependencyIDs.

Other factors

The PR description shows the mordant lint firing before the rename and clean after, and cargo check / cargo fmt --check passing. No new tests are needed since there is no behavioral change to observe — the existing isolated-install suite covers this code path. The one added doc comment on next_dep is accurate and useful (disambiguates from DependencyID). The deliberate decision to leave the stale always_unwrapped_option baseline line for a separate regenerate is reasonable scope hygiene.

[robobun]

^{Updated 11:05 AM PT - Aug 15th, 2026}

@robobun, your commit 705836d is building: #98345