Skip to content

Tag every recipe with a detected locale#11

Merged
dubadub merged 21 commits into
mainfrom
feat/recipe-locale-detection
Jul 12, 2026
Merged

Tag every recipe with a detected locale#11
dubadub merged 21 commits into
mainfrom
feat/recipe-locale-detection

Conversation

@dubadub

@dubadub dubadub commented Jul 12, 2026

Copy link
Copy Markdown
Member

Summary

Every recipe now carries a locale, resolved during indexing:

  • An author-declared Cooklang locale: key always wins (meta.locale(), normalized en_USen-US).
  • Otherwise the language is detected from the recipe's prose with whatlang. Detection runs over the parsed recipe, so Cooklang markup (@flour{200%g}) and quantities never reach the detector.
  • When detection isn't reliable, we store NULL rather than a guess.

Stored on the recipes row as locale (en, de, en-US) + locale_source (declared | detected), indexed in Tantivy as an exact-match field, and surfaced through:

  • APIGET /api/search?q=…&locale=de; locale / locale_source on the recipe detail response.
  • Web UI — language dropdown with counts on the search form, 🗣️ chip on recipe cards, a language pill on the detail page (qualified "(detected)" when we guessed), and inLanguage in the Schema.org JSON-LD.
  • CLIfederation backfill-locales [--force] tags recipes already in the database (content is already stored, so no refetching) and re-indexes them.

Filtering by a base language also matches its regional variants: locale=en finds en-US recipes, while locale=en-US matches only those. The filter is case-insensitive (?locale=EN and ?locale=en-us both work); the field is deliberately excluded from the free-text query parser, so searching for "de" doesn't return every German recipe.

Backfilled against a real 2,438-recipe database: 2,414 tagged (2,3xx en, 32 fr, 1 de, 1 declared en-US); the 24 left NULL are short/ambiguous entries like "Oats" where guessing would be worse than abstaining.

⚠️ Deploying this requires rebuilding the search index

This adds a field to the Tantivy schema, which makes any pre-existing index incompatible. SearchIndex::new previously called Index::open_in_dir, which opens using the on-disk schema and silently ignores the in-code one — so writing the new field would have panicked (out-of-bounds field id) the first time the GitHub indexer touched a recipe. That latent trap would have been sprung by any schema change; this branch now refuses to open a mismatched index with an actionable error instead.

rm -rf data/index        # or your INDEX_PATH
federation backfill-locales

Documented under "Upgrading" in the README.

Note: the feed crawler never wrote to the Tantivy index (a pre-existing gap — only the GitHub indexer did), and backfill-locales re-indexes every recipe it touches. So feed-crawled recipes become searchable for the first time, and search result counts will rise after the backfill.

Pre-existing bugs found but NOT fixed here

Worth their own issues:

  1. The GitHub indexer never refreshes recipes.content. When a .cook file changes upstream it updates only the file SHA (there's a stale // For now, we'll keep the existing recipe comment saying as much), while re-deriving ingredients/tags/locale from the new content — so the stored recipe body drifts from everything derived from it.
  2. /api/search's pagination.total saturates at the page window (total = top_docs.len() against a TopDocs::with_limit(limit + offset)) rather than reporting the true match count.

Test plan

  • cargo test — 112 tests pass, 0 failures (cargo clippy --all-targets -- -D warnings clean)
  • Unit tests: declared-beats-detected, region preserved (en-US) and never invented, ISO 639-3 → 639-1 mapping incl. the cmnzh / pesfa macrolanguage cases, too-short and non-linguistic input → None, ingredient names can't outvote the narrative
  • Search tests: locale filter excludes non-matching recipes, combines with a text query, base-language filter matches regional codes, case-insensitive filter, mismatched-schema index refuses to open
  • API test through the real router: ?locale=de returns only the German recipe; detail JSON carries both fields
  • Backfill integration tests: tags NULL-locale recipes and makes them searchable; leaves already-tagged rows alone unless --force; still indexes recipes whose locale can't be resolved (they must not vanish from search)
  • End-to-end against a running server: locale filter, base-language matching (locale=en returns the en-US recipe), dropdown with counts, card chips, and the detail pill verified in real rendered HTML for all four declared/detected × parsed/fallback combinations

🤖 Generated with Claude Code

