diff --git a/docs/SCAN_TO_ORDER.md b/docs/SCAN_TO_ORDER.md index 25bb4f5..b7b171a 100644 --- a/docs/SCAN_TO_ORDER.md +++ b/docs/SCAN_TO_ORDER.md @@ -8,6 +8,10 @@ recommend. When the customer taps a store and a size, a second call confirms the shelf still has it — and if it does not, names the next-nearest store that does. +When the label fits several products — `"britannia"` names 258 of them — it +answers with a short "did you mean?" list instead of picking one, because a +confident price on the wrong biscuit is worse than one extra tap. + Base path: `/live/api/v1/mob/scan`. Every response uses the usual envelope `{ code, status, message, details }`; the shapes below are `details`. @@ -17,7 +21,13 @@ Base path: `/live/api/v1/mob/scan`. Every response uses the usual envelope photo ──Lens──▶ label │ ▼ - POST /lookup ───▶ match + stores[] (recommended first) + POST /lookup ───▶ ambiguous:true + candidates[] "did you mean?" + │ │ + │ customer taps one candidate + │ │ + │ POST /lookup { brand, catalogueid } + │ │ + └───▶ match + stores[] (recommended first) ◀──┘ │ customer taps a store + a size │ @@ -26,11 +36,21 @@ photo ──Lens──▶ label ok:false + alternative → offer the other store ``` +**`/lookup` has two possible answers and the app must handle both.** A label +that names one product comes back with `match` + `stores`. A label that fits +several — a bare brand name like `"britannia"`, a generic word like +`"biscuits"` — comes back with `ambiguous: true` and `candidates`, and the +app asks the customer which one before any price is shown. Lens returns a +bare wordmark often, because it is usually the biggest thing printed on a +packet, so this is a normal path and not an error case. + `GET /stores` is for the "choose another shop" sheet: the customer's registered stores, nearest first, independent of any product. ## `POST /lookup` +Note the `//` notes below are annotations, not JSON — strip them. + ```json { "customerid": 5123, @@ -39,16 +59,25 @@ registered stores, nearest first, independent of any product. "longitude": 77.0290, "tenantids": [1135, 1140], // optional: what the app THINKS the customer joined "limit": 0 // optional: max stores, 0 = all + + // Instead of a label: name the product outright. This is how you resolve + // a candidate the customer tapped, and how a deep link or a "buy again" + // skips recognition. With both set, `label` is ignored. + // "brand": "britannia", "catalogueid": 7 } ``` +`label` is required **unless** `brand` and `catalogueid` are both given. + `tenantids` is verified, never trusted: the server intersects it with the `tenantcustomers` table. Ids the customer is not actually registered with come back in `unregistered_tenantids` — treat that as "refresh the local list". A list that matches nothing at all is treated as stale and all registered stores are used. -Response: +### Response A — one product identified + +`ambiguous: false`, `match` set, `candidates` empty. ```json { @@ -59,6 +88,8 @@ Response: "image": "https://…", "score": 0.94, "method": "vector+text" }, "catalogue_variants": [ { "…same shape…": "100 g" }, { "…": "200 g" } ], + "ambiguous": false, + "candidates": [], "confidence": 0.94, "available": true, "recommended_locationid": 20, @@ -83,12 +114,57 @@ Response: } ``` -How to read it: +### Response B — several products fit, none clearly -- `match == null` → nothing recognised; show `message` and let them retry. - `confidence` below ~0.5 → recognised but unsure; confirm the name with the - customer before showing prices. `method: "text"` means no embedding model - was involved (not configured, or it timed out) — be a little more cautious. +`ambiguous: true`, `match: null`, `stores: []`. Show a "did you mean?" list. + +```json +{ + "label": "britannia", + "match": null, + "ambiguous": true, + "candidates": [ + { "brand": "britannia", "catalogueid": 23, "product_name": "Britannia Marie Gold", + "size": "250 g", "image": "https://…", "score": 0.95, "method": "text", "available": true }, + { "brand": "britannia", "catalogueid": 22, "product_name": "Britannia Good Day Butter Cookies", + "image": "https://…", "score": 0.95, "method": "text" }, + { "brand": "britannia", "catalogueid": 21, "product_name": "Britannia Good Day Cashew Cookies", + "image": "https://…", "score": 0.95, "method": "text" } + ], + "confidence": 0.95, + "available": false, + "stores": [], + "catalogue_variants": [], + "message": "Which one is it? 1 of these 3 are in stock near you." +} +``` + +- **`confidence` is not low here, and that is not a bug.** "britannia" really + does appear in all three names, so relevance is high — what is missing is + *identification*. Gate on `ambiguous`, never on `confidence`: an app that + reads 0.95 as "sure enough to show a price" reintroduces the exact bug this + path exists to prevent. +- **`available` on a candidate** means at least one of the customer's + registered stores has it in stock right now. Candidates are ordered + available-first, so the list can show what is buyable before what is not + — and the field is absent (not `false`) when unavailable, so read it as + falsy, not as a required key. +- **To resolve a pick**, call `/lookup` again with that candidate's `brand` + and `catalogueid` and no label. You get Response A for that exact product, + with `method: "direct"` and `confidence: 1`. +- At most 10 candidates come back. + +### How to read either response + +- `match == null && !ambiguous` → nothing recognised; show `message` and let + them retry with a clearer photo. +- `ambiguous: true` → ask, do not guess. Never show a price on this path; + `stores` is deliberately empty. +- `confidence` below ~0.5 with a `match` → recognised but unsure; worth + confirming the name before showing prices. `method: "text"` means no + embedding model was involved (not configured, or it timed out) — be a + little more cautious. `method: "direct"` means the caller named the + product, so nothing was recognised at all. - `stores` is ordered **in-stock first, then nearest**. Exactly one store has `recommended: true` — the nearest with stock — and only when `available` is true. Stores that sell it but have nothing on the shelf are still listed @@ -205,6 +281,59 @@ Same `ScanStore` shape as inside `stores[]` above, without options. - **Identity** is the `customerid` in the body, like every other mobile endpoint here — there is no auth layer yet (see `SECURITY_HANDOFF.md`). +## Two decisions, and why + +Both come from a proposal (2026-09-23) to have the app send vectors it +computed on the phone. Recorded here because the next person will ask. + +### The app does not send `textvector` + +An on-device MiniLM vector is only comparable to the catalogue's if the app +ships the identical model *and* tokenizer *and* pooling *and* normalisation; +a quantised tflite build usually drifts, and the failure is silent — the +ranking just gets worse. There is also nothing to gain: the server-side +embed is ~30 ms warm and the result is cached in Redis by label, so one +model call serves every customer who scans that product. A client-supplied +vector *defeats* that cache (the key would have to be the vector, not the +label), and 384 floats is ~5 KB of upload against ~12 bytes for +`"Milk Bikis"`. If the field ever arrives it can be accepted and validated, +but the app should not be asked to compute it. + +**Send the full OCR text instead** if you want to give the server more to +work with — ~100 bytes, no model coupling, strictly more information than a +single label. + +### The app does not send `imagevector` — yet + +The catalogue *does* carry image vectors: every `brand_*` table has +`img_vector vector(1024)`, filled on 1885 of 2124 rows (empty in +`brand_haldirams`, `brand_kaleesuwari`, `brand_mdh`, `brand_zzsmoketest`). +That matches the proposed MobileNetV3-Small embedder, so the idea is +coherent and half-built — this flow simply does not read that column. + +It stays unread for now because **Google Lens is already the image +recogniser, and a far better one**: photo → Lens → label is Google's product +recognition, trained on billions of images. Putting a 137M-parameter +ImageNet backbone searching 1885 vectors *behind* that adds little where +Lens succeeds, and MobileNetV3-Small — which struggles to tell one blue +biscuit wrapper from another — is unlikely to rescue the cases where Lens +fails. There is also an unverified dependency: the preprocessing the app +would use (BGR → centre crop → 224×224 INTER_AREA → RGB → `/255.0`) has to +match whatever the catalogue pipeline actually ran, or the search returns +confidently-ranked noise. + +**What would change this:** the field data. Once live, count how often +`/lookup` returns `ambiguous: true` or nothing recognised. If Lens labels are +reliable, image search is polish; if that number is high, it becomes the +priority — and the first task is the cosine check (embed a known catalogue +product's image through the app's exact pipeline, compare with its stored +`img_vector`; ≈0.99 means the contract holds), not writing the query. + +There is one non-recognition argument for it worth remembering: on-device +inference is free and needs no Google dependency, which matters if Cloud +Vision costs start to bite at volume. That is a business reason, not a +quality one. + ## For backend developers ### Where the code is @@ -219,7 +348,7 @@ Same `ScanStore` shape as inside `stores[]` above, without options. | `utils/embedding.go` | `Embedder` interface, OpenAI-compatible and Gemini clients | | `utils/geo.go` | coordinate parsing, haversine, opening hours, label tokenising | | `config/config.go` | `EmbeddingConfig` and its validation | -| `scratch/cataloguedims` | read-only check of the catalogue's embedding width / fill | +| `scratch/cataloguedims` | read-only check of every catalogue vector column's width and fill | ### Try it locally @@ -238,11 +367,13 @@ anything; a schema-only dump does not. ### Tests -`go test ./services -run 'Lookup|Confirm|Stores|CatalogueFamily'` drives -the whole pipeline through a fake repository (`services/scan_test.go`); no -database. `go test ./utils` covers both HTTP clients against `httptest` -servers, and the geo helpers. Add a case to `scan_test.go`'s fixture when -you change ranking — it is the spec. +`go test ./services -run 'Lookup|Confirm|Stores|Brand|Ambiguous|Specific|TextScore|Distinct|Naming'` +drives the whole pipeline through a fake repository +(`services/scan_test.go`); no database. `go test ./utils` covers both HTTP +clients against `httptest` servers, and the geo helpers. Add a case to +`scan_test.go`'s fixture when you change ranking — it is the spec, and +`newBrandLabelFixture` in particular is the regression guard for the +brand-name bug described under Scoring. ### Knobs (constants in `scanService.go`) @@ -251,6 +382,8 @@ you change ranking — it is the spec. | `scanLookupTimeout` | 5 s | whole lookup, including the model call | | `scanCatalogueTopK` | 15 | rows taken from each brand table and from the merge | | `scanMinScore` | 0.50 | below this the best hit is not shown as a match | +| `scanAmbiguityMargin` | 0.06 | how close the runner-up may be before the answer becomes a question | +| `scanMaxCandidates` | 10 | longest "did you mean?" list | | `embedTimeout` (`utils/embedding.go`) | 4 s | one model call | | `scanVectorTTL` / `scanHitsTTL` (`scanRepository.go`) | 7 d / 30 min | cache lifetimes | @@ -274,6 +407,19 @@ Classic* at 0.9 (the "G" was dropped, so only "parle" matched either row), and the name tie-break handed it to Monaco because a space precedes a hyphen in ASCII. A confident, wrong answer — the kind no score floor can catch. +**When the substring rule ties, that tie is the answer.** A bare brand name +is a substring of every one of that brand's names, so all of them score 0.95 +— identically, at a high score no floor would ever catch. Rather than +scoring around it, `isAmbiguous` reads it: if the runner-up is within +`scanAmbiguityMargin` of the leader, the reply becomes `ambiguous: true` +with `candidates` instead of a match (see Response B). Erring towards asking +is deliberate — one tap on a picture against the wrong biscuit. A label that +names one product leaves the runner-up far behind, so the common case is +untouched, and `services/scan_test.go`'s +`TestABrandNameScoresItsProductsIdentically` guards the tie itself: a +formula that broke it on name length or word count would bring the bug +back. + ### Changing the embedding model 1. The catalogue team re-embeds `search_query` with the new model. diff --git a/models/scan.go b/models/scan.go index fc61af2..1b54bbf 100644 --- a/models/scan.go +++ b/models/scan.go @@ -11,8 +11,16 @@ package models type ScanLookupRequest struct { Customerid int `json:"customerid"` // What Lens read: "Milk Bikis", "Dabur Honey 500g". Free text, trimmed - // and capped by the service. + // and capped by the service. Not required when Brand and Catalogueid + // name a product outright. Label string `json:"label"` + // A product the customer has already chosen, by its catalogue key — + // which is how the app resolves a `candidates` list from an earlier + // ambiguous lookup, and how a deep link or a re-order skips recognition + // altogether. When both are set the label is ignored and no catalogue + // search runs. + Brand string `json:"brand"` + Catalogueid int64 `json:"catalogueid"` // Where the customer is right now. Optional: without it the customer's // saved primary address is used, and without that stores are listed in // registration order with no distance. @@ -86,9 +94,15 @@ type ScanCatalogueMatch struct { VariantKey string `json:"variant_key,omitempty"` Image string `json:"image,omitempty"` Score float64 `json:"score"` - // "vector", "vector+text" or "text" — how the score was produced. The app - // can be more cautious with a text-only match. + // "vector+text", "text" or "direct" — how the score was produced. The app + // can be more cautious with a text-only match; "direct" means the caller + // named the product by its catalogue key and nothing was recognised. Method string `json:"method"` + // Set only on entries of `candidates`: at least one of the customer's + // registered stores has this product in stock right now. Candidates are + // ordered with the available ones first, so a "did you mean?" list can + // show what is actually buyable before what is not. + Available bool `json:"available,omitempty"` } // ScanLookupResponse is the answer to a scan. @@ -96,10 +110,27 @@ type ScanLookupResponse struct { Label string `json:"label"` // The best catalogue product for the label, and the sizes of it the // catalogue knows about (each a separate catalogue row). + // + // Match is nil when nothing was recognised, and also when several + // products matched equally well — see Ambiguous. Match *ScanCatalogueMatch `json:"match"` Variants []ScanCatalogueMatch `json:"catalogue_variants"` + // Several products fit the label and no one of them is a clear winner — + // which is what a bare brand name ("britannia") or a generic word + // ("biscuits") produces, and Lens returns those often because a + // wordmark is the most legible thing on a packet. + // + // When true: Match is nil, Stores is empty, and Candidates holds the + // products to offer as "did you mean?". Picking one means calling + // /lookup again with that candidate's `brand` and `catalogueid`. + // + // Guessing instead would mean showing a confident price for a product + // the customer did not photograph. + Ambiguous bool `json:"ambiguous"` + Candidates []ScanCatalogueMatch `json:"candidates"` // 0..1. Below ~0.5 the app should confirm with the customer before - // showing prices. + // showing prices. With Ambiguous set this is the leader's score, which + // by definition the runner-up nearly equals. Confidence float64 `json:"confidence"` // Registered stores that stock the product, nearest first, in-stock // first. Empty with Available=false when none does. diff --git a/repositories/scanRepository.go b/repositories/scanRepository.go index b76b375..c326071 100644 --- a/repositories/scanRepository.go +++ b/repositories/scanRepository.go @@ -94,6 +94,9 @@ type ScanRepository interface { VectorSearch(ctx context.Context, vector []float32, limit int) ([]CatalogueHit, error) TextSearch(ctx context.Context, label string, limit int) ([]CatalogueHit, error) VectorSearchAvailable() bool + // CatalogueRef is one product named by its catalogue key, with its other + // pack sizes after it. Nothing is recognised or scored. + CatalogueRef(ctx context.Context, brand string, id int64) ([]CatalogueHit, error) // cache CachedVector(ctx context.Context, model, label string) ([]float32, bool) @@ -546,6 +549,72 @@ func (r *scanRepository) TextSearch(ctx context.Context, label string, limit int return hits, nil } +// tableFor resolves a brand the caller named to a real catalogue table. +// +// The lookup is against the tables discovered from information_schema, never +// a string built from the request: table names cannot be parameterised in +// SQL, so the discovered map is what keeps this from being an injection +// point. Both the table suffix ("britannia") and a display name ("24 Mantra" +// → brand_24_mantra) resolve. +func (r *scanRepository) tableFor(ctx context.Context, brand string) (string, map[string]bool, error) { + tables, err := r.brandTables(ctx) + if err != nil { + return "", nil, err + } + for _, candidate := range []string{ + "brand_" + strings.ToLower(strings.TrimSpace(brand)), + "brand_" + normaliseBrandKey(brand), + } { + if cols, ok := tables[candidate]; ok { + return candidate, cols, nil + } + } + return "", nil, ErrUnknownBrand +} + +// CatalogueRef reads one product by (brand, id) and appends its other pack +// sizes — same variant_key where the catalogue assigned one, same name +// otherwise, matching how the search groups a family. +// +// Distance is 0 on every row: nothing here was ranked, the caller said which +// product they meant. +func (r *scanRepository) CatalogueRef(ctx context.Context, brand string, id int64) ([]CatalogueHit, error) { + table, cols, err := r.tableFor(ctx, brand) + if err != nil { + return nil, err + } + suffix := strings.TrimPrefix(table, "brand_") + columns := hitColumns(suffix, cols) + + var self []CatalogueHit + err = r.catalogue.WithContext(ctx).Raw(fmt.Sprintf( + `SELECT %s, 0::float8 AS distance FROM %s WHERE id = ?`, columns, table), id).Scan(&self).Error + if err != nil { + return nil, err + } + if len(self) == 0 { + return nil, nil + } + + var siblings []CatalogueHit + if cols["variant_key"] && strings.TrimSpace(self[0].VariantKey) != "" { + err = r.catalogue.WithContext(ctx).Raw(fmt.Sprintf( + `SELECT %s, 0::float8 AS distance FROM %s WHERE variant_key = ? AND id <> ? ORDER BY id`, + columns, table), self[0].VariantKey, id).Scan(&siblings).Error + } else { + err = r.catalogue.WithContext(ctx).Raw(fmt.Sprintf( + `SELECT %s, 0::float8 AS distance FROM %s WHERE LOWER(product_name) = LOWER(?) AND id <> ? ORDER BY id`, + columns, table), self[0].ProductName, id).Scan(&siblings).Error + } + if err != nil { + // The product itself was found; losing its other sizes is the smaller + // failure and the caller asked for this one. + log.Printf("scan: could not read pack sizes of %s#%d: %v", brand, id, err) + return self, nil + } + return append(self, siblings...), nil +} + func sortedKeys(m map[string]map[string]bool) []string { keys := make([]string, 0, len(m)) for k := range m { diff --git a/scratch/cataloguedims/main.go b/scratch/cataloguedims/main.go index 1b39c1f..ff03f0c 100644 --- a/scratch/cataloguedims/main.go +++ b/scratch/cataloguedims/main.go @@ -53,28 +53,29 @@ func main() { var cols []struct { Relname string + Attname string Typname string Atttypmod int } if err := db.Raw(` - SELECT c.relname, t.typname, a.atttypmod + SELECT c.relname, a.attname, t.typname, a.atttypmod FROM pg_attribute a JOIN pg_class c ON c.oid = a.attrelid JOIN pg_type t ON t.oid = a.atttypid - WHERE a.attname = 'embedding' AND c.relname LIKE 'brand\_%' - ORDER BY c.relname`).Scan(&cols).Error; err != nil { + WHERE t.typname = 'vector' AND a.attnum > 0 AND c.relname LIKE 'brand\_%' + ORDER BY c.relname, a.attname`).Scan(&cols).Error; err != nil { log.Fatal(err) } if len(cols) == 0 { - fmt.Println("no brand_* table has an embedding column") + fmt.Println("no brand_* table has a vector column") return } - fmt.Printf("%-28s %-8s %5s %5s %5s\n", "table", "type", "dims", "rows", "embd") + fmt.Printf("%-24s %-16s %-8s %5s %5s %5s\n", "table", "column", "type", "dims", "rows", "filled") for _, c := range cols { var total, filled int64 db.Raw(fmt.Sprintf(`SELECT COUNT(1) FROM %s`, c.Relname)).Scan(&total) - db.Raw(fmt.Sprintf(`SELECT COUNT(1) FROM %s WHERE embedding IS NOT NULL`, c.Relname)).Scan(&filled) - fmt.Printf("%-28s %-8s %5d %5d %5d\n", c.Relname, c.Typname, c.Atttypmod, total, filled) + db.Raw(fmt.Sprintf(`SELECT COUNT(1) FROM %s WHERE %s IS NOT NULL`, c.Relname, c.Attname)).Scan(&filled) + fmt.Printf("%-24s %-16s %-8s %5d %5d %5d\n", c.Relname, c.Attname, c.Typname, c.Atttypmod, total, filled) } // nomic/bge emit unit vectors; a norm far from 1 means another pipeline. diff --git a/services/scanService.go b/services/scanService.go index 954fa8c..7566e15 100644 --- a/services/scanService.go +++ b/services/scanService.go @@ -10,6 +10,7 @@ import ( "nearle/repositories" "nearle/utils" "sort" + "strconv" "strings" "sync" "time" @@ -54,6 +55,12 @@ const ( // came back as "Paneer Makhni 500ml" (0.304) — a near-miss on an // unrelated row clears a floor set that close to the noise. scanMinScore = 0.50 + // How close the runner-up may be before the leader stops being an answer + // and the two become a question. See isAmbiguous. + scanAmbiguityMargin = 0.06 + // A "did you mean?" list longer than this is not a choice, it is a + // catalogue — the customer is standing in a shop holding a packet. + scanMaxCandidates = 10 ) // ScanErrors the controller maps to statuses. Everything else is a 500. @@ -82,11 +89,15 @@ func NewScanService(repo repositories.ScanRepository, embedder utils.Embedder) S func (s *scanService) Lookup(ctx context.Context, req models.ScanLookupRequest) (*models.ScanLookupResponse, error) { label := strings.TrimSpace(req.Label) + // The caller can name the product outright instead of describing it — + // how the app resolves a candidate the customer picked. + direct := strings.TrimSpace(req.Brand) != "" && req.Catalogueid > 0 + if req.Customerid <= 0 { return nil, fmt.Errorf("%w: customerid is required", ErrScanBadRequest) } - if label == "" { - return nil, fmt.Errorf("%w: label is required", ErrScanBadRequest) + if label == "" && !direct { + return nil, fmt.Errorf("%w: label, or brand and catalogueid, is required", ErrScanBadRequest) } if len(label) > scanMaxLabelLen { label = label[:scanMaxLabelLen] @@ -126,6 +137,10 @@ func (s *scanService) Lookup(ctx context.Context, req models.ScanLookupRequest) }() go func() { defer wg.Done() + if direct { + hits, method, matchErr = s.resolveRef(ctx, req.Brand, req.Catalogueid) + return + } hits, method, matchErr = s.searchCatalogue(ctx, label) }() wg.Wait() @@ -146,21 +161,35 @@ func (s *scanService) Lookup(ctx context.Context, req models.ScanLookupRequest) } resp := &models.ScanLookupResponse{ - Label: label, - Stores: []models.ScanStoreOffer{}, - Variants: []models.ScanCatalogueMatch{}, + Label: label, + Stores: []models.ScanStoreOffer{}, + Variants: []models.ScanCatalogueMatch{}, + Candidates: []models.ScanCatalogueMatch{}, } // Verify the app's idea of the customer's tenants against the truth. stores, resp.UnregisteredTenantids = restrictToTenants(stores, req.Tenantids) - if len(hits) == 0 || hits[0].score < scanMinScore { + switch { + case len(hits) == 0 && direct: + resp.Message = "That product is no longer in the catalogue." + return resp, nil + case len(hits) == 0, hits[0].score < scanMinScore: resp.Message = "We couldn't recognise that product. Try a clearer photo of the front of the pack." return resp, nil } - best := hits[0] - family := catalogueFamily(hits) + distinct := distinctProducts(hits) + + // Several products fit and none of them clearly wins — a bare brand name + // or a generic word. Ask rather than guess: naming one of them would put + // a confident price on a product the customer did not photograph. + if !direct && isAmbiguous(distinct) { + return s.candidatesResponse(ctx, resp, hits, distinct, method, stores) + } + + best := distinct[0] + family := catalogueFamily(hits, best) resp.Match = ptr(best.toMatch(method)) resp.Confidence = round3(best.score) for _, h := range family { @@ -179,11 +208,7 @@ func (s *scanService) Lookup(ctx context.Context, req models.ScanLookupRequest) keys = append(keys, repositories.CatalogueKey{Brand: h.Brand, Catalogueid: h.ID, Imageid: h.ImageID}) names = append(names, h.ProductName) } - locationids := make([]int, 0, len(stores)) - for _, st := range stores { - locationids = append(locationids, st.Locationid) - } - rows, err := s.repo.StoreOptions(ctx, locationids, keys, names) + rows, err := s.repo.StoreOptions(ctx, locationIDs(stores), keys, names) if err != nil { return nil, err } @@ -213,6 +238,137 @@ func (s *scanService) Lookup(ctx context.Context, req models.ScanLookupRequest) return resp, nil } +// candidatesResponse answers an ambiguous label with the products to choose +// between, marking which of them the customer can actually buy right now. +// +// The availability read is the same StoreOptions query the confident path +// runs, widened to every candidate — so a "did you mean?" list can put the +// three that are in stock above the five that are not, instead of sending +// somebody to a shelf that has none of them. +func (s *scanService) candidatesResponse(ctx context.Context, resp *models.ScanLookupResponse, + hits, distinct []scoredHit, method string, stores []models.ScanStore) (*models.ScanLookupResponse, error) { + + candidates := distinct + if len(candidates) > scanMaxCandidates { + candidates = candidates[:scanMaxCandidates] + } + resp.Ambiguous = true + resp.Confidence = round3(distinct[0].score) + + // Every catalogue row belonging to a candidate, and a way back from what + // a tenant's product row carries to the candidate it stands for. + inCandidates := make(map[string]bool, len(candidates)) + for _, c := range candidates { + inCandidates[c.productKey()] = true + } + var keys []repositories.CatalogueKey + var names []string + byImage := make(map[string]string) + byRef := make(map[string]string) + byName := make(map[string]string) + for _, h := range hits { + key := h.productKey() + if !inCandidates[key] { + continue + } + keys = append(keys, repositories.CatalogueKey{Brand: h.Brand, Catalogueid: h.ID, Imageid: h.ImageID}) + names = append(names, h.ProductName) + if h.ImageID != "" { + byImage[h.ImageID] = key + } + byRef[refKey(h.Brand, h.ID)] = key + if n := strings.ToLower(strings.TrimSpace(h.ProductName)); n != "" { + byName[n] = key + } + } + + stocked := make(map[string]bool) + if len(stores) > 0 && len(keys) > 0 { + rows, err := s.repo.StoreOptions(ctx, locationIDs(stores), keys, names) + if err != nil { + return nil, err + } + for _, row := range rows { + if row.Stock <= 0 { + continue + } + // Same precedence as optionFromRow: the stable key first. + if row.Imageid != "" { + if key, ok := byImage[row.Imageid]; ok { + stocked[key] = true + continue + } + } + if row.Catalogueid > 0 { + if key, ok := byRef[refKey(row.Productbrand, row.Catalogueid)]; ok { + stocked[key] = true + continue + } + } + if key, ok := byName[strings.ToLower(strings.TrimSpace(row.Productname))]; ok { + stocked[key] = true + } + } + } + + for _, c := range candidates { + m := c.toMatch(method) + m.Available = stocked[c.productKey()] + resp.Candidates = append(resp.Candidates, m) + } + // Buyable first; within each group the search's own ranking stands. + sort.SliceStable(resp.Candidates, func(i, j int) bool { + return resp.Candidates[i].Available && !resp.Candidates[j].Available + }) + + available := 0 + for _, c := range resp.Candidates { + if c.Available { + available++ + } + } + if available > 0 { + resp.Message = fmt.Sprintf("Which one is it? %d of these %d are in stock near you.", + available, len(resp.Candidates)) + } else { + resp.Message = fmt.Sprintf("Which one is it? We found %d products that could match.", + len(resp.Candidates)) + } + return resp, nil +} + +// resolveRef reads the product the caller named, with its pack sizes. No +// recognition, so every row scores 1 and the method says so. +func (s *scanService) resolveRef(ctx context.Context, brand string, id int64) ([]scoredHit, string, error) { + rows, err := s.repo.CatalogueRef(ctx, brand, id) + if err != nil { + switch { + case errors.Is(err, repositories.ErrCatalogueDBUnavailable): + return nil, "direct", ErrScanCatalogueDown + case errors.Is(err, repositories.ErrUnknownBrand): + return nil, "direct", fmt.Errorf("%w: unknown brand %q", ErrScanBadRequest, brand) + } + return nil, "direct", err + } + hits := make([]scoredHit, 0, len(rows)) + for _, row := range rows { + hits = append(hits, scoredHit{CatalogueHit: row, score: 1}) + } + return hits, "direct", nil +} + +func refKey(brand string, id int64) string { + return strings.ToLower(strings.TrimSpace(brand)) + "#" + strconv.FormatInt(id, 10) +} + +func locationIDs(stores []models.ScanStore) []int { + ids := make([]int, 0, len(stores)) + for _, st := range stores { + ids = append(ids, st.Locationid) + } + return ids +} + // ── Confirm ───────────────────────────────────────────────────────────────── func (s *scanService) Confirm(ctx context.Context, req models.ScanConfirmRequest) (*models.ScanConfirmResponse, error) { @@ -530,6 +686,11 @@ func (s *scanService) embed(ctx context.Context, label string) ([]float32, error // The whole label as a substring of the name is near-certain; otherwise the // share of label words found in name+title, scaled so that "all of them" // stops short of the substring case. +// +// A label that is a substring of MANY names — a bare brand, "britannia" — +// therefore scores them all 0.95, identically. That tie is not a flaw to +// score around: it is the signal, and isAmbiguous reads it to answer "did +// you mean?" rather than letting the sort order pick a winner. func textScore(h repositories.CatalogueHit, label string, tokens []string) float64 { name := strings.ToLower(h.ProductName) hay := name + " " + strings.ToLower(h.Title) @@ -561,28 +722,65 @@ func textScore(h repositories.CatalogueHit, label string, tokens []string) float return 0.8 * float64(found) / float64(len(tokens)) } -// catalogueFamily is the best hit and its other pack sizes: same brand, and -// the same variant_key when the catalogue assigned one, else the same name. -// Every member is a separate catalogue row a shop may have imported. -func catalogueFamily(hits []scoredHit) []scoredHit { - if len(hits) == 0 { - return nil +// productKey identifies a product across its pack sizes: the catalogue's own +// variant_key where it assigned one, the name otherwise, always within a +// brand. Two rows sharing it are 100 g and 200 g of one thing; two rows that +// do not are different products to choose between. +func productKey(h repositories.CatalogueHit) string { + if k := strings.TrimSpace(h.VariantKey); k != "" { + return h.Brand + "/" + strings.ToLower(k) } - best := hits[0] - family := []scoredHit{best} - for _, h := range hits[1:] { - if h.Brand != best.Brand { + return h.Brand + "/" + strings.ToLower(strings.TrimSpace(h.ProductName)) +} + +// productKey of a scored hit — scoredHit embeds the row it scored. +func (h scoredHit) productKey() string { return productKey(h.CatalogueHit) } + +// distinctProducts keeps the best-scoring row of each product, in rank +// order — the list of things the customer could actually be shown to choose +// between, as opposed to the same product listed four times in four sizes. +func distinctProducts(hits []scoredHit) []scoredHit { + seen := make(map[string]bool, len(hits)) + out := make([]scoredHit, 0, len(hits)) + for _, h := range hits { + key := h.productKey() + if seen[key] { continue } - switch { - case best.VariantKey != "" && h.VariantKey != "": - if h.VariantKey == best.VariantKey { - family = append(family, h) - } - case strings.EqualFold(strings.TrimSpace(h.ProductName), strings.TrimSpace(best.ProductName)): + seen[key] = true + out = append(out, h) + } + return out +} + +// isAmbiguous reports that naming the leader as THE match would be a guess +// dressed up as an answer, because something else is level with it. +// +// A margin rather than an absolute threshold: what matters is not how high +// the best score is but whether anything is tied with it. A bare brand name +// is a substring of every one of that brand's names, so textScore gives them +// all 0.95 — a perfect tie at a HIGH score, which no floor would catch. +// +// Erring towards asking is deliberate. Asking costs the customer one tap on +// a picture; guessing wrong costs them the wrong biscuit and costs us the +// belief that the scanner works. A label that names one product leaves the +// runner-up far behind, so the common case is unaffected. +func isAmbiguous(distinct []scoredHit) bool { + return len(distinct) >= 2 && distinct[1].score >= distinct[0].score-scanAmbiguityMargin +} + +// catalogueFamily is `of` and its other pack sizes, drawn from hits. +func catalogueFamily(hits []scoredHit, of scoredHit) []scoredHit { + key := of.productKey() + family := make([]scoredHit, 0, 4) + for _, h := range hits { + if h.productKey() == key { family = append(family, h) } } + if len(family) == 0 { + return []scoredHit{of} + } return family } diff --git a/services/scan_test.go b/services/scan_test.go index 7a41043..169ee3a 100644 --- a/services/scan_test.go +++ b/services/scan_test.go @@ -3,10 +3,13 @@ package services import ( "context" "errors" + "fmt" + "strings" "testing" "nearle/models" "nearle/repositories" + "nearle/utils" ) /* @@ -30,9 +33,13 @@ type fakeScanRepo struct { options []repositories.StoreOptionRow at map[int]*repositories.StoreOptionRow // productid → row + ref []repositories.CatalogueHit + refErr error + askedKeys []repositories.CatalogueKey askedNames []string askedLocs []int + askedRef string cachedHits map[string][]repositories.CatalogueHit } @@ -69,6 +76,10 @@ func (f *fakeScanRepo) TextSearch(context.Context, string, int) ([]repositories. return f.text, nil } func (f *fakeScanRepo) VectorSearchAvailable() bool { return f.hasVec } +func (f *fakeScanRepo) CatalogueRef(_ context.Context, brand string, id int64) ([]repositories.CatalogueHit, error) { + f.askedRef = fmt.Sprintf("%s#%d", brand, id) + return f.ref, f.refErr +} func (f *fakeScanRepo) CachedVector(context.Context, string, string) ([]float32, bool) { return nil, false } @@ -423,7 +434,7 @@ func TestCatalogueFamilyGroupsByVariantKeyThenName(t *testing.T) { {CatalogueHit: milkBikis200, score: 0.88}, {CatalogueHit: repositories.CatalogueHit{Brand: "parle", ProductName: "Milk Bikis", VariantKey: "milk_bikis"}, score: 0.5}, } - family := catalogueFamily(hits) + family := catalogueFamily(hits, hits[0]) if len(family) != 2 || family[1].ID != 8 { t.Fatalf("family should be the two britannia sizes, got %+v", family) } @@ -432,11 +443,229 @@ func TestCatalogueFamilyGroupsByVariantKeyThenName(t *testing.T) { a := scoredHit{CatalogueHit: repositories.CatalogueHit{Brand: "b", ID: 1, ProductName: "Honey"}} b := scoredHit{CatalogueHit: repositories.CatalogueHit{Brand: "b", ID: 2, ProductName: "honey "}} c := scoredHit{CatalogueHit: repositories.CatalogueHit{Brand: "b", ID: 3, ProductName: "Honey Lite"}} - if family := catalogueFamily([]scoredHit{a, b, c}); len(family) != 2 { + if family := catalogueFamily([]scoredHit{a, b, c}, a); len(family) != 2 { t.Errorf("name match should join 1 and 2 only, got %+v", family) } } +/* +Ambiguity. + +Lens hands back whatever was most legible on the packet, and on a packet that +is very often the brand wordmark alone. "britannia" fits 258 catalogue rows +equally well, so there is no best one. textScore gives all of them 0.95 — +correctly, the label IS in every one of those names — and with nothing to +read that tie, the sort order picked a winner and the customer was shown one +arbitrary biscuit with "confidence": 0.95 and a price. These tests are the +contract that it asks instead. +*/ + +// Three different Britannia products, of which the customer's stores stock +// one. Text-only: no embedder, which is also how production runs until the +// model is configured. +func newBrandLabelFixture() *fakeScanRepo { + cashew := repositories.CatalogueHit{Brand: "britannia", ID: 21, ProductName: "Britannia Good Day Cashew Cookies", VariantKey: "good_day_cashew", ImageID: "britannia_good_day_cashew"} + butter := repositories.CatalogueHit{Brand: "britannia", ID: 22, ProductName: "Britannia Good Day Butter Cookies", VariantKey: "good_day_butter", ImageID: "britannia_good_day_butter"} + marie := repositories.CatalogueHit{Brand: "britannia", ID: 23, ProductName: "Britannia Marie Gold", VariantKey: "marie_gold", ImageID: "britannia_marie_gold"} + + return &fakeScanRepo{ + exists: true, + stores: fixtureStores(), + text: []repositories.CatalogueHit{cashew, butter, marie}, + options: []repositories.StoreOptionRow{ + {Tenantid: 2, Locationid: 20, Productid: 220, Productname: "Britannia Marie Gold", + Productbrand: "britannia", Catalogueid: 23, Imageid: "britannia_marie_gold", Price: 30, Stock: 4}, + }, + } +} + +func TestABareBrandNameAsksInsteadOfGuessing(t *testing.T) { + repo := newBrandLabelFixture() + svc := NewScanService(repo, nil) + + resp, err := svc.Lookup(context.Background(), models.ScanLookupRequest{ + Customerid: 5, Label: "britannia", Latitude: "11.035", Longitude: "77.035", + }) + if err != nil { + t.Fatal(err) + } + + if !resp.Ambiguous { + t.Fatalf("a bare brand name must not resolve to one product, got match %+v", resp.Match) + } + if resp.Match != nil { + t.Errorf("Match must be nil while ambiguous, got %+v", resp.Match) + } + if len(resp.Stores) != 0 { + t.Errorf("no store or price may be quoted for a product the customer has not chosen, got %d offers", len(resp.Stores)) + } + if len(resp.Variants) != 0 { + t.Errorf("pack sizes belong to a chosen product, got %+v", resp.Variants) + } + if len(resp.Candidates) != 3 { + t.Fatalf("want the three distinct Britannia products, got %d: %+v", len(resp.Candidates), resp.Candidates) + } + + // The one the customer can actually buy is offered first. + if !resp.Candidates[0].Available || resp.Candidates[0].Catalogueid != 23 { + t.Errorf("the stocked product should lead the list, got %+v", resp.Candidates[0]) + } + for _, c := range resp.Candidates[1:] { + if c.Available { + t.Errorf("only Marie Gold is stocked, but %s reports available", c.ProductName) + } + } + // Note what confidence does NOT say here. The label appears verbatim in + // all three names, so relevance is high — and the answer is still a + // question. An app that gated on `confidence` instead of `ambiguous` + // would show a price for the wrong biscuit, which is the whole bug. + if resp.Confidence < 0.9 { + t.Errorf("a verbatim brand match scores high; %v suggests the scoring changed", resp.Confidence) + } + if !strings.Contains(resp.Message, "Which one") { + t.Errorf("the message should ask, got %q", resp.Message) + } +} + +func TestAnAmbiguousLabelWithNoStockStillLists(t *testing.T) { + repo := newBrandLabelFixture() + repo.options = nil + svc := NewScanService(repo, nil) + + resp, err := svc.Lookup(context.Background(), models.ScanLookupRequest{Customerid: 5, Label: "britannia"}) + if err != nil { + t.Fatal(err) + } + if !resp.Ambiguous || len(resp.Candidates) != 3 { + t.Fatalf("want three candidates, got ambiguous=%v %d", resp.Ambiguous, len(resp.Candidates)) + } + for _, c := range resp.Candidates { + if c.Available { + t.Errorf("%s cannot be available with no stock anywhere", c.ProductName) + } + } +} + +// The other half of the contract: a label that does name a product must not +// start asking questions. +func TestASpecificLabelStillWinsOutright(t *testing.T) { + repo := newBrandLabelFixture() + svc := NewScanService(repo, nil) + + resp, err := svc.Lookup(context.Background(), models.ScanLookupRequest{ + Customerid: 5, Label: "good day cashew", + }) + if err != nil { + t.Fatal(err) + } + if resp.Ambiguous { + t.Fatalf("a label naming one product should resolve, got candidates %+v", resp.Candidates) + } + if resp.Match == nil || resp.Match.Catalogueid != 21 { + t.Fatalf("want the cashew cookies, got %+v", resp.Match) + } + if len(resp.Candidates) != 0 { + t.Errorf("candidates belong to an ambiguous answer, got %+v", resp.Candidates) + } +} + +// The property the ambiguity check rests on: a label that is a substring of +// several names scores them EQUALLY. Nothing downstream can tell "did you +// mean?" from "found it" if a formula breaks that tie on name length, word +// count or anything else incidental — which is how one arbitrary Britannia +// biscuit used to come back with a price on it. +func TestABrandNameScoresItsProductsIdentically(t *testing.T) { + cashew := repositories.CatalogueHit{ProductName: "Britannia Good Day Cashew Cookies 200g"} + butter := repositories.CatalogueHit{ProductName: "Britannia Good Day Butter Cookies 100g"} + // Deliberately a much shorter name: length must not become a tie-breaker. + marie := repositories.CatalogueHit{ProductName: "Britannia Marie Gold"} + + tokens := utils.SearchTokens("britannia") + a, b, c := textScore(cashew, "britannia", tokens), textScore(butter, "britannia", tokens), textScore(marie, "britannia", tokens) + if a != b || b != c { + t.Fatalf("a brand must score its products equally, got %.3f / %.3f / %.3f", a, b, c) + } + if a == 0 { + t.Fatal("the brand name is in every one of those names; scoring it 0 would hide them all") + } + + // And a label that does name a product must NOT tie with its siblings, + // or everything would be a question. + specific := utils.SearchTokens("good day cashew") + if textScore(cashew, "good day cashew", specific) <= textScore(butter, "good day cashew", specific) { + t.Error("a label naming one product must outscore its siblings") + } + + if none := textScore(cashew, "dabur honey", utils.SearchTokens("dabur honey")); none != 0 { + t.Errorf("nothing in common should score 0, got %.3f", none) + } +} + +func TestDistinctProductsCollapsesPackSizes(t *testing.T) { + hits := []scoredHit{ + {CatalogueHit: milkBikis, score: 1}, + {CatalogueHit: milkBikis200, score: 0.9}, + {CatalogueHit: goodDay, score: 0.5}, + } + distinct := distinctProducts(hits) + if len(distinct) != 2 || distinct[0].ID != 7 || distinct[1].ID != 9 { + t.Fatalf("two sizes of one product are one choice, got %+v", distinct) + } +} + +// Picking a candidate: the app sends the key instead of a description, and +// nothing is recognised at all. +func TestNamingTheProductSkipsRecognition(t *testing.T) { + repo := newLookupFixture() + repo.ref = []repositories.CatalogueHit{milkBikis, milkBikis200} + // If recognition ran, these would decide the answer instead. + repo.text = []repositories.CatalogueHit{goodDay} + repo.vector = []repositories.CatalogueHit{goodDay} + svc := NewScanService(repo, fakeEmbedder{vec: []float32{0.1}}) + + resp, err := svc.Lookup(context.Background(), models.ScanLookupRequest{ + Customerid: 5, Brand: "britannia", Catalogueid: 7, + Latitude: "11.035", Longitude: "77.035", + }) + if err != nil { + t.Fatal(err) + } + if repo.askedRef != "britannia#7" { + t.Fatalf("the catalogue should have been asked for that exact product, got %q", repo.askedRef) + } + if resp.Match == nil || resp.Match.Catalogueid != 7 || resp.Match.Method != "direct" { + t.Fatalf("want a direct match on 7, got %+v", resp.Match) + } + if resp.Ambiguous || resp.Confidence != 1 { + t.Errorf("a named product is not a guess: ambiguous=%v confidence=%v", resp.Ambiguous, resp.Confidence) + } + if len(resp.Variants) != 2 { + t.Errorf("its pack sizes should come with it, got %+v", resp.Variants) + } + if !resp.Available || resp.RecommendedLocationid != 20 { + t.Errorf("stores are resolved exactly as for a recognised product, got %+v", resp.Stores) + } +} + +func TestAMissingCatalogueRefIsNotAMatch(t *testing.T) { + repo := newLookupFixture() + repo.ref = nil + svc := NewScanService(repo, nil) + + resp, err := svc.Lookup(context.Background(), models.ScanLookupRequest{ + Customerid: 5, Brand: "britannia", Catalogueid: 999, + }) + if err != nil { + t.Fatal(err) + } + if resp.Match != nil || resp.Ambiguous || len(resp.Stores) != 0 { + t.Fatalf("a product that is gone is not a match, got %+v", resp) + } + if !strings.Contains(resp.Message, "no longer") { + t.Errorf("the message should say the product is gone, got %q", resp.Message) + } +} + // A vector neighbour that is merely not-quite-unrelated used to clear the old // 0.30 floor: in production "Paracetamol" came back as "Paneer Makhni 500ml" // on a 0.304 similarity. Correct labels land near 0.92, so nothing this weak