[rooms] improve tag query performance

apps/rooms/migrations/0013_room_tag.sql

@@ -0,0 +1,43 @@
+-- Room tags as their own table, mirroring the `api` worker's `event_tag` (0010). One row
+-- per tag per room. Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) —
+-- keep in sync.
+--
+-- The table is AUTHORITATIVE and the blob's `Tags` key is removed below, the same
+-- arrangement 0007/0008 gave subrooms and their saves: one place a tag is stored, so the
+-- two can't drift. `serializeRoom` drops `Tags` on write and the reads re-attach it, so
+-- the room DTO the client sees is unchanged.
+--
+-- The point is the lookup. Every tag-filtered read — a discovery category row
+-- (`/algorithmiclists/quests_algoendpoint`), a `#tag` room search, the `base` template
+-- list — used to SELECT every room and parse each blob just to ask what it was tagged.
+-- They now narrow in SQL off `idx_room_tag_tag` and read only the blobs that match.
+--
+-- `tag` is stored lowercased: it is the lookup key, and every comparison in rooms-db.ts
+-- was already case-insensitive, so nothing downstream can tell. `type` is the client's
+-- tag-category int — 0 for a user tag, 2 for the auto-derived ones like `rro` — echoed
+-- back as stored.
+
+CREATE TABLE IF NOT EXISTS room_tag (
+  room_id INTEGER NOT NULL,
+  tag TEXT NOT NULL,
+  type INTEGER NOT NULL DEFAULT 0,
+  PRIMARY KEY (room_id, tag)
+  );
+CREATE INDEX IF NOT EXISTS idx_room_tag_tag ON room_tag (tag);
+
+-- Backfill from the blobs. `json_each` walks the `Tags` array; a room with no array (or a
+-- null one) contributes nothing, which is why this is a join rather than a correlated
+-- subquery. `INSERT OR IGNORE` collapses a room that somehow carries the same tag twice
+-- in different casing — the primary key is the lowercased name.
+INSERT OR IGNORE INTO room_tag (room_id, tag, type)
+  SELECT
+    r.room_id,
+    lower(json_extract(t.value, '$.Tag')),
+    COALESCE(json_extract(t.value, '$.Type'), 0)
+  FROM room r, json_each(r.data, '$.Tags') t
+  WHERE json_extract(t.value, '$.Tag') IS NOT NULL;
+
+-- Single source of truth: the tags now live in `room_tag`, so the copy in the blob goes.
+-- Leaving it would be a second answer to "what is this room tagged" that only the writes
+-- through toggleRoomTag keep current.
+UPDATE room SET data = json_remove(data, '$.Tags');

apps/rooms/src/test/integration/api.test.ts

@@ -2299,6 +2299,127 @@ describe('rooms endpoints', () => {
 		expect(tagsIn(byCoOwner)).toContain('spooky')
 	})
 
