From 77af832e3202d05a07fa54bf0d459e4e33c40ec8 Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Fri, 25 Sep 2026 13:25:18 -0400 Subject: [PATCH 1/2] fix: recompute the new subtree root max in rotateRight() After a right rotation the node that moved up ($x) must have its max recomputed, but the code recomputed $y a second time instead. The stale max on the new subtree root makes findIntersections() prune subtrees that do contain matches. Insert-only workloads mostly mask this because insert() re-walks the path to the root afterwards; the delete fix-up does not. --- src/IntervalTree.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IntervalTree.php b/src/IntervalTree.php index 08c2704..3972aa0 100644 --- a/src/IntervalTree.php +++ b/src/IntervalTree.php @@ -473,7 +473,7 @@ private function rotateRight(Node $y): void $x = $y->getParent(); if ($x !== null && $x !== $this->nilNode) { - $y->updateMax(); + $x->updateMax(); } } From d68ae8faedd9b20775d1c4263cd6e915830102ea Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Fri, 25 Sep 2026 13:25:18 -0400 Subject: [PATCH 2/2] fix: make deleteFixup() match CLRS RB-DELETE-FIXUP Two deviations from the textbook algorithm corrupted the tree on removal: * Case 2 must only apply when *both* of the sibling's children are black; the code tested only the near child. * After the case-3 rotation the sibling must be re-read from the parent (w = x.p.right / x.p.left); the code kept using the old sibling. Either one breaks the black-height or red-red invariant, after which later removals dereference the nil sentinel's (null) children and throw a TypeError, and queries return wrong results. Add a randomized test that checks findIntersections() and getSize() against a brute-force list across a mix of inserts and removals. On the previous code 45 of the 50 seeds fail. --- src/IntervalTree.php | 6 ++- tests/IntervalTreeRandomizedTest.php | 77 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 tests/IntervalTreeRandomizedTest.php diff --git a/src/IntervalTree.php b/src/IntervalTree.php index 3972aa0..d2aafce 100644 --- a/src/IntervalTree.php +++ b/src/IntervalTree.php @@ -290,7 +290,7 @@ private function deleteFixup(Node $fixNode): void $brotherNode = $currentNode->getParent()->getRight(); } - if ($brotherNode->getLeft()->getColor()->isBlack()) { + if ($brotherNode->getLeft()->getColor()->isBlack() && $brotherNode->getRight()->getColor()->isBlack()) { $brotherNode->setColor(NodeColor::red()); $currentNode = $currentNode->getParent(); } else { @@ -298,6 +298,7 @@ private function deleteFixup(Node $fixNode): void $brotherNode->setColor(NodeColor::red()); $brotherNode->getLeft()->setColor(NodeColor::black()); $this->rotateRight($brotherNode); + $brotherNode = $currentNode->getParent()->getRight(); } $brotherNode->setColor($currentNode->getParent()->getColor()); $currentNode->getParent()->setColor(NodeColor::black()); @@ -313,7 +314,7 @@ private function deleteFixup(Node $fixNode): void $this->rotateRight($currentNode->getParent()); $brotherNode = $currentNode->getParent()->getLeft(); } - if ($brotherNode->getRight()->getColor()->isBlack()) { + if ($brotherNode->getRight()->getColor()->isBlack() && $brotherNode->getLeft()->getColor()->isBlack()) { $brotherNode->setColor(NodeColor::red()); $currentNode = $currentNode->getParent(); } else { @@ -321,6 +322,7 @@ private function deleteFixup(Node $fixNode): void $brotherNode->setColor(NodeColor::red()); $brotherNode->getRight()->setColor(NodeColor::black()); $this->rotateLeft($brotherNode); + $brotherNode = $currentNode->getParent()->getLeft(); } $brotherNode->setColor($currentNode->getParent()->getColor()); $currentNode->getParent()->setColor(NodeColor::black()); diff --git a/tests/IntervalTreeRandomizedTest.php b/tests/IntervalTreeRandomizedTest.php new file mode 100644 index 0000000..e573ca3 --- /dev/null +++ b/tests/IntervalTreeRandomizedTest.php @@ -0,0 +1,77 @@ + + */ + public function provideSeeds(): array + { + $seeds = []; + for ($seed = 1; $seed <= 50; $seed++) { + $seeds["seed $seed"] = [$seed]; + } + return $seeds; + } + + /** + * @dataProvider provideSeeds + */ + public function testRandomInsertRemoveMatchesBruteForce(int $seed): void + { + mt_srand($seed); + /** @var IntervalTree $tree */ + $tree = new IntervalTree(); + /** @var array $live */ + $live = []; + for ($op = 1; $op <= 300; $op++) { + if (count($live) < 2 || mt_rand(0, 2) > 0) { + $low = mt_rand(0, 1000) * 1000 + $op; + $high = $low + mt_rand(0, 30000); + $tree->insert(new NumericInterval($low, $high), $op); + $live[$op] = [$low, $high]; + } else { + $value = array_rand($live); + [$low, $high] = $live[$value]; + self::assertTrue($tree->remove(new NumericInterval($low, $high), $value)); + unset($live[$value]); + } + + $queryLow = mt_rand(0, 1030000); + $queryHigh = $queryLow + mt_rand(0, 20000); + $expected = []; + foreach ($live as $value => [$low, $high]) { + if (!($high < $queryLow || $queryHigh < $low)) { + $expected[] = $value; + } + } + $actual = []; + foreach ($tree->findIntersections(new NumericInterval($queryLow, $queryHigh)) as $pair) { + $actual[] = $pair->getValue(); + } + sort($expected); + sort($actual); + self::assertSame($expected, $actual, "seed $seed, operation $op"); + self::assertSame(count($live), $tree->getSize()); + } + } +}