Files
memes_bot/PLAN.md
T
goyban 4be90a3223 Add implementation plan for TODO #2-#6 with settled decisions
Design-only document covering the moderation queue, public mode,
save-vs-extract vault, and federation. Records the four decisions
taken on the roadmap and the API claims verified against aiogram
3.15.0.

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

196 lines
13 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
Status: design only, nothing implemented. Branch `develop` created locally (TODO #1), not pushed.
---
## 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
On a forwarded message, ask with inline buttons: **[📥 Save whole post]** / **[✂️ Extract media]**. Extract = today's behavior. Save = the private vault.
**Albums are the hard part.** A post with caption + multiple media arrives as *several separate messages* sharing a `media_group_id`, with no "album complete" signal. Standard approach: buffer by `media_group_id` and debounce ~1s after the last message before prompting. Ask once per album, not once per file.
**Schema (migration v4)** — a separate table, not a `media` column, since one vault item holds N files:
```sql
vault_items(id, owner_id, caption TEXT, keywords TEXT, source_chat TEXT, created_at)
vault_files(item_id, file_id, file_unique_id, media_type, position)
```
Vault rows are **always private** and never touched by the federation sync in #4 — enforce that structurally by keeping them in a different table, so no `visibility` flag can ever be mis-set.
**Search should cover the caption, not just keywords** — the point of a vault is finding a post you saved without having tagged it. This is where the current `LIKE '%,kw%'` approach (`db.py:43`) stops being adequate; use an FTS5 virtual table over `caption + keywords`.
**Open question — recall UX.** Telegram **inline mode cannot send an album**; one inline result sends exactly one message. So a multi-file vault item can't come back through `@bot query` intact. Two options:
1. Inline returns the item's *first* media with the caption attached — works everywhere, but loses the other files.
2. A `/vault <query>` command in the bot's private chat that replies with the full album, which you then forward manually — preserves everything, costs one extra step.
These aren't exclusive; doing both (1 for speed, 2 for fidelity) is probably right, but it's your call. Single-file vault items are unaffected either way.
---
## #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.
### 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 | 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** — inline cannot send an album. For multi-file vault items, the inline result sends the **link to the original post** where one exists; otherwise it tells the user to open the bot and pull the full album there. Single-file items send directly as today.
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.