Claude finished @Yeraze's task —— View job

PR Review — refactor(test): migrate 4 remaining multi-backend test files to createTestDb helper

• Read changed files
• Check createTestDb helper pattern
• Analyze code quality and correctness
• Post review feedback

This is a clean, well-structured refactor. The approach is consistent with the other 25 files migrated earlier and the code quality is good. A few observations:

Positive Highlights

• SQLite path is now correct-by-construction. Using createTestDb() (which runs the real migration registry) means the SQLite backend tests will always match production schema. This is exactly the right fix — the drifted hand-rolled DDL was the root cause of feat(map): position-history markers at every fix, points-only mode, hover tooltip (#3492) #3495-style silent failures.
• Foreign key seeding in misc.test.ts is thoughtful. The beforeEach try/catch for inserting user id=1 handles the real FK constraint from migration 041 while gracefully skipping PG/MySQL where the test DDL doesn't include the users table.
• insertNodeSql in notifications.test.ts correctly adds nodeId and sourceId for the SQLite path, matching the real schema's NOT NULL and composite PK requirements.

Issues / Concerns

1. notifications.test.ts — messages schema divergence for PG/MySQL (minor, existing pattern)

The PG and MySQL POSTGRES_CREATE/MYSQL_CREATE blocks in notifications.test.ts define a messages table without a sourceId column (lines 125–145, 261–281). The production messages schema has sourceId. This PR doesn't introduce this gap, but it's worth noting as a follow-up: the PG/MySQL DDL for notifications.test.ts may drift again for the same reasons the SQLite DDL did. This pattern is already an accepted trade-off for PG/MySQL since those run only in CI with test containers.

2. notifications.test.ts — insertMessageSql SQLite vs PG/MySQL asymmetry (functional concern)

insertMessageSql (line 333) includes createdAt for SQLite only:

// SQLite:
`... timestamp, createdAt) VALUES (... ${timestamp}, ${timestamp})`
// PG/MySQL:
`... timestamp) VALUES (... ${timestamp})`

The PG messages DDL (line 136) has no createdAt column and no NOT NULL constraint for it, so PG tests pass. But if someone adds createdAt NOT NULL to the PG/MySQL DDL in the future, these inserts will silently fail. Not a bug today, just a flag for future maintainers.

3. notifications.test.ts — markChannelMessagesAsRead userId=null test (latent risk)

The test at line 741–754 inserts a message but passes userId = null to markChannelMessagesAsRead, with a comment that "For SQLite, userId 0 doesn't need to exist (no FK constraint enforced by default)". With createTestDb(), foreign_keys=ON is actually enabled by migration 041 — if read_messages has an FK on userId → users.id, inserting with userId=0 when there's no user with id=0 could fail or be silently masked. The current test uses expect(typeof count).toBe('number') which only checks no exception is thrown, but this tolerance could hide a real behavioral change. Worth a closer look at whether migration 041 adds that FK on read_messages.

4. misc.test.ts — ALL_TABLES cleanup list no longer needed for SQLite (minor, no harm)

The ALL_TABLES constant (lines 153–160) is used only in the PG/MySQL beforeEach for clearTable(). The SQLite backend now creates a fresh createTestDb() instance per test (lines 585–593), so ALL_TABLES isn't referenced in the SQLite path. This is correct behavior — just a note that the constant's scope is PG/MySQL only.

5. neighbors.test.ts — no schema drift concerns ✓

The simplest of the four; clean migration with no notable issues.

6. traceroutes.test.ts — no schema drift concerns ✓

Also clean. The DbTraceroute/DbRouteSegment type imports reinforce that the repository contract is being tested, not just raw SQL.

Summary

The core change is correct and beneficial. The SQLite path for all four repositories now uses the real migration-built schema, eliminating future drift. The PG/MySQL paths are unchanged (still hand-rolled DDL in POSTGRES_CREATE/MYSQL_CREATE constants), which is the accepted pattern for this codebase.

The one item worth a follow-up is point 3 above — verifying the userId=null behavior in the markChannelMessagesAsRead test works correctly now that foreign_keys=ON is active via createTestDb().

Verdict: Approve with optional follow-up on point 3.