dubadub added 20 commits July 12, 2026 14:46
Cooklang's meta.locale() only accepts its strict format (de, en_US) and
returns None for anything else. Unconditionally skipping the raw 'locale'
key when building 'custom' meant a value cooklang rejected -- such as the
hyphenated 'en-US' an author might plausibly type -- was dropped from both
the typed field and custom, losing it entirely.

Compute the typed locale once up front and only exclude the raw key from
'custom' when it actually parsed, so unparseable values still survive as
custom metadata.
Persists the resolved recipe locale (BCP-47-style code plus its
provenance: declared vs detected) on the recipes table. Adds
update_recipe_locale and list_locales, and threads the two new
columns through create_recipe and update_recipe_with_content.
Later tasks will populate real values from the crawler, GitHub
indexer, and a backfill command.
…dering

update_recipe_with_content's effect on locale was untested. Pins the
derived-state contract: a content update carries the newly-resolved
locale through, and a resolution to None clears the columns rather
than leaving a stale guess.

list_locales was only exercised against a single-locale result set.
Adds coverage for count-descending order, the alphabetical tie-break,
and the exclusion of NULL locales.
Parse the Cooklang content once per entry and reuse it for both the image
fallback and locale resolution, on the create and update paths alike.

Not covered by a crawler-level test: validate_url rejects loopback and
private addresses, so a mockito-backed feed cannot be crawled end to end.
The locale logic itself is covered by the indexer::locale unit tests.
Add a language dropdown to the search page (folding regional codes like
en-US into their base language), a language chip on recipe cards, a
locale pill on the recipe detail page, and inLanguage in the Schema.org
JSON-LD.
Tantivy pins field ids to the schema stored on disk, but SearchIndex::new
opened an existing index with Index::open_in_dir and then addressed it with
the in-code RecipeSchema. Once the two diverge every field id is wrong, and
writing a document panics in the fast-field writer rather than failing
cleanly -- which is what adding the `locale` field does to any index built
before it.

Compare the on-disk schema against ours at open time and refuse to proceed,
naming the rebuild command. Failing fast beats auto-wiping the index, which
would leave search silently returning zero results.
Two bugs in the same function:

1. Recipes whose locale couldn't be resolved (too little/ambiguous prose)
   were skipped entirely, including the index_recipe call. Following the
   README's upgrade path (rm -rf data/index && backfill-locales) permanently
   dropped those recipes from search, and every rerun re-skipped them
   deterministically. Locale resolution now only gates the DB write and the
   `updated` counter; every recipe with content is always indexed.

2. The Tantivy writer was committed once at the very end of the whole run,
   while each recipe's DB row was written immediately. A crash mid-run left
   DB rows pointing at a locale that was never actually committed to the
   index, and a plain rerun would skip them forever (their locale is
   already set). The writer is now created and committed per batch, before
   that batch's DB rows are written, so a crash leaves `locale IS NULL` and
   a rerun picks the batch back up. index_recipe deletes-then-adds by id,
   so redoing a batch is idempotent.
Locale codes are stored canonically as lowercase language + optional
uppercase region (en, de, en-US). The Tantivy locale field is untokenized
(exact match), so ?locale=EN or ?locale=en-us silently returned zero
results even though the recipe exists as "en"/"en-US". The web UI always
sends canonical values, but the API is public.

Add locale::normalize_code and apply it to the incoming filter in
SearchIndex::search, covering both the API and web UI in one place.

cooklang_parser's declared-`locale:` handling normalized the same way
independently; it now calls the shared helper so the two can't drift on
what "canonical" means.
@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review: Tag every recipe with a detected locale

Overall this is a well-scoped, thoroughly tested feature. The declared-vs-detected precedence, the narrative-text-before-ingredients fallback for detection (avoiding "Prosciutto" outvoting an English method), the macrolanguage mapping (cmnzh, pesfa), and the Tantivy schema-mismatch guard are all handled carefully, with unit tests targeting the exact edge cases called out in the PR description. Nice work.

Code quality & design

  • src/indexer/locale.rs is a clean, dependency-free, well-documented module — good separation of concerns (no I/O, easy to unit test).
  • The SearchIndex::new change to refuse opening a mismatched on-disk schema (src/indexer/search.rs) is a genuinely valuable defensive fix — it turns a would-be panic into an actionable startup error, with a matching test (test_opening_an_index_with_a_stale_schema_is_refused).
  • backfill_locales (src/cli/commands.rs) uses keyset pagination (id > ?) rather than OFFSET, correctly avoiding skipped rows as they drop out of the locale IS NULL predicate mid-scan. The per-batch "commit index, then write DB rows" ordering is documented with a clear rationale for the failure mode (redo work, not lose data).
  • Normalization (normalize_code) is centralized and reused between the declared-locale parser path and the search filter path, so they can't drift.