+	// Tags live in `room_tag`, not in the room blob (migration 0013). These pin the
+	// invariant that makes that safe: the table is the only copy, and the DTO is rebuilt
+	// from it on read.
+	it('stores tags in room_tag and never in the room blob', async () => {
+		await putForm('/rooms/2/tags', { tag: 'ghostly' }, '1')
+
+		// The blob must carry no `Tags` key at all — a second copy is what the table exists
+		// to eliminate, and it would only be kept current by the writes that go through
+		// toggleRoomTag.
+		const row = await env.DB.prepare('SELECT data FROM room WHERE room_id = 2').first<{
+			data: string
+		}>()
+		expect(JSON.parse(row!.data).Tags).toBeUndefined()
+
+		// The table has it, lowercased — `tag` is the lookup key the index is on.
+		const tagRow = await env.DB.prepare(
+			'SELECT tag, type FROM room_tag WHERE room_id = 2 AND tag = ?1'
+		)
+			.bind('ghostly')
+			.first<{ tag: string; type: number }>()
+		expect(tagRow).toEqual({ tag: 'ghostly', type: 0 })
+
+		// And the read re-attaches it, so the client sees an unchanged DTO.
+		const room = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as {
+			Tags: Array<{ Tag: string; Type: number }>
+		}
+		expect(room.Tags).toContainEqual({ Tag: 'ghostly', Type: 0 })
+
+		// Toggle back off: the row goes, rather than lingering to answer `#ghostly` searches.
+		await putForm('/rooms/2/tags', { tag: 'ghostly' }, '1')
+		expect(
+			await env.DB.prepare('SELECT COUNT(*) AS n FROM room_tag WHERE room_id = 2 AND tag = ?1')
+				.bind('ghostly')
+				.first<{ n: number }>()
+		).toEqual({ n: 0 })
+	})
+
+	// D1 caps a statement at 100 bound parameters. The seeded database has 45 rooms, so
+	// every `IN (…)` list built per-room fitted and the cap went unnoticed until a real
+	// server crossed it: the hot feed attaches tags to EVERY room, so the bind list grew
+	// with the database and the query failed with "variable number must be between ?1 and
+	// ?100". This seeds past the cap so the feeds are exercised above it.
+	it('serves the feeds when there are more rooms than D1 allows bound parameters', async () => {
+		const FIRST = 20000
+		const COUNT = 150
+		for (let i = 0; i < COUNT; i++) {
+			const roomId = FIRST + i
+			await env.DB.prepare('INSERT INTO room (data) VALUES (?1)')
+				.bind(
+					JSON.stringify({
+						RoomId: roomId,
+						Name: `BulkRoom${i}`,
+						CreatorAccountId: 700,
+						IsDorm: false,
+						Accessibility: 1,
+						CreatedAt: '2026-05-01T00:00:00Z',
+						SubRooms: [],
+					})
+				)
+				.run()
+			// Every third one tagged, so the tag map is non-trivial above the cap too.
+			if (i % 3 === 0) {
+				await env.DB.prepare('INSERT INTO room_tag (room_id, tag, type) VALUES (?1, ?2, 0)')
+					.bind(roomId, 'bulky')
+					.run()
+			}
+		}
+
+		// The feed that failed: it reads every room, so it attaches tags for all of them.
+		const hot = await SELF.fetch(`${ORIGIN}/rooms/hot?take=5`)
+		expect(hot.status).toBe(200)
+
+		// Rooms owned by one account — this hydrates the WHOLE result set rather than a page,
+		// so its subroom and save reads run above the cap too. Those had the same latent bug.
+		const mine = await SELF.fetch(`${ORIGIN}/rooms/createdby/me`, { headers: await bearer('700') })
+		expect(mine.status).toBe(200)
+		expect(((await mine.json()) as unknown[]).length).toBe(COUNT)
+
+		// And the tag lookup still answers correctly above the cap.
+		const tagged = (await (
+			await SELF.fetch(`${ORIGIN}/rooms/search?query=%23bulky&take=200`)
+		).json()) as { Results: Array<{ RoomId: number }> }
+		expect(tagged.Results.length).toBe(Math.ceil(COUNT / 3))
+
+		// Clean up so the room counts other tests assert stay put.
+		await env.DB.prepare('DELETE FROM room WHERE room_id >= ?1').bind(FIRST).run()
+		await env.DB.prepare('DELETE FROM room_tag WHERE room_id >= ?1').bind(FIRST).run()
+	})
+
+	it("drops a deleted room's tag rows", async () => {
+		// Throwaway room owned by account 1, seeded directly the way the DELETE test above
+		// does, so this doesn't disturb the imported rooms.
+		await env.DB.prepare('INSERT INTO room (data) VALUES (?1)')
+			.bind(
+				JSON.stringify({
+					RoomId: 9600,
+					Name: 'TagCascadeRoom',
+					CreatorAccountId: 1,
+					IsDorm: false,
+					Accessibility: 1,
+					SubRooms: [],
+				})
+			)
+			.run()
+		await putForm('/rooms/9600/tags', { tag: 'doomed' }, '1')
+
+		const count = async () =>
+			(await env.DB.prepare('SELECT COUNT(*) AS n FROM room_tag WHERE room_id = 9600').first<{
+				n: number
+			}>())!.n
+		expect(await count()).toBe(1)
+
+		expect(
+			(await SELF.fetch(`${ORIGIN}/rooms/9600`, { method: 'DELETE', headers: await bearer('1') }))
+				.status
+		).toBe(200)
+		// Otherwise the tag rows keep answering `#doomed` searches and category rows for a
+		// room nobody can open.
+		expect(await count()).toBe(0)
+	})
+
 	it('PUT /rooms/:id/name is auth-gated, owner-only, unique, and persists', async () => {
 		// No token → 401 (auth gate).
 		expect((await putForm('/rooms/2/name', { name: 'Whatever' })).status).toBe(401)