fix: stop legacy Places API writing mislabeled places into category buckets

.gitignore

@@ -5,3 +5,4 @@ bin/
 .idea
 .DS_Store
 node_modules/
+Vacation-planner

POI/categories.go

@@ -38,8 +38,6 @@ const (
 	LocationTypeBar           = LocationType("bar")
 	LocationTypeBakery        = LocationType("bakery")
 	LocationTypeMealTakeaway  = LocationType("meal_takeaway")
-	LocationTypeFastFood      = LocationType("fast_food_restaurant")
-	LocationTypeFoodCourt     = LocationType("food_court")
 	LocationTypeMuseum        = LocationType("museum")
 	LocationTypeGallery       = LocationType("art_gallery")
 	LocationTypeAmusementPark = LocationType("amusement_park")
@@ -58,26 +56,33 @@ const (
 	LocationTypePharmacy = LocationType("pharmacy")
 )
 
-// GetPlaceCategory maps a Google Maps place type back to its category. It is the inverse of
-// GetPlaceTypes and MUST stay consistent with it: the nearby-search cache writes each place
-// under EncodeNearbySearchRedisKey(GetPlaceCategory(place.LocationType), ...), so a type that
+// GetPlaceCategory maps a Google Maps place type back to its category, reporting whether
+// the type is mapped at all. It is the inverse of GetPlaceTypes and MUST stay consistent
+// with it: the nearby-search cache writes each place under
+// EncodeNearbySearchRedisKey(GetPlaceCategory(place.LocationType), ...), so a type that
 // resolves to a different category than the one it was searched under would never cache-hit.
-func GetPlaceCategory(placeType LocationType) (placeCategory PlaceCategory) {
+//
+// It deliberately has NO default category. An earlier version defaulted to Eatery, which
+// silently absorbed place types the legacy Nearby Search does not understand — two
+// Places-API-(New)-only types ("fast_food_restaurant", "food_court") were added to
+// GetPlaceTypes(Eatery), Google ignored the unenforceable filter, and prominence-ranked
+// hotels were written into the eatery geo buckets. Returning ok=false forces every caller
+// to decide what an unmapped type means, and makes TestPlaceCategoryRoundTrip able to fail.
+func GetPlaceCategory(placeType LocationType) (PlaceCategory, bool) {
 	switch placeType {
 	case LocationTypePark, LocationTypeAmusementPark, LocationTypeGallery, LocationTypeMuseum:
-		placeCategory = PlaceCategoryVisit
+		return PlaceCategoryVisit, true
 	case LocationTypeCafe, LocationTypeRestaurant, LocationTypeBar, LocationTypeBakery, LocationTypeMealTakeaway:
-		placeCategory = PlaceCategoryEatery
+		return PlaceCategoryEatery, true
 	case LocationTypeShoppingMall, LocationTypeDepartmentStore, LocationTypeSupermarket, LocationTypeClothingStore, LocationTypeStore:
-		placeCategory = PlaceCategoryShopping
+		return PlaceCategoryShopping, true
 	case LocationTypeLodging:
-		placeCategory = PlaceCategoryLodging
+		return PlaceCategoryLodging, true
 	case LocationTypeGym, LocationTypeSpa, LocationTypePharmacy:
-		placeCategory = PlaceCategoryWellness
+		return PlaceCategoryWellness, true
 	default:
-		placeCategory = PlaceCategoryEatery
+		return PlaceCategory(""), false
 	}
-	return
 }
 
 // GetPlaceTypes returns a set of types defined in Google Maps API given a location type
@@ -88,7 +93,7 @@ func GetPlaceTypes(placeCat PlaceCategory) (placeTypes []LocationType) {
 			[]LocationType{LocationTypePark, LocationTypeAmusementPark, LocationTypeGallery, LocationTypeMuseum}...)
 	case PlaceCategoryEatery:
 		placeTypes = append(placeTypes,
-			[]LocationType{LocationTypeCafe, LocationTypeRestaurant, LocationTypeBar, LocationTypeBakery, LocationTypeMealTakeaway, LocationTypeFastFood, LocationTypeFoodCourt}...)
+			[]LocationType{LocationTypeCafe, LocationTypeRestaurant, LocationTypeBar, LocationTypeBakery, LocationTypeMealTakeaway}...)
 	case PlaceCategoryShopping:
 		placeTypes = append(placeTypes,
 			[]LocationType{LocationTypeShoppingMall, LocationTypeDepartmentStore, LocationTypeSupermarket, LocationTypeClothingStore, LocationTypeStore}...)

docs/migrations/reclassify-buckets.md

@@ -0,0 +1,86 @@
+# Migration: purge misclassified places from category geo buckets
+
+One-time cleanup for the `fast_food_restaurant` incident (#446). The legacy Nearby
+Search ignores an unknown `?type=` instead of erroring, so searches for two
+Places-API-(New)-only types returned prominence-ranked establishments that were then
+stamped with the queried type — writing hotels into `placeIDs:eatery:level*`.
+
+`GET /v1/migrate/reclassify-buckets?category=Eatery` — admin only, **dry-run unless
+`apply=true`**. Any other value of `apply` (absent, empty, `TRUE`, `1`) is a dry run.
+
+## Run the fixes first
+
+This must run **after** the write-path fixes in this PR are deployed. `8644199` is still
+creating new bad records in production: every city/category/price-bucket whose
+`MapsLastSearchTime` marker passes the 14-day `MinMapsResultRefreshDuration` does a fresh
+cold search. Cleaning before the fix deploys just gets re-polluted.
+
+## The rule
+
+A member is removed only when its **primary** Google type maps to a *different*
+category — the exact inverse of the write rule. An unmapped primary type is not
+evidence of misclassification, because the write path would legitimately place it there.
+
+| Primary type | Resolves to | Action |
+| --- | --- | --- |
+| `lodging` | Lodging | removed |
+| `supermarket`, `department_store` | Shopping | removed |
+| `cafe`, `restaurant` | Eatery | kept |
+| `meal_delivery`, `night_club` | unmapped | kept |
+| no `types[]` at all | unmapped | kept |
+
+⚠️ This rule and the write rule (`SetPlacesAddGeoLocations`, which keys on the stamped
+`LocationType`) deliberately disagree. A refactor that "unifies" them reintroduces the
+incident. `TestRemoveMisclassifiedPlacesPrimaryTypeTruthTable` pins the dangerous direction.
+
+## Expected scale
+
+Measured against production on 2026-07-30 (24,203 bucket members scanned):
+
+| Category | Members | Candidates |
+| --- | --- | --- |
+| Eatery | 8,589 | 145 |
+| Visit | 14,674 | 0 |
+| Shopping | 498 | 12 |
+| Wellness | 319 | 4 |
+| Lodging | 123 | 3 |
+
+**164 total.** Of these, 123 are incident-attributable (stamped `fast_food_restaurant`,
+mostly San Francisco / Tulsa / Boise hotels); the other 41 are pre-existing
+misclassifications the rule also catches (hotels stamped `restaurant`, supermarkets
+stamped `bakery`).
+
+**Known residue:** 27 incident records are *not* removed because their primary type maps
+to no category — `university` ×5, `airport` ×2, `real_estate_agency` ×2, `stadium`,
+`night_club`, `hardware_store`, `doctor`, and 7 with no `types[]`. These stay in the
+eatery buckets. The trip-planning path (`planner/solver.go`) does not reclassify, so they
+remain reachable in generated plans. Broadening `GetPlaceCategory` to cover those types is
+tracked as follow-up work.
+
+Separately, ~328 bucket members have no backing `place_details` record. Pre-existing and
+unrelated; the migration skips them.
+
+## Steps
+
+```bash
+# 1. Dry run and READ THE OUTPUT before going further.
+curl -s -H "Authorization: Bearer $ADMIN_JWT" \
+  "https://best-vacation-planner.herokuapp.com/v1/migrate/reclassify-buckets?category=Eatery" | jq
+
+# 2. Apply.
+curl -s -H "Authorization: Bearer $ADMIN_JWT" \
+  "https://best-vacation-planner.herokuapp.com/v1/migrate/reclassify-buckets?category=Eatery&apply=true" | jq
+
+# 3. Re-run the dry run; misclassified should now be 0 (modulo the residue above).
+```
+
+Repeat per category as needed (`Shopping`, `Wellness`, `Lodging`). Reads are pipelined in
+batches of 100 and the report carries `bucket_sizes`/`total_members`; the full Eatery scan
+completed in ~1.3s against production, well inside Heroku's 30s H12.
+
+To force a cold search when spot-checking a city afterwards, drop its marker field
+(format `<country>:<admin area 1>:<city>:<category>:<price level>`):
+
+```bash
+redis-cli HDEL MapsLastSearchTime "united states:ca:los altos:eatery:0"
+```

iowrappers/data_migrations.go

@@ -2,13 +2,16 @@ package iowrappers
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 	"strconv"
 	"strings"
 	"sync"
 
 	"github.com/bobg/go-generics/set"
+	"github.com/redis/go-redis/v9"
 	"github.com/weihesdlegend/Vacation-planner/POI"
 )
 
@@ -189,6 +192,178 @@ func (s *PoiSearcher) AddUserRatingsTotal(context context.Context) error {
 	return nil
 }
 
+// bucketCleanupReadBatchSize is how many place records the bucket scan reads per pipeline
+// round trip, matching the batch size SetPlacesAddGeoLocations uses on the write side.
+const bucketCleanupReadBatchSize = 100
+
+// BucketCleanupReport summarizes a RemoveMisclassifiedPlacesFromCategoryBuckets run.
+// BucketSizes/TotalMembers are measured with ZCARD before the scan begins so a dry-run
+// report states the scale of the job even for buckets an operator has never sized.
+type BucketCleanupReport struct {
+	BucketSizes   map[string]int64 `json:"bucket_sizes"`
+	TotalMembers  int64            `json:"total_members"`
+	Scanned       int              `json:"scanned"`
+	Misclassified int              `json:"misclassified"`
+	Removed       int              `json:"removed"`
+	RemovedIDs    []string         `json:"removed_ids"`
+}
+
+// SetPlace stores a single place record. Exported wrapper over setPlace for migrations and tests.
+func (r *RedisClient) SetPlace(ctx context.Context, place POI.Place) error {
+	return r.setPlace(ctx, place)
+}
+
+// AddGeoLocation adds a place to a geo bucket under an explicit key. Exported for
+// migrations and tests that need to write buckets the normal write path would reject.
+func (r *RedisClient) AddGeoLocation(ctx context.Context, key string, place POI.Place) error {
+	loc := place.GetLocation()
+	_, err := r.client.GeoAdd(ctx, key, &redis.GeoLocation{
+		Name:      place.ID,
+		Latitude:  loc.Latitude,
+		Longitude: loc.Longitude,
+	}).Result()
+	return err
+}
+
+// RemoveMisclassifiedPlacesFromCategoryBuckets removes places from cat's geo buckets whose
+// PRIMARY Google type does not belong to cat. It repairs the fast_food_restaurant incident:
+// two Places-API-(New)-only types were searched against the legacy Nearby Search, which
+// ignored the unenforceable type filter and returned prominence-ranked establishments, and
+// those were stamped with the queried type and written into placeIDs:eatery:level*.
+//
+// The removal rule is the exact inverse of the WRITE rule, not of POI.ReclassifyForCategory.
+// SetPlacesAddGeoLocations files a place under GetPlaceCategory(place.LocationType) and
+// refuses to write anything whose type maps to no category, so this migration removes a
+// member only when its primary type positively maps to a DIFFERENT category (lodging ->
+// Lodging, supermarket -> Shopping). An UNMAPPED primary type is deliberately kept: types
+// like "meal_delivery" and "night_club" are legal legacy types that Google routinely lists
+// first for genuine eateries (a delivery-first restaurant, a bar that is also a club), as are
+// records with no Types at all (cached before Types was captured). Those places carry a
+// stamped LocationType the write path maps straight back to this category, so removing them
+// would only delete rows the next cold search re-creates — while shrinking the trip-planning
+// candidate pool for up to MinMapsResultRefreshDuration, because the trip-planning path
+// (planner/solver.go -> matching.NearbySearchForCategory) reads these buckets with no
+// reclassification at all.
+//
+// Note this is a broader keep-set than POI.ReclassifyForCategory applies on the merchant
+// endpoint: that function keeps a place only when its primary type is one of the category's
+// five search types. Divergence is intended — one function decides what to show in a single
+// response, this one decides what may exist in the shared cache.
+//
+// dryRun reports what would be removed without deleting anything. Always dry-run first.
+func (r *RedisClient) RemoveMisclassifiedPlacesFromCategoryBuckets(ctx context.Context, cat POI.PlaceCategory, dryRun bool) (BucketCleanupReport, error) {
+	report := BucketCleanupReport{RemovedIDs: make([]string, 0), BucketSizes: make(map[string]int64)}
+
+	levels := []POI.PriceLevel{POI.PriceLevelDefault}
+	if cat == POI.PlaceCategoryEatery {
+		levels = POI.AllPriceLevels
+	}
+
+	keys := make([]string, 0, len(levels))
+	for _, level := range levels {
+		keys = append(keys, POI.EncodeNearbySearchRedisKey(cat, level))
+	}
+
+	// Size the job before doing it. The scan cost is linear in bucket membership and the
+	// handler runs under the caller's request context, so an operator needs the member count
+	// to judge whether a dry run can complete inside their client/router timeout.
+	for _, key := range keys {
+		size, err := r.client.ZCard(ctx, key).Result()
+		if err != nil {
+			return report, fmt.Errorf("sizing geo bucket %s: %w", key, err)
+		}
+		report.BucketSizes[key] = size
+		report.TotalMembers += size
+	}
+
+	for _, key := range keys {
+		members, err := r.client.ZRange(ctx, key, 0, -1).Result()
+		if err != nil {
+			return report, fmt.Errorf("reading geo bucket %s: %w", key, err)
+		}
+		// Read place records in pipelined batches rather than one GET per member: a serial
+		// N+1 over a real bucket cannot finish inside a 30s request timeout, which would
+		// make the mandatory dry-run review impossible and defeat the safety property.
+		for start := 0; start < len(members); start += bucketCleanupReadBatchSize {
+			batch := members[start:min(start+bucketCleanupReadBatchSize, len(members))]
+			places, found, err := r.getPlacesPipelined(ctx, batch)
+			if err != nil {
+				return report, fmt.Errorf("reading place records for %s: %w", key, err)
+			}
+			for i, placeID := range batch {
+				report.Scanned++
+				if !found[i] {
+					// no place record backing this bucket member; leave it for RemovePlaces
+					Logger.Debugf("RemoveMisclassifiedPlacesFromCategoryBuckets: no record for %s in %s", placeID, key)
+					continue
+				}
+				place := places[i]
+				primary := POI.PrimaryLocationType(place.Types)
+				// Only remove members whose primary type positively belongs to a DIFFERENT
+				// category. An unmapped primary type (meal_delivery, night_club, or no Types
+				// at all) is not evidence of misclassification — the write path would
+				// legitimately place it here.
+				if c, ok := POI.GetPlaceCategory(primary); !ok || c == cat {
+					continue
+				}
+				report.Misclassified++
+				report.RemovedIDs = append(report.RemovedIDs, placeID)
+				Logger.Infof("RemoveMisclassifiedPlacesFromCategoryBuckets: %s (%q, LocationType=%q, primary=%q, Types=%v) does not belong in %s",
+					placeID, place.Name, place.LocationType, primary, place.Types, key)
+				if dryRun {
+					continue
+				}
+				if _, err := r.client.ZRem(ctx, key, placeID).Result(); err != nil {
+					return report, fmt.Errorf("removing %s from %s: %w", placeID, key, err)
+				}
+				report.Removed++
+			}
+		}
+	}
+	return report, nil
+}
+
+// getPlacesPipelined fetches the place records for placeIDs in a single round trip.
+// found[i] reports whether placeIDs[i] had a usable record: a bucket member with no (or an
+// unparsable) backing record is not this migration's problem to fix, so it is reported as
+// missing rather than failing the whole batch. A transport-level failure is returned as an
+// error, because then nothing in the batch was actually read.
+func (r *RedisClient) getPlacesPipelined(ctx context.Context, placeIDs []string) ([]POI.Place, []bool, error) {
+	cmds := make([]*redis.StringCmd, len(placeIDs))
+	_, err := r.client.Pipelined(ctx, func(pipe redis.Pipeliner) error {
+		for i, placeID := range placeIDs {
+			cmds[i] = pipe.Get(ctx, PlaceDetailsRedisKeyPrefix+placeID)
+		}
+		return nil
+	})
+	// Pipelined surfaces the first non-nil command error, and a missing key is reported as
+	// redis.Nil — an expected outcome here, not a failure.
+	if err != nil && !errors.Is(err, redis.Nil) {
+		return nil, nil, err
+	}
+
+	places := make([]POI.Place, len(placeIDs))
+	found := make([]bool, len(placeIDs))
+	for i, cmd := range cmds {
+		res, cmdErr := cmd.Result()
+		if cmdErr != nil {
+			continue
+		}
+		if unmarshalErr := json.Unmarshal([]byte(res), &places[i]); unmarshalErr != nil {
+			Logger.Debugf("getPlacesPipelined: cannot parse record for %s: %v", placeIDs[i], unmarshalErr)
+			continue
+		}
+		found[i] = true
+	}
+	return places, found, nil
+}
+
+// RemoveMisclassifiedPlacesFromCategoryBuckets forwards to the RedisClient method so the
+// admin handler can call it through the concrete PoiSearcher (p.Solver.Searcher).
+func (s *PoiSearcher) RemoveMisclassifiedPlacesFromCategoryBuckets(ctx context.Context, cat POI.PlaceCategory, dryRun bool) (BucketCleanupReport, error) {
+	return s.redisClient.RemoveMisclassifiedPlacesFromCategoryBuckets(ctx, cat, dryRun)
+}
+
 func (s *PoiSearcher) AddUrl(context context.Context) error {
 	placeIdToDetailedSearchResults, err := s.addDataFieldsToPlaces(context, "url", BatchSize)
 	if err != nil {