Skip to content

Index-tracked open-set heap: O(log n) decrease-key + sift-down fix#7

Merged
GenerelSchwerz merged 1 commit into
Minecraft-Pathfinding:2026-rewritefrom
XaXayo12:improve-open-set-heap
Jun 18, 2026
Merged

Index-tracked open-set heap: O(log n) decrease-key + sift-down fix#7
GenerelSchwerz merged 1 commit into
Minecraft-Pathfinding:2026-rewritefrom
XaXayo12:improve-open-set-heap

Conversation

@XaXayo12

Copy link
Copy Markdown

Open-set heap: fix sift-down + O(log n) decrease-key

A* uses a binary min-heap (src/abstract/heap.ts) as its open set. Two issues:

1. Correctness bug in pop() — the sift-down's right-child guard was smallerChild < size - 1, off by one: when the smaller child is the last slot it gets skipped, so pop() can leave the minimum off the root. A* then pops nodes out of f order — exploring extra nodes and able to return a worse path.

2. update() was O(n) — decrease-key (called whenever A* finds a cheaper route to an already-open node) did heap.indexOf(val), a linear scan over the whole open set → ~O(n²) across a search.

Fix

Every node now stores its slot in heapIdx, maintained on each sift, so update() is O(log n) with no scan. Sift-up/down are rewritten with the correct child bound; pop() uses Array.pop() instead of splice.

Tests

tests/heap.test.ts:

  • pops in ascending f order,
  • the boundary regression (the old sift-down returned [1, 8, …] instead of [1, 2, …]),
  • update() decrease-key,
  • heapIdx lifecycle (set on push, -1 after pop),
  • a 200-node randomized push + decrease-key stress test.

All existing pathfinder tests still pass, so real A* output is unchanged — just correct and faster. tsc build clean, ts-standard reports 0 problems.

…x sift-down

The A* open set is a binary min-heap. It had two issues:

1. Correctness: pop()'s sift-down used `smallerChild < size - 1`, which skips
   the right child when it is the final slot. pop() could therefore leave the
   minimum off the root, so A* popped nodes out of f-order -- exploring extra
   nodes and able to return a worse path.

2. Performance: update() (decrease-key, called whenever A* finds a cheaper
   route to an already-open node) did `heap.indexOf(val)` -- an O(n) scan over
   the whole open set, i.e. ~O(n^2) across a search.

Fix: each node stores its slot in `heapIdx`, maintained on every sift, so
update() is O(log n) with no scan. Sift-up/down are rewritten with the correct
child bound; pop() uses Array.pop() instead of splice.

Tests: tests/heap.test.ts (ascending order, the boundary regression,
decrease-key, heapIdx lifecycle, 200-node randomized stress). All existing
pathfinder tests still pass, so real A* output is unchanged -- just correct
and faster.
@GenerelSchwerz GenerelSchwerz merged commit 9fabef7 into Minecraft-Pathfinding:2026-rewrite Jun 18, 2026
1 check passed
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.

2 participants