Skip to content

Commit 6c27e1a

Browse files
authored
fix(core): exclude ranges that cover no byte (#12)
A parser produces zero-length ranges — a record covering no byte is a fact about the file, not a mistake — and one of them was enough to cost a consumer the whole view. `IntervalIndex` sorts its build on a centre test that `start === end` can never satisfy, so such a range fell to a child at every level and, once alone in a list, split into itself for ever: a stack overflow in the constructor. `IntervalColumns` did not hang but answered wrongly, reporting an empty range as overlapping any query that contained it, which `DecorationStore.between` handed to the renderer. Both indexes now leave out a range with `end <= start`. That is what half-open already means, so no answer changes, and it is what `DecorationStore.map` already did with a range an edit had emptied. A range reaching to infinity failed the same way by another route: its midpoint is not finite, and a centre of `Infinity` sits inside nothing, so every range went to one child. The centre falls back to a median start, which a range always covers, so both children come out strictly smaller whatever the bounds are.
1 parent 521aacc commit 6c27e1a

6 files changed

Lines changed: 134 additions & 14 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,23 @@ version lines would multiply into combinations nobody tests.
99

1010
## [Unreleased]
1111

12-
Nothing since 0.1.0.
12+
### Fixed
13+
14+
- **A range covering no byte no longer breaks the decoration indexes.** One zero-length
15+
range was enough to make `new IntervalIndex(...)` exhaust the stack: a range with
16+
`start === end` cannot satisfy the centre test the build sorts on, so it was handed
17+
down a level at every step and, once alone in a list, split into itself for ever. A
18+
parser produces such ranges — a zero-length record is a fact about the file — so a
19+
consumer lost the whole view over one of them. `IntervalColumns` did not hang, but it
20+
reported an empty range as overlapping any query that contained it, which
21+
`DecorationStore.between` passed on to the renderer. Both indexes now leave out a range
22+
with `end <= start`, which is what half-open already means and what
23+
`DecorationStore.map` already did with a range an edit had emptied.
24+
- **A range reaching to infinity is indexed instead of hanging.** Its midpoint is not
25+
finite, and a centre of `Infinity` pushes every range into one child — the same
26+
non-terminating split by another route. The centre falls back to a median start, which
27+
a range always covers, so the build makes progress whatever the bounds are. Finite
28+
ranges take the measured midpoint heuristic as before.
1329

1430
## [0.1.0] — 2026-08-08
1531

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ so a change that makes one of them wrong has to say what replaced it — otherwi
153153
next reader trusts a comment that is no longer true.
154154

155155
**An entry in `CHANGELOG.md`** under `[Unreleased]` if the change is one a consumer would
156-
notice. `0.1.0-next.0` is published, so there is a released version to describe changes
157-
against.
156+
notice. `0.1.0` is released, so there is a stable version to describe changes against.
158157

159158
## Releasing
160159

‎packages/core/src/interval-columns.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ export class IntervalColumns {
5252
private collect(node: number, low: number, high: number, from: number, limit: number, out: number[]): void {
5353
if (low >= limit || this.tree[node]! <= from) return;
5454
if (high - low === 1) {
55-
out.push(low);
55+
// A range covering no byte overlaps nothing, but the two bounds this walk checks
56+
// are met when the query contains it — `start` before `to`, `end` after `from` —
57+
// so it has to be excluded here rather than left to them.
58+
if (this.ends[low]! > this.starts[low]!) out.push(low);
5659
return;
5760
}
5861
const middle = (low + high) >> 1;

‎packages/core/src/interval-index.ts‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@ interface Node<T extends Interval> {
3232
*
3333
* Rebuilt whole rather than mutated: decorations arrive in bulk and are queried
3434
* once per row per frame, so the cost belongs on the write.
35+
*
36+
* Ranges that cover no byte are not indexed. A parser produces them — a zero-length
37+
* record is a fact about the file, not a mistake — and they would otherwise be carried
38+
* through a tree that can never answer with them.
3539
*/
3640
export class IntervalIndex<T extends Interval> {
3741
private readonly root: Node<T> | undefined;
3842

3943
constructor(items: readonly T[]) {
40-
this.root = build([...items]);
44+
this.root = build(items.filter(coversAByte));
4145
}
4246

4347
/** Ranges overlapping `[from, to)`, in no particular order. */
@@ -53,12 +57,22 @@ export class IntervalIndex<T extends Interval> {
5357
}
5458
}
5559

60+
/**
61+
* Whether a range covers a byte at all. `end > start` is the whole test: it excludes
62+
* the empty range, the inverted one, and a bound that is not a number, none of which a
63+
* half-open query can ever overlap — so leaving them out changes no answer, and
64+
* `IntervalColumns` and `DecorationStore.map` already treat them the same way.
65+
*
66+
* It is also what makes `build` terminate. `here` asks for `start <= centre < end`,
67+
* which an empty range cannot satisfy at any centre: it goes to a child every time, and
68+
* alone in a list it splits into itself for ever. One such range used to cost the whole
69+
* index a stack overflow at construction.
70+
*/
71+
const coversAByte = <T extends Interval>(item: T): boolean => item.end > item.start;
72+
5673
function build<T extends Interval>(items: T[]): Node<T> | undefined {
5774
if (items.length === 0) return undefined;
58-
// The median of the midpoints, so a run of ranges in one place does not tip
59-
// every level to one side.
60-
const midpoints = items.map((item) => (item.start + item.end) / 2).sort((left, right) => left - right);
61-
const center = midpoints[midpoints.length >> 1]!;
75+
const center = centerOf(items);
6276
const left: T[] = [];
6377
const right: T[] = [];
6478
const here: T[] = [];
@@ -80,6 +94,26 @@ function build<T extends Interval>(items: T[]): Node<T> | undefined {
8094
};
8195
}
8296

97+
/**
98+
* The median of the midpoints, so a run of ranges in one place does not tip every level
99+
* to one side.
100+
*
101+
* A range reaching to infinity has no midpoint to offer, and a centre that is not finite
102+
* sits inside nothing: every range would be handed to one child and the build would
103+
* recurse on the same list for ever. The median start is the fallback, because a range
104+
* covers its own start — so whichever range the median picks stays here, and both
105+
* children come out strictly smaller. Finite ranges never reach it.
106+
*/
107+
function centerOf<T extends Interval>(items: T[]): number {
108+
const midpoints = items.map((item) => (item.start + item.end) / 2).sort(ascending);
109+
const center = midpoints[midpoints.length >> 1]!;
110+
if (Number.isFinite(center)) return center;
111+
const starts = items.map((item) => item.start).sort(ascending);
112+
return starts[starts.length >> 1]!;
113+
}
114+
115+
const ascending = (left: number, right: number): number => left - right;
116+
83117
const sortByStart = <T extends Interval>(items: T[]): T[] => [...items].sort((left, right) => left.start - right.start);
84118
const sortByEnd = <T extends Interval>(items: T[]): T[] => [...items].sort((left, right) => right.end - left.end);
85119

‎packages/core/test/decorations.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ describe("DecorationStore", () => {
4848
expect(labels(nested().between(20, 32))).toEqual(["record"]);
4949
});
5050

51+
it("does not report a range that covers no byte", () => {
52+
// A parse result carries them — a zero-length record is a fact about the file — and a
53+
// row query contains one rather than touching its bounds, which is the case that used
54+
// to hand it to the renderer as if it overlapped.
55+
const store = new DecorationStore();
56+
store.addAll([{ start: 0, end: 48, label: "record" }, { start: 16, end: 16, label: "empty" }]);
57+
expect(labels(store.between(0, 32))).toEqual(["record"]);
58+
expect(labels(store.between(16, 17))).toEqual(["record"]);
59+
expect(labels(store.allAt(16))).toEqual(["record"]);
60+
expect(store.at(16, "structure")).toBeUndefined();
61+
// Still held and still reported as a range: the store does not decide it is a mistake.
62+
expect(store.size).toBe(2);
63+
expect(labels(store.all)).toEqual(["record", "empty"]);
64+
});
65+
5166
it("filters by kind", () => {
5267
const store = new DecorationStore();
5368
store.addAll([{ start: 0, end: 1, kind: "bookmark" }, { start: 0, end: 8, kind: "structure" }]);

‎packages/core/test/interval-index.test.ts‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@ import { IntervalIndex, type Interval } from "../src/interval-index";
55
const range = (start: number, end: number): Interval => ({ start, end });
66
const starts = (found: Interval[]) => found.map((item) => item.start).sort((left, right) => left - right);
77

8-
/** The answer a full pass would give, which the index has to match exactly. */
8+
/**
9+
* The answer a full pass would give, which the index has to match exactly. The pair of
10+
* comparisons is the usual overlap test but it only holds for a range that covers a
11+
* byte: an empty range sitting inside the query passes both and intersects nothing.
12+
*/
913
const byScan = (items: Interval[], from: number, to: number) =>
10-
starts(items.filter((item) => item.start < to && item.end > from));
14+
starts(items.filter((item) => item.end > item.start && item.start < to && item.end > from));
1115

1216
describe("IntervalIndex", () => {
1317
it("finds what overlaps a range and nothing else", () => {
@@ -40,13 +44,53 @@ describe("IntervalIndex", () => {
4044
expect(starts(index.covering(95))).toEqual([0]);
4145
});
4246

47+
it("leaves out a range that covers no byte", () => {
48+
// A zero-length range cannot satisfy `start <= centre < end` at any centre, so the
49+
// build used to hand it to a child at every level and, alone in a list, split it
50+
// into itself until the stack ran out. One of them cost a consumer the whole index.
51+
const empty = range(10, 10);
52+
expect(new IntervalIndex([empty]).covering(10)).toEqual([]);
53+
expect(new IntervalIndex([empty]).overlapping(0, 100)).toEqual([]);
54+
55+
// Wherever it sits among ranges that do cover bytes, and however many there are.
56+
const among: Interval[] = [range(0, 100), empty, range(20, 24), range(96, 400)];
57+
expect(starts(new IntervalIndex(among).overlapping(0, 500))).toEqual([0, 20, 96]);
58+
expect(starts(new IntervalIndex(among).covering(10))).toEqual([0]);
59+
60+
// The shape that found this: a 0-byte record's payload inside a parsed file.
61+
const parsed = [range(0, 5309808), range(5309808, 5309816), range(5309808, 5309808)];
62+
expect(starts(new IntervalIndex(parsed).covering(5309808))).toEqual([5309808]);
63+
});
64+
65+
it("leaves out a range whose bounds are not ordered numbers", () => {
66+
// Inverted and not-a-number fail `end > start` the same way an empty range does, and
67+
// a full pass drops them too — `end > from` is false for NaN.
68+
expect(new IntervalIndex([range(20, 10)]).overlapping(0, 100)).toEqual([]);
69+
expect(new IntervalIndex([range(0, Number.NaN)]).overlapping(0, 100)).toEqual([]);
70+
expect(new IntervalIndex([range(Number.NaN, 10)]).overlapping(0, 100)).toEqual([]);
71+
});
72+
73+
it("indexes a range that reaches to infinity rather than dropping it", () => {
74+
// It covers bytes, so it belongs in the answer. Its midpoint is not finite, though,
75+
// and a centre of `Infinity` puts every range in one child — which is the same
76+
// non-terminating split by another route, so the centre falls back to a median start.
77+
const items = [range(0, Number.POSITIVE_INFINITY), range(10, 20)];
78+
const index = new IntervalIndex(items);
79+
expect(starts(index.covering(5))).toEqual([0]);
80+
expect(starts(index.covering(15))).toEqual([0, 10]);
81+
expect(starts(index.covering(1e12))).toEqual([0]);
82+
});
83+
4384
it("agrees with a full pass over awkward data", () => {
4485
// A document-spanning outer range plus clusters, which is what a parsed
45-
// structure looks like and what the naive index gets wrong.
86+
// structure looks like and what the naive index gets wrong. Every cluster carries a
87+
// zero-length range as well, because a parse result carries them and the fuzz that
88+
// missed the hang above could not produce one.
4689
const items: Interval[] = [range(0, 4096)];
4790
for (let cluster = 0; cluster < 40; cluster++) {
4891
const base = cluster * 100;
4992
items.push(range(base, base + 64), range(base + 8, base + 16), range(base + 8, base + 9));
93+
items.push(range(base + 32, base + 32), range(base + 64, base + 64));
5094
}
5195
const index = new IntervalIndex(items);
5296
for (let from = 0; from < 4200; from += 7) {
@@ -83,13 +127,13 @@ describe("IntervalColumns", () => {
83127
for (let cluster = 0; cluster < 40; cluster++) {
84128
const base = cluster * 100;
85129
items.push(range(base, base + 64), range(base + 8, base + 16), range(base + 8, base + 9));
130+
items.push(range(base + 32, base + 32), range(base + 64, base + 64));
86131
}
87132
for (let sibling = 0; sibling < 200; sibling++) items.push(range(5000 + sibling * 16, 5000 + sibling * 16 + 12));
88133

89134
for (let from = 0; from < 8400; from += 13) {
90135
const to = from + (from % 3 === 0 ? 1 : 16);
91-
const expected = starts(items.filter((item) => item.start < to && item.end > from));
92-
expect(found(items, from, to), `overlapping(${from}, ${to})`).toEqual(expected);
136+
expect(found(items, from, to), `overlapping(${from}, ${to})`).toEqual(byScan(items, from, to));
93137
}
94138
});
95139

@@ -107,6 +151,15 @@ describe("IntervalColumns", () => {
107151
expect(found(items, 0, 1000)).toEqual([0, 400]);
108152
});
109153

154+
it("answers without a range that covers no byte, as the tree does", () => {
155+
// The store's index reaches this case by construction, and both indexes have to give
156+
// the same answer for the same ranges or a host cannot swap one for the other.
157+
const items = [range(0, 200), range(100, 100), range(150, 160)];
158+
expect(found(items, 100, 101)).toEqual([0]);
159+
expect(found(items, 99, 102)).toEqual([0]);
160+
expect(found(items, 0, 200)).toEqual([0, 150]);
161+
});
162+
110163
it("reuses the array it is handed", () => {
111164
const { index } = columns([range(0, 4), range(8, 12)]);
112165
const out: number[] = [];

0 commit comments

Comments
 (0)