W-569: stem query suffixes so inflected words reach their keywords - #184
W-569: stem query suffixes so inflected words reach their keywords#184wr wants to merge 5 commits into
Conversation
`:ghosted:` returned nothing while `:ghosting:` returned 👻. fzy rejects a needle longer than its haystack, so the match died before scoring — no amount of tuning reached it. Strip common English inflections and retry, but only when the query as typed found nothing. That gating is what makes the rules safe to keep crude: a wrong guess leaves an empty picker empty, and a query that already works keeps its exact ranking and pays nothing. Candidates go out most-likely-first, so in practice an inflected query costs one extra corpus scan, not one per rule. Refs: W-569
![wr-claude-reviewer[bot]](https://avatars.githubusercontent.com/in/3849589?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fallback-only gate (if trimmed.isEmpty) is exactly the right design here — a wrong guess leaves an empty picker empty, which can never be worse than the current result. The candidate ordering (most-likely first, early-exit on first hit) is efficient, and the "corpus decides which candidate exists" framing means crude rules are safe. Test coverage is thorough, including the ordering invariant (press before pres) and the double-s guard.
CI: only the ssot pipeline check is currently visible; no test suite run has appeared yet. That matches this repo's pre-push hook model, but worth confirming 293 tests still pass before merge.
Nit: import Foundation in QueryStemmer.swift appears unnecessary — the file uses only Swift stdlib types ([Character], Array, String), and String.contains(_: Character) is stdlib in Swift 5.7+. Safe to drop.
Stemming guesses several spellings and can't know which is a word, so taking the first candidate that returned anything let a nonsense stem win on an accidental fuzzy hit: `movies` → `movy` finds 🎑 (m-o-v-y inside "moon_viewing_ceremony") and shadowed `movie` → 🎥 behind it. `untied` → `unty` surfaced 📍 the same way. Gate each candidate on whether it starts some haystack — a plain character compare, no DP, so it costs far less than the fzy pass it guards. Refs: W-569
There was a problem hiding this comment.
Round 2 follow-up — previous findings addressed, one nit still open.
The isCorpusTerm addition in a25c083 is the right call. Gating on a prefix match before a full fzy scan means a wrong stem guess (movy) can't win just because it fuzzy-matches something unrelated (moon_viewing_ceremony), and the real candidate (movie) still gets its turn. inventedStemDoesNotShadowTheRealOne and stemsThatAreNotWordsYieldNothing cover the two failure modes neatly. starts(with:) over ~24k haystacks at stem length 3–7 is genuinely cheap vs the DP pass it gates.
CI: ssot pipeline in-progress; no failures visible. Pre-push hook model accounts for the absent xcodebuild run.
Nit (carry-over from round 1): import Foundation in QueryStemmer.swift is still there — the file uses only stdlib types ([Character], Array, String.contains(_: Character) is stdlib in Swift 5.7+). Safe to drop in a follow-up.
The corpus check rejects invented spellings, but not a trim that lands on a different real word: `hoped` → `hop` returns rabbits and shadows `hope`, `bared` → `bar` shadows `bare`. Order accepted candidates by length so the one retaining most of what was typed is searched first, keeping the stemmer's own order for ties. Refs: W-569
There was a problem hiding this comment.
Round 3 follow-up — previous findings partially addressed; one carry-over nit and one new nit, no blocking issues.
The length-preference ordering in acceptedStems is the right call. Filtering corpus terms first and then sorting by (length desc, original position asc) means the candidate that retains most of what the user typed is tried first, so hoping → hope wins over hoping → hop when both pass the corpus gate. The new tests (longerStemIsTriedFirstWhenBothAreRealTerms, longerStemChangesWhatSurfaces) pin exactly that invariant end-to-end, which is good.
CI: ssot pipeline check in-progress; no failures. Same pre-push hook situation as prior rounds.
Nits (grouped):
-
(Carry-over × 2)
import FoundationinSources/Mojito/EmojiDB/QueryStemmer.swiftis still unused — the file touches only[Character],Array, andString, all stdlib. Safe to drop. -
(New) The
acceptedStemsdocstring says "rather than calling the non-stablesort." Swift'ssort/sortedhas been stable since Swift 5.0, so "non-stable" is factually wrong about the language. The(length, position)composite key is still the right implementation (it makes the intent explicit and avoids any future surprise if the input order were to change), but the stated reason is misleading. Consider: "Composite key makes sort order explicit independent of input order" or simply drop the parenthetical.
Ordering purely by length broke `skies`: the longest accepted candidate is `skie`, which isn't a word but prefixes `skier`, so ⛷️ won and `sky` was never tried. Length was standing in for "is this real", and it's a bad proxy. Rank by match strength first — an exact corpus term beats one that merely prefixes a longer word — then by length, then by the stemmer's own order. `skies` → sky, ski, skie; `hoped` → hope, hop; `movies` → movie, movi. Refs: W-569
There was a problem hiding this comment.
First-pass review. No blocking findings.
What looks good
The fallback-only gate (if trimmed.isEmpty) is the right structural call. It makes the stemming rules provably safe: a wrong guess leaves an empty picker empty, and can never reshuffle a query that already works. The corpus-gated candidate ranking in acceptedStems / stemMatch is a clean second guard that prevents an invented spelling (e.g. movy) from shadowing the real one (movie) by winning the first fzy pass.
StemMatch: Int, Comparable with an early-return on .exact keeps stemMatch cheap relative to the fzy pass it gates. The (match, length, offset) sort key in acceptedStems is explicit about stability — good, given Swift.sort is stable but the intent isn't obvious without it.
Test coverage is solid: unit cases in QueryStemmerTests for each rule, end-to-end cases in FuzzyMatcherTests against the real corpus including the stemmingOnlyRunsWhenTheQueryFoundNothing guard, and the stemsAreOrderedByHowSolidlyTheyExist property test that pins the ranking.
Nit
// QueryStemmer.swift:62
for suffix in ["ies", "ied"] where hasSuffix(Array(suffix)) {
add(Array(needle.dropLast(3)) + ["y"])
}suffix appears only in the where clause — the body doesn't use it — and both iterations produce the same transformation. The for … where idiom reads like each element might do something different. An || guard makes the intent explicit at a glance:
if hasSuffix(["i","e","s"]) || hasSuffix(["i","e","d"]) {
add(Array(needle.dropLast(3)) + ["y"])
}They aren't equally trustworthy. `-ies` is reliably the plural of a `-y` noun — `skies` is sky + s, never ski + es, since the plural of ski is skis — so it outranks the generic trims. `-ied` carries no such guarantee: `carried` is carry + ied but `skied` is ski + ed, so it goes last and wins only when the plain trim found no real word. Treating them as one rule meant `skied` resolved to sky. Refs: W-569
There was a problem hiding this comment.
Round 5 follow-up — Round 4's nit addressed; two carry-over nits still open, no blocking issues.
The split of -ies and -ied into separate rules (commit 608b3c9) is the right call and actually goes further than what was asked. Positioning -ies before the generic trims and -ied after correctly captures the semantic difference: skies is reliably sky + plural so the Y-restore should win the ordering race, while skied is ambiguously ski + ed so the plain trim (ski) should get priority. The ("skies", ["sky", "ski", "skie"]) vs ("skied", ["ski", "sky", "skie"]) test cases in stemsAreOrderedByHowSolidlyTheyExist pin this invariant end-to-end.
CI: ssot pipeline in-progress, no failures visible. Consistent with the pre-push hook model.
Nits (carry-over × 2):
-
import FoundationinSources/Mojito/EmojiDB/QueryStemmer.swiftline 1 — still unused. The file touches only[Character],Array, andString;String.contains(_: Character)is stdlib in Swift 5.7+. Safe to drop in a follow-up. -
acceptedStemsdocstring inSources/Mojito/EmojiDB/FuzzyMatcher.swiftline 294 says "rather than the non-stablesort" — Swift'ssort/sortedhas been stable since Swift 5.0, so the stated reason is wrong. The composite(match, length, offset)key is still the right implementation (explicit intent, robust to input-order changes), but the parenthetical misleads. Suggest: "Composite key makes sort order explicit independent of input order" or just drop it.
1 participant
:ghosted:returned nothing, while:ghosting:returned 👻. The keyword was in the corpus the whole time — the query just couldn't reach it.FzyScorer.scorerejects any needle longer than its haystack:ghostedis 7 characters,ghostis 5. The match dies before the DP runs, so no scoring change reaches it. Trimming the suffix is the only way through.What works now
Design
QueryStemmer.stems(of:)returns candidate spellings, most likely first — not one canonical stem.-ingalone can't distinguishshipping(undo a doubled consonant) fromcelebrating(restore a droppede) fromblocking(plain trim), so all three go out and the corpus decides which exists.It runs only when the raw query found nothing. That gate is what makes the rules safe to keep this crude:
Deliberately not Porter/Snowball — a real stemmer's aggressive rules are wrong here (
ghosting→ghostis wanted, butcelebrate→celebrisn't a corpus term), and the fallback-only structure means precision matters more than recall.Cost
Measured over the real corpus, an inflected query costs one extra scan — candidates are ordered so the likeliest hits first:
ghostedghost,ghostecelebratingcelebrat,celebratepartiesparty,parti,partieshippingsmileAt ~1.3 ms per scan that's ~2.6 ms worst case for an inflected query, unchanged for everything else. Theoretical ceiling is 4 scans for a nonsense word ending in
-ingthat matches nothing.Test plan
QueryStemmerTests— 10 cases over the rules: plain stems,-ies/-ied→y, restorede, consonant doubling,pressnever becomingpres, uninflected queries yielding nothing, no stem shorter than 3 charactersFuzzyMatcherTests.inflectedQueryReachesItsKeyword— the four end-to-end cases above against the real bundled corpusFuzzyMatcherTests.stemmingOnlyRunsWhenTheQueryFoundNothing—:catsmatches 🐱 directly, so the-sstem must not fire and reshuffle rankingNote
Stacked on #183 — review that first. Base will retarget to
mainonce it lands.Refs W-569