WIP! A BB-style forum, on the ATmosphere! We're still working... we'll be back soon when we have something to show off!
node typescript hono htmx atproto

fix(indexer): strip title from reply records at index time (ATB-35) (#55)

* fix(indexer): strip title from reply records at index time (ATB-35)

When a space.atbb.post record carries both a reply ref and a title field,
the indexer now coerces title to null for both INSERT and UPDATE paths.
This enforces the lexicon invariant ("title is omitted for replies") at
the data-ingestion boundary rather than relying on the API or UI to ignore
the field.

ATB-36 is a duplicate of this issue and has been marked accordingly.

* fix(indexer): address PR review feedback (ATB-35)

- Add try-catch to getPostIdByUri, matching the structured error logging
pattern used by all other helper methods
- Warn when reply ref is present but missing $type (rootPostId/parentPostId
will be null; operators now have an observable signal)
- Reaction stub handlers: info → warn (events are being permanently
discarded, not successfully processed)
- Tests: extract createTrackingDb helper to eliminate inline mock duplication
- Tests: add topic-starter title-preserved assertions (right branch of ternary)
- Tests: assert full inserted shape on reply create (rootPostId/parentPostId
null, rootUri/parentUri populated) to document the $type-less behavior

authored by

Malpercio and committed by
GitHub
6586811b 26023224

+214 -41
+187 -30
apps/appview/src/lib/__tests__/indexer.test.ts
··· 59 59 } as unknown as Database; 60 60 }; 61 61 62 + /** 63 + * Builds a mock DB whose transaction captures values passed to insert/update. 64 + * selectResults controls what FK lookup queries return (e.g. board/forum IDs). 65 + */ 66 + function createTrackingDb(selectResults: any[] = []) { 67 + let insertedValues: any = null; 68 + let updatedValues: any = null; 69 + 70 + const db = createMockDb(); 71 + db.transaction = vi.fn().mockImplementation(async (callback) => { 72 + const txContext = { 73 + insert: vi.fn().mockImplementation((_table: any) => ({ 74 + values: vi.fn().mockImplementation((vals: any) => { 75 + insertedValues = vals; 76 + return Promise.resolve(undefined); 77 + }), 78 + })), 79 + select: vi.fn().mockReturnValue({ 80 + from: vi.fn().mockReturnValue({ 81 + where: vi.fn().mockReturnValue({ 82 + limit: vi.fn().mockResolvedValue(selectResults), 83 + }), 84 + }), 85 + }), 86 + update: vi.fn().mockImplementation((_table: any) => ({ 87 + set: vi.fn().mockImplementation((vals: any) => { 88 + updatedValues = vals; 89 + return { where: vi.fn().mockResolvedValue(undefined) }; 90 + }), 91 + })), 92 + delete: vi.fn(), 93 + }; 94 + return await callback(txContext); 95 + }); 96 + 97 + return { 98 + db, 99 + getInsertedValues: () => insertedValues, 100 + getUpdatedValues: () => updatedValues, 101 + }; 102 + } 103 + 62 104 describe("Indexer", () => { 63 105 let mockDb: Database; 64 106 let indexer: Indexer; ··· 129 171 }); 130 172 131 173 it("should handle post creation with board reference", async () => { 132 - // Track what values are inserted 133 - let insertedValues: any = null; 134 - 135 174 const mockBoardId = BigInt(123); 136 - const mockDbWithTracking = createMockDb(); 137 - mockDbWithTracking.transaction = vi.fn().mockImplementation(async (callback) => { 138 - const txContext = { 139 - insert: vi.fn().mockImplementation((_table: any) => { 140 - return { 141 - values: vi.fn().mockImplementation((vals: any) => { 142 - insertedValues = vals; 143 - return Promise.resolve(undefined); 144 - }), 145 - }; 146 - }), 147 - select: vi.fn().mockReturnValue({ 148 - from: vi.fn().mockReturnValue({ 149 - where: vi.fn().mockReturnValue({ 150 - limit: vi.fn().mockResolvedValue([{ id: mockBoardId }]), 151 - }), 152 - }), 153 - }), 154 - update: vi.fn(), 155 - delete: vi.fn(), 156 - }; 157 - return await callback(txContext); 158 - }); 159 - 160 - const indexer = new Indexer(mockDbWithTracking, mockLogger); 175 + const { db: trackingDb, getInsertedValues } = createTrackingDb([{ id: mockBoardId }]); 176 + const boardIndexer = new Indexer(trackingDb, mockLogger); 161 177 const boardUri = "at://did:plc:forum/space.atbb.forum.board/board1"; 162 178 163 179 const event: CommitCreateEvent<"space.atbb.post"> = { ··· 190 206 }, 191 207 }; 192 208 193 - await indexer.handlePostCreate(event); 209 + await boardIndexer.handlePostCreate(event); 194 210 195 211 // Verify the insert values include boardUri and boardId 212 + const insertedValues = getInsertedValues(); 196 213 expect(insertedValues).toBeDefined(); 197 214 expect(insertedValues.boardUri).toBe(boardUri); 198 215 expect(insertedValues.boardId).toBe(mockBoardId); ··· 234 251 expect(mockDb.insert).toHaveBeenCalled(); 235 252 }); 236 253 254 + it("strips title when indexing a reply on create (title: null regardless of record)", async () => { 255 + const { db: trackingDb, getInsertedValues } = createTrackingDb(); 256 + const replyIndexer = new Indexer(trackingDb, mockLogger); 257 + const rootParentUri = "at://did:plc:user1/space.atbb.post/topic1"; 258 + 259 + const event: CommitCreateEvent<"space.atbb.post"> = { 260 + did: "did:plc:test123", 261 + time_us: 1234567890, 262 + kind: "commit", 263 + commit: { 264 + rev: "abc", 265 + operation: "create", 266 + collection: "space.atbb.post", 267 + rkey: "reply1", 268 + cid: "cid456", 269 + record: { 270 + $type: "space.atbb.post", 271 + text: "Reply with a title (should be stripped)", 272 + title: "This should not be stored", 273 + // No $type on the reply ref — isReplyRef() returns false; 274 + // rootPostId/parentPostId are null while rootUri/parentUri are populated via optional chaining. 275 + reply: { 276 + root: { uri: rootParentUri, cid: "cidRoot" }, 277 + parent: { uri: rootParentUri, cid: "cidParent" }, 278 + }, 279 + createdAt: "2024-01-01T01:00:00Z", 280 + } as any, 281 + }, 282 + }; 283 + 284 + await replyIndexer.handlePostCreate(event); 285 + 286 + const vals = getInsertedValues(); 287 + expect(vals).toBeDefined(); 288 + expect(vals.title).toBeNull(); 289 + // $type-less reply ref: IDs not resolved, URIs populated via optional chaining 290 + expect(vals.rootPostId).toBeNull(); 291 + expect(vals.parentPostId).toBeNull(); 292 + expect(vals.rootUri).toBe(rootParentUri); 293 + expect(vals.parentUri).toBe(rootParentUri); 294 + }); 295 + 296 + it("strips title when indexing a reply on update (title: null regardless of record)", async () => { 297 + const { db: trackingDb, getUpdatedValues } = createTrackingDb(); 298 + const replyIndexer = new Indexer(trackingDb, mockLogger); 299 + 300 + const updateEvent: CommitUpdateEvent<"space.atbb.post"> = { 301 + did: "did:plc:test123", 302 + time_us: 1234567890, 303 + kind: "commit", 304 + commit: { 305 + rev: "abc", 306 + operation: "update", 307 + collection: "space.atbb.post", 308 + rkey: "reply1", 309 + cid: "cid789", 310 + record: { 311 + $type: "space.atbb.post", 312 + text: "Updated reply text", 313 + title: "Title that should be stripped on update", 314 + reply: { 315 + root: { uri: "at://did:plc:user1/space.atbb.post/topic1", cid: "cidRoot" }, 316 + parent: { uri: "at://did:plc:user1/space.atbb.post/topic1", cid: "cidParent" }, 317 + }, 318 + createdAt: "2024-01-01T01:00:00Z", 319 + } as any, 320 + }, 321 + }; 322 + 323 + await replyIndexer.handlePostUpdate(updateEvent); 324 + 325 + const vals = getUpdatedValues(); 326 + expect(vals).toBeDefined(); 327 + expect(vals.title).toBeNull(); 328 + }); 329 + 330 + it("preserves title when indexing a topic starter on create", async () => { 331 + const { db: trackingDb, getInsertedValues } = createTrackingDb(); 332 + const topicIndexer = new Indexer(trackingDb, mockLogger); 333 + 334 + const event: CommitCreateEvent<"space.atbb.post"> = { 335 + did: "did:plc:test123", 336 + time_us: 1234567890, 337 + kind: "commit", 338 + commit: { 339 + rev: "abc", 340 + operation: "create", 341 + collection: "space.atbb.post", 342 + rkey: "topic1", 343 + cid: "cid111", 344 + record: { 345 + $type: "space.atbb.post", 346 + text: "Topic body text", 347 + title: "My topic title", 348 + createdAt: "2024-01-01T00:00:00Z", 349 + } as any, 350 + }, 351 + }; 352 + 353 + await topicIndexer.handlePostCreate(event); 354 + 355 + const vals = getInsertedValues(); 356 + expect(vals).toBeDefined(); 357 + expect(vals.title).toBe("My topic title"); 358 + expect(vals.rootPostId).toBeNull(); 359 + expect(vals.parentPostId).toBeNull(); 360 + expect(vals.rootUri).toBeNull(); 361 + expect(vals.parentUri).toBeNull(); 362 + }); 363 + 364 + it("preserves title when indexing a topic starter on update", async () => { 365 + const { db: trackingDb, getUpdatedValues } = createTrackingDb(); 366 + const topicIndexer = new Indexer(trackingDb, mockLogger); 367 + 368 + const event: CommitUpdateEvent<"space.atbb.post"> = { 369 + did: "did:plc:test123", 370 + time_us: 1234567890, 371 + kind: "commit", 372 + commit: { 373 + rev: "abc", 374 + operation: "update", 375 + collection: "space.atbb.post", 376 + rkey: "topic1", 377 + cid: "cid222", 378 + record: { 379 + $type: "space.atbb.post", 380 + text: "Updated topic body", 381 + title: "Updated topic title", 382 + createdAt: "2024-01-01T00:00:00Z", 383 + } as any, 384 + }, 385 + }; 386 + 387 + await topicIndexer.handlePostUpdate(event); 388 + 389 + const vals = getUpdatedValues(); 390 + expect(vals).toBeDefined(); 391 + expect(vals.title).toBe("Updated topic title"); 392 + }); 393 + 237 394 it("should handle post update", async () => { 238 - 395 + 239 396 240 397 const event: CommitUpdateEvent<"space.atbb.post"> = { 241 398 did: "did:plc:test123",
+27 -11
apps/appview/src/lib/indexer.ts
··· 91 91 if (Post.isReplyRef(record.reply)) { 92 92 rootId = await this.getPostIdByUri(record.reply.root.uri, tx); 93 93 parentId = await this.getPostIdByUri(record.reply.parent.uri, tx); 94 + } else if (record.reply) { 95 + // reply ref present but $type omitted — rootPostId/parentPostId will be null 96 + this.logger.warn("Post reply ref missing $type — rootPostId/parentPostId not resolved", { 97 + operation: "Post CREATE", 98 + postDid: event.did, 99 + postRkey: event.commit.rkey, 100 + }); 94 101 } 95 102 96 103 // Look up board ID if board reference exists ··· 113 120 did: event.did, 114 121 rkey: event.commit.rkey, 115 122 cid: event.commit.cid, 116 - title: record.title ?? null, 123 + title: record.reply ? null : (record.title ?? null), 117 124 text: record.text, 118 125 forumUri: record.forum?.forum.uri ?? null, 119 126 boardUri: record.board?.board.uri ?? null, ··· 145 152 146 153 return { 147 154 cid: event.commit.cid, 148 - title: record.title ?? null, 155 + title: record.reply ? null : (record.title ?? null), 149 156 text: record.text, 150 157 forumUri: record.forum?.forum.uri ?? null, 151 158 boardUri: record.board?.board.uri ?? null, ··· 870 877 async handleReactionCreate( 871 878 event: CommitCreateEvent<"space.atbb.reaction"> 872 879 ) { 873 - this.logger.info("Reaction created (not implemented)", { did: event.did, rkey: event.commit.rkey }); 880 + this.logger.warn("Reaction created (not implemented)", { did: event.did, rkey: event.commit.rkey }); 874 881 // TODO: Add reactions table to schema 875 882 } 876 883 877 884 async handleReactionUpdate( 878 885 event: CommitUpdateEvent<"space.atbb.reaction"> 879 886 ) { 880 - this.logger.info("Reaction updated (not implemented)", { did: event.did, rkey: event.commit.rkey }); 887 + this.logger.warn("Reaction updated (not implemented)", { did: event.did, rkey: event.commit.rkey }); 881 888 // TODO: Add reactions table to schema 882 889 } 883 890 884 891 async handleReactionDelete( 885 892 event: CommitDeleteEvent<"space.atbb.reaction"> 886 893 ) { 887 - this.logger.info("Reaction deleted (not implemented)", { did: event.did, rkey: event.commit.rkey }); 894 + this.logger.warn("Reaction deleted (not implemented)", { did: event.did, rkey: event.commit.rkey }); 888 895 // TODO: Add reactions table to schema 889 896 } 890 897 ··· 982 989 const parsed = parseAtUri(postUri); 983 990 if (!parsed) return null; 984 991 985 - const result = await dbOrTx 986 - .select({ id: posts.id }) 987 - .from(posts) 988 - .where(and(eq(posts.did, parsed.did), eq(posts.rkey, parsed.rkey))) 989 - .limit(1); 992 + try { 993 + const result = await dbOrTx 994 + .select({ id: posts.id }) 995 + .from(posts) 996 + .where(and(eq(posts.did, parsed.did), eq(posts.rkey, parsed.rkey))) 997 + .limit(1); 990 998 991 - return result.length > 0 ? result[0].id : null; 999 + return result.length > 0 ? result[0].id : null; 1000 + } catch (error) { 1001 + this.logger.error("Database error in getPostIdByUri", { 1002 + operation: "getPostIdByUri", 1003 + postUri, 1004 + error: error instanceof Error ? error.message : String(error), 1005 + }); 1006 + throw error; 1007 + } 992 1008 } 993 1009 994 1010 /**