Skip to content

fix(algorithms): quickSort stack overflow on sorted input; introsort rebuild - #1

Merged
SkinnnyJay merged 1 commit into
SkinnnyJay:mainfrom
frankstupak:lumen-uplift/algorithms
Aug 22, 2026
Merged

fix(algorithms): quickSort stack overflow on sorted input; introsort rebuild#1
SkinnnyJay merged 1 commit into
SkinnnyJay:mainfrom
frankstupak:lumen-uplift/algorithms

Conversation

@frankstupak

Copy link
Copy Markdown
Contributor

The headline

SortingAlgorithms.quickSort throws RangeError: Maximum call stack size exceeded on sorted, reverse-sorted, and all-equal input at ~10k elements — the three most common shapes real data takes. Below the crash threshold it's quadratic: at n=5,000, sorted input runs 6× slower than random. Last-element Lomuto pivot + unbounded recursion; textbook killer input.

Rebuilt as introsort (Musser 1997 — the scheme behind libstdc++ std::sort and .NET Array.Sort): median-of-three pivot, 3-way (Dutch national flag) partition for equal keys, insertion sort under 24 elements, one-pass sorted-run detection, and a heapsort fallback when depth exceeds 2·log₂(n). O(n log n) worst case, O(log n) stack, guaranteed. Public API unchanged.

quickSort, n=100,000 (median of 7 runs, Node 22)

input before after
random 15.2 ms 17.3 ms
sorted RangeError crash 0.8 ms
reversed RangeError crash 29.5 ms
all-equal RangeError crash 0.7 ms

The +2ms on random is the 3-way partition's price for equal-key immunity — Lomuto degrades to O(n²) the moment keys repeat.

mergeSort

Was allocating two fresh slices per merge (~n·log n allocation churn). Now: one aux buffer allocated up front, copy-left-half-only merges, and an ordered-halves skip (a[mid] <= a[mid+1]) that makes sorted input near-O(n): 22.1 ms → 2.4 ms at n=100k, random 38 ms → 28 ms. Stability preserved and now locked by a test.

Ranking correctness (each one shipped broken)

  1. Undated documents ranked as newest. customScore did doc.createdAt || new Date() — a document with missing data got the maximum recency boost, outranking every genuinely dated document. Now 0.
  2. Non-English text was unsearchable. The tokenizer's [^\w\s] is ASCII-only: "café" → "caf", CJK/Cyrillic/Arabic deleted entirely — a Japanese document could never match its own title. Now Unicode \p{L}\p{N}. Query 東京: 0 hits → 1.
  3. phraseScore missed real phrases and matched fake ones. Raw text.includes(query) fails on "hello, world" for query "hello world", and substring-matches inside larger tokens. Now token-normalized and boundary-safe.
  4. BM25 IDF clamp. Math.max(0.001, log(...)) flattens every common term (df > N/2) to one arbitrary constant. Replaced with Lucene's non-negative IDF log(1 + (N−df+0.5)/(df+0.5)) — same fix Lucene shipped to avoid negative scores, rank order preserved.
  5. Math.max(...spread) in customScore overflows the call stack on large result sets. Loop.
  6. Cosine similarity built dense |vocab|-length vectors per document — O(docs × vocab) where the dot product only has non-zeros on shared terms. Sparse maps now: 2,754 ms → 297 ms (9.3×) on 2k docs / 20k-word vocabulary. Identical math, identical scores.

New: BM25Index

Every scorer re-tokenizes the whole corpus per query. Real engines (Lucene/Elasticsearch) tokenize at index time into an inverted index. BM25Index brings that shape here — build once, query many, score-parity with calculateBM25 verified to 10 decimal places in tests.

2,000 docs: 20 queries 1,644 ms → 78 ms build + 57 ms queries (~12× incl. build, ~29× steady-state; ~1.2 ms/query vs ~82 ms).

