Per-feed aggregation opt-out for status and announcement feeds #90

agent-pi · 2026-10-17T08:37:25Z

agent-pi commented

2026-10-17 08:37:25 +00:00

Status/announcement feeds (Ultra.cc, Shock Hosting, Opentrackers, Contabo) collapsed into one per-feed mega-aggregate on every full refresh because their items are not distinct news stories: the prod DB held a 46-article Ultra.cc aggregate spanning 2023–2026 alongside 24- and 20-article dumps, recreated each time refresh ran and polluting the UI until the 7-day TTL cleared them.

A feeds.aggregation_enabled flag (default on, alembic migration included) lets a feed opt out:

All three aggregation paths — incremental eligibility, consolidation input, and full refresh — filter opted-out feeds out, so their articles stay plain reading-list items. A user whose feeds have all opted out makes no LLM call at all.
The flag is a property of the shared feed row (status-feed-ness is about the feed, not the subscriber); any subscriber can toggle it from the Feeds page (CSRF-protected, strict true/false validation, subscriber-only).
Existing aggregates from an opted-out feed age out through normal TTL expiry rather than being deleted on opt-out.
Flip Ultra.cc / Shock Hosting / Opentrackers / Contabo off in the running instance after deploy; the current mega-aggregates will then expire naturally.

TDD throughout: 15 new tests across db (flag persistence + get_aggregating_feed_ids), service (three exclusion paths + opted-in control), and web (toggle both ways, non-subscriber rejected, invalid value 400, page state). Full nox green.

Closes #87

Status/announcement feeds (Ultra.cc, Shock Hosting, Opentrackers, Contabo) collapsed into one per-feed mega-aggregate on every full refresh because their items are not distinct news stories: the prod DB held a 46-article Ultra.cc aggregate spanning 2023–2026 alongside 24- and 20-article dumps, recreated each time refresh ran and polluting the UI until the 7-day TTL cleared them. A `feeds.aggregation_enabled` flag (default **on**, alembic migration included) lets a feed opt out: - All three aggregation paths — incremental eligibility, consolidation input, and full refresh — filter opted-out feeds out, so their articles stay plain reading-list items. A user whose feeds have all opted out makes no LLM call at all. - The flag is a property of the shared feed row (status-feed-ness is about the feed, not the subscriber); any subscriber can toggle it from the Feeds page (CSRF-protected, strict `true`/`false` validation, subscriber-only). - Existing aggregates from an opted-out feed age out through normal TTL expiry rather than being deleted on opt-out. - Flip Ultra.cc / Shock Hosting / Opentrackers / Contabo off in the running instance after deploy; the current mega-aggregates will then expire naturally. TDD throughout: 15 new tests across db (flag persistence + `get_aggregating_feed_ids`), service (three exclusion paths + opted-in control), and web (toggle both ways, non-subscriber rejected, invalid value 400, page state). Full nox green. Closes #87

agent-pi added 1 commit

2026-10-17 08:37:25 +00:00

🔀 Merge branch 'feat/issue-85-86-cross-run-story-dedup'

/ gitleaks (pull_request) Successful in 40s

Details

/ pr-review (pull_request) Successful in 2m11s

Details

/ checks (pull_request) Successful in 22m27s

Details

/ publish-container (pull_request) Has been skipped

Details

/ deploy-docs (pull_request) Has been skipped

Details

/ container (pull_request) Successful in 1m48s

Details

67bcbafe56

forgejo-actions reviewed

2026-10-17 08:39:20 +00:00

forgejo-actions left a comment

WuMing

Found 1 issue(s). See inline comments below.

## WuMing Found **1** issue(s). See inline comments below.

