goyban
206 lines
14 KiB
Markdown
206 lines
14 KiB
Markdown
# 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`
|
||
- [ ] **#5** save vs. extract (vault)
|
||
- [ ] **#4** federation
|
||
|
||
Test suites: `test_db.py`, `test_bot.py`, `test_access.py` — 20 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
|
||
|
||
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.
|