diff --git a/api-docs.yaml b/api-docs.yaml index b0e1f5f..58385be 100644 --- a/api-docs.yaml +++ b/api-docs.yaml @@ -1887,10 +1887,42 @@ components: type: integer example: 1 + AddressMatch: + type: object + description: >- + Why a row was returned and how good the hit is. Present on results from + /addresses; absent on a lookup by id, where there is nothing to have + matched. + properties: + tier: + type: string + enum: [prefix, fuzzy, filter, none] + description: >- + Which pass produced the row. `prefix` - the full-text prefix index + matched every query word. `fuzzy` - only the trigram fallback + matched, so the query was misspelled or truncated. `filter` - no + text query; the row matched structured filters alone. `none` - a + text query was supplied but yielded no usable search terms, so + nothing was matched on. + example: prefix + confidence: + type: number + format: float + minimum: 0 + maximum: 1 + description: >- + How good the hit is, within its tier. Absent for `filter` and + `none`, where there is no text to score against. Not comparable + across tiers: a fuzzy 0.9 is a strong typo match, not a better + answer than a prefix 0.7. + example: 0.0476 + OhioAddress: type: object description: Complete Ohio address information with geographic coordinates properties: + match: + $ref: '#/components/schemas/AddressMatch' id: type: integer description: Unique address identifier diff --git a/models/address.go b/models/address.go index dc009b7..1125a8e 100644 --- a/models/address.go +++ b/models/address.go @@ -20,8 +20,47 @@ type OhioAddress struct { Latitude float64 `json:"latitude" db:"latitude"` Longitude float64 `json:"longitude" db:"longitude"` CreatedAt time.Time `json:"created_at" db:"created_at"` + + // Match describes why this row was returned. Populated by the /addresses + // search path; absent on a lookup by id, where there is nothing to have + // matched, and absent on /addresses/search, which runs a different set of + // passes that do not yet report a tier. + Match *AddressMatch `json:"match,omitempty"` +} + +// AddressMatch tells a caller how good a hit is, not just that it is a hit. +// +// Search already knew all of this -- which pass produced the row, and how well +// it scored -- and threw it away, returning a flat list in which a typo rescue +// that barely cleared the similarity threshold is indistinguishable from an +// address that matched every word exactly. A caller matching addresses +// automatically has to decide whether to accept a result, and that decision +// needs this. +type AddressMatch struct { + // Tier is which pass produced the row: + // + // prefix the full-text prefix index matched every query word + // fuzzy only the trigram fallback matched, so the query was misspelled + // or truncated + // filter no text query; the row matched structured filters alone + // none a text query was supplied but yielded no usable search terms, + // so nothing was matched on and these rows mean little + Tier string `json:"tier"` + + // Confidence is 0..1 within the tier, and is absent when there is no text + // query to score against. It is not comparable across tiers: a fuzzy 0.9 + // is a strong typo match, not a better answer than a prefix 0.7. + Confidence *float64 `json:"confidence,omitempty"` } +// Match tiers. +const ( + MatchTierPrefix = "prefix" + MatchTierFuzzy = "fuzzy" + MatchTierFilter = "filter" + MatchTierNone = "none" +) + // AddressSearchParams represents search parameters for address queries type AddressSearchParams struct { Query string `json:"query" form:"query"` // General search query diff --git a/services/address_service.go b/services/address_service.go index 6ff7c2b..0374613 100644 --- a/services/address_service.go +++ b/services/address_service.go @@ -110,6 +110,12 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP var selectFields []string argIndex := 1 hasRelevanceScore := false + hasFuzzySimilarity := false + // Where the predicate's own parameters landed, so relevance can be scored + // on exactly what matched rather than on a parallel expression that is + // free to disagree with it. + tsQueryArg := 0 + var fuzzyWordArgs []int // queryWords outlives this block. The relevance score it feeds lives in the // SELECT clause, and its parameters have to be numbered after every WHERE @@ -150,11 +156,13 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP for _, word := range queryWords { conditions = append(conditions, fmt.Sprintf("$%d <%% full_address", argIndex)) args = append(args, word) + fuzzyWordArgs = append(fuzzyWordArgs, argIndex) argIndex++ } } else { conditions = append(conditions, fmt.Sprintf("fts @@ to_tsquery('simple', $%d)", argIndex)) args = append(args, buildPrefixTSQuery(queryWords)) + tsQueryArg = argIndex argIndex++ } } @@ -209,36 +217,54 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP // to parse with "could not determine data type of parameter $1". whereArgCount := len(args) - // Build relevance score for ranking results. These CASE arms are evaluated - // only for rows the index already matched, so the ILIKEs here cost nothing - // like they did in the WHERE clause. - if len(queryWords) > 0 { - var scoreComponents []string - - for _, word := range queryWords { - wordPattern := "%" + word + "%" - - // Score: full_address match gets highest priority, then specific fields - scoreComponents = append(scoreComponents, fmt.Sprintf(` - CASE - WHEN full_address ILIKE $%d THEN 150 - WHEN street ILIKE $%d THEN 100 - WHEN (house_number || ' ' || street) ILIKE $%d THEN 90 - WHEN house_number ILIKE $%d THEN 80 - WHEN city ILIKE $%d THEN 60 - WHEN postcode ILIKE $%d THEN 50 - WHEN county ILIKE $%d THEN 30 - ELSE 0 - END`, argIndex, argIndex, argIndex, argIndex, argIndex, argIndex, argIndex)) - - args = append(args, wordPattern) - argIndex++ - } - - selectFields = append(selectFields, "("+strings.Join(scoreComponents, " + ")+") as relevance_score") + // Relevance. + // + // This was a sum of CASE arms over ILIKE, and it was inert. The first arm + // tests `full_address ILIKE '%word%'` for 150, which is the maximum, and + // full_address already contains house number, street, unit, city, region + // and postcode. The prefix predicate runs over + // to_tsvector('simple', full_address), so every row it admits has every + // query word inside full_address: the 150 arm always fired, the lower arms + // were unreachable, and the score was the constant 150 x len(queryWords) + // for every row. Confirmed against seeded data -- exactly one distinct + // value across every hit. ORDER BY relevance_score DESC therefore sorted + // nothing and fell through to alphabetical, and any confidence derived + // from it was always 1.0. + // + // ts_rank_cd ranks on the same tsvector and the same tsquery the predicate + // used, so the score cannot disagree with what matched -- which also + // removes a second defect: the CASE interpolated the *raw* word while the + // predicate used the *sanitized* one, so a query with trailing punctuation + // scored 0 on a row it had correctly matched. Normalisation flag 32 + // divides the rank by itself plus one, bounding it to 0..1 with no + // hand-maintained maximum to drift out of sync. + if len(queryWords) > 0 && tsQueryArg > 0 { + selectFields = append(selectFields, + fmt.Sprintf("ts_rank_cd(fts, to_tsquery('simple', $%d), 32) as relevance_score", tsQueryArg)) hasRelevanceScore = true } + // The fuzzy predicate applies `word <% full_address` to each word + // independently, so the score has to aggregate the same way. Scoring the + // joined phrase instead looks for one contiguous ordered extent, and would + // report a confidence below the very threshold that admitted the row + // whenever the matched words sit apart or out of order. + // + // LEAST is the right aggregate for an AND: a row is only as trustworthy as + // its weakest matching word. + if len(fuzzyWordArgs) > 0 { + parts := make([]string, 0, len(fuzzyWordArgs)) + for _, argPos := range fuzzyWordArgs { + parts = append(parts, fmt.Sprintf("word_similarity($%d, full_address)", argPos)) + } + expr := parts[0] + if len(parts) > 1 { + expr = "LEAST(" + strings.Join(parts, ", ") + ")" + } + selectFields = append(selectFields, expr+" as fuzzy_similarity") + hasFuzzySimilarity = true + } + // Ordering. Distance ordering takes lat/lng as fresh parameters, numbered // after the SELECT ones so placeholder numbers keep matching positions in // fullQueryArgs. @@ -263,6 +289,12 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP ) ASC, id`, argIndex, argIndex+1) orderByArgs = append(orderByArgs, params.Lng, params.Lat) argIndex += 2 + } else if hasFuzzySimilarity { + // Fuzzy rows score 0 on relevance_score by construction, so ordering by + // it put the alphabetically-first county on top while the reported + // confidence rose and fell arbitrarily down the list. Order by the + // number actually being reported, so results[0] is the closest match. + orderBy = "ORDER BY fuzzy_similarity DESC, county, city, street, house_number, id" } else if hasRelevanceScore { // Order by relevance score (highest first) orderBy = "ORDER BY relevance_score DESC, county, city, street, house_number, id" @@ -337,7 +369,8 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP var addresses []models.OhioAddress for rows.Next() { var addr models.OhioAddress - var relevanceScore *int // May or may not be present + var relevanceScore *float64 // ts_rank_cd, already bounded 0..1 + var fuzzySimilarity *float64 var rowTotal int // Scan targets are assembled in the same order the SELECT list was @@ -351,6 +384,9 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP if hasRelevanceScore { dest = append(dest, &relevanceScore) } + if hasFuzzySimilarity { + dest = append(dest, &fuzzySimilarity) + } if useWindowCount { dest = append(dest, &rowTotal) } @@ -363,6 +399,7 @@ func (s *AddressService) searchAddresses(q querier, params models.AddressSearchP if useWindowCount { total = rowTotal } + addr.Match = buildMatch(fuzzy, params.Query, len(queryWords), relevanceScore, fuzzySimilarity) addresses = append(addresses, addr) } @@ -1115,3 +1152,47 @@ func buildPrefixTSQuery(words []string) string { } return strings.Join(terms, " & ") } + +// buildMatch turns what the query already computed into something a caller can +// act on. +func buildMatch(fuzzy bool, queryText string, queryWords int, score, similarity *float64) *models.AddressMatch { + if queryWords == 0 { + // A query was supplied but every word was dropped as too short to + // yield a usable prefix term, so no text predicate was applied at all + // and these rows matched nothing in particular. Calling that "filter" + // would tell a caller they were looking at a deliberate structured + // result and invite them to trust it. + if strings.TrimSpace(queryText) != "" { + return &models.AddressMatch{Tier: models.MatchTierNone} + } + // Structured filters only. There is no text to have matched well or + // badly, so a confidence here would be invented. + return &models.AddressMatch{Tier: models.MatchTierFilter} + } + + if fuzzy { + match := &models.AddressMatch{Tier: models.MatchTierFuzzy} + if similarity != nil { + c := clamp01(*similarity) + match.Confidence = &c + } + return match + } + + match := &models.AddressMatch{Tier: models.MatchTierPrefix} + if score != nil { + c := clamp01(*score) + match.Confidence = &c + } + return match +} + +func clamp01(v float64) float64 { + if v < 0 { + return 0 + } + if v > 1 { + return 1 + } + return v +} diff --git a/services/match_confidence_integration_test.go b/services/match_confidence_integration_test.go new file mode 100644 index 0000000..11307c7 --- /dev/null +++ b/services/match_confidence_integration_test.go @@ -0,0 +1,230 @@ +package services + +import ( + "testing" + + "geocoding-api/models" +) + +// A caller matching addresses automatically has to decide whether to accept a +// result. Before this, a trigram rescue that barely cleared the similarity +// threshold and an address that matched every word exactly came back +// indistinguishable, in one flat list. +func TestMatchTierDistinguishesExactFromTypo(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + exact, _, err := svc.SearchAddresses(models.AddressSearchParams{Query: "Barendt", Limit: 5}) + if err != nil { + t.Fatalf("exact search: %v", err) + } + if len(exact) == 0 { + t.Fatal("expected hits for a correctly spelled street") + } + + // "barendtt" doubles a letter. The prefix index cannot match it, so only + // the trigram fallback can. Deliberately not a transposition ("barnedt"): + // those score 0.375, below the 0.6 threshold, and are documented as not + // recovered -- using one would make the fuzzy assertions below unreachable. + typo, _, err := svc.SearchAddresses(models.AddressSearchParams{Query: "barendtt", Limit: 5}) + if err != nil { + t.Fatalf("typo search: %v", err) + } + + if exact[0].Match == nil { + t.Fatal("match metadata missing on a search result") + } + if exact[0].Match.Tier != models.MatchTierPrefix { + t.Errorf("exact hit tier = %q, want %q", exact[0].Match.Tier, models.MatchTierPrefix) + } + if exact[0].Match.Confidence == nil { + t.Fatal("no confidence on a text search hit") + } + t.Logf("exact %-34s tier=%s confidence=%.3f", + exact[0].FullAddress, exact[0].Match.Tier, *exact[0].Match.Confidence) + + // Asserted, not guarded. An `if len(typo) > 0` here would let every fuzzy + // assertion below silently vanish the moment the fallback stopped working, + // which is the exact regression this test exists to catch. + if len(typo) == 0 { + t.Fatal("the trigram fallback returned nothing for a recoverable typo") + } + { + if typo[0].Match == nil { + t.Fatal("match metadata missing on the fuzzy result") + } + if typo[0].Match.Tier != models.MatchTierFuzzy { + t.Errorf("typo hit tier = %q, want %q", typo[0].Match.Tier, models.MatchTierFuzzy) + } + // The fuzzy pass matched precisely because the literal substring is + // absent, so an ILIKE-derived score would be zero. A real similarity + // has to come back instead. + if typo[0].Match.Confidence == nil { + t.Fatal("fuzzy hit carries no confidence") + } + if *typo[0].Match.Confidence <= 0 { + t.Errorf("fuzzy confidence = %.3f, want > 0 -- the ILIKE score was used instead of word_similarity", + *typo[0].Match.Confidence) + } + t.Logf("typo %-34s tier=%s confidence=%.3f", + typo[0].FullAddress, typo[0].Match.Tier, *typo[0].Match.Confidence) + } +} + +// With no query there is nothing to have matched well or badly, so inventing a +// confidence would be worse than omitting it. +func TestFilterOnlySearchReportsNoConfidence(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + rows, _, err := svc.SearchAddresses(models.AddressSearchParams{City: "Columbus", Limit: 3}) + if err != nil { + t.Fatalf("filter search: %v", err) + } + if len(rows) == 0 { + t.Fatal("expected rows for a city filter") + } + + if rows[0].Match == nil { + t.Fatal("match metadata missing on a filtered search") + } + if rows[0].Match.Tier != models.MatchTierFilter { + t.Errorf("tier = %q, want %q", rows[0].Match.Tier, models.MatchTierFilter) + } + if rows[0].Match.Confidence != nil { + t.Errorf("confidence = %v, want absent -- there is no text to score against", *rows[0].Match.Confidence) + } +} + +// Confidence has to order results the way a human would rank them, or it is +// just a number riding along. +func TestConfidenceIsBoundedAndOrdered(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + rows, _, err := svc.SearchAddresses(models.AddressSearchParams{Query: "Barendt", Limit: 20}) + if err != nil { + t.Fatalf("search: %v", err) + } + + if len(rows) == 0 { + t.Fatal("no rows to check ordering against; the assertions below would pass vacuously") + } + + var prev float64 = 2 + for i, r := range rows { + if r.Match == nil || r.Match.Confidence == nil { + t.Fatalf("row %d has no confidence", i) + } + c := *r.Match.Confidence + if c < 0 || c > 1 { + t.Errorf("row %d confidence = %f, outside 0..1", i, c) + } + // Results are ordered by relevance, so confidence must not increase + // as we walk down the list. + if c > prev { + t.Errorf("row %d confidence %.3f exceeds the row above it (%.3f); "+ + "confidence disagrees with the ordering", i, c, prev) + } + prev = c + } +} + +// The relevance score this confidence is built on used to be the constant +// 150 x len(queryWords) for every row -- full_address contains every other +// scored column, so the top CASE arm always fired. Confidence was therefore +// always exactly 1.0, and the ordering it fed sorted nothing. +func TestPrefixConfidenceVariesBetweenRows(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + // The shared fixture seeds rows of one shape, so every hit is genuinely an + // equally good match and an equal confidence is the right answer. To see + // whether the score carries any signal at all, the rows have to differ in + // match quality: same two query words, very different densities. + if _, err := db.Exec(` + INSERT INTO ohio_addresses (hash, house_number, street, unit, city, district, region, postcode, county, geom, full_address) + VALUES + ('rank-tight', '7', 'Barendt Road', '', 'Columbus', 'FRA', 'OH', '43004', 'Franklin', + ST_SetSRID(ST_MakePoint(-83.0, 40.0), 4326), + '7 Barendt Road, Columbus, OH 43004'), + ('rank-diffuse', '7', 'Barendt Road', '', 'Columbus', 'FRA', 'OH', '43004', 'Franklin', + ST_SetSRID(ST_MakePoint(-83.0, 40.0), 4326), + '7 Barendt Road Extension Seventeen Industrial Park Building Nine, Columbus, OH 43004') + `); err != nil { + t.Fatalf("seed contrasting rows: %v", err) + } + + rows, _, err := svc.SearchAddresses(models.AddressSearchParams{ + Query: "Barendt Columbus", Limit: 100, + }) + if err != nil { + t.Fatalf("search: %v", err) + } + + confidence := map[string]float64{} + for _, r := range rows { + if r.Match == nil || r.Match.Confidence == nil { + t.Fatal("missing confidence") + } + confidence[r.Hash] = *r.Match.Confidence + } + + tight, okTight := confidence["rank-tight"] + diffuse, okDiffuse := confidence["rank-diffuse"] + if !okTight || !okDiffuse { + t.Fatalf("seeded rows missing from results (tight=%t diffuse=%t)", okTight, okDiffuse) + } + + t.Logf("tight %.4f diffuse %.4f", tight, diffuse) + if tight == diffuse { + t.Errorf("both rows report %.4f; the score carries no signal", tight) + } + if tight < diffuse { + t.Errorf("the row where the query words sit close together (%.4f) scores below the diffuse one (%.4f)", + tight, diffuse) + } +} + +// Punctuation used to break this: the predicate matched on the sanitized word +// while the score ILIKE'd the raw one, so a correct row scored zero. +func TestTrailingPunctuationDoesNotZeroConfidence(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + rows, _, err := svc.SearchAddresses(models.AddressSearchParams{Query: "Barendt.", Limit: 5}) + if err != nil { + t.Fatalf("search: %v", err) + } + if len(rows) == 0 { + t.Fatal("a trailing full stop should not prevent the match") + } + if rows[0].Match == nil || rows[0].Match.Confidence == nil { + t.Fatal("missing confidence") + } + if *rows[0].Match.Confidence <= 0 { + t.Errorf("confidence = %.4f on a correctly matched row; the score is scoring a different string than the predicate matched", + *rows[0].Match.Confidence) + } + t.Logf("query with trailing punctuation: confidence=%.4f", *rows[0].Match.Confidence) +} + +// A query whose words are all too short adds no text predicate at all, so the +// rows returned matched nothing in particular. Reporting that as "filter" tells +// a caller they are looking at a deliberate structured result. +func TestDroppedQueryWordsAreNotReportedAsAFilterMatch(t *testing.T) { + db := setupCountTestDB(t) + svc := NewAddressService(db) + + rows, _, err := svc.SearchAddresses(models.AddressSearchParams{Query: "I 5", Limit: 3}) + if err != nil { + t.Fatalf("search: %v", err) + } + if len(rows) == 0 { + t.Skip("no rows returned; nothing to label") + } + if rows[0].Match.Tier != models.MatchTierNone { + t.Errorf("tier = %q, want %q -- a query that matched nothing must not look like a structured filter result", + rows[0].Match.Tier, models.MatchTierNone) + } +}