Skip to content

Add search text matcher boundary#13

Merged
BigAkins merged 4 commits into
mainfrom
Ak/search-quality-boundary
May 12, 2026
Merged

Add search text matcher boundary#13
BigAkins merged 4 commits into
mainfrom
Ak/search-quality-boundary

Conversation

@BigAkins

Copy link
Copy Markdown
Owner

Summary

This cleanup adds a small pure helper boundary for AkFit search text behavior.

It moves current search normalization, alias handling, simple plural stemming, edit distance, match scoring, dessert/processed penalty behavior, and type-ahead suggestion matching into SearchTextMatcher.

What changed

Added

  • AkFit/AkFit/Search/SearchTextMatcher.swift
  • AkFit/AkFitTests/SearchTextMatcherTests.swift

Updated

  • SupabaseFoodSearchService now delegates normalization/filtering/scoring behavior to SearchTextMatcher
  • SearchView now delegates type-ahead matching to SearchTextMatcher

Why

Search behavior was becoming dense across SupabaseFoodSearchService and SearchView.

This PR creates a small testable boundary before future search/ranking work, reducing the chance that future feature work duplicates or changes search behavior accidentally.

Behavior

Search behavior is intended to be preserved exactly.

This PR does not:

  • redesign search
  • change ranking behavior
  • change UI
  • change Supabase schema
  • add RPCs
  • touch food seed data
  • change search routing

What is still not solved

This does not yet:

  • improve ranking quality
  • add new aliases
  • change fuzzy matching tolerance
  • fix seed-data gaps
  • add server-side type-ahead
  • solve broader restaurant/branded-food coverage

Those should be handled in future feature-specific branches.

Verification

Verified with the PR-safe AkFit test plan.

Result:

  • 45 tests passed
  • 0 failed

Copilot AI review requested due to automatic review settings May 12, 2026 04:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 701307a35d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .memsearch/memory/2026-05-11.md Outdated
Comment on lines +9 to +10
<!-- session:019e1a49-efbb-7911-936e-90e6dfd8b17e rollout:/Users/aki/.codex/sessions/2026/05/11/rollout-2026-05-11T22-45-13-019e1a49-efbb-7911-936e-90e6dfd8b17e.jsonl -->
- User asked: Implement the next AkFit cleanup-only PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drop committed local session log from repository

Remove this .memsearch memory file from the commit: it contains local machine paths and session metadata that are unrelated to app behavior, and committing it leaks developer-environment details while adding persistent noise to version control. These generated artifacts should be excluded (for example via ignore rules) rather than tracked.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors shared search normalization/matching/scoring into a new pure helper (SearchTextMatcher) and updates both Supabase-backed search and SearchView type-ahead to delegate to it, with new unit tests to preserve behavior.

Changes:

  • Added SearchTextMatcher with normalization, aliasing, stemming, scoring, and suggestion-matching helpers.
  • Updated SupabaseFoodSearchService and SearchView to delegate search behavior to the helper.
  • Added SearchTextMatcherTests to lock in expected normalization and suggestion ranking behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
AkFit/AkFit/Search/SearchTextMatcher.swift Introduces centralized pure search-text normalization/matching/scoring/suggestions logic.
AkFit/AkFit/Search/SupabaseFoodSearchService.swift Delegates query normalization, filtering, and scoring to SearchTextMatcher.
AkFit/AkFit/Views/Search/SearchView.swift Delegates type-ahead suggestion matching to SearchTextMatcher.
AkFit/AkFitTests/SearchTextMatcherTests.swift Adds unit tests for normalization, stemming, and suggestion ranking behavior.
.memsearch/memory/2026-05-11.md Adds a local session log file (appears unrelated to app code).
.memsearch/.index.pid Adds a transient PID artifact (appears unrelated to app code).
Comments suppressed due to low confidence (1)

