Repository navigation
docs: drop the unimplemented report feature and reframe base64 storage - #18
Merged
Merged
Conversation
Two entries under "Known limitations" were not limitations. The report mechanism was never built. Carrying it as an open gap invited the reading that the app is unfinished, when in fact the feature is simply not part of it. Removed from the README, from the firestore.rules comment, and from the rules test that used it as an example. The test itself stays — that an unmatched path fails closed is a real security property, worth keeping whether or not `reports` ever existed — but it now asserts the general rule instead of naming a collection nobody will recognise. Base64 file storage is a decision, not an unresolved problem. Firebase Storage needs the Blaze plan; this project stays on Spark; encoding the file into the note document is how uploads work at all under that constraint. It moves to Key decisions alongside the other trade-offs. The ~650 KB ceiling stays documented word for word — it is the first thing a reviewer will ask about, and the app is built around the number rather than surprised by it: images are downscaled to fit, larger PDFs are rejected with a message that says why. Also removes the shebang from backfill-created-at.js. The repo normalises line endings, so a Windows checkout gives the file CRLF, and a shebang ending in \r is one esbuild does not strip — which made the test importing it fail to parse with a SyntaxError pointing at the import statement. CI never saw it, because Linux checkouts get LF; it only broke after the merged branch was checked out locally. The script is always invoked through node or the npm script, so the line bought nothing. A comment now says why it must not come back. Firestore suite: 40 tests pass. Android suite: 107. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Two entries under Known limitations were not limitations.
The report mechanism
Never built. Carrying it as an open gap invited the reading that the app is unfinished, when in fact the feature is simply not part of it.
Removed from the README, from the
firestore.rulescomment, and from the rules test that used it as an example. The test itself stays — that an unmatched path fails closed is a real security property, worth keeping whether or notreportsever existed — but it now asserts the general rule instead of naming a collection nobody will recognise.Base64 file storage
A decision, not an unresolved problem. Firebase Storage needs the Blaze plan; this project stays on Spark; encoding the file into the note document is how uploads work at all under that constraint.
It moves to Key decisions alongside the other trade-offs. The ~650 KB ceiling stays documented word for word — it is the first thing a reviewer will ask about, and the app is built around the number rather than surprised by it: images are downscaled to fit, larger PDFs are rejected with a message that says why.
A latent bug this uncovered
backfill-created-at.jsloses its shebang.The repo normalises line endings, so a Windows checkout gives the file CRLF — and a shebang ending in
\ris one esbuild does not strip, which made the test importing it fail to parse with aSyntaxErrorpointing at the import statement.CI never saw it, because Linux checkouts get LF. It only surfaced after the merged branch from #17 was checked out locally. The script is always invoked through
nodeor the npm script, so the line bought nothing; a comment now says why it must not come back.Verification
Firestore suite: 40 tests pass. Android suite: 107.
🤖 Generated with Claude Code