docs(ai): correct eight claims the code no longer supports - #996
Conversation
Every claim on the page was checked against release/2.2.0. The model of the feature holds -- the schema-shaped guarantee, the routing, the timeouts, the probe taxonomy and all 27 code-map rows survived. Eight sentences did not. Two are drift rather than error: the page landed on 2026-08-27, metadata stripping (#914) hours later the same day, `portion` (#864) three days after. - **The schema has four fields, not three.** `portion` was missing from the prose and from the sequence diagram, and the page's own excerpt elided the comment defending it. It is not inert -- it is a lookup key into the matched food's own portion list, so the model's word chooses which row is preset. The grams still come from the database, but a page that tells the reader to audit the schema has to state what is in it. - **The encoder fallback does not send the file unmodified.** It strips every metadata block first -- EXIF GPS above all -- and refuses a file it cannot parse. The page understated its own protection while asserting something false, and "unmodified" is the word a privacy reviewer would quote back. Third bound added; diagram node corrected. - **1024 px bounds the shortest edge, not the longest.** flutter_image_compress takes the smaller ratio, so a 4:3 frame leaves at about 1365x1024. The source comment in meal_photo_encoder.dart carries the same misreading and still needs its own fix. - **There is no gallery source.** Only ImageSource.camera is offered; "Camera or picker" read as "camera or gallery". - **The photo path has a second gate.** For a server you run it appears only once the probe's photo leg has passed. - **Two requests carry a retention instruction on the wire** -- `store: false` to OpenAI, `data_collection: "deny"` to OpenRouter -- so retention is not purely a policy question deferred to the README. - **The probe is not once.** It re-runs on every confirm, deliberately: the address and model can be identical and the machine behind them different. - **`rejected` is the provider refusing, not a guardrail.** A plaintext-guard refusal lands on `unknown`, because nothing was sent and nothing was learned. - **The schema row cited a test that does not pin it.** The contract test named asserts a quantity ceiling and would still pass if `calories` were added. Repointed at the test that actually enumerates the exposed fields.
There was a problem hiding this comment.
🟢 Approval recommended
Changes are documentation-only and appear consistent with the referenced implementation and tests, with only a minor clarity/wording nit noted.
Pull request overview
Updates docs/ai-architecture.md to realign eight claims with the current AI meal assistance implementation, focusing on schema surface area, photo handling, probe gating, and retention parameters so the published documentation matches shipped behavior.
Changes:
- Corrects the meal-items tool schema description to include the
portionfield and explains its semantics. - Updates the photo pipeline description (camera-only source, shortest-edge resize, and metadata-stripping fallback behavior).
- Clarifies provider retention instructions (
store: false,data_collection: "deny"), probe re-run behavior, and the meaning ofrejected.
File summaries
| File | Description |
|---|---|
| docs/ai-architecture.md | Fixes factual drift in AI architecture documentation around schema fields, photo encoding/fallback behavior, probe gating, and retention signaling. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| a retention instruction of their own. The direct OpenAI call sends `store: false`, because | ||
| Responses stores by default and silence there is not a no-op. The OpenRouter routing block sends |
Second pass over the same review: the GAP and OVERSTATED findings, after the factually-wrong set. Every addition was verified against release/2.2.0 first, and three of the reviewer's suggestions turned out to be wrong. What the page now answers that it did not: - **What travels with a request.** Model id, system prompt, the line you typed, the tool and its schema, the answer cap -- and the app language appended to the prompt as one sentence. Nothing from the diary, profile or history, no earlier request, no identifier. The negative is structural: `requestItems` takes a system string and a `MealContent`, so there is no seam to attach one. - **The other half of the litre story.** Putting `l` in the enum opened the mirror-image failure -- a model answering with a unit nobody typed, 470 kcal logged for a glass of milk. `textStatesAUnit` closed it by corroborating any unit against the text the model was given. - **What a garbled or truncated reply does.** One bad entry is dropped and the batch survives; a reply where every entry drops is refused rather than passed on as an empty list, which would be indistinguishable from "no food here". Truncation is not a case the app recognises, and the page now says so. - **Two requests reach a server you run**, and the model-list one goes out before the consent screen -- it hangs off the address field, not the save path. - **What the app asks each destination not to keep** -- `store: false`, `data_collection: deny` -- with a guardrail row for the latter. - **The probe sends a fixed line and a bundled photograph**, never anything of yours, matching the consent screen's wording from #985. - The `failed --> passed` edge the state diagram was missing, the photo-only count rule, the onboarding entry point, and the consent invariant's real home in credential storage rather than in two widgets. Narrowed where the page claimed more than the code delivers: retraction covers the photo row only; "the disclosure shown before anything is stored" becomes "before any credential is stored", since a provider tag and a model id are written first; and the blanket no-logging sentence now matches what the contract test actually pins about the three clients. The page's own guard is hardened alongside it, since it is the only thing still watching once the page is lifted to the wiki: the link scan read one link form out of several and silently missed reference-style definitions, the slug function diverged from GitHub's on underscores and on runs of spaces, a link resolving outside the repository was mis-resolved rather than failed, and the canary floor sat at 25 against a real 49. Verified by adding a broken reference-style link and watching it fail, which it did not before.
The page said the plaintext check "is the whole of the enforcement". That was overstated while redirects escaped it, and is true again now: the sentence lands here rather than in the documentation PR because it only becomes accurate when this change does. Says why it is whole rather than asserting that it is -- a redirect is not followed, so no hop is made on an approval granted to a different destination -- and covers the https case, which the guard otherwise waves through. This branch carries the doc-accuracy work merged in, so the paragraph sits in its corrected context; #996 remains the PR that reviews it.
There was a problem hiding this comment.
🟡 Changes recommended
The new fenced-block stripping helper does not handle indented code fences, so the link/heading scan can still read fenced content and produce incorrect results.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| String withoutFences(String markdown) => markdown.replaceAll( | ||
| RegExp(r'^(```|~~~)[^\n]*\n.*?^\1[^\n]*$', multiLine: true, dotAll: true), | ||
| '', | ||
| ); |
…ead indented fences Copilot caught both. The retention sentence had no subject: it now says the Responses API retains request and response content by default, which is the point of sending `store: false`. `withoutFences` anchored its fence pattern at column 0, so a fence indented inside a list item was invisible to it and the sample links and headings in that fence were scanned as though they were the page's own. CommonMark allows up to three spaces before a fence, and the closing run needs the same allowance.
There was a problem hiding this comment.
🟡 Changes recommended
The updated doc test’s fence-stripping logic fails to ignore deeper-indented fenced blocks that exist in this repo, and the doc contains inconsistent quoting for an on-wire data_collection value.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/ai-architecture.md:343
- This section alternates between
data_collection: "deny"anddata_collection: 'deny'. Since this is describing an on-the-wire field/value, using single quotes is inconsistent and can be read as invalid JSON; prefer double quotes for the string value everywhere here.
**Every request through OpenRouter carries `data_collection: 'deny'`** inside its routing block.
OpenRouter's routing default is `allow`, which makes providers that store input non-transiently
and may train on it eligible to serve the request. It is sent as a policy field rather than
inferred from the model slug, because free *usage* is what triggers those clauses and not the
`:free` suffix — a slug check would look like enforcement and would not be it.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| RegExp( | ||
| r'^ {0,3}(```|~~~)[^\n]*\n.*?^ {0,3}\1[^\n]*$', | ||
| multiLine: true, | ||
| dotAll: true, | ||
| ), |
* docs(ai): correct eight claims the code no longer supports Every claim on the page was checked against release/2.2.0. The model of the feature holds -- the schema-shaped guarantee, the routing, the timeouts, the probe taxonomy and all 27 code-map rows survived. Eight sentences did not. Two are drift rather than error: the page landed on 2026-08-27, metadata stripping (#914) hours later the same day, `portion` (#864) three days after. - **The schema has four fields, not three.** `portion` was missing from the prose and from the sequence diagram, and the page's own excerpt elided the comment defending it. It is not inert -- it is a lookup key into the matched food's own portion list, so the model's word chooses which row is preset. The grams still come from the database, but a page that tells the reader to audit the schema has to state what is in it. - **The encoder fallback does not send the file unmodified.** It strips every metadata block first -- EXIF GPS above all -- and refuses a file it cannot parse. The page understated its own protection while asserting something false, and "unmodified" is the word a privacy reviewer would quote back. Third bound added; diagram node corrected. - **1024 px bounds the shortest edge, not the longest.** flutter_image_compress takes the smaller ratio, so a 4:3 frame leaves at about 1365x1024. The source comment in meal_photo_encoder.dart carries the same misreading and still needs its own fix. - **There is no gallery source.** Only ImageSource.camera is offered; "Camera or picker" read as "camera or gallery". - **The photo path has a second gate.** For a server you run it appears only once the probe's photo leg has passed. - **Two requests carry a retention instruction on the wire** -- `store: false` to OpenAI, `data_collection: "deny"` to OpenRouter -- so retention is not purely a policy question deferred to the README. - **The probe is not once.** It re-runs on every confirm, deliberately: the address and model can be identical and the machine behind them different. - **`rejected` is the provider refusing, not a guardrail.** A plaintext-guard refusal lands on `unknown`, because nothing was sent and nothing was learned. - **The schema row cited a test that does not pin it.** The contract test named asserts a quantity ceiling and would still pass if `calories` were added. Repointed at the test that actually enumerates the exposed fields. * fix(net): stop the plaintext guard being bypassed by a redirect GuardedPlaintextClient approves the URL the app builds, but redirects are followed inside `dart:io`, below `BaseClient.send`. A hop therefore never re-entered the guard and never met `approve`. A private server answering 30x with a public `http://` Location had that connection made -- out of a check that had just reported the destination private. The https pass-through had the same hole from the other direction: `approve` waves https straight through, and an encrypted first hop says nothing about where a `Location` points, so an https -> http://public redirect was followed unchecked too. Setting the flag only in `_reboundTo`, as first suggested, would have missed that half; it is set in `send` for every request instead. The guard cannot vouch for a hop it never sees, so it does not let one happen: `followRedirects` is off and a 30x is returned to the caller as the response it is. `AiModelListApi` already treats a non-200 as a failed request, and an OpenAI-compatible endpoint that redirects its own API path is not a case worth following blindly to a destination nothing checked. Two tests pin it -- one per branch, both failing without the change. A third records what a caller sees; it passes either way, because the following happens below the fake, and it says so rather than looking like a regression test. * docs(ai): answer the questions the page left open, and harden its guard Second pass over the same review: the GAP and OVERSTATED findings, after the factually-wrong set. Every addition was verified against release/2.2.0 first, and three of the reviewer's suggestions turned out to be wrong. What the page now answers that it did not: - **What travels with a request.** Model id, system prompt, the line you typed, the tool and its schema, the answer cap -- and the app language appended to the prompt as one sentence. Nothing from the diary, profile or history, no earlier request, no identifier. The negative is structural: `requestItems` takes a system string and a `MealContent`, so there is no seam to attach one. - **The other half of the litre story.** Putting `l` in the enum opened the mirror-image failure -- a model answering with a unit nobody typed, 470 kcal logged for a glass of milk. `textStatesAUnit` closed it by corroborating any unit against the text the model was given. - **What a garbled or truncated reply does.** One bad entry is dropped and the batch survives; a reply where every entry drops is refused rather than passed on as an empty list, which would be indistinguishable from "no food here". Truncation is not a case the app recognises, and the page now says so. - **Two requests reach a server you run**, and the model-list one goes out before the consent screen -- it hangs off the address field, not the save path. - **What the app asks each destination not to keep** -- `store: false`, `data_collection: deny` -- with a guardrail row for the latter. - **The probe sends a fixed line and a bundled photograph**, never anything of yours, matching the consent screen's wording from #985. - The `failed --> passed` edge the state diagram was missing, the photo-only count rule, the onboarding entry point, and the consent invariant's real home in credential storage rather than in two widgets. Narrowed where the page claimed more than the code delivers: retraction covers the photo row only; "the disclosure shown before anything is stored" becomes "before any credential is stored", since a provider tag and a model id are written first; and the blanket no-logging sentence now matches what the contract test actually pins about the three clients. The page's own guard is hardened alongside it, since it is the only thing still watching once the page is lifted to the wiki: the link scan read one link form out of several and silently missed reference-style definitions, the slug function diverged from GitHub's on underscores and on runs of spaces, a link resolving outside the repository was mis-resolved rather than failed, and the canary floor sat at 25 against a real 49. Verified by adding a broken reference-style link and watching it fail, which it did not before. * docs(ai): state the redirect rule now that the guard enforces it The page said the plaintext check "is the whole of the enforcement". That was overstated while redirects escaped it, and is true again now: the sentence lands here rather than in the documentation PR because it only becomes accurate when this change does. Says why it is whole rather than asserting that it is -- a redirect is not followed, so no hop is made on an approval granted to a different destination -- and covers the https case, which the guard otherwise waves through. This branch carries the doc-accuracy work merged in, so the paragraph sits in its corrected context; #996 remains the PR that reviews it. * docs(ai): address review — name what the Responses API retains, and read indented fences Copilot caught both. The retention sentence had no subject: it now says the Responses API retains request and response content by default, which is the point of sending `store: false`. `withoutFences` anchored its fence pattern at column 0, so a fence indented inside a list item was invisible to it and the sample links and headings in that fence were scanned as though they were the page's own. CommonMark allows up to three spaces before a fence, and the closing run needs the same allowance. * test(net): the fake keeps the request association, and sentCount stops overclaiming Copilot, both correct. The injected response dropped `request:`, so an assertion on `response.request` would have read differently depending on whether a test supplied its own response. And the `sentCount` doc comment implied the counter could detect a redirect being followed; it cannot, because that happens inside `dart:io` below this fake. It pins that the guard sends once, which is a smaller claim and the true one. Carries the updated doc branch in as well. * fix(net): keep the port in the Host header when rebinding Rebinding `http://ollama.lan:11434` to its resolved address kept the name in the Host header, but wrote it as `ollama.lan` — dropping the `:11434`. That is a different authority from the one the user typed. A reverse proxy or a strict server routes by what that header says, so it can reject or misroute the request, and this is the common case rather than a corner one: a local model server is essentially never on 80. The port is written only when the URL carried an explicit one, so a plain `http://ollama.lan` still sends the bare name rather than a redundant `:80`. Reported against the release branch, where the guard is otherwise unchanged. * test(net): put the redirect rule to a real socket The existing redirect tests assert against a recording fake, which cannot follow a redirect — `dart:io` does that below `BaseClient.send`, where no fake reaches. So they pin that `followRedirects` was set to false, but not that setting it false actually stops the second hop. Review of the release branch made exactly that objection. These run against a real `HttpServer` on loopback and a real `dart:io` client, on both branches through the guard: the literal pass-through and the rebound path. A control test proves the setup has teeth by showing an unguarded client really does follow the 302 to `/second` — without it the other two would pass just as well against a server that never redirected. A fourth reads the Host header off the wire at the server, which is the only place that proves it survives `dart:io` setting a Host of its own from the connection URI. `flutter_test` installs an HttpOverrides that answers every request with a canned 400 instead of opening a socket; the group clears it and puts it back.
#1001) Three comments claimed `minWidth`/`minHeight` cap the longest edge. They bound the shortest one. flutter_image_compress takes `max(1, min(width/minWidth, height/minHeight))` as its scale factor (BitmapCompressExt.kt:62-66; UIImage+scale.m:12-21 picks the same branch), so the smaller ratio wins and the edge landing on 1024 is the short one. A 4080x3072 camera frame comes out 1360x1024, not 1024x768 -- about 1.8x the pixels the comments promised, and more again at 16:9. Comments only; the calls are unchanged and the behaviour was always this. Corrected in meal_photo_encoder.dart and user_image_storage.dart, where the claim appeared twice each (dartdoc summary and the call site), and in recipe_builder_screen.dart, which repeated it when explaining why it picks at full resolution. Found while checking docs/ai-architecture.md, which stated the same thing and is corrected in #996.
* docs(ai): correct eight claims the code no longer supports Every claim on the page was checked against release/2.2.0. The model of the feature holds -- the schema-shaped guarantee, the routing, the timeouts, the probe taxonomy and all 27 code-map rows survived. Eight sentences did not. Two are drift rather than error: the page landed on 2026-08-27, metadata stripping (#914) hours later the same day, `portion` (#864) three days after. - **The schema has four fields, not three.** `portion` was missing from the prose and from the sequence diagram, and the page's own excerpt elided the comment defending it. It is not inert -- it is a lookup key into the matched food's own portion list, so the model's word chooses which row is preset. The grams still come from the database, but a page that tells the reader to audit the schema has to state what is in it. - **The encoder fallback does not send the file unmodified.** It strips every metadata block first -- EXIF GPS above all -- and refuses a file it cannot parse. The page understated its own protection while asserting something false, and "unmodified" is the word a privacy reviewer would quote back. Third bound added; diagram node corrected. - **1024 px bounds the shortest edge, not the longest.** flutter_image_compress takes the smaller ratio, so a 4:3 frame leaves at about 1365x1024. The source comment in meal_photo_encoder.dart carries the same misreading and still needs its own fix. - **There is no gallery source.** Only ImageSource.camera is offered; "Camera or picker" read as "camera or gallery". - **The photo path has a second gate.** For a server you run it appears only once the probe's photo leg has passed. - **Two requests carry a retention instruction on the wire** -- `store: false` to OpenAI, `data_collection: "deny"` to OpenRouter -- so retention is not purely a policy question deferred to the README. - **The probe is not once.** It re-runs on every confirm, deliberately: the address and model can be identical and the machine behind them different. - **`rejected` is the provider refusing, not a guardrail.** A plaintext-guard refusal lands on `unknown`, because nothing was sent and nothing was learned. - **The schema row cited a test that does not pin it.** The contract test named asserts a quantity ceiling and would still pass if `calories` were added. Repointed at the test that actually enumerates the exposed fields. * docs(ai): answer the questions the page left open, and harden its guard Second pass over the same review: the GAP and OVERSTATED findings, after the factually-wrong set. Every addition was verified against release/2.2.0 first, and three of the reviewer's suggestions turned out to be wrong. What the page now answers that it did not: - **What travels with a request.** Model id, system prompt, the line you typed, the tool and its schema, the answer cap -- and the app language appended to the prompt as one sentence. Nothing from the diary, profile or history, no earlier request, no identifier. The negative is structural: `requestItems` takes a system string and a `MealContent`, so there is no seam to attach one. - **The other half of the litre story.** Putting `l` in the enum opened the mirror-image failure -- a model answering with a unit nobody typed, 470 kcal logged for a glass of milk. `textStatesAUnit` closed it by corroborating any unit against the text the model was given. - **What a garbled or truncated reply does.** One bad entry is dropped and the batch survives; a reply where every entry drops is refused rather than passed on as an empty list, which would be indistinguishable from "no food here". Truncation is not a case the app recognises, and the page now says so. - **Two requests reach a server you run**, and the model-list one goes out before the consent screen -- it hangs off the address field, not the save path. - **What the app asks each destination not to keep** -- `store: false`, `data_collection: deny` -- with a guardrail row for the latter. - **The probe sends a fixed line and a bundled photograph**, never anything of yours, matching the consent screen's wording from #985. - The `failed --> passed` edge the state diagram was missing, the photo-only count rule, the onboarding entry point, and the consent invariant's real home in credential storage rather than in two widgets. Narrowed where the page claimed more than the code delivers: retraction covers the photo row only; "the disclosure shown before anything is stored" becomes "before any credential is stored", since a provider tag and a model id are written first; and the blanket no-logging sentence now matches what the contract test actually pins about the three clients. The page's own guard is hardened alongside it, since it is the only thing still watching once the page is lifted to the wiki: the link scan read one link form out of several and silently missed reference-style definitions, the slug function diverged from GitHub's on underscores and on runs of spaces, a link resolving outside the repository was mis-resolved rather than failed, and the canary floor sat at 25 against a real 49. Verified by adding a broken reference-style link and watching it fail, which it did not before. * docs(ai): address review — name what the Responses API retains, and read indented fences Copilot caught both. The retention sentence had no subject: it now says the Responses API retains request and response content by default, which is the point of sending `store: false`. `withoutFences` anchored its fence pattern at column 0, so a fence indented inside a list item was invisible to it and the sample links and headings in that fence were scanned as though they were the page's own. CommonMark allows up to three spaces before a fence, and the closing run needs the same allowance. (cherry picked from commit 209e8ee)
#1001) Three comments claimed `minWidth`/`minHeight` cap the longest edge. They bound the shortest one. flutter_image_compress takes `max(1, min(width/minWidth, height/minHeight))` as its scale factor (BitmapCompressExt.kt:62-66; UIImage+scale.m:12-21 picks the same branch), so the smaller ratio wins and the edge landing on 1024 is the short one. A 4080x3072 camera frame comes out 1360x1024, not 1024x768 -- about 1.8x the pixels the comments promised, and more again at 16:9. Comments only; the calls are unchanged and the behaviour was always this. Corrected in meal_photo_encoder.dart and user_image_storage.dart, where the claim appeared twice each (dartdoc summary and the call site), and in recipe_builder_screen.dart, which repeated it when explaining why it picks at full resolution. Found while checking docs/ai-architecture.md, which stated the same thing and is corrected in #996. (cherry picked from commit 202a21b)
Five independent passes over
docs/ai-architecture.md, 164 claims checked againstorigin/release/2.2.0.The page's model of the feature is correct. The schema-shaped guarantee, the routing, the timeouts, the probe taxonomy, the plaintext ranges and all 27 code-map rows survived. Eight sentences did not, and this PR fixes those.
Two of them are drift rather than error — the page landed 2026-08-27 07:24, metadata stripping (#914) at 19:18 the same day,
portion(#864) three days later.The two that mattered
"The permitted fields are exactly three" — there are four.
portionis inmealItemsToolSchemaand the page's own excerpt elides the comment defending it. Three of five passes found this independently. It is not inert:portionis a lookup key into the matched food's own portion list, so the model's word chooses which portion row is preset. The grams still come from the database — the one rule holds — but a page that hands the reader an audit procedure ("did anyone add a field here?") has to state the right count.The encoder fallback said the file goes out "unmodified".
meal_photo_encoder.dartstrips EXIF, XMP and IPTC — including the GPS block — and refuses a file it cannot parse. The page understated its own protection and asserted something false, in the one paragraph about the riskiest branch.The rest
store: falseanddata_collection: "deny"are retention instructions on the wire, not policy deferred to the READMErejectedis the provider refusing, not a guardrail; a plaintext-guard refusal lands onunknowncalorieswere added to the schemaNot fixed here
meal_photo_encoder.dart:196anduser_image_storage.dartrepeat the shortest/longest-edge misreading. Code comments, separate change.dart:iowithout re-entering it. That is an OVERSTATED finding with a one-line code fix (followRedirects = false), not a documentation fix, so it is left for its own PR.Test plan
flutter test test/unit_test/ai_architecture_doc_test.dart— passes; every link and anchor still resolvesopenrouter_meal_items_api_test.dart→ exposes only query, quantity, unit and portionorigin/release/2.2.0, not taken from the reviewWorth reaching 2.2.0.
RELEASING.mdsays this page is published to the wiki on the first release shipping AI meal assistance — once it is over there, nothing runs against it.