Skip to content

fix(vocab): stop reading the whole vocabulary for similar terms (#277) - #279

Merged
HugoFara merged 1 commit into
developfrom
fix/277-similar-terms-memory
Aug 23, 2026
Merged

fix(vocab): stop reading the whole vocabulary for similar terms (#277)#279
HugoFara merged 1 commit into
developfrom
fix/277-similar-terms-memory

Conversation

@HugoFara

Copy link
Copy Markdown
Owner

Fixes #277.

Root cause

FindSimilarTerms::execute() selected every term of the language into PHP and built a LetterPairProfile for each — letter pairs plus phonetic normalisation — before the 0.33 threshold threw almost all of them away. Fine for a hand-built vocabulary; fatal for one seeded from a dictionary import. The reported fatal at PreparedStatement.php:209 is the row-accumulation loop in fetchAll(), exactly as @sarahmccuan guessed in the issue.

Both branches of GET /api/v1/terms/for-edit reach it through getSimilarTermsForEdit(), so every word click paid for it.

Fix

The admission test moves into SQL, so the rows that come back are the ones that stood a chance. Two ways in, matching the two ways rankByCoverage() admits a candidate:

  • Word familyWoLemmaLC = ? OR WoTextLC = ?, mirroring sharesWordFamily(), so irregular forms still get in whatever they score on spelling.
  • Letter-pair overlap — Dice is 2|A∩B| / (|A|+|B|), so clearing a threshold t needs |A∩B| >= t(|A|+|B|)/2; dropping the candidate's own pair count, unknown until it is read, leaves the weaker t|B|/2 that a sum of LIKE tests can check. A term pair never spans a space, so it belongs to a candidate's pair set exactly when it is a substring of the candidate.

A LIMIT backs that up, with the scan ordered by shared-pair count so a vocabulary large enough to reach the cap loses its weakest matches rather than whichever have the lowest WoID. The rows then go back into WoID order before ranking, because rankByCoverage() breaks ties on whichever candidate it saw first — below the cap the results are unchanged.

QueryBuilder::whereRaw() and selectRaw() gained an optional $bindings array. The prefilter needs placeholders, and hand-writing the SQL would have gone around the builder's automatic WoUsID multi-user scoping. Both signatures stay backward compatible — the one existing whereRaw() caller passes SQL only.

Measured, on a seeded 200k-term vocabulary

before after
peak memory 174 MB 8 MB
latency 0.8–1.2 s 0.2–0.5 s
suggestions identical on all 5 probes

At PHP's default memory_limit=128M the old path reproduces the reported Allowed memory size ... exhausted; the new one returns its 10 suggestions at a 4 MB peak.

Known narrowing

The bound covers the character half of the score. A term whose spelling has almost nothing in common with the searched one but whose pronunciation does is no longer admitted on phonetics alone. With the defaults (minRanking = 0.33, phoneticWeight = 0.3) the phonetic side alone tops out at 0.30 and cannot clear the threshold anyway. It is documented on buildCandidateFilter().

Tests

Eight new tests: five for the prefilter — including a property test that it never drops a term the ranking would keep, plus word-family admission, the threshold scaling with term length, the short-term short circuit, and %/_ escaping — and two for whereRaw() binding order.

Psalm 0 errors, PHPCS 0 errors and 0 warnings, PHPUnit 9098 pass. Live checks also confirm 100%_off returns nothing rather than erroring, and a term too short for a letter pair short-circuits without a query.

Opening the term editor selected every term of the language into PHP and
profiled each one — letter pairs plus phonetic normalisation — before the
0.33 threshold threw almost all of them away. That is affordable for the
few thousand terms a reader adds by hand, but a vocabulary seeded from a
dictionary import runs to hundreds of thousands, and clicking a word spent
a minute before dying on the memory limit in PreparedStatement::fetchAll().

Move the admission test into SQL so the rows that come back are the ones
that stood a chance. Two ways in, matching how rankByCoverage() admits a
candidate: the same word family, or enough shared letter pairs. Dice is
2|A∩B| / (|A|+|B|), so clearing a threshold t needs |A∩B| >= t(|A|+|B|)/2,
and dropping the candidate's own pair count leaves the weaker t|B|/2 that
a sum of LIKE tests can check. A cap backs that up, with the scan ordered
by shared pairs so a vocabulary large enough to reach it loses its weakest
matches; the rows go back into WoID order before ranking, since ties are
broken on whichever candidate was seen first. Below the cap the results
are unchanged.

The prefilter needs placeholders, and hand-writing the SQL would have lost
QueryBuilder's automatic WoUsID scoping, so whereRaw() and selectRaw() take
an optional bindings array. Both stay backward compatible.

On a seeded 200k-term vocabulary: 174 MB peak becomes 8 MB, 0.8-1.2s becomes
0.2-0.5s, and the suggestions are identical. At the default memory_limit of
128M the old path reproduces the reported fatal; the new one peaks at 4 MB.

The one narrowing is documented: the bound covers the character half of the
score, so a term whose spelling has almost nothing in common is no longer
admitted on pronunciation alone. With the defaults the phonetic side alone
tops out at 0.30 and cannot clear the 0.33 threshold anyway.
@HugoFara HugoFara added bug Something isn't working database Database-related issues labels Aug 23, 2026
@HugoFara HugoFara linked an issue Aug 23, 2026 that may be closed by this pull request
@HugoFara
HugoFara merged commit bd77bc5 into develop Aug 23, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working database Database-related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Add Term" fails if dictionary size > php memory_limit

1 participant