Skip to content

fix(utils): bump BoundedMap access order on overwrite - #1153

Open
MsfPablo wants to merge 1 commit into
Nano-Collective:mainfrom
MsfPablo:fix/bounded-map-overwrite-order-v2
Open

fix(utils): bump BoundedMap access order on overwrite#1153
MsfPablo wants to merge 1 commit into
Nano-Collective:mainfrom
MsfPablo:fix/bounded-map-overwrite-order-v2

Conversation

@MsfPablo

@MsfPablo MsfPablo commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1143.

BoundedMap.set(key, value) on an existing key updated the value but did not change the key's position in the underlying Map's iteration order. Since eviction picks the oldest key (this.map.keys().next().value), a frequently-overwritten key stayed at the oldest position and was evicted before newer entries — the opposite of the intended LRU behavior.

Change

In set(), when the key already exists, delete it before re-inserting so Map moves it to the most-recent (last-evicted) position:

if (this.map.has(key)) {
    this.map.delete(key);
}

The existing "skip eviction when overwriting" guard (!this.map.has(key)) is unchanged, so overwriting still never evicts a different entry — it now also stops evicting the overwritten key itself.

Test

Adds a regression test (overwrite bumps access order so the key is not evicted) that fails on the pre-fix code:

  • maxSize: 2; insert a, b; overwrite a; insert c.
  • Pre-fix: a is evicted (it kept the oldest position). ✘
  • Post-fix: b is evicted, a survives. ✔

All 22 bounded-map.spec.ts tests pass; the new test was verified to fail without the fix.

Checklist

  • Tests added / updated
  • Changeset added (.changeset/fix-bounded-map-overwrite-order.md)
  • test:ava, test:types, lint, and format clean on the touched files

set() on an existing key updated the value but left the key at its
original insertion position, so a frequently-overwritten key stayed
oldest and was evicted before newer entries. Delete-then-reinsert on
overwrite to move the key to most-recent, matching the intended LRU
eviction order.

Adds a regression test that fails on the pre-fix code.

Closes Nano-Collective#1143.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] BoundedMap.set doesn't update access order on overwrite

1 participant