feat: AI-assisted meal logging — multi-item text entry, offline and model-assisted (#599) - #648
Open
simonoppowa wants to merge 18 commits into
Open
feat: AI-assisted meal logging — multi-item text entry, offline and model-assisted (#599)#648simonoppowa wants to merge 18 commits into
simonoppowa wants to merge 18 commits into
Conversation
The `pull_request` trigger listed only `main` and `develop`, so retargeting #616 onto `feature/ai-assisted-meal-logging` silently left it with no checks at all. Nothing failed — `gh pr checks` simply reported none, while the PR still showed as mergeable, which is an easy absence to miss. Tier 0 stages three PRs on that branch before it reaches `develop` as one merge, so those PRs need the same validation everything else gets. Scoped to `feature/**` rather than `**` so this covers deliberate integration branches without firing a full run on every PR between two arbitrary topic branches.
* feat(add-meal): scaffold meal-text parser with segmentation Adds ParsedMealItem/MealTextParseResult and parseMealText(), which for now only segments free text on comma/semicolon/newline/plus (no quantity or unit extraction yet). Implements the decimal-vs-separator comma rule from #600: a comma with a digit on both sides is a decimal point, every other comma is a separator. Part of #600. * feat(add-meal): extract leading/trailing quantity and unit per segment Handles both 100g toast and toast 100g, with or without a space before the unit letters (1.5 l milk). Only g/kg/ml/l/oz are recognized as unit symbols; an unrecognized letter run right after a number (100xyz toast) is left as part of the query instead of being guessed at, since it's more likely a product code than a unit this parser doesn't know. Part of #600. * feat(add-meal): normalize kg/l to g/ml before emitting UnitDropdownItem.fromString has no kg/l entry and silently falls back to g/ml for anything unrecognized, so emitting kg/l unchanged would have turned '1.5 l milk' into 1.5 g/ml instead of 1500 ml — a silent 1000x error. Converting here, before the value leaves the parser, closes that gap. Part of #600. * feat(add-meal): reject letter-less segments via FoodNameValidator Reuses the existing FoodNameValidator (issues #211/#214) instead of duplicating its rule. Errors are indexed by item number, counting only segments actually attempted — an empty segment from a trailing comma doesn't consume a number, so 'Item 1' always refers to the same thing a user would count by eye. Part of #600. * feat(add-meal): enforce the 10000 quantity bound after unit conversion Matches meal_detail_bottom_sheet.dart's manual-entry check. Order matters: the bound is checked after kg/l->g/ml conversion, so '15kg flour' is rejected as 15000 g instead of passing as an under-the- bound 15. Part of #600. * feat(add-meal): assert the unit invariant and add a dedicated test Adds a debug-mode assert in parseMealText plus a test that sweeps a batch of inputs and checks every non-null unit is one of the six UnitDropdownItem.toString() values. This is the guard #600 asks for so a future unit addition to UnitDropdownItem can't reintroduce the silent g/ml coercion this parser was built to avoid. Part of #600. * test(add-meal): close remaining checklist coverage from #600 Adds the explicit comma-decimal end-to-end value check (1,5 l milk -> 1500 ml, not just item count) and a test exercising all four separators together in one input. Part of #600. * test(add-meal): document negative-number behavior Neither quantity regex matches a leading '-', so a segment like '-5g sugar' isn't parsed as quantity -5 — it falls through to the query as-is. Adds a test making that explicit rather than leaving it an unverified side effect of the regex. Part of #600. * docs(add-meal): expand parseMealText's top-level doc comment Adds a worked example (verified against real output) and explains the tier-0/#599 framing, the never-invent-estimates constraint, and why the parser is locale-independent by construction. Mirrors the file-level doc comment style already used in json_meal_importer.dart. Part of #600. * refactor(add-meal): stop calling _normalizeUnitSymbol twice per branch _extractQuantityAndUnit checked _normalizeUnitSymbol(rawUnit) for null in the if-condition, then recomputed the same value inline for the returned unit. Hoists it into a local instead. No behavior change — verified against the full test file. Part of #600. * fix(add-meal): keep quantities on multi-word foods, leave names untouched Two defects in the new text parser, both silent — no error, no flagged row, just a wrong search intent handed downstream. Quantities were dropped from any multi-word food. The unit group was a bare `([a-zA-Z]*)`, so it greedily took the first word of the name as a candidate unit and the whole match was then discarded when that word turned out not to be one: "2 chicken breasts" -> query="2 chicken breasts" qty=null "2 boiled eggs" -> query="2 boiled eggs" qty=null "2 eggs" survived only by accident: with a single trailing word there is nothing left for `\s+` to match, which forced the engine back into the no-unit reading. Adding one more word removed that pressure. Making the group optional *and* restricting it to the five known symbols lets the engine find that reading itself, so the guard in both branches collapses to a lower-case and `_normalizeUnitSymbol` is gone. Food names were also being rewritten. Decimal commas were replaced with `.` across the whole input before segmentation, which caught commas inside the name too, so the food search received characters the user never typed: "yoghurt 3,5% fat" -> query="yoghurt 3.5% fat" "Omega 3,6,9 capsules" -> query="Omega 3.6.9 capsules" The separator regex now declines to match a decimal comma rather than rewriting it, and the conversion happens only on the number actually parsed. Both are common in the locales the comma rule exists for. Eight regression tests across two groups, each with the failure it guards written down so the constraint survives a later refactor. * fix(add-meal): reject negative quantities instead of searching for them Raised by Copilot on #616. The number pattern only matched unsigned digits, so a leading '-' stopped the number being recognised as a quantity at all rather than being caught by the `> 0` bound: "-5g sugar" -> query="-5g sugar" qty=null The segment then reached the food search as a literal query, matched nothing, and the review row would have applied the default amount — the user types -5 and silently gets 100. `0g water` already errors, and there is no reason for -5 to behave differently. Matching the sign routes the input into the existing bound, which is checked after unit conversion so "-5kg flour" is rejected too. A hyphen only reads as a sign when glued to the digits of a quantity, so hyphenated names are untouched: "low-fat milk" and "Coca-Cola" still parse as names, and a lone "-123" is still rejected for having no letters rather than for its sign. Five tests cover those boundaries. --------- Co-authored-by: Simon Oppowa <24407484+simonoppowa@users.noreply.github.com>
* feat(add-meal): scaffold meal-text parser with segmentation Adds ParsedMealItem/MealTextParseResult and parseMealText(), which for now only segments free text on comma/semicolon/newline/plus (no quantity or unit extraction yet). Implements the decimal-vs-separator comma rule from #600: a comma with a digit on both sides is a decimal point, every other comma is a separator. Part of #600. * feat(add-meal): extract leading/trailing quantity and unit per segment Handles both 100g toast and toast 100g, with or without a space before the unit letters (1.5 l milk). Only g/kg/ml/l/oz are recognized as unit symbols; an unrecognized letter run right after a number (100xyz toast) is left as part of the query instead of being guessed at, since it's more likely a product code than a unit this parser doesn't know. Part of #600. * feat(add-meal): normalize kg/l to g/ml before emitting UnitDropdownItem.fromString has no kg/l entry and silently falls back to g/ml for anything unrecognized, so emitting kg/l unchanged would have turned '1.5 l milk' into 1.5 g/ml instead of 1500 ml — a silent 1000x error. Converting here, before the value leaves the parser, closes that gap. Part of #600. * feat(add-meal): reject letter-less segments via FoodNameValidator Reuses the existing FoodNameValidator (issues #211/#214) instead of duplicating its rule. Errors are indexed by item number, counting only segments actually attempted — an empty segment from a trailing comma doesn't consume a number, so 'Item 1' always refers to the same thing a user would count by eye. Part of #600. * feat(add-meal): enforce the 10000 quantity bound after unit conversion Matches meal_detail_bottom_sheet.dart's manual-entry check. Order matters: the bound is checked after kg/l->g/ml conversion, so '15kg flour' is rejected as 15000 g instead of passing as an under-the- bound 15. Part of #600. * feat(add-meal): assert the unit invariant and add a dedicated test Adds a debug-mode assert in parseMealText plus a test that sweeps a batch of inputs and checks every non-null unit is one of the six UnitDropdownItem.toString() values. This is the guard #600 asks for so a future unit addition to UnitDropdownItem can't reintroduce the silent g/ml coercion this parser was built to avoid. Part of #600. * test(add-meal): close remaining checklist coverage from #600 Adds the explicit comma-decimal end-to-end value check (1,5 l milk -> 1500 ml, not just item count) and a test exercising all four separators together in one input. Part of #600. * test(add-meal): document negative-number behavior Neither quantity regex matches a leading '-', so a segment like '-5g sugar' isn't parsed as quantity -5 — it falls through to the query as-is. Adds a test making that explicit rather than leaving it an unverified side effect of the regex. Part of #600. * docs(add-meal): expand parseMealText's top-level doc comment Adds a worked example (verified against real output) and explains the tier-0/#599 framing, the never-invent-estimates constraint, and why the parser is locale-independent by construction. Mirrors the file-level doc comment style already used in json_meal_importer.dart. Part of #600. * refactor(add-meal): stop calling _normalizeUnitSymbol twice per branch _extractQuantityAndUnit checked _normalizeUnitSymbol(rawUnit) for null in the if-condition, then recomputed the same value inline for the returned unit. Hoists it into a local instead. No behavior change — verified against the full test file. Part of #600. * fix(add-meal): keep quantities on multi-word foods, leave names untouched Two defects in the new text parser, both silent — no error, no flagged row, just a wrong search intent handed downstream. Quantities were dropped from any multi-word food. The unit group was a bare `([a-zA-Z]*)`, so it greedily took the first word of the name as a candidate unit and the whole match was then discarded when that word turned out not to be one: "2 chicken breasts" -> query="2 chicken breasts" qty=null "2 boiled eggs" -> query="2 boiled eggs" qty=null "2 eggs" survived only by accident: with a single trailing word there is nothing left for `\s+` to match, which forced the engine back into the no-unit reading. Adding one more word removed that pressure. Making the group optional *and* restricting it to the five known symbols lets the engine find that reading itself, so the guard in both branches collapses to a lower-case and `_normalizeUnitSymbol` is gone. Food names were also being rewritten. Decimal commas were replaced with `.` across the whole input before segmentation, which caught commas inside the name too, so the food search received characters the user never typed: "yoghurt 3,5% fat" -> query="yoghurt 3.5% fat" "Omega 3,6,9 capsules" -> query="Omega 3.6.9 capsules" The separator regex now declines to match a decimal comma rather than rewriting it, and the conversion happens only on the number actually parsed. Both are common in the locales the comma rule exists for. Eight regression tests across two groups, each with the failure it guards written down so the constraint survives a later refactor. * fix(add-meal): reject negative quantities instead of searching for them Raised by Copilot on #616. The number pattern only matched unsigned digits, so a leading '-' stopped the number being recognised as a quantity at all rather than being caught by the `> 0` bound: "-5g sugar" -> query="-5g sugar" qty=null The segment then reached the food search as a literal query, matched nothing, and the review row would have applied the default amount — the user types -5 and silently gets 100. `0g water` already errors, and there is no reason for -5 to behave differently. Matching the sign routes the input into the existing bound, which is checked after unit conversion so "-5kg flour" is rejected too. A hyphen only reads as a sign when glued to the digits of a quantity, so hyphenated names are untouched: "low-fat milk" and "Coca-Cola" still parse as names, and a lone "-123" is still rejected for having no letters rather than for its sign. Five tests cover those boundaries. * feat(add-meal): resolve parsed meal items against the food search Closes #601. Takes the parser's search intents and turns each one into real food-database entries, reusing the existing search stack. Per item, both existing entry points on SearchProductsUseCase run in parallel, then mergeAndRankMeals does what only it does — dedup across sources, near-duplicate collapsing, and keeping the user's own custom meals and recipes in a tier above remote results. What it does not do is trust that ordering for selection. The shared ranker compares token sets for exact equality, so a query and a record differing only by an inflectional suffix do not intersect at all: "eggs" against a record named "Egg" scores exactly 0.0, while "Cadbury Creme Eggs" contains the literal plural and scores well above it. Typed search hides this because the user sees the list and picks. This use case does not — it auto-selects — so the same flaw becomes a silently wrong food in the diary. meal_relevance_ranker.dart is deliberately untouched. It backs the live search screen, where current behaviour is what people already rely on; changing it there is a separate change with its own risk. The resolver re-orders within the tiers that ranker established, using its own score. That score compares tokens by shared prefix rather than by equality, so a suffix difference costs a little similarity instead of all of it. No plural rules: stripping a trailing "s" works in one of nine locales and reintroduces the per-language word lists parseMealText was built to avoid. Prefix agreement is locale-independent — en eggs/Egg, de Eier/Ei, it uova/uovo, tr yumurtalar/yumurta all resolve. A floor stops apple matching apricot, relaxed for tokens shorter than the floor so "Ei" is not excluded by its own length. The comparison is symmetric, and that is load-bearing. Scoring only the query's tokens would rank "Cadbury Creme Eggs" (1.0) above "Egg" (0.75). Counting the record's unmatched tokens too drops it to 0.5. Each source is guarded separately rather than sharing one Future.wait, which fails fast: a source erroring must narrow the candidate list, never fail the item or the batch. SearchProductsUseCase already degrades internally, so this should not trigger today — it is here so a change there cannot turn a transient network error into a lost row. Items the search cannot resolve come back unresolved rather than dropped, and carry the parsed item so the review screen can show and fix them. A silent drop is the one failure mode that loses data without telling anyone. ResolvedMealItem also carries a confidence score so #602 can flag a weak match instead of presenting a guess as settled. 24 tests: 13 pure fixtures on the scorer, 11 on the use case against a hand-written fake. One asserts the shared ranker still scores eggs/Egg at 0.0, so if that file is ever fixed this fails and forces a revisit rather than passing for a new reason. * style(locator): drop unrelated reformatting from this PR Running `dart format` on locator.dart to format the new registration also rewrapped the TrendsBloc and CustomMealsBloc registrations, which this PR has no business touching — the repo has pre-existing format drift and reformatting neighbours turns a 7-line change into a noisy one. --------- Co-authored-by: Lauren <laasliu@ucsc.edu>
* feat(add-meal): scaffold meal-text parser with segmentation Adds ParsedMealItem/MealTextParseResult and parseMealText(), which for now only segments free text on comma/semicolon/newline/plus (no quantity or unit extraction yet). Implements the decimal-vs-separator comma rule from #600: a comma with a digit on both sides is a decimal point, every other comma is a separator. Part of #600. * feat(add-meal): extract leading/trailing quantity and unit per segment Handles both 100g toast and toast 100g, with or without a space before the unit letters (1.5 l milk). Only g/kg/ml/l/oz are recognized as unit symbols; an unrecognized letter run right after a number (100xyz toast) is left as part of the query instead of being guessed at, since it's more likely a product code than a unit this parser doesn't know. Part of #600. * feat(add-meal): normalize kg/l to g/ml before emitting UnitDropdownItem.fromString has no kg/l entry and silently falls back to g/ml for anything unrecognized, so emitting kg/l unchanged would have turned '1.5 l milk' into 1.5 g/ml instead of 1500 ml — a silent 1000x error. Converting here, before the value leaves the parser, closes that gap. Part of #600. * feat(add-meal): reject letter-less segments via FoodNameValidator Reuses the existing FoodNameValidator (issues #211/#214) instead of duplicating its rule. Errors are indexed by item number, counting only segments actually attempted — an empty segment from a trailing comma doesn't consume a number, so 'Item 1' always refers to the same thing a user would count by eye. Part of #600. * feat(add-meal): enforce the 10000 quantity bound after unit conversion Matches meal_detail_bottom_sheet.dart's manual-entry check. Order matters: the bound is checked after kg/l->g/ml conversion, so '15kg flour' is rejected as 15000 g instead of passing as an under-the- bound 15. Part of #600. * feat(add-meal): assert the unit invariant and add a dedicated test Adds a debug-mode assert in parseMealText plus a test that sweeps a batch of inputs and checks every non-null unit is one of the six UnitDropdownItem.toString() values. This is the guard #600 asks for so a future unit addition to UnitDropdownItem can't reintroduce the silent g/ml coercion this parser was built to avoid. Part of #600. * test(add-meal): close remaining checklist coverage from #600 Adds the explicit comma-decimal end-to-end value check (1,5 l milk -> 1500 ml, not just item count) and a test exercising all four separators together in one input. Part of #600. * test(add-meal): document negative-number behavior Neither quantity regex matches a leading '-', so a segment like '-5g sugar' isn't parsed as quantity -5 — it falls through to the query as-is. Adds a test making that explicit rather than leaving it an unverified side effect of the regex. Part of #600. * docs(add-meal): expand parseMealText's top-level doc comment Adds a worked example (verified against real output) and explains the tier-0/#599 framing, the never-invent-estimates constraint, and why the parser is locale-independent by construction. Mirrors the file-level doc comment style already used in json_meal_importer.dart. Part of #600. * refactor(add-meal): stop calling _normalizeUnitSymbol twice per branch _extractQuantityAndUnit checked _normalizeUnitSymbol(rawUnit) for null in the if-condition, then recomputed the same value inline for the returned unit. Hoists it into a local instead. No behavior change — verified against the full test file. Part of #600. * fix(add-meal): keep quantities on multi-word foods, leave names untouched Two defects in the new text parser, both silent — no error, no flagged row, just a wrong search intent handed downstream. Quantities were dropped from any multi-word food. The unit group was a bare `([a-zA-Z]*)`, so it greedily took the first word of the name as a candidate unit and the whole match was then discarded when that word turned out not to be one: "2 chicken breasts" -> query="2 chicken breasts" qty=null "2 boiled eggs" -> query="2 boiled eggs" qty=null "2 eggs" survived only by accident: with a single trailing word there is nothing left for `\s+` to match, which forced the engine back into the no-unit reading. Adding one more word removed that pressure. Making the group optional *and* restricting it to the five known symbols lets the engine find that reading itself, so the guard in both branches collapses to a lower-case and `_normalizeUnitSymbol` is gone. Food names were also being rewritten. Decimal commas were replaced with `.` across the whole input before segmentation, which caught commas inside the name too, so the food search received characters the user never typed: "yoghurt 3,5% fat" -> query="yoghurt 3.5% fat" "Omega 3,6,9 capsules" -> query="Omega 3.6.9 capsules" The separator regex now declines to match a decimal comma rather than rewriting it, and the conversion happens only on the number actually parsed. Both are common in the locales the comma rule exists for. Eight regression tests across two groups, each with the failure it guards written down so the constraint survives a later refactor. * fix(add-meal): reject negative quantities instead of searching for them Raised by Copilot on #616. The number pattern only matched unsigned digits, so a leading '-' stopped the number being recognised as a quantity at all rather than being caught by the `> 0` bound: "-5g sugar" -> query="-5g sugar" qty=null The segment then reached the food search as a literal query, matched nothing, and the review row would have applied the default amount — the user types -5 and silently gets 100. `0g water` already errors, and there is no reason for -5 to behave differently. Matching the sign routes the input into the existing bound, which is checked after unit conversion so "-5kg flour" is rejected too. A hyphen only reads as a sign when glued to the digits of a quantity, so hyphenated names are untouched: "low-fat milk" and "Coca-Cola" still parse as names, and a lone "-123" is still rejected for having no letters rather than for its sign. Five tests cover those boundaries. * feat(add-meal): resolve parsed meal items against the food search Closes #601. Takes the parser's search intents and turns each one into real food-database entries, reusing the existing search stack. Per item, both existing entry points on SearchProductsUseCase run in parallel, then mergeAndRankMeals does what only it does — dedup across sources, near-duplicate collapsing, and keeping the user's own custom meals and recipes in a tier above remote results. What it does not do is trust that ordering for selection. The shared ranker compares token sets for exact equality, so a query and a record differing only by an inflectional suffix do not intersect at all: "eggs" against a record named "Egg" scores exactly 0.0, while "Cadbury Creme Eggs" contains the literal plural and scores well above it. Typed search hides this because the user sees the list and picks. This use case does not — it auto-selects — so the same flaw becomes a silently wrong food in the diary. meal_relevance_ranker.dart is deliberately untouched. It backs the live search screen, where current behaviour is what people already rely on; changing it there is a separate change with its own risk. The resolver re-orders within the tiers that ranker established, using its own score. That score compares tokens by shared prefix rather than by equality, so a suffix difference costs a little similarity instead of all of it. No plural rules: stripping a trailing "s" works in one of nine locales and reintroduces the per-language word lists parseMealText was built to avoid. Prefix agreement is locale-independent — en eggs/Egg, de Eier/Ei, it uova/uovo, tr yumurtalar/yumurta all resolve. A floor stops apple matching apricot, relaxed for tokens shorter than the floor so "Ei" is not excluded by its own length. The comparison is symmetric, and that is load-bearing. Scoring only the query's tokens would rank "Cadbury Creme Eggs" (1.0) above "Egg" (0.75). Counting the record's unmatched tokens too drops it to 0.5. Each source is guarded separately rather than sharing one Future.wait, which fails fast: a source erroring must narrow the candidate list, never fail the item or the batch. SearchProductsUseCase already degrades internally, so this should not trigger today — it is here so a change there cannot turn a transient network error into a lost row. Items the search cannot resolve come back unresolved rather than dropped, and carry the parsed item so the review screen can show and fix them. A silent drop is the one failure mode that loses data without telling anyone. ResolvedMealItem also carries a confidence score so #602 can flag a weak match instead of presenting a guess as settled. 24 tests: 13 pure fixtures on the scorer, 11 on the use case against a hand-written fake. One asserts the shared ranker still scores eggs/Egg at 0.0, so if that file is ever fixed this fails and forces a revisit rather than passing for a new reason. * feat(add-meal): add the multi-item review screen and entry point Closes #602. Ships the tier-0 feature: type a whole meal, confirm what it matched, log it in one go. The review step is deliberate and load-bearing. Nothing reaches the diary that the user has not looked at. Today that is a correctness concern — the resolver auto-selects and can be wrong. Once a model tier lands it is also the human review AI Act Art. 50(4) and both providers' usage policies expect, so the class doc says so where someone would later be tempted to collapse it into a one-tap log. Rows carry the user's edits separately from what the resolver produced, so re-picking a candidate never discards a typed quantity. Unresolved items stay visible and skippable rather than being dropped; skipping is reversible until the batch is written. Two behaviours the issue left open, decided here: The #212 duplicate guard is intentionally not applied. It lives in MealDetailBottomSheet, not in addIntake, so a direct loop bypasses it anyway — but the interaction is also wrong for a batch, which legitimately contains repeats (two coffees, rice at lunch and dinner). A per-item "you already added this today" dialog would turn one confirmation into several. The review screen is the confirmation. Low confidence clears once the user picks a candidate. The flag reflects the resolver's doubt about its own first choice; showing it against a human selection would be noise. The whole batch is validated before any of it is written. addIntake calls double.parse with no guard, so one bad row would otherwise throw mid-loop and leave the rows before it already logged, with no rollback. Every row is checked with tryParse and the >0 && <=10000 bounds up front. Quantity and unit defaults match MealDetailScreen exactly — serving when the food has one, else 100 metric / 1 imperial — so a bulk row and a hand-added row of the same food start from the same number. 11 strings across all nine locales, appended textually to keep the ARB formatting and the existing 921-key parity intact. * fix(add-meal): convert amounts before logging and write the batch in order Review findings on #619, five from Copilot and three found while checking them. Two would have put wrong numbers in the diary. Amounts reached addIntake unconverted. Nutriment values are per gram or millilitre and IntakeEntity.totalKcal is just `amount * energyPerUnit`, so an amount entered in oz, fl.oz or servings has to be converted first. The bottom sheet does this by passing MealDetailState.totalQuantityConverted; the bulk screen passed the raw text, which stored 4 g for 4 oz of steak — roughly a 28x under-count, silent. The conversion is now a shared function both paths call, so they cannot drift apart again. Intakes were written concurrently. addIntake was `void ... async` and so could not be awaited, it did not await _updateTrackedDay, and that did not await the two calls accumulating the day's totals. Those accumulate with a read-modify-write, so overlapping writes interleave their reads and lose updates — a batch silently under-counted the day. addIntake now returns Future<void>, awaits its tracked-day update, and the submit loop awaits each row in turn. This also fixes the same pre-existing race in the QR importer's loop, which is the only other caller that writes in a loop. Also from the review: - Rows show the kcal they will log. #602 asked for it and it was missing; confirming a batch without seeing its size defeats the review step. - Unresolved rows offer Quick Add. #602 asked for it; without it an unmatched item is a dead end. - Batch validation uses the same `^\d+([.,]\d{0,2})?$` shape manual entry enforces, as an input formatter and again on submit, instead of a bare tryParse that accepted scientific notation and unbounded decimals. - The bloc is closed in dispose — registerFactory hands out a fresh instance per navigation, so the screen owns it. - Row titles use AutoSizeText with maxLines 1 and ellipsis, per the row-overflow convention in AGENTS.md. - The candidate picker's list carries a Semantics identifier. - A failed write resets the submitting flag and surfaces a message rather than leaving the button disabled. 8 tests on the converter, including the 4 oz case specifically. * fix(add-meal): address bulk review findings * style(locator): drop unrelated reformatting from this PR Running `dart format` on locator.dart to format the new registration also rewrapped the TrendsBloc and CustomMealsBloc registrations, which this PR has no business touching — the repo has pre-existing format drift and reformatting neighbours turns a 7-line change into a noisy one. * style: drop unrelated reformatting picked up by dart format Formatting the files this PR touches also rewrapped code it has no business changing — a ChoiceChip block in add_meal_screen, the MealDetailInitial super call and an emit() in meal_detail_bloc. The repo has pre-existing format drift, so running the formatter on a file reformats its neighbours too and turns a small change into a noisy one. Restored the untouched code and reapplied only the real edits: add_meal_screen drops from 96 changed lines to 34, meal_detail_bloc from 56 to 39. Also puts the meal_quantity_converter import back in alphabetical order, where replacing the unit_calc import in place had left it out of sequence. --------- Co-authored-by: Lauren <laasliu@ucsc.edu>
) * fix(l10n): make the Ukrainian and Chinese bulk-add hints parseable The placeholder in the bulk-add field is the only place the syntax is explained, so a hint the parser cannot read teaches a format that does not work. Two of the nine did exactly that: uk "2 яйця, 100г тост, чорна кава" zh "2个鸡蛋、100克吐司、黑咖啡" The Ukrainian "г" is Cyrillic ge (U+0433), not Latin "g", so "100г тост" kept its digits in the food name and went to the search as the literal query "100г тост". The Chinese hint uses 克 for gram and 、 as the separator, neither of which the parser recognises, so the entire line collapsed to one unparsed item. Ukrainian now uses a Latin "g"; the food names are untouched. Chinese drops the count and demonstrates two weights instead: "100g 吐司, 250ml 牛奶, 黑咖啡". A count cannot be shown there — Chinese puts the counter between number and noun and uses no spaces, so 2个鸡蛋 parses as one token, and forcing "2 个鸡蛋" only moves the counter into the food name. Teaching an unnatural word order to demonstrate a feature is worse than demonstrating the part that works. The new test reads every ARB off disk and asserts each hint parses into the three items it depicts, with at least two amounts recognised and no digits stranded in a food name. It fails on both old strings with the reason spelled out. * feat(add-meal): accept CJK list separators A Chinese, Japanese or Korean keyboard emits 、 ， and ； by default, so a user typing a list the only way their keyboard offers got one unparsed row no matter what the placeholder demonstrated. Fixing the hint alone would have told them to reach for a punctuation mark their keyboard does not produce. These are punctuation, not vocabulary, so this does not reintroduce the per-locale word lists the parser was designed to avoid: the set is fixed, tiny, and does not grow when a language is added. None carries a decimal meaning either, so unlike ',' they need no lookaround. The Chinese placeholder now uses ，— the comma that keyboard actually produces — rather than an ASCII one. Two things fell out for free. U+3000, the ideographic space, is already matched by \s, so "100g 吐司" resolves without further work. And the original broken hint now yields three separate rows the user can fix individually instead of one unparsable row, even though the counts in it still cannot be read. Tests cover each separator, the ideographic space, CJK and ASCII mixed in one line, and that a number on both sides of a fullwidth comma still splits — the ASCII comma cases are unchanged.
* fix(add-meal): read a bare count as servings, not grams Closes #622. Typing "2 eggs" logged two *grams* of egg — 3 kcal — because a quantity stated without a unit fell through to the metric default. A bare count means "N of them", and when the matched food carries serving data that is precisely what a serving is. So a stated quantity with no unit now selects `serving`: two eggs is two servings. The unit dropdown already offers `serving` only for foods that have the data, so this picks what was reachable anyway rather than inventing anything. When the food has no serving data there is nothing for the count to count, and no guess is better than another. Those rows keep the weight fallback and are flagged instead, so the user's eye lands on the one row that needs a decision rather than on a plausible-looking wrong number. They stay loggable — the amount and unit are editable, and blocking the batch over a unit would be worse than the wrong unit. The flag is deliberately not raised on unresolved rows: they already say they matched nothing and cannot be logged, so a second complaint about their amount is noise. Cases with no quantity at all are untouched — those still fall back to one serving expressed the way the record expresses it, matching the single-item meal-detail screen. One string across all nine locales. Five tests: the serving case, the flagged case, that a stated unit is never second-guessed, that the no-quantity default did not move, and that unresolved rows stay unflagged. * fix(add-meal): gate the count-as-serving default on a scalable serving Review on #627 caught that `hasServingValues` is not the right gate. It is true when a record carries only `servingSize` text, but `convertQuantityToBaseUnit` scales on `servingQuantity` and leaves the amount untouched without it. So `2 eggs` against such a record was relabelled `serving` and still logged 2 g -- the #622 bug wearing a unit that reads correct, with the warning suppressed because the same getter gated that too. Those records are not an edge case: Open Food Facts derives `serving_quantity` by parsing the `serving_size` text, and strings like "1 egg" or "1 slice" do not parse. The failing set is exactly the counted foods this fix is about. Gate both the unit default and the flag on `servingQuantity` instead. `hasServingValues` stays where it belongs, deciding what the dropdown offers. Also derive `amountNeedsCheck` rather than storing it. The stored flag was carried forward unchanged by `copyWith`, so switching to a candidate without serving data kept a stale "all clear" and left the wrong number unmarked. Computing it from the selected candidate removes the staleness rather than patching one path. Picking a unit by hand clears it -- the user has answered the question it was asking. * fix(add-meal): re-derive a row's amount and unit when its candidate changes Found while driving the bulk-add screen on a Pixel 6, not by any test. Making `amountNeedsCheck` derived fixed the stale flag but left the unit stored, so the two could disagree. Picking a countable record after an uncountable one cleared the "check the unit" warning while the row kept the previous candidate's weight unit: "2 chicken breasts" read 2 oz, 99 kcal, with no warning. That is the #622 failure again with the signal switched off -- worse than before, because the vanishing warning reads as confirmation. Re-derive both defaults against the newly selected food. A row that is still pristine gets the same treatment it would have had if the resolver had picked that candidate first; on device the same case now reads 2 Portion, 472 kcal. Only for a pristine row. Re-deriving under a hand-typed amount would reinterpret it -- an amount of 150 entered as grams would silently become 150 servings -- so an edited amount or a chosen unit stops the re-derive. Both directions are covered by tests and were checked on the device. * fix(add-meal): stop offering a serving unit that cannot be scaled Fourth review finding on #627, and the sharpest, because the warning this PR adds leads the user straight into it. `allowedUnits` offered `serving` whenever `hasServingValues` was true, which includes a record carrying nothing but `servingSize` text. `convertQuantityToBaseUnit` cannot scale those, so the option is a no-op: record: servingSize "1 egg", servingQuantity null initial g/ml, amountNeedsCheck = true user reads "check the unit", picks the obvious one after serving amountNeedsCheck = false, logged = 2.0 The row says "check the unit", the user picks the one that plainly means "two of them", and the app relabels it, clears the warning and logs two grams. Quieter than the bug it replaced. Gate the entry on `servingQuantity`, the same gate `_initialUnit` and `amountNeedsCheck` already use. A unit that cannot be converted is not offered; `effectiveUnit` already falls back for anything absent. Three of the four findings on this PR share one root: `hasServingValues` is not a proxy for "can be scaled", and this feature was reading it as one. The same dropdown in meal_detail_bottom_sheet.dart gates on `hasServingValues` too and has the same no-op, but it is on develop and outside this diff. Filed separately.
The screen was the one part of tier 0 with no automated coverage. The bloc had 26 tests and the parser and resolver their own, but everything the screen itself does -- validating the batch before writing, converting the amount, the sequential write loop, skip, the candidate picker -- was exercised only by driving a device by hand. That does not run in CI. Eleven tests, over the real BulkAddBloc with only the search and the write target faked, so the parse -> resolve -> row pipeline under test is the shipping one. The write assertions record what `addIntake` was actually handed rather than what the row displays. A row can read "4 oz" on screen and still hand "4" to the store, which is how 4 oz came to be logged as 4 g. Two tests document limits rather than guarantees: a store that fails mid-loop leaves earlier rows written with no rollback, and a rejected item is reported by position instead of being silently dropped. Both are the current behaviour and worth pinning so a change to either is deliberate. Checked the tests fail when the code is wrong, rather than trusting green: dropping the unit conversion fails 2, writing skipped rows fails 1, and never flagging a bare count fails 1.
This reverts commit 80f897e. Reverting the delivery, not the work. The commit was pushed straight to this integration branch, which skips the only validation this repo runs: `push` is scoped to `main` on purpose, so a direct push here gets no CI at all, and there is no review surface either. Re-landed as a PR into this branch so it gets checks and a diff to read, matching how every other change in this feature arrived.
The screen was the one part of tier 0 with no automated coverage. The bloc had 26 tests and the parser and resolver their own, but everything the screen itself does -- validating the batch before writing, converting the amount, the sequential write loop, skip, the candidate picker -- was exercised only by driving a device by hand. That does not run in CI. Eleven tests, over the real BulkAddBloc with only the search and the write target faked, so the parse -> resolve -> row pipeline under test is the shipping one. The write assertions record what `addIntake` was actually handed rather than what the row displays. A row can read "4 oz" on screen and still hand "4" to the store, which is how 4 oz came to be logged as 4 g. Two tests document limits rather than guarantees: a store that fails mid-loop leaves earlier rows written with no rollback, and a rejected item is reported by position instead of being silently dropped. Both are the current behaviour and worth pinning so a change to either is deliberate. Checked the tests fail when the code is wrong, rather than trusting green: dropping the unit conversion fails 2, writing skipped rows fails 1, and never flagging a bare count fails 1.
* fix(l10n): localize the bulk-add parser errors Closes #631. `parseMealText` built its complaints as English literals and the screen rendered them verbatim, so a user on any of the other eight locales saw "Item 2: not a valid food name" in an otherwise translated screen. Observed on a Pixel 6 in German for a stray `123`, a `0g` quantity and a value over the 10000 bound -- ordinary typos in a free-text box, not rare paths. The parser cannot reach `S.of(context)`: it is pure by design so it can be unit-tested with no I/O, and that is worth keeping. So it now reports *what* was wrong with *which* item -- a `MealTextParseError` carrying a kind, a 1-based item number and the bound it exceeded -- and the screen builds the sentence where a BuildContext exists. Three ARB keys across nine locales, with the item number as a placeholder rather than a concatenated prefix: "Item 2:" is not a prefix in every language, and Turkish wants "2. öğe" while Chinese wants "第 2 项". Also covers the gap that let this ship: a widget test pumps the screen in German and asserts the German string is shown and the English one is not, and an ARB test fails if any locale drops {number} or {bound} -- a translation that loses the index still compiles and still renders, it just stops saying which item was rejected. Verified the tests fail without the fix: rendering the raw error objects instead of the localized text fails 2 of them. * refactor(add-meal): make an unbounded "too large" error unrepresentable Review on #632: the render site read `(error.bound ?? 0).toInt()`, so a `quantityTooLarge` arriving without its bound would have told the user their quantity must be "0 or less" -- coherent, wrong, and impossible to act on. No path constructs it that way today; the shape simply allowed it. The cause was mine: one class with a `kind` enum beside a nullable `bound` that only one kind uses. That makes the invalid pairing constructible, and the `?? 0` was me noticing the smell and defaulting instead of fixing it. An assert would have caught it only in debug, leaving the misleading message in release. `MealTextParseError` is now sealed, with the data each reason needs on the subtype: only QuantityTooLargeError carries a bound, and it is required and non-null. The compiler enforces both halves -- constructing that error without a bound does not compile, and the screen's switch is exhaustive, so a fourth reason later forces the render site to handle it. `bound` is an int rather than a double: it is a cap, not a measurement, and the string it feeds takes an integer placeholder, so the conversion at the render site is gone along with the truncation it could have hidden. No behaviour change. 1082 tests still pass.
* feat(add-meal): add a model-assisted meal-text interpreter Implements #633, the first part of tier 1b of #599: a model does the language work, the database still supplies every number. `MealTextInterpreter` returns the same `MealTextParseResult` the deterministic parser returns, so an implementation is a drop-in alternative to `parseMealText` -- the resolver, the review screen and the write path do not know or care which produced the items. The provenance guarantee is structural rather than a convention. The tool schema handed to the model has no macro fields, so it has nowhere to put a calorie count and cannot supply one; a test asserts the schema exposes only query, quantity and unit. Whatever does come back then passes through `validateParsedMealItems`, the bounds `parseMealText` already enforces, so a model can never write to the diary under looser rules than a regex. That is why this tier does not reopen #250: no macro is ever invented, and every number still comes from Open Food Facts / USDA / BLS. The prompt asks for a quantity only when the user stated one. Inferring that half an avocado is about 100 g is estimation rather than parsing, and estimating mass is the first step back toward estimating nutrition. With nothing stated the field is omitted and the review row's existing serving-size default fills it, exactly as for the deterministic parser. The tool call is forced, so a prose reply is not a shape any caller has to handle. Anything unexpected -- a missing tool call, a malformed body, a non-200 -- raises `MealTextInterpreterException` rather than guessing, and carries no response body, since provider payloads can echo the submitted text into logs. Auth failures are marked non-transient so a caller can tell "your key is wrong" from "try again later". No UI, no key storage and no wiring: the key is a parameter for now. Those are #634 and #635. Nothing in the app calls this yet. * fix(add-meal): harden the interpreter against unexpected provider replies Three findings from review on #636, all confirmed. **A forced cast bypassed the exception surface.** `(block['input'] as Map?)` throws a TypeError if the provider ever returns something other than an object there, which is exactly the case the doc comment claims raises cleanly. #635 relies on `MealTextInterpreterException` to fall back to the deterministic parser; a TypeError would crash the caller instead. Checked rather than cast. **No timeout.** The other remote data sources in this repo use `.timeout(_timeoutDuration)`; this did not, so a stalled connection would hang until the OS gave up and the user would wait rather than falling back. 20 seconds, matching them. **A unit could survive with no quantity.** `ParsedMealItem` documents the two as stated together and `parseMealText` cannot produce one without the other, so downstream code was written against that. A model can produce it, and honouring a unit nobody attached a number to presents a guess as though the user typed it. Dropped, with the reasoning recorded where it happens. Three tests added, each mutation-checked: restoring the cast fails one, removing the timeout fails one, keeping the orphan unit fails one. * test(add-meal): make the interpreter timeout injectable The stalled-connection test spent the real twenty seconds proving the timeout fires, taking that file from ~4s to ~24s. Injecting the duration lets it use 50ms instead, and stops a future increase to the production value quietly making the suite slower. `defaultTimeout` is public and asserted by its own test, so what ships is still pinned at the twenty seconds the other remote data sources use — the injection point is for tests, not a way to change production behaviour by accident. Both halves mutation-checked: removing the `.timeout` fails the hang test, and shipping a 50ms default fails the pin. This came out of a review comment on #636 whose stated reason was wrong -- it claimed the test used `fakeAsync` without importing it, and the file has never referenced `fakeAsync` at all. The slow test it implied was real, so the observation was worth acting on even though the diagnosis was not.
…#634) (#640) * feat(settings): store a bring-your-own API key for AI meal assistance Implements #634, the second part of tier 1b of #599. Nothing calls the key yet -- the interpreter is #633 and the wiring is #635 -- so this changes no behaviour beyond adding a settings entry. The key is the user's own. It lives in the platform keystore via the existing hardened `FlutterSecureStorage` options, including `resetOnError: false`, so a keystore hiccup cannot silently discard it. The enabled flag is stored beside the key rather than in ConfigDBO. It is not a secret, but keeping both in one place means they cannot disagree: there is no state where the app believes the feature is on while the key it needs has been cleared. It also avoids a Hive schema migration for one bool. `isEnabled()` returns false whenever no key is stored, so no caller has to check two things to answer one question. Saving a key turns the feature on, because a second switch afterwards is a step that exists only to be forgotten. Pausing keeps the key so re-enabling does not mean finding the credential again. The key is write-only in the UI: once saved it renders as a fixed-length mask, never the value and never its real length, which would leak the provider's key format. The field is obscured with autocorrect and keyboard suggestions off, since suggestion history is a plausible place for a pasted credential to end up. The disclosure sits above the field rather than behind a link. Saving a key changes what leaves the device, and the README makes a falsifiable claim about that -- so the privacy table moves from three destinations to four, with Anthropic marked as reaching out only when a key is saved, and states plainly that only the typed line is sent, never the diary, and that every number still comes from the food databases. Nine strings across nine locales, 945 keys, no drift. Sixteen tests, and both halves were checked by mutation: unmasking the key fails a test, and letting a cleared credential stay "enabled" fails five. * style: undo an unintended reformat of settings_screen.dart The previous commit ran `dart format` over settings_screen.dart, which rewrote 993 lines for roughly 30 lines of real change. This repo's pinned SDK formats differently from what is committed, so formatting a whole file buries the actual edit and makes review impossible. Restores the committed formatting and re-applies only the real edits: the two imports, the credential field, the initState refresh, the three helpers and the settings tile. Same for locator.dart. Net effect on the tree is identical; the diff is now 40 lines instead of 993. No behaviour change, 1098 tests still pass. * fix(settings): address review on the AI key dialog Three findings from review on #640, all confirmed. **A dismissed dialog lost the change.** The switch and the remove button write immediately, but `showDialog` defaults to barrier-dismissible, so a tap outside popped `null` and the caller was told nothing had changed -- leaving the settings tile describing state that no longer existed. The dialog is no longer barrier-dismissible, and the caller now refreshes unconditionally rather than trusting the answer, so neither half can reintroduce it alone. **The README linked to a file this branch does not contain.** `anthropic_meal_text_interpreter.dart` only exists on #636's branch, so merging this one first would have shipped a broken link on the front page. Points at #633 instead, which resolves whatever the merge order turns out to be. **A Row label could overflow.** AGENTS.md is explicit that a title in a Row has to survive a long localized string and a large system font; "Ключ збережено" plus the mask is materially wider than the English. Wrapped in Expanded with maxLines and ellipsis, with a test at 2x text scale on a narrow viewport. Removing the Expanded fails it.
…red (#635) (#643) * feat(add-meal): add a model-assisted meal-text interpreter Implements #633, the first part of tier 1b of #599: a model does the language work, the database still supplies every number. `MealTextInterpreter` returns the same `MealTextParseResult` the deterministic parser returns, so an implementation is a drop-in alternative to `parseMealText` -- the resolver, the review screen and the write path do not know or care which produced the items. The provenance guarantee is structural rather than a convention. The tool schema handed to the model has no macro fields, so it has nowhere to put a calorie count and cannot supply one; a test asserts the schema exposes only query, quantity and unit. Whatever does come back then passes through `validateParsedMealItems`, the bounds `parseMealText` already enforces, so a model can never write to the diary under looser rules than a regex. That is why this tier does not reopen #250: no macro is ever invented, and every number still comes from Open Food Facts / USDA / BLS. The prompt asks for a quantity only when the user stated one. Inferring that half an avocado is about 100 g is estimation rather than parsing, and estimating mass is the first step back toward estimating nutrition. With nothing stated the field is omitted and the review row's existing serving-size default fills it, exactly as for the deterministic parser. The tool call is forced, so a prose reply is not a shape any caller has to handle. Anything unexpected -- a missing tool call, a malformed body, a non-200 -- raises `MealTextInterpreterException` rather than guessing, and carries no response body, since provider payloads can echo the submitted text into logs. Auth failures are marked non-transient so a caller can tell "your key is wrong" from "try again later". No UI, no key storage and no wiring: the key is a parameter for now. Those are #634 and #635. Nothing in the app calls this yet. * feat(settings): store a bring-your-own API key for AI meal assistance Implements #634, the second part of tier 1b of #599. Nothing calls the key yet -- the interpreter is #633 and the wiring is #635 -- so this changes no behaviour beyond adding a settings entry. The key is the user's own. It lives in the platform keystore via the existing hardened `FlutterSecureStorage` options, including `resetOnError: false`, so a keystore hiccup cannot silently discard it. The enabled flag is stored beside the key rather than in ConfigDBO. It is not a secret, but keeping both in one place means they cannot disagree: there is no state where the app believes the feature is on while the key it needs has been cleared. It also avoids a Hive schema migration for one bool. `isEnabled()` returns false whenever no key is stored, so no caller has to check two things to answer one question. Saving a key turns the feature on, because a second switch afterwards is a step that exists only to be forgotten. Pausing keeps the key so re-enabling does not mean finding the credential again. The key is write-only in the UI: once saved it renders as a fixed-length mask, never the value and never its real length, which would leak the provider's key format. The field is obscured with autocorrect and keyboard suggestions off, since suggestion history is a plausible place for a pasted credential to end up. The disclosure sits above the field rather than behind a link. Saving a key changes what leaves the device, and the README makes a falsifiable claim about that -- so the privacy table moves from three destinations to four, with Anthropic marked as reaching out only when a key is saved, and states plainly that only the typed line is sent, never the diary, and that every number still comes from the food databases. Nine strings across nine locales, 945 keys, no drift. Sixteen tests, and both halves were checked by mutation: unmasking the key fails a test, and letting a cleared credential stay "enabled" fails five. * style: undo an unintended reformat of settings_screen.dart The previous commit ran `dart format` over settings_screen.dart, which rewrote 993 lines for roughly 30 lines of real change. This repo's pinned SDK formats differently from what is committed, so formatting a whole file buries the actual edit and makes review impossible. Restores the committed formatting and re-applies only the real edits: the two imports, the credential field, the initState refresh, the three helpers and the settings tile. Same for locator.dart. Net effect on the tree is identical; the diff is now 40 lines instead of 993. No behaviour change, 1098 tests still pass. * fix(settings): address review on the AI key dialog Three findings from review on #640, all confirmed. **A dismissed dialog lost the change.** The switch and the remove button write immediately, but `showDialog` defaults to barrier-dismissible, so a tap outside popped `null` and the caller was told nothing had changed -- leaving the settings tile describing state that no longer existed. The dialog is no longer barrier-dismissible, and the caller now refreshes unconditionally rather than trusting the answer, so neither half can reintroduce it alone. **The README linked to a file this branch does not contain.** `anthropic_meal_text_interpreter.dart` only exists on #636's branch, so merging this one first would have shipped a broken link on the front page. Points at #633 instead, which resolves whatever the merge order turns out to be. **A Row label could overflow.** AGENTS.md is explicit that a title in a Row has to survive a long localized string and a large system font; "Ключ збережено" plus the mask is materially wider than the English. Wrapped in Expanded with maxLines and ellipsis, with a test at 2x text scale on a narrow viewport. Removing the Expanded fails it. * fix(add-meal): harden the interpreter against unexpected provider replies Three findings from review on #636, all confirmed. **A forced cast bypassed the exception surface.** `(block['input'] as Map?)` throws a TypeError if the provider ever returns something other than an object there, which is exactly the case the doc comment claims raises cleanly. #635 relies on `MealTextInterpreterException` to fall back to the deterministic parser; a TypeError would crash the caller instead. Checked rather than cast. **No timeout.** The other remote data sources in this repo use `.timeout(_timeoutDuration)`; this did not, so a stalled connection would hang until the OS gave up and the user would wait rather than falling back. 20 seconds, matching them. **A unit could survive with no quantity.** `ParsedMealItem` documents the two as stated together and `parseMealText` cannot produce one without the other, so downstream code was written against that. A model can produce it, and honouring a unit nobody attached a number to presents a guess as though the user typed it. Dropped, with the reasoning recorded where it happens. Three tests added, each mutation-checked: restoring the cast fails one, removing the timeout fails one, keeping the orphan unit fails one. * feat(add-meal): read the meal line with a model when a key is configured Implements #635, the last part of tier 1b of #599. Closes #623. With no key stored, nothing changes anywhere: `parseMealText` remains the only path and the screen behaves exactly as it did. `ReadMealTextUseCase` owns the choice. Keeping the policy out of the bloc means it can be tested without a widget tree, and the bloc keeps one collaborator rather than three. Whatever the outcome, it returns a result: a missing key, a paused switch, no network, a rejected credential, a rate limit, a changed API or an outright bug in the interpreter all land the user on the tier 0 experience with no error to dismiss. The model is an improvement to a working feature, never a dependency of it. It also prefers the parser when the model returns nothing the parser did not. `100g toast` is unambiguous, and an empty screen after a network round trip is worse than the offline answer. Model-read batches are marked in the review list. The confirmation step was already load-bearing -- see the doc comment on BulkAddScreen -- but it is only meaningful if the user knows what did the reading, and once a model is involved that step is also the human review AI Act Art. 50(4) expects. Closes #623 as a side effect. Counts in languages with no spaces between words (`2个鸡蛋`) cannot be expressed by the deterministic parser without per-locale word lists, which #600 refused on the grounds that they cost nine files every time they grow. A model needs none. No provenance change: no new MealSourceEntity value, no export-schema change, no "estimated" marker. Every number still comes from the food databases, so there is nothing to flag. Nine tests, mutation-checked: removing the fallback fails two, ignoring the disabled switch fails one. * test(add-meal): make the interpreter timeout injectable The stalled-connection test spent the real twenty seconds proving the timeout fires, taking that file from ~4s to ~24s. Injecting the duration lets it use 50ms instead, and stops a future increase to the production value quietly making the suite slower. `defaultTimeout` is public and asserted by its own test, so what ships is still pinned at the twenty seconds the other remote data sources use — the injection point is for tests, not a way to change production behaviour by accident. Both halves mutation-checked: removing the `.timeout` fails the hang test, and shipping a 50ms default fails the pin. This came out of a review comment on #636 whose stated reason was wrong -- it claimed the test used `fakeAsync` without importing it, and the file has never referenced `fakeAsync` at all. The slow test it implied was real, so the observation was worth acting on even though the diagnosis was not. * style: match the surrounding registration style in locator.dart Raised as a suppressed comment on #640, and correct. I hand-wrote this registration in the pre-3.x style after the formatter rewrote settings_screen.dart wholesale on this branch, on the assumption that locator.dart was in that older style too. It is not: 96 lines in the file already use the trailing-comma form and only two did not, one of them mine, sitting directly between two registrations that use it. So the line was the outlier rather than the match, and would have reflowed on the next format run. Fixed by hand rather than by formatting the file -- the formatter still wants 12 other lines here, none of them mine. The lesson from the earlier churn was "match the surrounding committed code", and I over-applied it into "assume everything is the old style".
…) (#645) * fix(add-meal): reuse one http client, and reject non-finite quantities Two findings from review on #643. Both landed on the integration branch before the review was read, so they are fixed here rather than there. **A new http.Client per interpret call.** The locator's interpreter factory built one each time, and nothing closes it. Every tap of Search on the bulk screen would leave another connection pool behind for the life of the process. Registered as a lazy singleton and reused. **NaN slipped past the bounds checks.** `quantity <= 0` and `quantity > _maxQuantity` are both *false* for NaN, so a non-finite value passed validation untouched and reached the row, where `_trimZeros` renders it as the literal text "NaN". This is reachable: `double.tryParse('NaN')` returns NaN, so a model answering with that string is enough. Non-finite quantities are now treated as no quantity at all -- dropped rather than rejected, matching how an unrecognized unit is handled, so the food stays usable and the row's own default fills in. Infinity and negative infinity go the same way; only NaN was actually slipping through, since the infinities are caught by the existing comparisons. The screen's submit validation would have blocked "NaN" from reaching the diary, so this was a broken row rather than a bad number in the log. Fixing it at the validator keeps every source held to one gate. Mutation-checked: removing the finite check fails two tests. * fix(add-meal): parse and rank languages that write without spaces Closes #623. Probing the shipped behaviour turned up more than the issue described, and one thing it got wrong. The issue said `zh` supported "weights with Latin unit symbols". It did not: `100g吐司` produced no quantity either, because `_leadingQuantity` requires whitespace between the unit and the food. A Chinese user got nothing unless they typed a space they would never write. And the ranker had the same blind spot, which nobody had looked at: score(query=鸡蛋 vs meal=土鸡蛋) = 0.000 A superstring of the query scored zero, so even a perfectly clean CJK query could miss the product it was looking at. `_tokenize` splits on non-letters, so a CJK phrase arrives as one token and the shared-prefix rule reads it as a single long word. Two fixes, neither a vocabulary list: - A Unicode *script* boundary counts as a token boundary, so a number sits flush against what it counts. This is the property that ruled number-words out in #600 -- a script set does not grow when a language is added. - CJK tokens are compared by character bigrams rather than shared prefixes, the way whitespace tokens are compared in Latin scripts. The second one is what removes any need for a list of measure words: `个鸡蛋` matches `鸡蛋` at 0.667, so the counter the parser leaves behind stops mattering. Also adds 克 / 毫升 / 千克 / 升 as unit symbols. This is a small per-script addition rather than per-language, and without it `100克吐司` parses as a bare count of 100 -- which the review row can read as 100 servings, worse than not parsing at all. Latin behaviour is unchanged and re-checked, since the boundary rule touches the regex the #616 backtracking bug lived in. All three changes mutation-checked: removing the boundary fails five tests, the Han symbols two, the bigrams two. * fix(add-meal): hang the unit off the sanitized quantity, not the raw one Review on #644, and the finding is against my own fix in this PR. Sanitizing a non-finite quantity to null left the unit check reading `candidate.quantity` — the raw value — so `{quantity: NaN, unit: 'g'}` came out as a null quantity with a `g` still attached. That is exactly the unit-without-quantity state the rule three lines above was added to prevent, two PRs ago, at the same reviewer's request. Gated on the sanitized value. Mutation-checked: reading the raw quantity again fails the new test. * docs: stop the unit-symbol comment claiming there are five Review on #645. Adding the Han symbols took the list to nine and left the comment saying "five", which would have misled the next person to touch it. Rewritten to describe the ordering rule instead of counting entries, so it stays true the next time one is added: longest-first, `毫升` before `升` and `千克` before `克`.
#644) * fix(add-meal): reuse one http client, and reject non-finite quantities Two findings from review on #643. Both landed on the integration branch before the review was read, so they are fixed here rather than there. **A new http.Client per interpret call.** The locator's interpreter factory built one each time, and nothing closes it. Every tap of Search on the bulk screen would leave another connection pool behind for the life of the process. Registered as a lazy singleton and reused. **NaN slipped past the bounds checks.** `quantity <= 0` and `quantity > _maxQuantity` are both *false* for NaN, so a non-finite value passed validation untouched and reached the row, where `_trimZeros` renders it as the literal text "NaN". This is reachable: `double.tryParse('NaN')` returns NaN, so a model answering with that string is enough. Non-finite quantities are now treated as no quantity at all -- dropped rather than rejected, matching how an unrecognized unit is handled, so the food stays usable and the row's own default fills in. Infinity and negative infinity go the same way; only NaN was actually slipping through, since the infinities are caught by the existing comparisons. The screen's submit validation would have blocked "NaN" from reaching the diary, so this was a broken row rather than a bad number in the log. Fixing it at the validator keeps every source held to one gate. Mutation-checked: removing the finite check fails two tests. * fix(add-meal): hang the unit off the sanitized quantity, not the raw one Review on #644, and the finding is against my own fix in this PR. Sanitizing a non-finite quantity to null left the unit check reading `candidate.quantity` — the raw value — so `{quantity: NaN, unit: 'g'}` came out as a null quantity with a `g` still attached. That is exactly the unit-without-quantity state the rule three lines above was added to prevent, two PRs ago, at the same reviewer's request. Gated on the sanitized value. Mutation-checked: reading the raw quantity again fails the new test.
) * fix(add-meal): stop the model mis-mapping units it was not offered Found by running 105 probes against the live API — the first time any request in this feature has actually been sent. Every one of these passed the offline suite, because a faked client cannot tell you what the model does with a schema it finds constraining. The tool schema's unit enum held only the app's *output* units, and the prompt said "never convert". Given `1.5 l milk` the model could not report a litre and would not convert, so it reported `1.5 ml` — a thousandfold under-count. Nothing flagged it, because a unit *was* stated. 1.5 l milk 1.5 ml -> 1500 ml 0.5 l water 0.5 ml -> 500 ml 1kg flour 1 -> 1000 g 1kg 500g flour 1.5 -> 1500 g 2 tbsp olive oil 2 g/ml -> 2 (bare count, and now flagged) 1 tsp sugar 1 g/ml -> 1 (bare count) Three changes: - `validateParsedMealItems` normalizes kg/l/lb the way parseMealText does, before the bounds are applied, so `15 kg` is still rejected as 15000 g. Previously it dropped them on the reasoning that nothing had normalized them, which was true of the parser and false of a model. - The schema offers the units the app can convert, and the prompt tells the model to give the number and omit the unit when the user's unit is not among them. A bare `2` is a number the review row already questions; `2 g` for two tablespoons is not. - `amountNeedsCheck` now also fires when a stated unit is *substituted*. The model answers "three slices of bread" as `3 serving`, and on a record with no scalable serving `effectiveUnit` quietly made that 3 g/ml. The old condition only looked for a *missing* unit, so nothing warned. Residual, documented rather than fixed: `1 pound of mince` still comes back as `1 oz` even with `lb` in the enum. That one needs a prompt example, and the run-to-run variation on `3x100g` shows the same input can pick different units on different calls — so the flag matters more than any single prompt wording. Probe harness in tool/, deliberately outside test/ so the suite stays offline. Findings in docs/live-interpreter-probe.md. * test(add-meal): probe the interpreter with a 1000-line generated corpus Per-case expectations do not scale past a few dozen lines, so this checks invariants that must hold for *any* input, a differential against the deterministic parser, and repeat stability. 1000 lines across nine locales, built from foods people actually log (griechischer Joghurt, борщ, mercimek çorbası, 饺子) crossed with the phrasings they use. Results with the unit fix in place: call failures 0 parser disagreements 0 invariant violations 0 (see below) unstable on repeat 1 of 40 latency median 976ms, p95 3387ms, max 9249ms The differential is the one that matters. It compares the model against `parseMealText` wherever the parser extracted a quantity and unit confidently, and it is exactly the check that would have caught `1.5 l -> 1.5 ml` automatically rather than by eye. Zero disagreements over 1000 lines is real evidence the unit handling now holds. The run reported five violations, all of which were my check being wrong rather than the model: `protein` matched "protein shake", "protein tozu" and "Proteinshake" — foods the user typed, reported as leaked nutrition values. The pattern now covers only words that never appear in a food name. A check that cries wolf on ordinary input is worse than no check. Instability is down to 1 in 40 but not gone: the same line answered with a quantity once and without it the next time. That is the standing argument for the row-level flag over prompt wording — the model's answer is not stable, so the review row has to catch a bad one. * fix(add-meal): actually put lb in the schema enum Review on #646 caught that the PR description and the code disagreed, and the description was the wrong one. The edit that was meant to add `lb` targeted a multi-line form of the enum that the formatter had already collapsed onto one line. It was written without an assertion, so it matched nothing and changed nothing, silently. The test I added at the same time asserted the enum *without* `lb`, so it passed and confirmed the wrong thing. The consequence was a false claim: I reported that "1 pound of mince" came back as "1 oz" despite `lb` being available. It was never available. With the enum actually carrying it: 1 pound of mince -> mince 453.59237 g 2 lb chicken -> chicken 907.18474 g 1.5 pounds of beef -> beef 680.388555 g 8 oz steak -> steak 8 oz (unchanged) So the sixteenfold under-count is gone, and the residual I documented as a model limitation was my own missing enum entry. Also fixes the probe's usage line, which said `<key>` while the script expects a path to a key file — an invitation to paste a credential onto a command line, which is the one thing these tools are built to avoid. * chore: stop committing the interpreter probe reports They are generated snapshots of a non-deterministic model. The corpus report states "1 line in 40 unstable"; another run gives a different number. A committed file asserting "0 violations, 0 disagreements" is true for one run of one model version against one prompt, and becomes quietly wrong the moment any of those moves — with nobody able to refresh it without an API key. The tools are the durable artifact. The findings belong in the PR that caused them, where they are dated and attached to the change, and they are already there. Ignored rather than deleted, so a future run does not put a stale snapshot back by accident. * chore: drop the corpus harness from the tree It answered its question — 1000 generated lines, zero disagreements with the deterministic parser — and that answer is recorded in #646 where it is dated and attached to the change that needed it. Keeping it costs more than it returns. It needs a paid API key, so nobody but the maintainer can run it; it cannot be a CI gate because the thing it measures is non-deterministic; and it imports the interpreter and the validator directly, so every refactor of those breaks 459 lines that no signal tells anyone how to fix. Nine locales of food lists and phrasing templates would read as stale within a year. `live_interpreter_probe.dart` stays. It encodes expected behaviour — "half an avocado" has no quantity, no macro word may appear in a query, the schema exposes exactly query/quantity/unit — which documents the model contract whether or not it is ever run. The corpus tool encodes a generator, which teaches nothing when read and cannot be run. Recoverable from this commit's parent if the question ever needs asking again. * chore: keep the live probe harnesses out of the repo Neither harness belongs in the tree. Both need a paid API key, so nobody but the maintainer can run them; neither can gate CI, because what they measure moves between identical calls; and both import the interpreter and validator directly, so every refactor of those breaks code that no signal tells anyone how to fix. What they found is recorded in #646 — the thousandfold litre under-count, the unit substitution, the missing `lb` — attached to the change that fixed each one, which is where a point-in-time measurement belongs. The behaviour worth keeping is already asserted offline and runs in CI: the tool schema exposes exactly query/quantity/unit and nothing else, kg/l/lb normalize before the bounds are applied, an unconvertible unit is dropped while the number survives, and a substituted unit raises the row's check flag. Also drops the .gitignore stanza for the reports, which described output of tools that are no longer here. * fix(add-meal): extract pounds offline, and do not trust unit casing Four findings from review on #646, all confirmed by running the code. Two matter. **`lb` was normalizable but not extractable.** Adding it to `_normalizeUnitAndQuantity` without adding it to `_unitSymbol` left the deterministic parser unable to see it, so `2 lb chicken` sent the query "lb chicken" to the food search with the amount as a bare count. Added to the symbol list, before the single-letter `l` so the alternation cannot match `l` and strand a `b`. **`validateParsedMealItems` trusted the casing it was given.** It documents its input as untrusted and `parseMealText` lower-cases what it extracts, but this entry point did not — so a model answering `KG` had its unit dropped and its number survive as a bare count. Two instead of 2000 g: the thousandfold under-count this PR exists to prevent, reached through casing alone. The other two are comments that had drifted from the code: the `amountNeedsCheck` doc described only the missing-unit case although the logic now also flags a substituted one, and the schema rationale named `l` and `kg` after `lb` joined them. Both behaviour fixes are mutation-checked: removing `lb` from the symbol list fails two tests, removing the lower-casing fails one.
Found on a Pixel 6 with a real key. Typing "meine Steuererklärung und ein Tacker" produced a row for "Meine Käsebrötchen", flagged as a doubtful match, which the user then has to skip. The model had it right: it returned an empty list, because a tax return and a stapler are not food. `ReadMealTextUseCase` then discarded that and used the parser instead, and the parser will turn any sentence with a letter in it into a search query. The rule conflated two different things. A model that *could not answer* — no network, a rejected key, a malformed reply — should fall back, and still does. A model that *did* answer, with an empty list, made the judgment it was asked to make, and overriding it means the app claims to have found food in a sentence about stationery. I wrote that rule thinking about `100g toast`, where an empty screen after a network round trip is worse than the offline answer. I did not consider that empty can be correct. Worth weighing against this: the 1000-line corpus returned 29 empty results on lines that were generated from food templates, so a model answering nothing about real food is not rare. Those users now see "nothing to log" rather than parser rows. That is an honest failure they can see and act on, rather than a silent substitution of a different reader whose output they cannot distinguish — but it is a real trade and worth a second opinion. Mutation-checked: restoring the override fails the new test.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces AI-assisted (opt-in BYO Anthropic key) and fully-offline multi-item meal logging to reduce add-meal friction, while keeping all nutrition provenance anchored to existing food databases.
Changes:
- Adds a new bulk-add flow (parser → resolver → review UI → sequential logging) to log multiple items from one text entry.
- Adds optional model-based text interpretation (Anthropic) gated by secure key storage + Settings UI, with hard fallback to deterministic parsing on any failure.
- Refactors quantity conversion + intake writing to avoid unit-conversion drift and to make multi-write flows await tracked-day updates.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit_test/resolver_relevance_test.dart | Unit tests for resolver-specific relevance scoring (inflection/script tolerance). |
| test/unit_test/resolve_parsed_meals_usecase_test.dart | Unit tests for resolving parsed items via existing search stack (merge/rank, isolation, concurrency). |
| test/unit_test/read_meal_text_usecase_test.dart | Unit tests for parser-vs-model selection and fallback policy. |
| test/unit_test/meal_quantity_converter_test.dart | Unit tests for base-unit conversion and kcal scaling. |
| test/unit_test/bulk_add_hint_test.dart | Verifies ARB placeholder hints remain parseable across locales and preserve error placeholders. |
| test/unit_test/bulk_add_bloc_test.dart | Unit tests for BulkAddBloc row derivation/editing/warning behavior. |
| test/unit_test/anthropic_meal_text_interpreter_test.dart | Unit tests for Anthropic request/response parsing, schema constraints, validation, and failure modes. |
| test/unit_test/ai_credential_storage_test.dart | Unit tests for secure key storage semantics (masking, enable/disable, clear). |
| test/features/settings/presentation/ai_assist_dialog_test.dart | Widget tests for the AI assistance settings dialog behavior and layout safety. |
| test/features/add_meal/presentation/bulk_add_screen_test.dart | Widget tests for bulk-add UI flow and correctness of writes (including conversions). |
| README.md | Updates privacy “what leaves your device” table and adds AI assistance disclosure. |
| lib/main.dart | Registers the new bulk-add route. |
| lib/l10n/intl_zh.arb | Adds bulk-add + AI assistance strings (ZH). |
| lib/l10n/intl_uk.arb | Adds bulk-add + AI assistance strings (UK). |
| lib/l10n/intl_tr.arb | Adds bulk-add + AI assistance strings (TR). |
| lib/l10n/intl_sk.arb | Adds bulk-add + AI assistance strings (SK). |
| lib/l10n/intl_pl.arb | Adds bulk-add + AI assistance strings (PL). |
| lib/l10n/intl_it.arb | Adds bulk-add + AI assistance strings (IT). |
| lib/l10n/intl_en.arb | Adds bulk-add + AI assistance strings + placeholders (EN). |
| lib/l10n/intl_de.arb | Adds bulk-add + AI assistance strings (DE). |
| lib/l10n/intl_cs.arb | Adds bulk-add + AI assistance strings (CS). |
| lib/features/settings/settings_screen.dart | Adds Settings tile + state refresh for AI assistance configuration. |
| lib/features/settings/presentation/widgets/ai_assist_dialog.dart | Adds dialog to save/pause/remove the user’s Anthropic API key with disclosure + semantics IDs. |
| lib/features/meal_detail/util/meal_quantity_converter.dart | New shared converter used by meal-detail and bulk-add paths. |
| lib/features/meal_detail/presentation/bloc/meal_detail_bloc.dart | Uses shared quantity converter; makes addIntake async/await tracked-day updates. |
| lib/features/add_meal/util/resolver_relevance.dart | Adds resolver-only ranking/scoring tolerant of inflection/suffixes and unspaced scripts. |
| lib/features/add_meal/util/meal_text_parser.dart | Adds deterministic multi-item parser + shared validation to enforce invariants for untrusted sources. |
| lib/features/add_meal/presentation/screens/bulk_add_screen.dart | New bulk-add screen with review UI, warnings, semantics IDs, and sequential intake writing. |
| lib/features/add_meal/presentation/bloc/bulk_add_state.dart | Bulk-add bloc states including parse errors + “read by model” flag. |
| lib/features/add_meal/presentation/bloc/bulk_add_event.dart | Bulk-add bloc events for parsing and row edits. |
| lib/features/add_meal/presentation/bloc/bulk_add_bloc.dart | BulkAddBloc row modeling, default derivation, and edit semantics. |
| lib/features/add_meal/presentation/add_meal_screen.dart | Adds app-bar action to open bulk-add screen. |
| lib/features/add_meal/domain/usecase/resolve_parsed_meals_usecase.dart | Resolves parsed items via OFF + Supabase search, merges, and ranks for auto-selection. |
| lib/features/add_meal/domain/usecase/read_meal_text_usecase.dart | Centralizes “use model or parser” policy with safe fallback behavior. |
| lib/features/add_meal/domain/meal_text_interpreter.dart | Defines interpreter contract + exception surface for fallback. |
| lib/features/add_meal/data/anthropic_meal_text_interpreter.dart | Anthropic Messages API implementation with forced tool schema and strict parsing. |
| lib/core/utils/navigation_options.dart | Adds bulkAddRoute. |
| lib/core/utils/locator.dart | Registers secure AI credential storage, shared http client, interpreter use case, bulk-add bloc, resolver use case. |
| lib/core/utils/ai_credential_storage.dart | Implements secure storage for API key + enabled flag with masking. |
| .github/workflows/default_workflow.yml | Enables CI on feature/** branches for long-lived integration branches. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1
to
+4
| import 'package:opennutritracker/core/utils/calc/unit_calc.dart'; | ||
| import 'package:opennutritracker/features/add_meal/domain/entity/meal_entity.dart'; | ||
| import 'package:opennutritracker/features/meal_detail/presentation/bloc/meal_detail_bloc.dart'; | ||
|
|
Comment on lines
+568
to
+572
| if (!mounted) return; | ||
| setState(() => _submitting = false); | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar(content: Text(S.of(context).bulkAddSearchFailedLabel)), | ||
| ); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lib/features/meal_detail/util/meal_quantity_converter.dart:3
meal_quantity_converter.dartdepends onmeal_detail_bloc.dartjust to accessUnitDropdownItem. This couples a util to a presentation-layer bloc and makes reuse harder; the converter can compare the serialized unit strings directly and avoid the import entirely.
import 'package:opennutritracker/core/utils/calc/unit_calc.dart';
import 'package:opennutritracker/features/add_meal/domain/entity/meal_entity.dart';
import 'package:opennutritracker/features/meal_detail/presentation/bloc/meal_detail_bloc.dart';
lib/features/meal_detail/util/meal_quantity_converter.dart:27
convertQuantityToBaseUnitcurrently matches units viaUnitDropdownItem.*.toString(). If other call sites pass the alternative spellingfl oz(which exists elsewhere in the codebase), this will skip the conversion. Matching on the canonical strings directly (and accepting bothfl ozandfl.oz) keeps conversion robust while removing the dependency onUnitDropdownItem.
if (unit == UnitDropdownItem.serving.toString()) {
final servingQuantity = meal.servingQuantity;
// A meal with no serving data can't be scaled — leave the amount alone
// rather than guessing, matching UpdateKcalEvent.
return servingQuantity != null ? quantity * servingQuantity : quantity;
lib/features/add_meal/presentation/bloc/bulk_add_bloc.dart:168
BulkAddRow'spropsomits most ofresolved(candidates, confidence, parsed quantity/unit, etc.). BecauseBulkAddLoadedStateusesEquatableandBlocBuilderrebuilds based on state equality, re-parsing/resolving that produces different candidates or confidence can be treated as "no change" and leave the UI showing stale data (including low-confidence warnings and candidate-picker content).
@override
List<Object?> get props => [
resolved.parsed.query,
selectedIndex,
amountText,
#650) * refactor(add-meal): give the Anthropic paths one request and one schema Pulls the endpoint, headers, forced tool call, response parsing and — most importantly — the tool schema out of AnthropicMealTextInterpreter into AnthropicMealItemsApi. The interpreter is now its prompt and nothing else. The schema is the reason. It is the app's provenance guarantee in executable form: it has no macro fields, so a model has nowhere to put a calorie count. That guarantee is only worth having if reviewing it is one question — "did anyone add a field to this schema?" — rather than one question per caller. A second path with its own copy would make it two, and the copies would drift. Also renames MealTextInterpreterException to MealInterpreterException, since nothing about it was ever text-specific. The old name is re-exported from meal_text_interpreter.dart so existing imports are untouched. No behaviour change: all 34 existing tests pass with only the mechanical rename applied to them. * feat(add-meal): read a meal photo into the multi-item screen (tier 2b) Photograph a plate, get the foods back as review rows. The camera action appears on the multi-item add screen only when an API key is enabled; everything downstream — resolving against Open Food Facts / USDA / BLS, the review rows, the write path — is the pipeline tier 1b already uses. The reader changes, the row logic does not. A photo may count, never measure -------------------------------- The text path can return "100 g" because the user typed it. A photograph states nothing, so a gram figure read off a picture of a plate is estimation wearing the costume of a measurement — and it reaches the review screen looking exactly as confident as a number the user typed. Counting discrete items is a different act: two eggs are two eggs, and the user can see whether the count is right. So the prompt asks for counts only, and the interpreter discards any amount that comes back carrying a unit. Dropping only the *unit* would be worse than useless: an estimated `200 g` of rice stripped to a bare `200` reads downstream as a count, and BulkAddBloc turns a bare count into servings. The quantity goes with it, and the row falls back to the same default an unquantified item gets. That rule is enforced in code, not asked for in the prompt. Mutation- checked both ways: stripping only the unit, and skipping the filter entirely, each fail tests. Failures are shown, not swallowed --------------------------------- ReadMealTextUseCase falls back to the deterministic parser because text always has one underneath it. A photo does not — there is no offline way to turn a picture into food names — so ReadMealPhotoUseCase returns a sealed result instead, and the screen says what happened. A rejected key reads differently from a rate limit: "try again later" is the wrong advice for a wrong key, and following it never stops being wrong. An empty answer stays an answer, not a failure (#647): the model looked and found no food. The photo is never stored ------------------------- Encoded to WebP in memory and sent for one request. Deliberately not UserImageStorage, which this otherwise resembles — sharing that code would mean sharing its file write, and a meal photo landing in the documents directory is a photo the export zip picks up and the user never asked to keep. README privacy table and the settings disclosure updated to match, in all 9 locales. Adds no dependency: image_picker and flutter_image_compress are already here for meal and recipe photos. Refs #599 * fix(add-meal): stop offering a refused photo as retryable A 149-image corpus run against the live API turned up JPEGs the provider refuses outright: a photograph carrying Adobe APP14 markers came back 400 on every attempt, while the same picture re-encoded went through. Those landed on "couldn't read the photo, check your connection and try again" — advice that can never work, because nothing about the connection is wrong and the next attempt fails identically. They now land on "couldn't use that image, try another photo", which is the one thing that does work. MealPhotoFailed carried a bool, which had room for exactly two outcomes and this is a third. It now carries MealPhotoFailure, so the bloc switches exhaustively and a fourth outcome cannot be quietly folded into an existing one. Worth noting what this says about MealPhotoEncoder: re-encoding to WebP is not only a size optimisation, it normalises images the provider would otherwise reject. The raw-bytes fallback, taken when a device has no WebP encoder, is the path that can still hit this — and now it fails legibly. Refs #599 * fix(add-meal): delete the picker's photo copy after reading it Driving the flow on a Pixel 6 falsified the disclosure this feature ships with. `image_picker` does not hand back the user's original file: it copies the chosen photo into the app's cache directory and returns that path, and it never cleans the copy up. After one pick the full JPEG was still sitting in cache/, byte for byte. The settings text says the photo is "never saved — not on this device", and the README said the app "never writes it to disk". Both were false, in the one part of this project where a privacy claim is meant to be checkable. MealPhotoEncoder.encodeAndDiscardSource now removes that copy once the photo is encoded, in a finally so it goes whether or not the request succeeded — a photo the provider rejected is exactly as unwelcome on disk as one it read. Re-verified on the device: the picker's temp directory is empty after a read, and no image exists anywhere in the app's storage outside the food-thumbnail cache. The README now describes the mechanism rather than asserting the absence, since "the picker makes a copy and we delete it" is the checkable claim and "nothing is ever written" was not. The documents directory was never involved, so the "not in your exports" half of the claim was true throughout. Refs #599 * feat(l10n): say when a photo contained no food A photo the model found nothing in showed "Nothing to log yet" — the same line an untouched screen shows. Confirmed on a Pixel 6: the user hands the app a photograph, waits, and gets back a message that reads as though nothing happened. It is worse than merely vague. The notice that says a model did the reading is only drawn when there are rows, so the empty case was the one screen where a machine had answered and nothing on it said so. Adds bulkAddPhotoNoFoodLabel across all nine locales and branches only when the read came from a photo, so the text path keeps its own wording. Parse errors still outrank both — a bad segment explains itself. Covered by two widget tests, one per direction: the photo line appears where it should and stays off the text path. Both mutation-checked. Refs #599 * fix(add-meal): treat a fraction as a measurement, and stop the disclosure overclaiming Three findings from review. **A fractional quantity is not a count.** `_countsOnly` dropped amounts carrying a unit but let a bare `1.5` through, which becomes "1.5 serving" downstream — a proportion the model estimated, wearing the confidence of something it counted. A 447-call corpus over 149 photos returned 109 quantities and every one was an integer, so this has never happened. That is the same reason `_countsOnly` itself has never fired, and the same reason to keep it: the guarantee is what stays true when a model version changes. The check is on the value rather than the JSON, since 2 and 2.0 are indistinguishable on the wire. **The disclosure overclaimed.** It said the photo is "never saved — not on this device", but the user picked it out of their own gallery, so of course it is on the device. The claim the app can actually make is that it keeps no copy of its own and the photo never reaches an export. Reworded in all nine locales; the README already said it this way after the Pixel run. **`_rawBytes` loaded oversized files before discarding them.** Now the length is checked first. Nothing observable changes — `_fitting` already rejected them — but the fallback path is the device with no WebP encoder, handling the camera's raw output, and that is the worst place to allocate eight megabytes in order to throw it away. A fourth comment claimed the `switch (reading)` cases fail to compile without an explicit break. That is the Dart 2 rule; Dart 3 pattern switches do not fall through. Left as written — it analyzes clean, four tests drive all three branches, and this commit's build ran on a Pixel 6. Refs #599 * fix(add-meal): align photo capture with review
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
lib/features/add_meal/domain/usecase/resolve_parsed_meals_usecase.dart:75
resolve()starts one Future per item (and each item starts two searches), which can burst into 2×N parallel remote calls for long inputs. That can easily trigger OFF/Supabase rate limits and makes the feature less reliable on slow connections. Consider bounding concurrency (e.g., resolve in batches) so a pasted list can’t overwhelm the network stack.
Future<List<ResolvedMealItem>> resolve(List<ParsedMealItem> items) async {
if (items.isEmpty) return const [];
return Future.wait(items.map(_resolveOne));
}
lib/features/meal_detail/util/meal_quantity_converter.dart:34
convertQuantityToBaseUnitonly converts fluid ounces for the'fl.oz'spelling. Elsewhere the codebase accepts both'fl oz'and'fl.oz'(seeUnitDropdownItem.fromStringand other conversion helpers), so a persisted/imported unit of'fl oz'would skip conversion and store an incorrect base amount.
if (unit == UnitDropdownItem.oz.toString()) {
return UnitCalc.ozToG(quantity);
}
if (unit == UnitDropdownItem.flOz.toString()) {
return UnitCalc.flOzToMl(quantity);
}
Comment on lines
+150
to
+155
| /// Returns a future so callers writing more than one intake can await | ||
| /// each in turn. The tracked-day totals are accumulated with a | ||
| /// read-modify-write (`TrackedDayDataSource.addDayCaloriesTracked`), so | ||
| /// overlapping calls interleave their reads and silently lose updates — | ||
| /// a batch logged concurrently under-counts the day. | ||
| Future<void> addIntake( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the AI-assisted meal logging feature (#599) to
developas one reviewable change. Sixteen commits, staged onfeature/ai-assisted-meal-loggingsince 9 August so it could be reviewed whole rather than in fragments.41 files, +6369/−41. Zero commits behind
develop.What a user gets
An appbar action on the add-meal screen opens a new multi-item screen. Type
100g Toast, 2 Eier, schwarzer Kaffee, confirm one screen, and three intakes are logged — instead of three full search-and-confirm cycles.Optionally, with their own Anthropic API key, the same box accepts free text:
zwei Eier und drei Scheiben Brot,I had a chicken caesar salad and a latte,2个鸡蛋，200ml牛奶.The claim this feature does not break
Every calorie and macro still comes from Open Food Facts / USDA / BLS. Nothing here estimates nutrition.
That holds in both tiers, and it is enforced rather than documented:
validateParsedMealItems, the same bounds the regex enforces. A model cannot write to the diary under looser rules than a parser.So no
MealSourceEntityvalue was added, no export-schema change, no "estimated" marker. The provenance apparatus agreed for a macro-emitting tier stays deferred, and #250 does not need reopening for this.Tier 0 — offline, no key, no network
meal_text_parser.dartresolver_relevance.darteggsagainstEggat 0.0resolve_parsed_meals_usecase.dartbulk_add_screen.dart+ blocmeal_quantity_converter.dartTier 1b — the user's own key, and only language work
The model reads language; the database still supplies every number. It handles what a regex cannot: number-words across nine locales, free text, and counts in scripts without spaces.
The key lives in the platform keystore, is write-only in the UI (fixed-length mask), and can be paused without being deleted. With no key configured nothing changes anywhere. Every failure — no network, rejected key, rate limit, changed API, an outright bug — falls back to the deterministic parser with no error shown.
Privacy
The README's destination table moves from three to four, with Anthropic reached only when the user saves a key. Only the typed line and the app language are sent; never the diary, profile or history. The project never sees the key and does not pay for its use.
Verification
develop. 24 new test files.Live testing found three things unit tests could not: a thousandfold litre under-count, a unit substitution that bypassed the row's warning, and a composition bug where a correct empty model answer was overridden. All three are fixed; the last is #647, which should merge before this.
Before merging
Closesonly fires on the default branch.feature/**CI trigger indefault_workflow.ymlwas added so PRs into this branch got checks at all. Keep it if the branch lives on for tier 1a; drop it if this is the end of the line.Not included, deliberately
Tier 1a — a model emitting macros — is not here and cannot be until decision #5 is settled against #250. #587 raises the same question through a share intent and is blocked on the same answer.