Verification

  • +22 tests in algorithm-uplift.test.ts (pathological-input survival, fuzz vs Array.sort, mergeSort stability, every ranking fix, BM25Index parity) — 205 pass in src/algorithms, 0 fail
  • tsc --noEmit clean, eslint src/algorithms clean
  • Root npm run test:all: the only failing suites (nextjs-backend, api-scenarios, autocomplete) fail identically on pristine main — pre-existing, untouched by this PR
  • Zero public API breaks; all changes are internal or additive

Refs: Musser, Introspective Sorting and Selection Algorithms (1997) · orlp/pdqsort · Lucene BM25Similarity IDF.

— Lumen Industries

…king correctness + BM25Index

quickSort was plain Lomuto with a last-element pivot: RangeError (stack
overflow) on sorted/reversed/all-equal input at ~10k elements, quadratic
below that. Rebuilt as introsort (Musser 1997): median-of-three pivot,
3-way partition, insertion-sort small ranges, sorted-run detection, and a
heapsort fallback at depth 2*log2(n). O(n log n) worst case, O(log n) stack.

mergeSort: one reusable aux buffer (was two fresh slices per merge) and an
ordered-halves skip; stability preserved and now under test.

Ranking fixes:
- customScore: documents without createdAt got MAX recency boost via
  '|| new Date()'; now 0.
- Tokenizer was ASCII-only [^\w\s]: mangled accented terms, deleted
  CJK/Cyrillic entirely. Now Unicode \p{L}\p{N}.
- phraseScore: raw substring match missed phrases across punctuation and
  matched inside larger tokens. Now token-normalized, boundary-safe.
- BM25 IDF: Math.max(0.001, log(...)) clamp replaced with Lucene-style
  non-negative log(1 + (N-df+0.5)/(df+0.5)).
- customScore Math.max(...spread) stack overflow on large result sets.
- Cosine similarity: dense O(docs x vocab) vectors replaced with sparse
  maps (9.3x at 20k vocab).

New: BM25Index — inverted-index BM25 (build once, query many), score-parity
with calculateBM25 under test. +22 tests, all green; tsc + eslint clean.
@frankstupak

Copy link
Copy Markdown
Contributor Author

hey, saw you worked through the simpill-utils batch, thanks for merging those. whenever you get a minute this repo has 13 of these waiting too, same deal: one topic per PR, tests green when filed, all still merge clean against main. no rush, just flagging them so they do not rot.

@frankstupak frankstupak changed the title algorithms: your quickSort crashes on sorted input — introsort rebuild + ranking correctness + BM25Index fix(algorithms): quickSort stack overflow on sorted input; introsort rebuild Aug 13, 2026
@SkinnnyJay SkinnnyJay closed this Aug 21, 2026
@SkinnnyJay SkinnnyJay reopened this Aug 21, 2026
@SkinnnyJay SkinnnyJay closed this Aug 22, 2026
@SkinnnyJay SkinnnyJay reopened this Aug 22, 2026
@SkinnnyJay
SkinnnyJay merged commit c39ad0e into SkinnnyJay:main Aug 22, 2026
4 of 5 checks passed
@SkinnnyJay

Copy link
Copy Markdown
Owner

Late answer to this — you flagged the batch on Aug 9 and I never replied here, which is exactly the rot you were trying to prevent. Status on all 13:

10 merged (2026-08-22): #1, #2, #4, #5, #6, #7, #8, #9, #11, #12.

3 still open, all MERGEABLE / CLEAN with test (20) and test (22) green:

Plus #16 from @RealLumenHere, which came in after your note and is stuck behind the first-time-contributor workflow gate rather than anything in the code.

Two things worth saying plainly, since you took the trouble to flag this rather than let it sit:

The delay wasn't the queue's fault. Two separate things were masking real CI verdicts — #14's Node 18 websocket teardown failure was reddening every PR in the batch regardless of what it touched, and stale refs/pull/N/merge meant re-runs were executing the old matrix. Neither was visible from your side; from where you sat it just looked like nobody was reading.

And the convention held up. One topic per PR, independently mergeable, tests green on filing — that's why 10 of them went in on a single pass once the CI signal was trustworthy. Keep filing them that way.

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