[obvious-autobuild]

fleet-packet: burst6-cont-fridgeluck-38

Test scan correction learner + confidence router — ported onto alpha

Covers the two recognition-logic services flagged in #4 with unit tests, and fixes one real bug found while writing them. The original base fix/scan-failure-report is stale against the repository's current integration branch alpha, so the branch is now ported onto alpha and this PR targets alpha. Still a draft.

What changed and why

• Port onto alpha. fix/scan-failure-report (3e6d321) is an ancestor of alpha (0309dd1); the branch's five commits sat on that base while alpha moved 63 commits ahead, with zero overlap in touched files. The port is a merge of origin/alpha into the branch as commit 4832043 — chosen over a rebase so the original commit hashes survive and no force-push is needed. git diff --name-only origin/alpha HEAD is exactly the three PR files. PR base retargeted fix/scan-failure-report → alpha.
• apps/ios/Tests/LearningServiceTests.swift — 13 tests. Correction threshold (auto-correct at 2, suggestion at 1), split/mixed correction targets, case-insensitivity, whitespace normalization, cache reload agreement with the database (ORDER BY determinism incl. rowid tie-break), highest-count wins after reload, telemetry counts/hit-rate/persistence (UserDefaults reset in setUp).
• apps/ios/Tests/ConfidenceRouterTests.swift — 12 tests. Thresholds by source (Vision 0.82/0.45, OCR exact 0.90/0.60, OCR fuzzy 0.55 confirm-only), inclusive boundary values via nextDown on the Thresholds constants, ocrMatchKind == nil defaulting to exact rules, manual-source always-auto, categorize order preservation, exact explanation strings + rounding, bucket labels.
• apps/ios/Capability/Core/Recognition/LearningService.swift — minimal fix (details below).
• fe6a8e6 wraps one over-length line in LearningServiceTests.swift so the repo's swift format lint --strict pre-commit gate passes; formatting only.

Fix: normalization in the correction learner

recordCorrection(visionLabel:) lowercased but did not trim, so recordCorrection("tomato", 1) then recordCorrection("TOMATO ", 1) landed on two different cache keys and the label never reached the auto-correct threshold — correctedIngredientId(for: "tomato") stayed nil. Failing-first repro:

XCTAssertEqual failed: ("nil") is not equal to ("Optional(1)") — LearningServiceTests.testCorrectionTrimsWhitespace

Fix: a single normalizedLabel helper (trim + lowercase) applied at every boundary — record, both lookups, cache load, and the stored vision_label. No schema migration; legacy untrimmed rows stay separate DB rows but the cache now merges them at read time (best-effort).

Same patch makes both cache queries share one deterministic order — ORDER BY correction_count DESC, last_used_at DESC, rowid DESC — so a fresh LearningService instance keeps the same winner per label as the record-time read-back instead of whichever row SQLite returns first on a count tie. (A second pre-fix failure — testMixedTargetCorrectionsDoNotAutoCorrect picking the older row on a same-second last_used_at tie — is the read-back sibling of this and is fixed by the same change.)

Audit note: where corrections apply (unchanged by this PR)

• VisionService passes LearningService.correctedIngredientId(for:) into IngredientIdentityResolution.resolveLabel — corrections take precedence over curated/catalog matches.
• The OCR resolution path does not consult user corrections; learned corrections apply only to the Vision classification path.
• IngredientReviewView is the write path: recordCorrection + suggestedCorrection pre-selection + shown/accepted telemetry.
• ReverseScanService / RecommendationEngine use ConfidenceLearningService (confidence calibration — separate service, out of scope here).

How I verified

