Files
memes_bot/PLAN.md
T
goyban 0732f2c34b Record what may be shared, ahead of federation
The bot-side half of TODO #4. Nothing federates yet; this establishes
what would be eligible to, and proves the boundary before there is a
hub that could leak across it.

- Schema v5 adds visibility, defaulting to private. Upgrading an
  existing library therefore opts nothing into sharing, which a test
  on the legacy upgrade path pins down.
- list_syncable() is the only function that will ever feed a hub, and
  it requires both public and approved. Tests force rows into states
  set_visibility refuses, so the filter is what protects the boundary
  rather than the caller being careful.
- set_visibility refuses anything not approved: media still in the
  moderation queue cannot be marked shared.
- /share lists memes with their current state and asks before making
  one public, since un-sharing stops future copies but cannot recall
  those already taken. The wording describes what sharing will mean
  once a hub exists, because by then warning is too late.
- /list shows the same markers.

Vault posts have no path into the sync set at all; being in a separate
table is what guarantees it, and there is a test saying so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-01 22:09:37 +00:00

234 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Implementation plan — TODO #2–#6
Progress on branch `develop` (not pushed):
- [x] **#1** develop branch
- [x] **Step 0** migration harness, per-owner uniqueness — `b55cee1`
- [x] **#6** full media coverage — `efc2be5`
- [x] **#3** moderation queue — `0a8c81d`
- [x] **#2** public mode + membership gate — `82b5da4`
- [x] **#5** save vs. extract (vault) — forward-based
- [~] **#4** federation — bot-side visibility done; hub, channel and sync client outstanding
Test suites: `test_db.py`, `test_bot.py`, `test_access.py`, `test_vault.py` — 34 cases, all passing.
---
## Step 0 — Prerequisites (blocking for #2, #3, #4, #5)
Two things in the current schema block almost everything else, so they come first.
**0a. There is no migration mechanism.** `db.init_db()` (`db.py:7-21`) only runs `CREATE TABLE IF NOT EXISTS`, so it can never alter an existing deployment's table. Every item below adds columns. Add a `schema_version` table and run ordered migration steps:
```python
async def migrate(db):
(v,) = await fetch_one("PRAGMA user_version")
if v < 2: ... # ALTER TABLE media ADD COLUMN ...
await db.execute(f"PRAGMA user_version = {TARGET}")
```
SQLite's `PRAGMA user_version` is enough here — no extra table needed.
**0b. `file_unique_id` is globally UNIQUE** (`db.py:14`). That is correct for a single-owner bot and wrong for every multi-user mode in this TODO: two users cannot each save the same meme. It also makes `INSERT OR REPLACE` (`db.py:28`) silently *delete and reinsert* a colliding row, changing its `id` (AUTOINCREMENT + REPLACE), which quietly breaks any `/delete <id>` the user had noted.
Change to `UNIQUE(owner_id, file_unique_id)`. SQLite can't drop a column constraint in place — this needs the create-new-table / copy / rename dance inside migration v2.
**0c. Related, worth fixing while in there:** `search_media()` (`db.py:34`) never filters by `owner_id`, so today every allowed user sees every user's media. Harmless for a personal bot, unacceptable once #2 opens the bot up. Search needs an explicit scope argument from the start.
---
## #6 — Full media coverage *(do this first: small, independent, immediate value)*
Currently handled (`bot.py:157-190`): photo, animation, video, voice, sticker.
Missing: **audio (mp3), document (file), video_note (round video)**.
**Refactor.** The five handlers are near-identical. Replace with one handler and an extractor table:
```python
EXTRACTORS = {
"photo": lambda m: m.photo[-1],
"gif": lambda m: m.animation,
"video": lambda m: m.video,
"voice": lambda m: m.voice,
"sticker": lambda m: m.sticker,
"audio": lambda m: m.audio,
"document": lambda m: m.document,
"video_note":lambda m: m.video_note,
}
```
One `@dp.message(F.photo | F.animation | ... , StateFilter(None))` that walks the table. This also fixes the duplicated filter list at `bot.py:225`, which currently misses any newly added type.
**Inline results** (`bot.py:395-416`) need matching branches: `InlineQueryResultCachedAudio` and `InlineQueryResultCachedDocument` (the latter requires a non-empty `title`).
**Constraint to accept:** Telegram has **no cached inline result type for `video_note`**. Round videos can be stored and listed, but cannot be returned via inline search. Options: store them as `video` (loses the round presentation), or accept that they're `/list`-only. Recommend the latter and say so in the README.
Also note `F.animation` catches most GIFs, but Telegram sometimes delivers a GIF as `document` with an image mime-type — the document branch will pick those up once it exists.
---
## #3 — Moderation queue *(before #2, since public mode depends on it)*
**Schema (migration v3):** add to `media`:
`status TEXT NOT NULL DEFAULT 'approved'` ∈ {`pending`, `approved`, `rejected`}, plus `submitted_by INTEGER`, `reviewed_by INTEGER`, `reviewed_at TIMESTAMP`.
Existing rows default to `approved`, so current deployments are unaffected. Every read path (`search_media`, `list_media`, `count_media`) gains `WHERE status = 'approved'`.
**Review flow — `/review` (moderators only):** pulls the *oldest pending* row, sends the media itself followed by its keywords and an inline keyboard:
`[✅ Approve] [❌ Reject] [✏️ Fix keywords] [⏸ Stop]`
**The "continue later if they are too much" requirement falls out for free**, and this is the key design choice: the queue position is not stored in FSM state — each button press just re-queries for the next `pending` row. A moderator can stop mid-queue, restart days later, or have several moderators working simultaneously, with no state to resume. Include a `(14 left)` counter in each message.
Guard against two moderators grabbing the same item with a conditional update — `UPDATE media SET status=? WHERE id=? AND status='pending'` and check `rowcount`.
---
## #2 — Public mode with membership conditions
**Config:** `PUBLIC_MODE=true`, `REQUIRED_CHATS=@channel,-100123456`, `REQUIRE_MODE=all|any`.
**Gate:** `getChatMember(chat_id, user_id)`, accepting status ∈ {`creator`, `administrator`, `member`, and `restricted` with `is_member=true`}. The bot must be an admin in each required chat for this call to work — that's a setup requirement for the README.
**Caching is not optional here.** Inline queries fire on *every keystroke*. An uncached membership check means several Telegram API calls per typed character, which will hit rate limits fast. Cache per `(user_id, chat_id)` with a ~5 minute TTL in memory.
**Rejection UX:** for inline queries, don't return an empty list — use `answerInlineQuery`'s `button` parameter (`InlineQueryResultsButton`, supported in aiogram 3.15) to show a tappable "Join @channel to use this bot" bar above the results. In direct chat, reply with the channel links.
With `PUBLIC_MODE=true`, non-admin submissions land as `status='pending'` from #3 rather than being ignored.
---
## #5 — Save vs. extract on forward — **implemented**
Built on forwarding rather than the file_id copy originally planned here, which
removed most of the work.
**The bot already holds the post.** When you forward something, Telegram puts a
copy in your private chat with the bot, so the bot can forward *that* back later
by `message_id`. It never needs to reach the original channel, store media, or
re-upload anything.
**`forwardMessages` (plural) preserves albums.** It takes up to 100 message ids
and returns them as one intact album, so the caption + N media case that made
this item large is handled by Telegram itself. Ids must be strictly ascending.
**One table, not two:**
```sql
vault_items(id, owner_id, chat_id, message_ids TEXT, caption, keywords, created_at)
```
Every vault query takes an `owner_id` and filters on it, so a guessed id cannot
reach another user's post. Vault rows live in their own table and are never
searched by inline mode, never moderated, and never federated.
**Flow.** A forwarded message prompts *Save whole post* / *Extract media*
(extract only appears when there is media). Save collects keywords and stores
the pointer; extract runs the existing add path, now generalised to a list so a
whole album shares one set of keywords. Albums are buffered by `media_group_id`
with a 1s debounce, since Telegram gives no "album complete" signal.
**Recall is inside the bot only**, as decided: `/vault [search]` lists matching
posts with a retrieve button and a delete button. Matching covers keywords and
the original caption. The save confirmation tells the user plainly that vault
posts do not appear in inline search.
**The tradeoff accepted:** a `message_id` is a pointer into the bot's chat with
the owner, not self-contained like a `file_id`. If that history is cleared, the
pointer breaks; retrieval catches the failure and tells the user to forward the
post again.
---
## #4 — Federation
### The constraint that decides the design
**A `file_id` is only valid for the bot that received it.** A different bot cannot send, download, or resolve another bot's `file_id`. `file_unique_id` *is* stable across bots, but cannot be used to send anything — it's an identity key only.
So "sync the public database every hour" **cannot work by copying rows between instances.** Each peer bot must independently obtain its *own* `file_id` for every shared meme. Any design that just ships database rows around will produce a table full of file_ids that throw `wrong file identifier` on use. This is the single most important thing to get right before writing code.
### Recommended architecture: shared channel + metadata hub
Split media transport from metadata sync:
**Media → a private Telegram "federation channel" that you own.** Every participating bot is added as an admin. When an instance marks media public, it copies that media into the channel. Peers, being admins, receive the `channel_post` update and extract a `file_id` *valid for them*, storing it against the shared `file_unique_id`. Telegram stores the bytes once; nobody hosts files; no media ever transits your server.
**Metadata → a small hub API** (`POST /sync` / `GET /since?ts=`) carrying only `file_unique_id`, `media_type`, `keywords`, `channel_message_id`, `origin_instance`. Rows keyed by `file_unique_id`, which is the one identifier that means the same thing everywhere.
**Backfill for a newly joined instance:** a bot can't read channel history, so it won't have seen older posts. It resolves them on demand — `forwardMessage(from_chat_id=<fed_channel>, message_id=<from hub metadata>, chat_id=<own log chat>)` returns a full `Message` containing a `file_id` valid for that bot. Note this must be `forwardMessage`, not `copyMessage`: `copyMessage` returns only a `MessageId` with no file data. Do this lazily on first use, not as an hour-long startup storm.
### Privacy and safety
- `visibility TEXT NOT NULL DEFAULT 'private'` ∈ {`private`, `public`} on `media` (migration v5). The sync job reads **only** `visibility='public' AND status='approved'`. Private rows have no code path to the hub — assert this in a test.
- Marking public is explicit and per-item, and should be **irreversible in practice**: once shared, peers have copies. Say so in the confirmation prompt.
- Each instance gets an `INSTANCE_ID` + bearer token you issue; HTTPS only; an instance may only write rows it owns. You keep revocation and a takedown endpoint that tombstones a `file_unique_id` so peers delete on next sync.
- Rate-limit per instance. A compromised peer token should be able to spam, at worst, its own rows.
- Practical note: a federated pool of user-submitted media needs a takedown path for illegal content, and as the hub operator that lands on you. Worth deciding the policy before launching, not after.
### Status
Done in this repo: the `visibility` column (private by default), `/share` to
change it, and `list_syncable()` — the single function that defines what is
ever allowed to leave this instance. It is guarded by tests that force rows
into states the writer would refuse, so the filter is proven rather than
assumed. Nothing calls it yet.
Outstanding, and all of it depends on decisions only the owner can make:
the federation channel, the hub deployment, the instance tokens, and the sync
client that pushes to and pulls from the hub.
### Open question
The hub is a separate deployable (small FastAPI + SQLite would do). Do you want it in this repo as `hub/`, or as its own repository? It has a different release cadence and a different threat model, which argues for separate.
---
## Suggested order
| # | Item | Depends on | Size |
|---|---|---|---|
| 0 | Migration harness + unique-constraint fix | — | S |
| 6 | Full media coverage | 0 (only for the refactor) | S |
| 3 | Moderation queue | 0 | M |
| 2 | Public mode + membership gate | 0, 3 | M |
| 5 | Save vs. extract + vault | 0 | M (was L) |
| 4 | Federation | 0, 3, hub built | XL |
#6 is shippable on its own and touches nothing else — good first commit on `develop`. #4 should stay last: it's the only item that's hard to reverse once other people are running instances against your hub.
## Decisions — settled 2026-09-01
1. **#5 recall UX** — settled further during implementation: the vault is **forward-based and retrievable only inside the bot**. Inline search does not surface vault posts at all; the save confirmation says so.
2. **#4 hub location** — **separate repository.** Channel name and hub hosting are the owner's call, not decided here.
3. **#6 video_note** — **store as `video`**, accepting the loss of round presentation, so round videos stay inline-searchable. See the risk note below.
4. **#4 architecture** — **shared federation channel accepted.** Media rides the channel, metadata rides the hub API.
### Risk carried by decision 3
A Telegram `file_id` encodes its own file type. Storing a `video_note` file_id and later returning it inside an `InlineQueryResultCachedVideo` may be rejected with `wrong file identifier`, because the cached-video slot expects a file_id minted as a video. This cannot be verified without a live bot token — **send the bot a round video and try to recall it inline before trusting this path.** If Telegram rejects it, the fallback is to treat video notes as `/list`-only (the option not chosen), which needs a one-line change to the extractor table.
### Process
Each step below lands as its own git commit on `develop`, so any step can be reverted independently.
---
## Verification
The API claims above were checked against a real `aiogram==3.15.0` install (the version pinned in `requirements.txt`), not asserted from memory:
| Claim | Result |
|---|---|
| `InlineQueryResultsButton` exists (used by #2) | ✅ present |
| `answerInlineQuery` accepts `button` | ✅ present |
| `InlineQueryResultCachedAudio` / `...CachedDocument` exist (#6) | ✅ both present |
| `InlineQueryResultCachedVideoNote` exists | ❌ **does not exist** — confirms the video_note limitation |
| `CachedDocument` requires a `title` | ✅ required fields are `id`, `title`, `document_file_id` |
| `copyMessage` returns file data (#4 backfill) | ❌ returns bare `MessageId` |
| `forwardMessage` returns file data (#4 backfill) | ✅ returns full `Message` |
The two ❌ rows are why the plan routes around `video_note` inline results and specifies `forwardMessage` for federation backfill.
Not verified here (needs a live bot token and a real chat): `getChatMember` behavior for #2, and the album `media_group_id` debounce timing for #5.