Potential issues

  1. docs/superpowers/plans/2026-07-12-recipe-locale-detection.md (1912 lines) and docs/superpowers/specs/2026-07-12-recipe-locale-design.md are brand-new files checked into the repo under docs/. These read as an AI-agent working plan/spec rather than user-facing project documentation. Worth confirming this is intentional — planning artifacts like this often stay out of version control rather than permanently living in the docs tree.
  2. GitHub indexer locale refresh on the update path (src/github/indexer.rs): when a .cook file's SHA changes, locale is now re-resolved from the newly fetched content and written via update_recipe_locale, but (per the PR's own "pre-existing bugs" note) recipes.content itself is never refreshed on this path — so locale/locale_source can end up describing content that differs from what's stored in recipes.content. Since this is explicitly flagged as a known, deliberately-unfixed issue already, just worth double-checking it's the intended scope boundary.
  3. backfill_locales's batch loop does a get_github_recipe_by_recipe_id + get_tags_for_recipe + get_ingredients_for_recipe per recipe (N+1-ish) inside each 500-row batch. Fine for a one-off CLI backfill (confirmed by the real 2,438-recipe run described in the PR), but worth batching if this pattern gets reused somewhere hotter later.
  4. Search pagination links in search.html interpolate locale the same un-encoded way query already was (?q={{ query }}&locale={{ locale }}&page=...). Askama HTML-escapes these so it's not an XSS risk, but a value containing & or # would still produce a malformed link. Pre-existing pattern, not a regression, but locale filtering makes it a bit more likely to surface.

Performance

  • No obvious hot-path regressions; the new Tantivy field is STRING | STORED and excluded from default query-parser fields, so free-text search is unaffected.
  • SearchIndex::commit now also calls self.reader.reload() after every commit, fixing a real eventual-consistency race (a search immediately after commit could previously read stale state). This affects all callers, not just the new backfill path — seems like the right call, but worth flagging as a behavior change.

Security

  • No injection concerns — all SQL uses bound parameters throughout, including the new update_recipe_locale/list_locales queries.
  • New dependencies (whatlang, isolang) are pure, offline, no-network crates — low supply-chain risk.

Test coverage

  • Excellent breadth: unit tests for locale resolution (declared vs. detected, region preservation, macrolanguage mapping, narrative-vs-ingredient-name precedence), search-layer filter tests (exact match, base-language matching, case-insensitivity, combined with text query), an API-level integration test through the real router, and backfill integration tests covering the "must not vanish from the index" invariant specifically.

Operational note

  • The README's "Upgrading" section and the schema-mismatch error message are a good, proactive way to handle the required index rebuild.

@dubadub

dubadub commented Jul 12, 2026

Copy link
Copy Markdown
Member Author

Thanks — addressed the one actionable item; notes on the rest.

1. Planning docs under docs/superpowers/ — fixed. These are intentional (this repo already tracks design docs: docs/plans/2025-12-29-recipe-jsonld-design.md, …-implementation.md, …-feed-validation-design.md), but they landed in the wrong place. Moved them to match the existing convention:

  • docs/plans/2026-07-12-recipe-locale-design.md
  • docs/plans/2026-07-12-recipe-locale-implementation.md

2. GitHub indexer: locale re-resolved from new content while recipes.content stays stale. Confirmed intended scope boundary. Worth being precise about why it isn't a new inconsistency: the code after that branch already re-extracts ingredients, cookware, and tags from the newly-fetched content, so the row has mixed fresh-derived data with a stale content column since well before this PR. Locale just follows the existing convention. The real bug is that the update path never refreshes content at all (there's a stale // For now, we'll keep the existing recipe comment admitting it) — that deserves its own issue rather than a drive-by fix here.

3. N+1 in the backfill batch loop. Agreed, and deliberately left: it's a one-off CLI command, and the real 2,438-recipe run was not I/O-bound in a way worth optimizing. If this pattern moves anywhere hotter, it should be batched.