src/yunjin/db/feeds.py Outdated

					
				@ -229,0 +257,4 @@

				    placeholders = ", ".join("?" for _ in feed_ids)

				    cursor = conn.cursor()

				    cursor.execute(

				        f"SELECT id FROM feeds WHERE aggregation_enabled = 1 AND id IN ({placeholders})",  # noqa: S608

code [HIGH]

SQLite has a maximum of 999 bound variables per statement. If a user subscribes to more than 999 feeds, this query will fail with an OperationalError. The parameterised IN clause should be split into batches (e.g., by iterating sub-lists) to stay under the limit.

**code** [HIGH] SQLite has a maximum of 999 bound variables per statement. If a user subscribes to more than 999 feeds, this query will fail with an OperationalError. The parameterised IN clause should be split into batches (e.g., by iterating sub-lists) to stay under the limit.

agent-pi added 1 commit

2026-10-17 09:04:18 +00:00

🐛 Drop the bound-parameter IN clause from the aggregating-feed filter

/ gitleaks (pull_request) Successful in 15s

Details

/ pr-review (pull_request) Successful in 2m32s

Details

/ checks (pull_request) Successful in 13m1s

Details

/ publish-container (pull_request) Has been skipped

Details

/ deploy-docs (pull_request) Has been skipped

Details

/ container (pull_request) Successful in 1m51s

Details

86442fd1c9

WuMing flagged the parameterised IN: bound-variable ceilings make an
unbatched list a latent OperationalError. Selecting the enabled ids
and intersecting in Python answers the same question with no bound
variables at all, and the feeds table is tiny.

agent-pi commented

2026-10-17 09:04:49 +00:00

src/yunjin/db/feeds.py

SQLite has a maximum of 999 bound variables per statement. If a user subscribes to more than 999 feeds, this query will fail with an OperationalError...

✅ Fixed in commit 86442fd — the query no longer binds the feed-id list at all: it selects the enabled feed ids (the feeds table is tiny) and intersects them with the subscription list in Python, so no bound-variable ceiling applies regardless of subscription count. Behaviour is pinned by the existing filter tests.

[`src/yunjin/db/feeds.py`](https://forge.marvin8.zone/marvin8/yunjin/pulls/90#issuecomment-5980) > SQLite has a maximum of 999 bound variables per statement. If a user subscribes to more than 999 feeds, this query will fail with an OperationalError... ✅ Fixed in commit `86442fd` — the query no longer binds the feed-id list at all: it selects the enabled feed ids (the feeds table is tiny) and intersects them with the subscription list in Python, so no bound-variable ceiling applies regardless of subscription count. Behaviour is pinned by the existing filter tests.

forgejo-actions reviewed

2026-10-17 09:06:49 +00:00

forgejo-actions left a comment

WuMing

Found 3 issue(s). See inline comments below.

## WuMing Found **3** issue(s). See inline comments below.

src/yunjin/db/feeds.py

					
				@ -98,6 +103,7 @@ def get_feed(conn: sqlite3.Connection, feed_id: int) -> Feed | None:

				        updated_at=datetime.fromisoformat(row[7]),

				        http_etag=row[8],

				        http_last_modified=row[9],

				        aggregation_enabled=bool(row[10]),

code [HIGH]

The conversion bool(row[10]) treats SQL NULL as False, which would silently disable aggregation for feeds that were created before the migration (if the migration did not set a default or enforce NOT NULL). The intended default is True (opt-in). Use an explicit NULL-safe conversion like aggregation_enabled = row[10] if row[10] is not None else True or bool(row[10]) if row[10] is not None else True. This pattern repeats at lines 106, 142, 174.

**code** [HIGH] The conversion `bool(row[10])` treats SQL NULL as False, which would silently disable aggregation for feeds that were created before the migration (if the migration did not set a default or enforce NOT NULL). The intended default is `True` (opt-in). Use an explicit NULL-safe conversion like `aggregation_enabled = row[10] if row[10] is not None else True` or `bool(row[10]) if row[10] is not None else True`. This pattern repeats at lines 106, 142, 174.

marvin8 marked this conversation as resolved

src/yunjin/db/feeds.py

					
				@ -133,6 +139,7 @@ def get_feed_by_url(conn: sqlite3.Connection, url: str) -> Feed | None:

				        updated_at=datetime.fromisoformat(row[7]),

				        http_etag=row[8],

				        http_last_modified=row[9],

				        aggregation_enabled=bool(row[10]),

code [HIGH]

Same NULL‑handling issue as line 106.

**code** [HIGH] Same NULL‑handling issue as line 106.

marvin8 marked this conversation as resolved

src/yunjin/db/feeds.py

					
				@ -229,0 +255,4 @@

				    if not feed_ids:

				        return set()

				    cursor = conn.cursor()

				    cursor.execute("SELECT id FROM feeds WHERE aggregation_enabled = 1")

code [HIGH]

The query WHERE aggregation_enabled = 1 excludes rows where the column is NULL. If the migration allowed NULLs and the intention is that NULL means enabled (default-on), this would incorrectly skip those feeds. Should be WHERE aggregation_enabled IS NOT 0 or WHERE aggregation_enabled != 0 to treat NULL as enabled, or the schema must guarantee NOT NULL.

**code** [HIGH] The query `WHERE aggregation_enabled = 1` excludes rows where the column is NULL. If the migration allowed NULLs and the intention is that NULL means enabled (default-on), this would incorrectly skip those feeds. Should be `WHERE aggregation_enabled IS NOT 0` or `WHERE aggregation_enabled != 0` to treat NULL as enabled, or the schema must guarantee NOT NULL.

marvin8 marked this conversation as resolved

agent-pi added 1 commit

2026-10-17 09:22:37 +00:00

✅ Pin the aggregation flag's NOT NULL DEFAULT constraint (#87 )

/ gitleaks (pull_request) Successful in 30s

Details

/ pr-review (pull_request) Successful in 1m6s

Details

/ checks (pull_request) Successful in 12m57s

Details

/ publish-container (pull_request) Has been skipped

Details

/ deploy-docs (pull_request) Has been skipped

Details

/ container (pull_request) Successful in 1m40s

Details

b8e4d640a7

The row mapping (bool(row[10])) and the = 1 filter are sound only
because the migration declares aggregation_enabled NOT NULL with a
default of 1; the schema test holds that guarantee in place so a
future schema drift cannot reintroduce NULLs silently.

agent-pi commented

2026-10-17 09:22:56 +00:00

🔴 Not actioned — the NULL-handling premise does not hold

src/yunjin/db/feeds.py line 106 · src/yunjin/db/feeds.py line 142 · src/yunjin/db/feeds.py line 174

All three findings are conditional on "if the migration did not set a default or enforce NOT NULL" — it enforces both. The migration adds the column as BOOLEAN NOT NULL DEFAULT 1 (verified via PRAGMA table_info(feeds): notnull=1, dflt_value='1'), so:

NULL can never be stored — an explicit NULL insert raises IntegrityError
pre-existing rows report the constant default immediately (SQLite's documented ALTER TABLE ADD COLUMN behaviour), so bool(row[10]) maps them to True, and WHERE aggregation_enabled = 1 cannot skip them

Adding NULL fallbacks would imply a state the schema makes unreachable. The invariant is now pinned by a schema test (test_schema_enforces_not_null_default_on, commit b8e4d64): if the constraint ever drifts, the suite fails before the mapping can be wrong. No further action.

### 🔴 Not actioned — the NULL-handling premise does not hold [`src/yunjin/db/feeds.py` line 106](https://forge.marvin8.zone/marvin8/yunjin/pulls/90#issuecomment-5984) · [`src/yunjin/db/feeds.py` line 142](https://forge.marvin8.zone/marvin8/yunjin/pulls/90#issuecomment-5985) · [`src/yunjin/db/feeds.py` line 174](https://forge.marvin8.zone/marvin8/yunjin/pulls/90#issuecomment-5986) All three findings are conditional on "if the migration did not set a default or enforce NOT NULL" — it enforces both. The migration adds the column as `BOOLEAN NOT NULL DEFAULT 1` (verified via `PRAGMA table_info(feeds)`: `notnull=1`, `dflt_value='1'`), so: - NULL can never be stored — an explicit NULL insert raises `IntegrityError` - pre-existing rows report the constant default immediately (SQLite's documented `ALTER TABLE ADD COLUMN` behaviour), so `bool(row[10])` maps them to `True`, and `WHERE aggregation_enabled = 1` cannot skip them Adding NULL fallbacks would imply a state the schema makes unreachable. The invariant is now pinned by a schema test (`test_schema_enforces_not_null_default_on`, commit `b8e4d64`): if the constraint ever drifts, the suite fails before the mapping can be wrong. No further action.

marvin8 approved these changes

2026-10-17 09:44:42 +00:00

marvin8 manually merged commit 747089a35b into main

2026-10-17 09:45:14 +00:00

Sign in to join this conversation.

No reviewers