mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-09-21 11:13:20 +08:00
Lee Jaeha / Adam
b4331cbe1e
## Description `PriorityQueue.bubbleDown` compares `swap` against `null`, but `swap` is initialized to `-1` (`priority-queue.ts:19`), and the loop's own exit check uses `swap === -1` (`:43`). So `swap === null` is always false and `swap !== null` always true: the right child is only ever compared with the left child, never with the element being sunk. A node can sink below a child larger than itself, breaking the heap invariant. Randomized check on the current code: 394 of 20,000 random heaps dequeue out of order. Smallest reproducer is 7 elements — `[1, 2, 3, 7, 6, 5, 4]` dequeues as `[1, 2, 3, 5, 4, 6, 7]`. The queue backs `AStarRunner` (`a-star.ts:4`, its only consumer), which routes orthogonal connectors via `ConnectorPathGenerator.updatePath` — edgeless connector re-routing, mindmap rendering, and auto-complete. The A* heuristic is admissible, so a path is still produced; the effect is that nodes are expanded in the wrong order, so the shortest / fewest-bends guarantee is lost rather than the connector breaking. ### Why this needed a config change too The package's `vitest.config.ts` was dropped in the #10702 package reorg and never recreated at `blocksuite/affine/blocks/surface/`, so `vitest` collects none of its six test files: ``` $ yarn vitest run blocksuite/affine/blocks/surface/src/__tests__/priority-queue.unit.spec.ts No test files found, exiting with code 1 ``` They have not run since 2025-03-08, which is why this survived. Restoring the config re-enables all six files (40 tests, all passing) so the regression test added here actually guards the fix. Verified by reverting the one-line fix with the config in place: ``` FAIL should not sink a node past children that are all larger than it expected [1,2,3,5,4,6,7] to deeply equal [1,2,3,4,5,6,7] FAIL should dequeue in ascending order regardless of insertion order Tests 2 failed | 38 passed (40) ``` ## Checklist - [x] I have signed the [AFFiNE Contributor License Agreement](https://cla-assistant.io/toeverything/AFFiNE) — required before merge; the `license/cla` check must be green ([how it works](https://github.com/toeverything/AFFiNE/blob/canary/docs/BUILDING.md#sign-the-cla-first)) - [x] The PR targets the `canary` branch and its title follows [Conventional Commits](https://www.conventionalcommits.org/) - [x] Tests are added or updated where it makes sense - [x] `yarn lint` and `yarn typecheck` pass locally - [x] If the PR code includes AI-generated edits, I have carefully reviewed it to ensure that the scope of the PR is consistent with what is claimed in the title and description, and that there are no redundant or invalid implementations. - [x] I agree that maintainers may close the PR without discussion if they consider the code quality to be too low. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Corrected priority queue ordering behavior to ensure items are dequeued in ascending order across insertion sequences. - **Tests** - Added coverage for priority queue behavior when comparing nodes with two larger child values. - Added automated test configuration and coverage reporting for the surface components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->