4. Un-encoded locale in pagination links. Real but not reachable in practice: the values come from list_locales(), which returns codes we wrote ourselves (en, de, en-US) — they can't contain & or #. The pre-existing q interpolation next to it is the one that could actually produce a malformed link. Also its own issue.

5. SearchIndex::commit now reloads the reader — correct to flag as a behavior change. It's intentional. The reader uses ReloadPolicy::OnCommitWithDelay, so a search issued right after a commit could read a stale index; the helper now delivers what its name promises. Note it had zero production callers before this PR (the GitHub indexer calls writer.commit() directly), so the only behavior change is for backfill-locales — which needs it.

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review: Tag every recipe with a detected locale

Overall this is a well-scoped, carefully tested feature. A few things stood out worth calling out.

Correctness

src/github/indexer.rs:367-384 — locale is derived from content that never gets persisted.
On the "existing recipe, SHA changed" update path, the indexer downloads the new raw .cook content and now resolves/stores its locale — but (per the stale // For now, we'll keep the existing recipe comment) recipes.content itself is never updated on this path. So locale/locale_source can end up describing text that differs from what's actually stored and rendered for that recipe. The PR description already flags the underlying stale-content bug as a known, unfixed issue, but this change adds a new column that is unconditionally kept in sync with content that isn't persisted, so the mismatch becomes directly user-visible (e.g. a "🇩🇪 German (detected)" pill on a recipe whose displayed body is still the old English text). Worth either fixing alongside this PR, or explicitly deferring the locale write too until the content-refresh bug is fixed.

Locale silently clears on any parse hiccup during a content update (by design — worth double-checking).
update_recipe_with_content/the crawler path sets locale/locale_source to NULL whenever the new content fails to parse or detection is unreliable (well covered by test_update_with_content_rederives_locale). This is documented and intentional ("we don't store a guess"), but it also means a recipe that already had a good declared/detected locale will silently lose it if a later crawl hits a transient upstream parse error (e.g. a temporary syntax typo). That's a reasonable trade-off, just want to confirm it's the intended behavior vs. keeping the last-known-good locale on parse failure.

Performance

backfill_locales (src/cli/commands.rs) issues ~3 extra DB round-trips per recipe inside the batch loop (get_github_recipe_by_recipe_id, get_tags_for_recipe, get_ingredients_for_recipe) plus the locale update itself. Fine for a one-off maintenance command against thousands of rows (as demonstrated against the real 2,438-recipe DB in the PR description), but if this ever needs to run against a much larger corpus it'd be worth batching those lookups instead of one-per-recipe.

Minor

search.html's pagination links now interpolate locale directly into the query string (?q={{ query }}&locale={{ locale }}&page=...) without URL-encoding — same pre-existing pattern as query, just now duplicated for the new param. A locale value containing & wouldn't come from the dropdown, so this is low risk, but worth keeping in mind if locale ever becomes free-text.

What's good

  • The Tantivy schema-mismatch guard (SearchIndex::new now refuses to open an index built with a different schema, with an actionable error) is a genuinely nice fail-fast fix — the old behavior would have panicked in the fast-field writer the first time a document was indexed.
  • backfill_locales's per-batch commit-before-DB-write ordering is a thoughtful crash-safety design, and it's directly tested (test_backfill_still_indexes_recipes_with_undetectable_locale, test_backfill_skips_recipes_that_already_have_a_locale).
  • Locale resolution itself (src/indexer/locale.rs) is careful: declared beats detected, narrative text is tried before falling back to ingredient names (avoids culturally-marked ingredient names outvoting an unambiguous English method — nicely tested with the Italian-ingredients case), macrolanguage codes (cmn, pes) are mapped explicitly since isolang can't bridge them, and case/region normalization is centralized in one normalize_code helper shared between the parser and the search filter so the two can't drift.
  • Test coverage is thorough across every layer touched: unit tests for locale resolution, DB persistence/rederivation/list_locales ordering, search filtering (case-insensitivity, base-language matching, combining with free text), the API route end-to-end, and backfill idempotency/crash-safety.

Security

No concerns — the locale filter is applied via a parameterized Tantivy TermQuery, not string-interpolated into the query parser, and all new SQL uses sqlx bound parameters throughout.

@dubadub dubadub merged commit 17275e2 into main Jul 12, 2026
9 checks passed
@dubadub dubadub deleted the feat/recipe-locale-detection branch July 12, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant