docs(readme): a server you run needs no key, and the cache delete is best effort - #1003
Merged
Conversation
…best effort Three claims in the Privacy section, found while reviewing docs/ai-architecture.md against the shipping code. The architecture page was the accurate surface in each case; the README was the one overstating. - **"off until you supply your own API key"** is wrong for a server you run. AiCredentialStorage defines that provider as usable on an endpoint and a model id, with no key at all -- ai_credential_storage.dart, `AiProvider.ownServer => endpoint != null && resolveEndpoint(endpoint) != null && modelId != null`. A self-hosting reader was being told to find a credential that does not exist. - **The photo camera has a second gate** for that provider: it appears only once the setup check has read a photograph successfully. The paragraph above already explains that check; this sentence contradicted it by naming the key as the whole condition. - **"the app deletes that as soon as the photo is encoded"** is stated absolutely. meal_photo_encoder.dart swallows a failed delete on purpose -- "a cache file we could not remove is not worth failing the user's meal entry over" -- so the honest claim is best effort. The architecture page already says so, and the two surfaces could not both be right.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two sentences still read as guaranteeing behaviors (API key presence / cache-file deletion) that the updated text itself describes as conditional or best-effort.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the README’s AI meal assistance section to match the more accurate behavior documented in docs/ai-architecture.md, specifically for self-hosted (“own server”) setups and photo-cache handling.
Changes:
- Clarifies that AI meal assistance can be enabled either via an API key or by configuring a self-hosted server (address + model name).
- Fixes the camera availability description to account for the self-hosted setup-check photo gate.
- Documents that deleting the system picker’s cache copy is best-effort rather than guaranteed.
File summaries
| File | Description |
|---|---|
| README.md | Corrects/clarifies AI meal assistance setup requirements, camera gating, and best-effort cache deletion semantics. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…an attempt Copilot, both fair. The paragraph now covers self-hosting, so 'The key is held in the Keystore' read as though every setup has one; a server of your own has an address stored the same way instead. And 'deletes' asserted an outcome the code only attempts -- 'attempts to delete' is what `meal_photo_encoder` actually does, since it swallows the failure on purpose.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are limited to documentation and were verified against the corresponding implementation behavior (own-server usability rules, photo probe gate, and best-effort cache deletion).
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
simonoppowa
added a commit
that referenced
this pull request
Sep 1, 2026
…best effort (#1003) * docs(readme): a server you run needs no key, and the cache delete is best effort Three claims in the Privacy section, found while reviewing docs/ai-architecture.md against the shipping code. The architecture page was the accurate surface in each case; the README was the one overstating. - **"off until you supply your own API key"** is wrong for a server you run. AiCredentialStorage defines that provider as usable on an endpoint and a model id, with no key at all -- ai_credential_storage.dart, `AiProvider.ownServer => endpoint != null && resolveEndpoint(endpoint) != null && modelId != null`. A self-hosting reader was being told to find a credential that does not exist. - **The photo camera has a second gate** for that provider: it appears only once the setup check has read a photograph successfully. The paragraph above already explains that check; this sentence contradicted it by naming the key as the whole condition. - **"the app deletes that as soon as the photo is encoded"** is stated absolutely. meal_photo_encoder.dart swallows a failed delete on purpose -- "a cache file we could not remove is not worth failing the user's meal entry over" -- so the honest claim is best effort. The architecture page already says so, and the two surfaces could not both be right. * docs(readme): the keystore sentence assumed a key, and the delete is an attempt Copilot, both fair. The paragraph now covers self-hosting, so 'The key is held in the Keystore' read as though every setup has one; a server of your own has an address stored the same way instead. And 'deletes' asserted an outcome the code only attempts -- 'attempts to delete' is what `meal_photo_encoder` actually does, since it swallows the failure on purpose. (cherry picked from commit 410eec2)
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.
Found while reviewing
docs/ai-architecture.mdagainstorigin/release/2.2.0. In each of these the architecture page was the accurate surface and the README was the one overstating — so the fix lands here, not there.A server you run needs no key
AiCredentialStoragedefines that provider as usable on an endpoint and a model id, with no key at all:A self-hosting reader was being told to go and find a credential that doesn't exist for their setup. Now: "your own API key, or the address of a server you run … a server of your own needs an address and a model name, not a key."
The same sentence is wrong in the published privacy policy (
en.txt:63,de.txt:184); that correction is in the iubenda paste-list rather than this PR, since those files record what is currently published.The camera has a second gate
For a server you run it appears only once the setup check has read a photograph successfully. The paragraph immediately above already explains that check and says the camera "is offered only after a photograph has actually been read once" — so this sentence contradicted its own neighbour.
The cache delete is best effort
Stated absolutely.
meal_photo_encoder.dartswallows a failed delete deliberately — "a cache file we could not remove is not worth failing the user's meal entry over, and the OS clears it later". The architecture page states the caveat; the README stated the outcome. Both could not be true, and the one a user reads at the moment they send a photo is the one that has to be right.ai_architecture_doc_test.dartpasses — it asserts the README still links to the architecture page, and that link is untouchedRelated: #996 corrects the architecture page itself.