.memsearch/.index.pid:2

  • .index.pid looks like a transient runtime artifact (PID) that will change between machines/runs and cause needless churn/merge conflicts. Suggest removing it from the PR and ignoring it (e.g. via .gitignore for .memsearch/).
55537


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .memsearch/memory/2026-05-11.md Outdated
Comment on lines +2 to +17
## Session 22:45

### 22:45
<!-- session:019e1a49-fa43-7912-b11c-9fd8c96a3034 rollout: -->
- Codex: {"title":"Add search helper tests"}

### 22:52
<!-- session:019e1a49-efbb-7911-936e-90e6dfd8b17e rollout:/Users/aki/.codex/sessions/2026/05/11/rollout-2026-05-11T22-45-13-019e1a49-efbb-7911-936e-90e6dfd8b17e.jsonl -->
- User asked: Implement the next AkFit cleanup-only PR.
- Codex: Implemented the cleanup-only search boundary. No commit made.

**Files Changed**
- [SearchTextMatcher.swift](/Users/aki/Documents/AkTalkToEM/AkCodeForEm/Project%20Workspace/AkPerso/AkFit/AkFit/AkFit/Search/SearchTextMatcher.swift) new pure helper
- [SupabaseFoodSearchService.swift](/Users/aki/Documents/AkTalkToEM/AkCodeForEm/Project%20Workspace/AkPerso/AkFit/AkFit/AkFit/Search/SupabaseFoodSearchService.swift) now delegates normalization/filtering/scoring
- [SearchView.swift](/Users/aki/Documents/AkTalkToEM/AkCodeForEm/Project%20Workspace/AkPerso/AkFit/AkFit/AkFit/Views/Search/SearchView.swift) type-ahead matching now delegates to helper
- [SearchTextMatcherTests.swift](/Users/aki/Documents/AkTalkToEM/AkCodeForEm/Project%20Workspace/AkPerso/AkFit/AkFit/AkFitTests/SearchTextMatcherTests.swif...
Comment on lines +74 to +85
/// Returns true when every normalized query word matches the term either
/// directly or through the rough plural stem.
static func matchesAllQueryWords(term: String, normalizedQuery: String) -> Bool {
guard !normalizedQuery.isEmpty else { return false }
let words = normalizedQuery.split(separator: " ").map(String.init)
let stemmedWords = words.map { stemWord($0) }
let normalizedTerm = normalizeForSearch(term)
let stemmedTerm = stemmedForm(normalizedTerm)
return words.allSatisfy { normalizedTerm.contains($0) } ||
stemmedWords.allSatisfy { stemmedTerm.contains($0) }
}

Comment on lines 52 to 57
rows = candidates.filter { row in
let n = Self.normalizeForSearch(row.foodName)
let nStemmed = Self.stemmedForm(n)
return words.allSatisfy { w in n.contains(w) } ||
stemmedWords.allSatisfy { sw in nStemmed.contains(sw) }
SearchTextMatcher.matchesAllQueryWords(
term: row.foodName,
normalizedQuery: normalized
)
}
@BigAkins

Copy link
Copy Markdown
Owner Author

Patched the review comments.

Changes:

  • removed .memsearch files from Git tracking
  • added .memsearch/ to .gitignore
  • added SearchTextMatcher.QueryMatch so query normalization, words, stems, and plain-food query state are computed once per query
  • updated Supabase search filtering and SearchView type-ahead matching to reuse the precomputed query match data
  • added tests comparing the precomputed helper path against the existing string-based helper path

Verification:

  • PR-safe AkFit test plan passed
  • 47 tests passed, 0 failed
  • git diff --check passed
  • .memsearch/ is now ignored and no .memsearch files remain tracked

@BigAkins

Copy link
Copy Markdown
Owner Author

@copilot

@Codex

Codex AI commented May 12, 2026

Copy link
Copy Markdown

@codex[agent]

No further changes needed on my side—latest commits bdbe855 (QueryMatch + reuse) and 6332426 (.memsearch ignored) cover the review items. Let me know if you want any follow-up tweaks.

@BigAkins

Copy link
Copy Markdown
Owner Author

@claude

Repository owner deleted a comment from chatgpt-codex-connector Bot May 12, 2026
@claude

claude Bot commented May 12, 2026

Copy link
Copy Markdown

Claude finished @BigAkins's task in 2m 10s —— View job


Code Review

  • Gather context (CLAUDE.md, git diff, changed files)
  • Review SearchTextMatcher.swift
  • Review SupabaseFoodSearchService.swift
  • Review SearchView.swift
  • Review SearchTextMatcherTests.swift
  • Post final review

The refactor is well-executed. The QueryMatch struct directly addresses the Copilot feedback about per-row recomputation, SearchView shrinks from 55 lines of logic to 2, and the .memsearch/ artifacts are now properly ignored. A few things worth looking at before merging:


1. Implicit safety assumption in matchScoreqStems[0] without bounds check

SearchTextMatcher.swift:155-156

if qWords.count <= 1 {
    let qStem = qStems[0]   // ← unchecked access

If qWords.count == 0, this crashes. In practice it's unreachable today because Swift.String.hasPrefix("") is always true, so the n.hasPrefix(q) early-return on line 148 fires first when the query is empty. But that's an implicit invariant, not an explicit guard — it breaks silently if the control flow above ever changes. A simple fix:

if qWords.count == 1 {   // exact check instead of <= 1
    let qStem = qStems[0]

Or add a guard before the branch. Fix this →


2. applyQueryAliases rebuilds the tuple array on every call

SearchTextMatcher.swift:38-41

let aliases: [(from: String, to: String)] = [
    ("in and out", "in n out"),
    ("innout", "in n out"),
]

This allocates a new array on every normalization call. Move to private static let:

private static let queryAliases: [(from: String, to: String)] = [
    ("in and out", "in n out"),
    ("innout", "in n out"),
]

Fix this →


3. Redundant entry in dessertKeywords

SearchTextMatcher.swift:187

isDessertOrProcessed uses substring contains, so "milkshake" already triggers on "shake". The "milkshake" entry does no additional work. Not a bug, just noise.


4. Sort comparator still normalizes inside O(n log n) comparisons

SupabaseFoodSearchService.swift:70-75

return rows.map(FoodItem.init).sorted { a, b in
    ...
    if SearchTextMatcher.isDessertOrProcessed(SearchTextMatcher.normalizeForSearch(a.name)) { sa += 1 }
    if SearchTextMatcher.isDessertOrProcessed(SearchTextMatcher.normalizeForSearch(b.name)) { sb += 1 }

normalizeForSearch is called inside the comparator, which runs O(n log n) times. This was the same behavior before this PR (not a regression), but it's worth noting: if the candidate set ever grows, a pre-sort .map { ($0, SearchTextMatcher.normalizeForSearch($0.name)) } before the sort would avoid the repeated work.


5. Test coverage gaps

The test file does a good job covering normalization, stemming, alias handling, ranking, and the precomputed-vs-string parity. A few paths have no coverage:

Gap Why it matters
editDistance Directly tests the O(n*m) routine in isolation
Fuzzy fallback path (< 3 exact matches, query ≥ 3 chars, single-char typo) The fallback branch in suggestions is never exercised by existing tests
matchScore for all score bands (0–4) Verifies the scoring tiers explicitly rather than relying on end-to-end suggestion order

Overall

Solid PR. The QueryMatch struct is the right design, the two-overload pattern (String + QueryMatch) makes the parity tests clean, and the delegation in SearchView is a meaningful simplification. Items 1 and 2 above are quick fixes; the rest are minor quality notes that could reasonably go in a follow-up.
| Branch: Ak/search-quality-boundary

@BigAkins BigAkins merged commit a44efe8 into main May 12, 2026
4 of 5 checks passed
@BigAkins BigAkins deleted the Ak/search-quality-boundary branch May 12, 2026 05:39
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.

3 participants