Linux sandbox, Swift 6.4 (swift-6.4-RELEASE), GRDB 7.10.0 @ 36e30a6f — same pin as apps/ios/Package.resolved. There is no Xcode here, so the Swift sources were exercised through a scratch SwiftPM check harness (same approach as the original PR run, like the repo's own Tools/* mirror packages): it mirrors the production sources unmodified (LearningService.swift, ConfidenceRouter.swift, ScanContracts.swift, Detection.swift, Migrations.swift, plus ScanBenchmarkModels.swift for the ScanRequestFailure typealias) under a module named FridgeLuck, with a two-type CoreGraphics shim (CGImage, CGFloat — Foundation on Linux already provides CGRect/CGPoint/CGSize) and the two new test files.

• swift format lint --strict --configuration .swift-format on the three changed files → clean (after fe6a8e6).
• swift test -j 4 -Xswiftc -DSQLITE_DISABLE_SNAPSHOT on the merged head, before and after fe6a8e6 → Executed 25 tests, with 0 failures (0 unexpected) (13 LearningServiceTests + 12 ConfidenceRouterTests).
• Negative control on the same harness: swapped in the pre-fix LearningService.swift (blob afa86dfc from 3e6d321) → Executed 25 tests, with 5 failures — 3× whitespace normalization (testCorrectionTrimsWhitespace, testWhitespaceAndCaseAcrossNewInstance) and 2× tie-ordering (testMixedTargetCorrectionsDoNotAutoCorrect, testNewInstanceKeepsMostRecentSuggestionOnCountTie), matching the original failing-first evidence; restored the fixed file → 25/25 again. The tests fail without the fix.
• git diff --name-only origin/alpha HEAD after the merge → exactly the three PR files, confirming the port introduces nothing else.

What is not verified

• No macOS/Xcode build was possible: the two test files run here through the scratch harness, not the repo's FridgeLuckTests Xcode target. Hosted CI on this head fe6a8e6 (run 38757069606) is green: build-and-test pass (6m54s), dead-code-advisory pass (1m50s); CodeRabbit skipped (bot user not eligible).
• No iOS device or simulator run and no UI verification; the change is service logic plus unit tests only.
• The repo's apps/ios SwiftPM package (AppModuleTests) does not compile on Linux (39 test files @testable import FridgeLuck, a module SwiftPM does not provide) — pre-existing and untouched by this PR.

Assumptions

• "Port onto alpha" means the branch now sits on alpha and the PR's base becomes alpha; the original base branch fix/scan-failure-report was not touched, and the PR stays a draft.
• Merging origin/alpha into the branch (preserving the five original commits, fast-forward push, no force-push) is an acceptable port; the zero-overlap check and the three-file diff against alpha are the evidence.
• The scratch harness is verification-only scaffolding kept out of the repo; the only non-repo code in it is the CoreGraphics shim, which the tests never assert on.

🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3599f8ec-b4d3-4456-bb93-a70b2b803fb3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[obvious-autobuild]

Independent review (Obvious, Autobuild + Max)

Verdict: ready

I did not author this PR (different thread); this review was performed autonomously per the task brief. Draft PR, base alpha, head obv/fridgeluck-043 @ fe6a8e6. Every checkable claim in the body verified true. No blocking findings — two LOW items, both fine as follow-ups.

Findings (ranked by severity)

1. LOW — record-time cache overwrite can drop the merged legacy count until restart. The record-time read-back (apps/ios/Capability/Core/Recognition/LearningService.swift:110, WHERE vision_label = ?) sees only the exact stored key, while loadCache() merges all whitespace/case variants of a label (merge guard at :72). With a legacy untrimmed row from a pre-fix install — e.g. ' cherry tomato ' with count 5 — init merges it under key cherry tomato and auto-correct is active; a new correction on that label then upserts a fresh normalized row (count 1) and :125 (cache[key] = top) overwrites the merged winner with the count-1 row, silently deactivating auto-correct for that label until the next LearningService init. After a restart the legacy row in turn shadows the newer correction (higher count wins). Self-healing, affects only installs with pre-fix untrimmed rows; the body's "best-effort" disclosure covers the mechanism but not this window. Smallest fix: after recording, re-derive the key's cache entry the way loadCache() does (re-run loadCache(), or keep the higher count mirroring the :72 guard). Fine as a follow-up — the threshold path for freshly recorded labels still works.

2. LOW — title undersells a production behavior change. The title says "Test scan correction learner + confidence router", but the PR also changes runtime behavior (LearningService.swift: trim+lowercase normalization, deterministic rowid DESC tie-break). The body discloses it prominently, so this is a discoverability nit; smallest fix: mention the fix in the title when converting from draft (e.g. "+ fix label normalization").

Housekeeping note: PR #53's base is this branch (obv/fridgeluck-043); when this merges, #53 will need its base retargeted.

What I verified (commands and results, all on head fe6a8e6)

• Body's structural claims, verified live: git diff --name-only origin/alpha HEAD → exactly the 3 PR files; git merge-base --is-ancestor 3e6d321 origin/alpha → true; pre-fix blob check git show 3e6d321:apps/ios/Capability/Core/Recognition/LearningService.swift | git hash-object --stdin → afa86dfc… as the body states; gh run view 38757069606 --json headSha,conclusion → success at fe6a8e6 (build-and-test 6m54s, dead-code-advisory 1m50s).
• Repo gate: swift format lint --strict --configuration .swift-format on the three changed files → clean.
• Independent Linux harness — built from scratch for this review, not the author's: production sources verbatim (LearningService.swift, ConfidenceRouter.swift, Detection.swift, ScanContracts.swift, Migrations.swift, ScanBenchmarkModels.swift) plus a one-line CGImage shim, GRDB pinned to the repo's exact resolved revision 7.10.0 @ 36e30a6f (matches apps/ios/Package.resolved). swift build → exit 0. swift test -j 4 -Xswiftc -DSQLITE_DISABLE_SNAPSHOT → Executed 25 tests, with 0 failures (0 unexpected) — 13 LearningServiceTests + 12 ConfidenceRouterTests, matching the body.
• Negative control (do the new tests fail without the fix?): swapped the pre-fix LearningService.swift blob (afa86dfc) into the harness → Executed 25 tests, with 5 failures (0 unexpected) — testCorrectionTrimsWhitespace (1 assert), testWhitespaceAndCaseAcrossNewInstance (2 asserts), testMixedTargetCorrectionsDoNotAutoCorrect (1), testNewInstanceKeepsMostRecentSuggestionOnCountTie (1) — the same four tests and the same assertion-message text the body quotes; all 12 ConfidenceRouterTests still passed. Restored the fixed file (verified byte-identical to the repo head with diff) → 25/25 again. The tests genuinely bind to production behavior.
• The tests also run in macOS CI: project.yml:39 includes the whole apps/ios/Tests directory in the FridgeLuckTests target and the scheme's testTargets includes it, so the green CI run compiled and executed the new tests on macOS, not only in my Linux harness.
• Code-level checks: the two tie-breaks agree — loadCache()'s first-wins over correction_count DESC, last_used_at DESC, rowid DESC with the existing.count >= candidate.count guard (:72) yields exactly the record-time LIMIT 1 winner; the upsert's ON CONFLICT(vision_label, corrected_ingredient_id) target is backed by uniqueKey(["vision_label", "corrected_ingredient_id"]) (apps/ios/Platform/Persistence/Database/Migrations.swift:60); callers share the same label pipeline (VisionService.swift:201 and :285, write path IngredientReviewView.swift:52), matching the body's audit note, and the OCR path indeed does not consult corrections; Detection's convenience flags delegate to ConfidenceRouter.bucket; the pinned Thresholds constants and exact explanation strings match the implementation.

What I couldn't verify

• No Xcode or device in this environment: I could not run the FridgeLuckTests Xcode target on macOS myself (the hosted CI run is the macOS evidence), and there was no simulator/UI verification. Service logic plus unit tests only, as the body states.
• The legacy-row scenario in finding 1 is derived from the code paths (pre-fix recordCorrection stored lowercased-but-untrimmed keys); I had no populated legacy database to reproduce it against a real data shape.
• Concurrent-recordCorrection interleavings (write, then separate read + cache update, with catch {} swallowing errors) are unchanged from before this PR — pre-existing design, not re-validated here.