From 8da4ac549308af1046ef08d97b64c6a61bbe0da9 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:35:14 +0200 Subject: [PATCH 01/27] Proper pluralisation for num records --- libraries/utils/src/slackNotifications.test.ts | 6 +++--- libraries/utils/src/slackNotifications.ts | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index 6f7a46914..5a2893f47 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -151,7 +151,7 @@ describe('slackNotifications', () => { const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); expect(callBody.text).toContain('test-app:'); - expect(callBody.text).toContain('This error occurred 3 times affecting 3 record(s)'); + expect(callBody.text).toContain('This error occurred 3 times affecting 3 records'); expect(callBody.text).toContain('rec3BGObwkLPSskvb'); expect(callBody.text).toContain('rec3qTvZcFGYccFcl'); expect(callBody.text).toContain('rec3060zKybkbm9UH'); @@ -281,7 +281,7 @@ describe('slackNotifications', () => { await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 2 record(s)'); // Only unique records + expect(callBody.text).toContain('affecting 2 records'); // Only unique records expect(callBody.text).toContain('rec1AbCdEfGhIjKl'); expect(callBody.text).toContain('rec2MnOpQrStUvWx'); }); @@ -301,7 +301,7 @@ describe('slackNotifications', () => { await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 15 record(s)'); + expect(callBody.text).toContain('affecting 15 records'); expect(callBody.text).toContain('and 5 more'); // 15 - 10 = 5 }); diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index b2b29b44e..0156a004c 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -234,8 +234,9 @@ const flushBatcher = async (batcher: BatcherState) => { if (batch.affectedRecords.size > 0) { const recordList = Array.from(batch.affectedRecords).slice(0, 10).join(', '); const moreRecords = batch.affectedRecords.size > 10 ? ` and ${batch.affectedRecords.size - 10} more` : ''; + const recordNoun = batch.affectedRecords.size === 1 ? 'record' : 'records'; - messages.push(`${header} affecting ${batch.affectedRecords.size} record(s):\n${recordList}${moreRecords}`); + messages.push(`${header} affecting ${batch.affectedRecords.size} ${recordNoun}:\n${recordList}${moreRecords}`); } else { messages.push(`${header}.`); } From 0f444a1bd468c14d4e21368d46e936f9eac3c6a1 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:43:05 +0200 Subject: [PATCH 02/27] Extract `BatchGroup` to abstract knowledged of how prose is batched --- libraries/utils/src/slackNotifications.ts | 89 ++++++++++++----------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index 0156a004c..fdc87e284 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -4,25 +4,44 @@ type SlackAlertEnv = { ALERTS_SLACK_CHANNEL_ID: string; }; +// Domain-neutral batching metadata. Callers describe how to group and summarise their +// messages without the util needing to know anything about the message contents. +type BatchGroup = { + // Grouping key: messages sharing a signature collapse into one batch. Defaults to the + // message text, so callers wanting to group by e.g. error type (ignoring embedded IDs) + // must supply a stable signature themselves. + signature?: string; + // Distinct affected items. Deduplicated across the window, counted, and listed in the + // batched summary ("affecting N …", first 10 + "and M more"). + dedupeKeys?: string[]; + // Noun for the count line, e.g. 'record'. Pluralised automatically. Defaults to 'item'. + itemNoun?: string; + // Extra header fragments appended to the batched summary, each rendered as ` (fragment)`, + // e.g. 'Table: tbl123'. + annotations?: string[]; +}; + type SlackAlertOptions = { channelId?: string; batchKey?: string; flushIntervalMs?: number; - // When a batched signature affects at least this many distinct records within a + // When a batched signature affects at least this many distinct items within a // flush window, cross-post the batched summary to escalationChannelId. Lets a // low-priority side channel stay quiet for routine drift while a sudden base-wide // spike still reaches the main alerts channel. spikeThreshold?: number; escalationChannelId?: string; + // How to group and summarise batched messages. Only used when batchKey is set. + batchGroup?: BatchGroup; }; type MessageBatch = { signature: string; occurrences: number; messages: string[]; - tableId: string | null; - fieldId: string | null; - affectedRecords: Set; + annotations: string[]; + itemNoun: string; + dedupedItems: Set; lastSeen: number; }; @@ -69,6 +88,7 @@ export const slackAlert = async ( addToBatch(getBatcherKey(options.batchKey, env, channelId), env, messages, channelId, flushIntervalMs, { spikeThreshold: options.spikeThreshold, escalationChannelId: options.escalationChannelId, + batchGroup: options.batchGroup, }); return; } @@ -95,15 +115,16 @@ const addToBatch = ( messages: string[], channelId: string, flushIntervalMs: number, - spikeOptions: { spikeThreshold?: number; escalationChannelId?: string }, + spikeOptions: { spikeThreshold?: number; escalationChannelId?: string; batchGroup?: BatchGroup }, ) => { const [mainMessage] = messages; if (!mainMessage) { return; } - const { tableId, fieldId, recordIds } = extractAirtableIds(mainMessage); - const signature = getMessageSignature(mainMessage); + const { batchGroup } = spikeOptions; + const signature = batchGroup?.signature ?? mainMessage; + const dedupeKeys = batchGroup?.dedupeKeys ?? []; let batcher = batchers.get(batchKey); @@ -128,17 +149,17 @@ const addToBatch = ( if (existing) { existing.occurrences += 1; existing.lastSeen = Date.now(); - recordIds.forEach((id) => { - existing.affectedRecords.add(id); + dedupeKeys.forEach((id) => { + existing.dedupedItems.add(id); }); } else { batcher.batches.set(signature, { signature, occurrences: 1, messages, - tableId, - fieldId, - affectedRecords: new Set(recordIds), + annotations: batchGroup?.annotations ?? [], + itemNoun: batchGroup?.itemNoun ?? 'item', + dedupedItems: new Set(dedupeKeys), lastSeen: Date.now(), }); } @@ -149,21 +170,6 @@ const addToBatch = ( }); }; -const getMessageSignature = (message: string) => { - // Match Airtable IDs (tbl/fld/rec + 10+ alphanumeric chars) - return message - .replace(/\btbl[A-Za-z0-9]{10,}/g, 'tbl***') - .replace(/\bfld[A-Za-z0-9]{10,}/g, 'fld***') - .replace(/\brec[A-Za-z0-9]{10,}/g, 'rec***'); -}; - -const extractAirtableIds = (message: string) => { - const tableId = (/\btbl[A-Za-z0-9]{10,}/.exec(message))?.[0] ?? null; - const fieldId = (/\bfld[A-Za-z0-9]{10,}/.exec(message))?.[0] ?? null; - const recordIds = message.match(/\brec[A-Za-z0-9]{10,}/g) ?? []; - return { tableId, fieldId, recordIds }; -}; - const shouldForceFlush = (batcher: BatcherState, flushIntervalMs: number) => { // Hard upper bound: flush at most 5× the interval after the first message // This prevents never flushing if messages keep coming in at a high rate, while still allowing for bursts of messages to be batched together @@ -225,18 +231,17 @@ const flushBatcher = async (batcher: BatcherState) => { // Build main message if (batch.occurrences > 1) { - const tableInfo = batch.tableId ? ` (Table: ${batch.tableId})` : ''; - const fieldInfo = batch.fieldId ? ` (Field: ${batch.fieldId})` : ''; - const header = `${mainMessage}${tableInfo}${fieldInfo}\n\n⚠️ This error occurred ${batch.occurrences} times`; - - // Only Airtable-derived warnings carry record IDs; other batched errors - // (e.g. tRPC server errors) have none, so drop the record clause for them. - if (batch.affectedRecords.size > 0) { - const recordList = Array.from(batch.affectedRecords).slice(0, 10).join(', '); - const moreRecords = batch.affectedRecords.size > 10 ? ` and ${batch.affectedRecords.size - 10} more` : ''; - const recordNoun = batch.affectedRecords.size === 1 ? 'record' : 'records'; - - messages.push(`${header} affecting ${batch.affectedRecords.size} ${recordNoun}:\n${recordList}${moreRecords}`); + const annotationInfo = batch.annotations.map((a) => ` (${a})`).join(''); + const header = `${mainMessage}${annotationInfo}\n\n⚠️ This error occurred ${batch.occurrences} times`; + + // Only callers that pass dedupeKeys carry affected items; others (e.g. tRPC + // server errors) have none, so drop the item clause for them. + if (batch.dedupedItems.size > 0) { + const itemList = Array.from(batch.dedupedItems).slice(0, 10).join(', '); + const moreItems = batch.dedupedItems.size > 10 ? ` and ${batch.dedupedItems.size - 10} more` : ''; + const noun = batch.dedupedItems.size === 1 ? batch.itemNoun : `${batch.itemNoun}s`; + + messages.push(`${header} affecting ${batch.dedupedItems.size} ${noun}:\n${itemList}${moreItems}`); } else { messages.push(`${header}.`); } @@ -272,9 +277,9 @@ const maybeEscalateSpike = async (batcher: BatcherState, batch: MessageBatch, ma return; } - // Distinct records is the blast radius; fall back to raw occurrences when a - // warning carries no record IDs (e.g. a retry loop on the same broken field). - const spikeCount = batch.affectedRecords.size > 0 ? batch.affectedRecords.size : batch.occurrences; + // Distinct affected items is the blast radius; fall back to raw occurrences when a + // batch carries no dedupeKeys (e.g. a retry loop on the same broken field). + const spikeCount = batch.dedupedItems.size > 0 ? batch.dedupedItems.size : batch.occurrences; if (spikeCount < spikeThreshold) { return; } From c958211bf444e510851497eae306ba54fcc5bdf1 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:43:47 +0200 Subject: [PATCH 03/27] `formatAirtableWarning` in DB --- libraries/db/src/index.ts | 3 + libraries/db/src/lib/formatAirtableWarning.ts | 59 +++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 libraries/db/src/lib/formatAirtableWarning.ts diff --git a/libraries/db/src/index.ts b/libraries/db/src/index.ts index 5ef73ef7a..a8bd12f01 100644 --- a/libraries/db/src/index.ts +++ b/libraries/db/src/index.ts @@ -117,5 +117,8 @@ export type { DeprecationSafeTable } from './lib/db-core'; export { AirtableTsError, ErrorType } from 'airtable-ts/dist/AirtableTsError'; +export { formatAirtableWarning } from './lib/formatAirtableWarning'; +export type { FormattedAirtableWarning } from './lib/formatAirtableWarning'; + // TODO: restrict what's exported export * from 'drizzle-orm'; diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts new file mode 100644 index 000000000..91094931a --- /dev/null +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -0,0 +1,59 @@ +/** + * airtable-ts surfaces read-validation failures as one deeply-nested error string, e.g.: + * Failed to map record from Airtable format for table 'self_serve_course_registration' + * (tbl…) and record rec…: Failed to map field fullName (fld…) from Airtable: Cannot + * convert value from airtable type 'multipleLookupValues' to 'string | null', as the + * Airtable API provided a 'object'. Suggestion: Update the types... + * + * `formatAirtableWarning` turns that into concise prose plus domain-neutral batching + * metadata (`slackAlert`'s `batchGroup`), so alerts read cleanly and group by + * table/field/error rather than by record. Returns null when the shape can't be parsed, + * letting callers fall back to the raw message. + */ +export type FormattedAirtableWarning = { + message: string; + batchGroup: { + signature: string; + dedupeKeys: string[]; + itemNoun: string; + annotations: string[]; + }; +}; + +export const formatAirtableWarning = (raw: string): FormattedAirtableWarning | null => { + const record = /for table '(?[^']*)' \((?tbl[A-Za-z0-9]+)\) and record (?rec[A-Za-z0-9]+)/.exec(raw); + if (!record?.groups) { + return null; + } + const { tableName, tableId, recordId } = record.groups; + + const field = /Failed to map field (?.+?) \((?fld[A-Za-z0-9]+)\)/.exec(raw); + const types = /from airtable type '(?[^']*)' to '(?[^']*)', as the Airtable API provided a '(?[^']*)'/.exec(raw); + + const location = field?.groups + ? `Field \`${field.groups.fieldName}\` on \`${tableName}\` (record ${recordId})` + : `Record ${recordId} on \`${tableName}\``; + + const reason = types?.groups + ? `can't map Airtable ${types.groups.fromType} → ${types.groups.tsType} (got ${types.groups.providedType})` + : raw; + + const message = `${location}: ${reason}. Set to undefined.`; + + const annotations = [ + `Table: ${tableId}`, + ...(field?.groups ? [`Field: ${field.groups.fieldId}`] : []), + ]; + + return { + message, + batchGroup: { + // Group by table/field/error, not by record: strip the one varying ID from the + // human-readable message so the same failure on different records collapses. + signature: message.replace(recordId!, 'REC'), + dedupeKeys: [recordId!], + itemNoun: 'record', + annotations, + }, + }; +}; From 665f3a790c6b1a3c619448594fa061add812c309 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:43:59 +0200 Subject: [PATCH 04/27] `formatAirtableWarning` tests --- .../db/src/lib/formatAirtableWarning.test.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 libraries/db/src/lib/formatAirtableWarning.test.ts diff --git a/libraries/db/src/lib/formatAirtableWarning.test.ts b/libraries/db/src/lib/formatAirtableWarning.test.ts new file mode 100644 index 000000000..e290fb65c --- /dev/null +++ b/libraries/db/src/lib/formatAirtableWarning.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from 'vitest'; +import { formatAirtableWarning } from './formatAirtableWarning'; + +describe('formatAirtableWarning', () => { + const fullMessage = "Failed to map record from Airtable format for table 'self_serve_course_registration' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type 'multipleLookupValues' to 'string | null', as the Airtable API provided a 'object'. Suggestion: Update the types in your table definition to compatible types for your Airtable base."; + + test('parses a field-level warning into concise prose and batch metadata', () => { + const formatted = formatAirtableWarning(fullMessage); + + expect(formatted?.message).toBe( + 'Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.', + ); + expect(formatted?.batchGroup.dedupeKeys).toEqual(['rec1ArmjQ8wUmBwDC']); + expect(formatted?.batchGroup.itemNoun).toBe('record'); + expect(formatted?.batchGroup.annotations).toEqual(['Table: tbla338CpAd0FF96g', 'Field: fldsS2lCVlk1WDSDw']); + // Prose carries no raw table/field IDs; those live in annotations only. + expect(formatted?.message).not.toContain('tbla338CpAd0FF96g'); + expect(formatted?.message).not.toContain('fldsS2lCVlk1WDSDw'); + expect(formatted?.message).not.toContain('Suggestion'); + }); + + test('signature ignores the record so same failure on different records groups together', () => { + const other = fullMessage.replace('rec1ArmjQ8wUmBwDC', 'rec2XCb0ZcK3urHOY'); + const a = formatAirtableWarning(fullMessage); + const b = formatAirtableWarning(other); + + expect(a?.batchGroup.signature).toBe(b?.batchGroup.signature); + expect(a?.batchGroup.signature).not.toContain('rec1ArmjQ8wUmBwDC'); + }); + + test('degrades gracefully for a record-level warning with no field/type detail', () => { + const message = "Failed to map record from Airtable format for table 'exercise' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened"; + const formatted = formatAirtableWarning(message); + + expect(formatted?.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']); + expect(formatted?.batchGroup.annotations).toEqual(['Table: tblXXXXXXXXXXXXXX']); + expect(formatted?.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); + }); + + test('returns null when the table/record shape is absent (raw fallback)', () => { + expect(formatAirtableWarning('Some unrelated error with no airtable ids')).toBeNull(); + }); +}); From 9a852f6e56d4ce620e710e3066b151228b10c11c Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:44:17 +0200 Subject: [PATCH 05/27] Website uses new `formatAirtableWarning` wrapper --- apps/website/src/lib/api/db/index.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/website/src/lib/api/db/index.ts b/apps/website/src/lib/api/db/index.ts index 34b4803c4..dc5d4ece5 100644 --- a/apps/website/src/lib/api/db/index.ts +++ b/apps/website/src/lib/api/db/index.ts @@ -1,4 +1,4 @@ -import { PgAirtableDb, createTestDbClients } from '@bluedot/db'; +import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db'; import { slackAlert } from '@bluedot/utils'; import env from '../env'; @@ -10,9 +10,14 @@ export default new PgAirtableDb({ ...(isTest ? createTestDbClients() : {}), async onWarning(warning: unknown) { const err = warning instanceof Error ? warning : new Error(String(warning)); + const formatted = formatAirtableWarning(err.message); + const message = formatted?.message ?? err.message; await slackAlert(env, [ - `Airtable validation warning encountered, attempting to proceed by setting the affected fields to undefined. Warning message: ${err.message}`, + message, ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : []), - ], { batchKey: 'airtable-validation' }); + ], { + batchKey: 'airtable-validation', + ...(formatted ? { batchGroup: formatted.batchGroup } : {}), + }); }, }); From 82304dd7f3dfe0d19d8a067baefd450d56103d13 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:44:34 +0200 Subject: [PATCH 06/27] pg-sync uses `formatAirtableWarning` helper --- apps/pg-sync-service/src/lib/db.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/pg-sync-service/src/lib/db.ts b/apps/pg-sync-service/src/lib/db.ts index 53dfd1d9b..8cf6a7b41 100644 --- a/apps/pg-sync-service/src/lib/db.ts +++ b/apps/pg-sync-service/src/lib/db.ts @@ -1,4 +1,4 @@ -import { PgAirtableDb, createTestDbClients } from '@bluedot/db'; +import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db'; import { slackAlert } from '@bluedot/utils'; import env from '../env'; @@ -18,7 +18,8 @@ export const db = new PgAirtableDb({ ...(isTest ? createTestDbClients() : {}), onWarning: async (warning: unknown) => { const err = warning instanceof Error ? warning : new Error(String(warning)); - const message = `Airtable validation warning encountered, attempting to proceed by setting the affected fields to undefined. Warning message: ${err.message}`; + const formatted = formatAirtableWarning(err.message); + const message = formatted?.message ?? err.message; // eslint-disable-next-line no-console console.warn(message); @@ -31,6 +32,7 @@ export const db = new PgAirtableDb({ batchKey: 'airtable-validation', spikeThreshold: VALIDATION_WARNING_SPIKE_THRESHOLD, escalationChannelId: env.ALERTS_SLACK_CHANNEL_ID, + ...(formatted ? { batchGroup: formatted.batchGroup } : {}), }); }, }); From b2070f5f76b685f3a43d455be2fe0f9432ed43c9 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:44:45 +0200 Subject: [PATCH 07/27] Update slack notification tests --- .../utils/src/slackNotifications.test.ts | 65 ++++++++++++++++--- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index 5a2893f47..e0c402a59 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -124,15 +124,18 @@ describe('slackNotifications', () => { }); describe('batching mode', () => { - test('should batch similar messages by signature', async () => { - const message1 = 'Failed to map field unitNumber (fldL42M2hgchJYIdD) from Airtable for table exercise (tbla7lc2MtSSbWVvS) and record rec3BGObwkLPSskvb'; - const message2 = 'Failed to map field unitNumber (fldL42M2hgchJYIdD) from Airtable for table exercise (tbla7lc2MtSSbWVvS) and record rec3qTvZcFGYccFcl'; - const message3 = 'Failed to map field unitNumber (fldL42M2hgchJYIdD) from Airtable for table exercise (tbla7lc2MtSSbWVvS) and record rec3060zKybkbm9UH'; + test('should batch by shared signature and render dedupeKeys and annotations', async () => { + const message = 'Field `unitNumber` on `exercise`: cannot map value. Set to undefined.'; + const batchGroupFor = (recordId: string) => ({ + signature: 'exercise/unitNumber', + dedupeKeys: [recordId], + itemNoun: 'record', + annotations: ['Table: tbla7lc2MtSSbWVvS', 'Field: fldL42M2hgchJYIdD'], + }); - // Add messages to batch - slackAlert(mockEnv, [message1], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); - slackAlert(mockEnv, [message2], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); - slackAlert(mockEnv, [message3], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); + slackAlert(mockEnv, [message], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: batchGroupFor('rec3BGObwkLPSskvb') }); + slackAlert(mockEnv, [message], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: batchGroupFor('rec3qTvZcFGYccFcl') }); + slackAlert(mockEnv, [message], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: batchGroupFor('rec3060zKybkbm9UH') }); // No messages sent yet expect(fetchMock).not.toHaveBeenCalled(); @@ -200,9 +203,51 @@ describe('slackNotifications', () => { expect(callBody.text).not.toContain('affecting'); }); + test('should collapse differing messages that share an explicit signature', async () => { + // Distinct message bodies but the same signature must land in one batch. + slackAlert(mockEnv, ['Error affecting Alice'], { + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'same-group', dedupeKeys: ['a'] }, + }); + slackAlert(mockEnv, ['Error affecting Bob'], { + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'same-group', dedupeKeys: ['b'] }, + }); + + fetchMock.mockResolvedValueOnce({ + ok: true, + json: async () => ({ ok: true, ts: '1.0' }), + }); + + await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); + // First message wins as the batch's main message. + expect(callBody.text).toContain('Error affecting Alice'); + expect(callBody.text).toContain('This error occurred 2 times affecting 2 items'); + }); + + test('should default itemNoun to a pluralised "item"', async () => { + slackAlert(mockEnv, ['boom'], { + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: ['x'] }, + }); + slackAlert(mockEnv, ['boom'], { + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: ['y'] }, + }); + + fetchMock.mockResolvedValueOnce({ + ok: true, + json: async () => ({ ok: true, ts: '1.0' }), + }); + + await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); + + const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); + expect(callBody.text).toContain('affecting 2 items'); + }); + test('should preserve first reply in batched messages', async () => { - const message1 = ['Main error message rec1AbCdEfGhIjKl', 'Stack trace here']; - const message2 = ['Main error message rec2MnOpQrStUvWx', 'Different stack trace']; + const message1 = ['Main error message', 'Stack trace here']; + const message2 = ['Main error message', 'Different stack trace']; slackAlert(mockEnv, message1, { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); slackAlert(mockEnv, message2, { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); From 8513e87268365396f83d3c14c303e55ad146a09d Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:45:05 +0200 Subject: [PATCH 08/27] `parseAirtableWarning` tests --- apps/pg-sync-service/src/lib/db.test.ts | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 apps/pg-sync-service/src/lib/db.test.ts diff --git a/apps/pg-sync-service/src/lib/db.test.ts b/apps/pg-sync-service/src/lib/db.test.ts new file mode 100644 index 000000000..6104426c0 --- /dev/null +++ b/apps/pg-sync-service/src/lib/db.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from 'vitest'; +import { parseAirtableWarning } from './db'; + +describe('parseAirtableWarning', () => { + const fullMessage = 'Failed to map record from Airtable format for table \'self_serve_course_registration\' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type \'multipleLookupValues\' to \'string | null\', as the Airtable API provided a \'object\'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.'; + + test('parses field-level warning into concise prose and structured IDs', () => { + const parsed = parseAirtableWarning(fullMessage); + + expect(parsed).not.toBeNull(); + expect(parsed?.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); + expect(parsed?.airtableIds).toEqual({ + tableId: 'tbla338CpAd0FF96g', + fieldId: 'fldsS2lCVlk1WDSDw', + recordIds: ['rec1ArmjQ8wUmBwDC'], + }); + // No raw table/field IDs leak into the human-readable prose. + expect(parsed?.message).not.toContain('tbla338CpAd0FF96g'); + expect(parsed?.message).not.toContain('fldsS2lCVlk1WDSDw'); + expect(parsed?.message).not.toContain('Suggestion'); + }); + + test('degrades gracefully for a record-level warning with no field/type detail', () => { + const message = 'Failed to map record from Airtable format for table \'exercise\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened'; + const parsed = parseAirtableWarning(message); + + expect(parsed).not.toBeNull(); + expect(parsed?.airtableIds).toEqual({ + tableId: 'tblXXXXXXXXXXXXXX', + fieldId: undefined, + recordIds: ['recYYYYYYYYYYYYYY'], + }); + expect(parsed?.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); + }); + + test('returns null when the table/record shape is absent (raw fallback)', () => { + expect(parseAirtableWarning('Some unrelated error with no airtable ids')).toBeNull(); + }); +}); From 9588a0d86ee63f3b682c7991a11d09fe8f0f5d4a Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:45:58 +0200 Subject: [PATCH 09/27] Update old Slack notification tests --- .../utils/src/slackNotifications.test.ts | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index e0c402a59..f0f6b5061 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -309,14 +309,12 @@ describe('slackNotifications', () => { expect(reply2Body.text).toBe('test-app: Reply 2'); }); - test('should track unique record IDs across batches', async () => { - const message1 = 'Error rec1AbCdEfGhIjKl'; - const message2 = 'Error rec1AbCdEfGhIjKl'; // Same record - const message3 = 'Error rec2MnOpQrStUvWx'; + test('should deduplicate items across batches', async () => { + const dedupe = (key: string) => ({ batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: [key], itemNoun: 'record' } }); - slackAlert(mockEnv, [message1], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); - slackAlert(mockEnv, [message2], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); - slackAlert(mockEnv, [message3], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); + slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); + slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); // Same key + slackAlert(mockEnv, ['Error'], dedupe('rec2MnOpQrStUvWx')); fetchMock.mockResolvedValueOnce({ ok: true, @@ -326,16 +324,16 @@ describe('slackNotifications', () => { await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 2 records'); // Only unique records + expect(callBody.text).toContain('affecting 2 records'); // Only unique keys expect(callBody.text).toContain('rec1AbCdEfGhIjKl'); expect(callBody.text).toContain('rec2MnOpQrStUvWx'); }); - test('should limit record list to 10 records', async () => { + test('should limit the item list to 10', async () => { const recordIds = Array.from({ length: 15 }, (_, i) => `rec${i}AbCdEfGhIjKl`); for (const recordId of recordIds) { - slackAlert(mockEnv, [`Error ${recordId}`], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS }); + slackAlert(mockEnv, ['Error'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: [recordId], itemNoun: 'record' } }); } fetchMock.mockResolvedValueOnce({ @@ -440,16 +438,16 @@ describe('slackNotifications', () => { describe('spike escalation', () => { const escalationChannelId = 'alerts-channel'; - test('cross-posts batched summary to escalation channel when record count hits threshold', async () => { - const messages = ['rec1AbCdEfGhIjKl', 'rec2MnOpQrStUvWx', 'rec3ZzYyXxWwVvUu'] - .map((recordId) => `Validation warning ${recordId}`); + test('cross-posts batched summary to escalation channel when item count hits threshold', async () => { + const recordIds = ['rec1AbCdEfGhIjKl', 'rec2MnOpQrStUvWx', 'rec3ZzYyXxWwVvUu']; - for (const message of messages) { - slackAlert(mockEnv, [message], { + for (const recordId of recordIds) { + slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, spikeThreshold: 3, escalationChannelId, + batchGroup: { signature: 'g', dedupeKeys: [recordId] }, }); } @@ -489,16 +487,16 @@ describe('slackNotifications', () => { }); test('does not escalate when escalation channel equals batch channel', async () => { - const messages = ['rec1AbCdEfGhIjKl', 'rec2MnOpQrStUvWx', 'rec3ZzYyXxWwVvUu'] - .map((recordId) => `Validation warning ${recordId}`); + const recordIds = ['rec1AbCdEfGhIjKl', 'rec2MnOpQrStUvWx', 'rec3ZzYyXxWwVvUu']; - for (const message of messages) { - slackAlert(mockEnv, [message], { + for (const recordId of recordIds) { + slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, spikeThreshold: 3, channelId: 'test-channel', escalationChannelId: 'test-channel', + batchGroup: { signature: 'g', dedupeKeys: [recordId] }, }); } @@ -535,15 +533,17 @@ describe('slackNotifications', () => { test('uses spike options from a later call in the same window', async () => { // First call omits spike options; a later call in the same window supplies them. // The later options should take effect rather than being silently ignored. - slackAlert(mockEnv, ['Validation warning rec1AbCdEfGhIjKl'], { + slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, + batchGroup: { signature: 'g', dedupeKeys: ['rec1AbCdEfGhIjKl'] }, }); - slackAlert(mockEnv, ['Validation warning rec2MnOpQrStUvWx'], { + slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, spikeThreshold: 2, escalationChannelId, + batchGroup: { signature: 'g', dedupeKeys: ['rec2MnOpQrStUvWx'] }, }); fetchMock From 485afaff92fc88245a3740fe4c09c940d8431ea2 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:46:24 +0200 Subject: [PATCH 10/27] remove db util tests --- apps/pg-sync-service/src/lib/db.test.ts | 39 ------------------------- 1 file changed, 39 deletions(-) delete mode 100644 apps/pg-sync-service/src/lib/db.test.ts diff --git a/apps/pg-sync-service/src/lib/db.test.ts b/apps/pg-sync-service/src/lib/db.test.ts deleted file mode 100644 index 6104426c0..000000000 --- a/apps/pg-sync-service/src/lib/db.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { describe, expect, test } from 'vitest'; -import { parseAirtableWarning } from './db'; - -describe('parseAirtableWarning', () => { - const fullMessage = 'Failed to map record from Airtable format for table \'self_serve_course_registration\' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type \'multipleLookupValues\' to \'string | null\', as the Airtable API provided a \'object\'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.'; - - test('parses field-level warning into concise prose and structured IDs', () => { - const parsed = parseAirtableWarning(fullMessage); - - expect(parsed).not.toBeNull(); - expect(parsed?.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); - expect(parsed?.airtableIds).toEqual({ - tableId: 'tbla338CpAd0FF96g', - fieldId: 'fldsS2lCVlk1WDSDw', - recordIds: ['rec1ArmjQ8wUmBwDC'], - }); - // No raw table/field IDs leak into the human-readable prose. - expect(parsed?.message).not.toContain('tbla338CpAd0FF96g'); - expect(parsed?.message).not.toContain('fldsS2lCVlk1WDSDw'); - expect(parsed?.message).not.toContain('Suggestion'); - }); - - test('degrades gracefully for a record-level warning with no field/type detail', () => { - const message = 'Failed to map record from Airtable format for table \'exercise\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened'; - const parsed = parseAirtableWarning(message); - - expect(parsed).not.toBeNull(); - expect(parsed?.airtableIds).toEqual({ - tableId: 'tblXXXXXXXXXXXXXX', - fieldId: undefined, - recordIds: ['recYYYYYYYYYYYYYY'], - }); - expect(parsed?.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); - }); - - test('returns null when the table/record shape is absent (raw fallback)', () => { - expect(parseAirtableWarning('Some unrelated error with no airtable ids')).toBeNull(); - }); -}); From 4bd2013c5477d7c67cca74877d8ce0c7fc6bc052 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:46:40 +0200 Subject: [PATCH 11/27] lint fixes --- libraries/db/src/lib/formatAirtableWarning.test.ts | 8 +++----- libraries/db/src/lib/formatAirtableWarning.ts | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.test.ts b/libraries/db/src/lib/formatAirtableWarning.test.ts index e290fb65c..2bf6305f5 100644 --- a/libraries/db/src/lib/formatAirtableWarning.test.ts +++ b/libraries/db/src/lib/formatAirtableWarning.test.ts @@ -2,14 +2,12 @@ import { describe, expect, test } from 'vitest'; import { formatAirtableWarning } from './formatAirtableWarning'; describe('formatAirtableWarning', () => { - const fullMessage = "Failed to map record from Airtable format for table 'self_serve_course_registration' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type 'multipleLookupValues' to 'string | null', as the Airtable API provided a 'object'. Suggestion: Update the types in your table definition to compatible types for your Airtable base."; + const fullMessage = 'Failed to map record from Airtable format for table \'self_serve_course_registration\' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type \'multipleLookupValues\' to \'string | null\', as the Airtable API provided a \'object\'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.'; test('parses a field-level warning into concise prose and batch metadata', () => { const formatted = formatAirtableWarning(fullMessage); - expect(formatted?.message).toBe( - 'Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.', - ); + expect(formatted?.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); expect(formatted?.batchGroup.dedupeKeys).toEqual(['rec1ArmjQ8wUmBwDC']); expect(formatted?.batchGroup.itemNoun).toBe('record'); expect(formatted?.batchGroup.annotations).toEqual(['Table: tbla338CpAd0FF96g', 'Field: fldsS2lCVlk1WDSDw']); @@ -29,7 +27,7 @@ describe('formatAirtableWarning', () => { }); test('degrades gracefully for a record-level warning with no field/type detail', () => { - const message = "Failed to map record from Airtable format for table 'exercise' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened"; + const message = 'Failed to map record from Airtable format for table \'exercise\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened'; const formatted = formatAirtableWarning(message); expect(formatted?.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']); diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts index 91094931a..c12a6f6fd 100644 --- a/libraries/db/src/lib/formatAirtableWarning.ts +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -25,6 +25,7 @@ export const formatAirtableWarning = (raw: string): FormattedAirtableWarning | n if (!record?.groups) { return null; } + const { tableName, tableId, recordId } = record.groups; const field = /Failed to map field (?.+?) \((?fld[A-Za-z0-9]+)\)/.exec(raw); From 4ff9c2606204d309fbf227dc492c85978fda246b Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:48:25 +0200 Subject: [PATCH 12/27] Minor doc updates --- libraries/utils/src/slackNotifications.test.ts | 2 +- libraries/utils/src/slackNotifications.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index f0f6b5061..2c23f1d21 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -508,7 +508,7 @@ describe('slackNotifications', () => { }); test('falls back to occurrence count when messages carry no record IDs', async () => { - // No rec IDs -> affectedRecords stays empty -> spikeCount uses raw occurrences. + // No dedupeKeys -> dedupedItems stays empty -> spikeCount uses raw occurrences. for (let i = 0; i < 3; i++) { slackAlert(mockEnv, ['Validation warning with no record id'], { batchKey: 'test', diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index fdc87e284..c2c7e510e 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -62,15 +62,16 @@ const batchers = new Map(); * Sends Slack message(s) to our prod/dev channels * - By default, messages are sent immediately to ALERTS_SLACK_CHANNEL_ID * - To send to a different channel, provide a channelId - * - To enable batching, provide a batchKey - messages will be grouped by signature (same error type/table/field) + * - To enable batching, provide a batchKey - messages are grouped by signature (the message text by default, or batchGroup.signature) * - Batching uses a rolling window: messages are sent N ms after the last message (not from the first) * - If multiple messages are provided, the first is sent as a new message, and the rest are sent as replies in a thread * * @param options.channelId - If provided, sends to this channel instead of ALERTS_SLACK_CHANNEL_ID * @param options.batchKey - If provided, enables batching with this key as the batch group identifier * @param options.flushIntervalMs - Time window for batching in ms (default: 60000, only used when batchKey is provided). Uses a rolling window - each new message resets the timer. - * @param options.spikeThreshold - If provided, when a batched signature affects at least this many distinct records within a flush window, cross-post the batched summary to escalationChannelId. + * @param options.spikeThreshold - If provided, when a batched signature affects at least this many distinct items within a flush window, cross-post the batched summary to escalationChannelId. * @param options.escalationChannelId - If provided, specifies the channel to which batched summaries are cross-posted when the spikeThreshold is exceeded. + * @param options.batchGroup - If provided, controls how batched messages are grouped and summarised (signature, dedupeKeys, itemNoun, annotations). */ export const slackAlert = async ( env: SlackAlertEnv, From 3a9f7f7197f9a9bfeb4cea74a1df5fb72911fc90 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:51:36 +0200 Subject: [PATCH 13/27] More descriptive signature keys for tests --- libraries/utils/src/slackNotifications.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index 2c23f1d21..d7f0e4964 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -228,10 +228,10 @@ describe('slackNotifications', () => { test('should default itemNoun to a pluralised "item"', async () => { slackAlert(mockEnv, ['boom'], { - batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: ['x'] }, + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: ['x'] }, }); slackAlert(mockEnv, ['boom'], { - batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: ['y'] }, + batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: ['y'] }, }); fetchMock.mockResolvedValueOnce({ @@ -310,7 +310,7 @@ describe('slackNotifications', () => { }); test('should deduplicate items across batches', async () => { - const dedupe = (key: string) => ({ batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: [key], itemNoun: 'record' } }); + const dedupe = (key: string) => ({ batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [key], itemNoun: 'record' } }); slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); // Same key @@ -333,7 +333,7 @@ describe('slackNotifications', () => { const recordIds = Array.from({ length: 15 }, (_, i) => `rec${i}AbCdEfGhIjKl`); for (const recordId of recordIds) { - slackAlert(mockEnv, ['Error'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'g', dedupeKeys: [recordId], itemNoun: 'record' } }); + slackAlert(mockEnv, ['Error'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [recordId], itemNoun: 'record' } }); } fetchMock.mockResolvedValueOnce({ @@ -447,7 +447,7 @@ describe('slackNotifications', () => { flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, spikeThreshold: 3, escalationChannelId, - batchGroup: { signature: 'g', dedupeKeys: [recordId] }, + batchGroup: { signature: 'shared-batch', dedupeKeys: [recordId] }, }); } @@ -496,7 +496,7 @@ describe('slackNotifications', () => { spikeThreshold: 3, channelId: 'test-channel', escalationChannelId: 'test-channel', - batchGroup: { signature: 'g', dedupeKeys: [recordId] }, + batchGroup: { signature: 'shared-batch', dedupeKeys: [recordId] }, }); } @@ -536,14 +536,14 @@ describe('slackNotifications', () => { slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, - batchGroup: { signature: 'g', dedupeKeys: ['rec1AbCdEfGhIjKl'] }, + batchGroup: { signature: 'shared-batch', dedupeKeys: ['rec1AbCdEfGhIjKl'] }, }); slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, spikeThreshold: 2, escalationChannelId, - batchGroup: { signature: 'g', dedupeKeys: ['rec2MnOpQrStUvWx'] }, + batchGroup: { signature: 'shared-batch', dedupeKeys: ['rec2MnOpQrStUvWx'] }, }); fetchMock From e11bcd798eb5819eab5ca23aebcd5e407d214d21 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:53:58 +0200 Subject: [PATCH 14/27] Remove `itemNoun`, `annotations` keys These just add unnecessary abstraction --- libraries/utils/src/slackNotifications.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index c2c7e510e..d37ab9653 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -12,13 +12,8 @@ type BatchGroup = { // must supply a stable signature themselves. signature?: string; // Distinct affected items. Deduplicated across the window, counted, and listed in the - // batched summary ("affecting N …", first 10 + "and M more"). + // batched summary ("affecting N item(s)", first 10 + "and M more"). dedupeKeys?: string[]; - // Noun for the count line, e.g. 'record'. Pluralised automatically. Defaults to 'item'. - itemNoun?: string; - // Extra header fragments appended to the batched summary, each rendered as ` (fragment)`, - // e.g. 'Table: tbl123'. - annotations?: string[]; }; type SlackAlertOptions = { @@ -39,8 +34,6 @@ type MessageBatch = { signature: string; occurrences: number; messages: string[]; - annotations: string[]; - itemNoun: string; dedupedItems: Set; lastSeen: number; }; @@ -71,7 +64,7 @@ const batchers = new Map(); * @param options.flushIntervalMs - Time window for batching in ms (default: 60000, only used when batchKey is provided). Uses a rolling window - each new message resets the timer. * @param options.spikeThreshold - If provided, when a batched signature affects at least this many distinct items within a flush window, cross-post the batched summary to escalationChannelId. * @param options.escalationChannelId - If provided, specifies the channel to which batched summaries are cross-posted when the spikeThreshold is exceeded. - * @param options.batchGroup - If provided, controls how batched messages are grouped and summarised (signature, dedupeKeys, itemNoun, annotations). + * @param options.batchGroup - If provided, controls how batched messages are grouped and summarised (signature, dedupeKeys). */ export const slackAlert = async ( env: SlackAlertEnv, @@ -158,8 +151,6 @@ const addToBatch = ( signature, occurrences: 1, messages, - annotations: batchGroup?.annotations ?? [], - itemNoun: batchGroup?.itemNoun ?? 'item', dedupedItems: new Set(dedupeKeys), lastSeen: Date.now(), }); @@ -232,17 +223,15 @@ const flushBatcher = async (batcher: BatcherState) => { // Build main message if (batch.occurrences > 1) { - const annotationInfo = batch.annotations.map((a) => ` (${a})`).join(''); - const header = `${mainMessage}${annotationInfo}\n\n⚠️ This error occurred ${batch.occurrences} times`; + const header = `${mainMessage}\n\n⚠️ This error occurred ${batch.occurrences} times`; // Only callers that pass dedupeKeys carry affected items; others (e.g. tRPC // server errors) have none, so drop the item clause for them. if (batch.dedupedItems.size > 0) { const itemList = Array.from(batch.dedupedItems).slice(0, 10).join(', '); const moreItems = batch.dedupedItems.size > 10 ? ` and ${batch.dedupedItems.size - 10} more` : ''; - const noun = batch.dedupedItems.size === 1 ? batch.itemNoun : `${batch.itemNoun}s`; - messages.push(`${header} affecting ${batch.dedupedItems.size} ${noun}:\n${itemList}${moreItems}`); + messages.push(`${header} affecting ${batch.dedupedItems.size} item(s):\n${itemList}${moreItems}`); } else { messages.push(`${header}.`); } From 532b7d1315d6bdadbc7ea48a114d33af60e288e5 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:54:24 +0200 Subject: [PATCH 15/27] Rename `spikeOptions` -> `batchOptions` --- libraries/utils/src/slackNotifications.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index d37ab9653..e3838cfa1 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -109,14 +109,14 @@ const addToBatch = ( messages: string[], channelId: string, flushIntervalMs: number, - spikeOptions: { spikeThreshold?: number; escalationChannelId?: string; batchGroup?: BatchGroup }, + batchOptions: { spikeThreshold?: number; escalationChannelId?: string; batchGroup?: BatchGroup }, ) => { const [mainMessage] = messages; if (!mainMessage) { return; } - const { batchGroup } = spikeOptions; + const { batchGroup } = batchOptions; const signature = batchGroup?.signature ?? mainMessage; const dedupeKeys = batchGroup?.dedupeKeys ?? []; @@ -128,15 +128,15 @@ const addToBatch = ( flushTimer: null, env, channelId, - spikeThreshold: spikeOptions.spikeThreshold, - escalationChannelId: spikeOptions.escalationChannelId, + spikeThreshold: batchOptions.spikeThreshold, + escalationChannelId: batchOptions.escalationChannelId, }; batchers.set(batchKey, batcher); } else { // Keep spike settings current: a later call in the same window should not be // silently ignored just because the batcher already exists. - batcher.spikeThreshold = spikeOptions.spikeThreshold; - batcher.escalationChannelId = spikeOptions.escalationChannelId; + batcher.spikeThreshold = batchOptions.spikeThreshold; + batcher.escalationChannelId = batchOptions.escalationChannelId; } const existing = batcher.batches.get(signature); From 550f74dfef506a4464b98c9b74355b5aa8d7ab8b Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:55:55 +0200 Subject: [PATCH 16/27] Simplify `formatAirtableWarning`, never returns null --- libraries/db/src/lib/formatAirtableWarning.ts | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts index c12a6f6fd..ad9266b74 100644 --- a/libraries/db/src/lib/formatAirtableWarning.ts +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -5,56 +5,53 @@ * convert value from airtable type 'multipleLookupValues' to 'string | null', as the * Airtable API provided a 'object'. Suggestion: Update the types... * - * `formatAirtableWarning` turns that into concise prose plus domain-neutral batching + * `formatAirtableWarning` turns an `onWarning` value into concise prose plus batching * metadata (`slackAlert`'s `batchGroup`), so alerts read cleanly and group by - * table/field/error rather than by record. Returns null when the shape can't be parsed, - * letting callers fall back to the raw message. + * table/field rather than by record. When the shape can't be parsed, it falls back to + * the raw message with no batching metadata. */ export type FormattedAirtableWarning = { message: string; + // message plus a stack-trace reply, ready to pass straight to slackAlert + messages: string[]; batchGroup: { - signature: string; - dedupeKeys: string[]; - itemNoun: string; - annotations: string[]; + signature?: string; + dedupeKeys?: string[]; }; }; -export const formatAirtableWarning = (raw: string): FormattedAirtableWarning | null => { +export const formatAirtableWarning = (warning: unknown): FormattedAirtableWarning => { + const err = warning instanceof Error ? warning : new Error(String(warning)); + const raw = err.message; + const withStack = (message: string) => [message, ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : [])]; + const record = /for table '(?[^']*)' \((?tbl[A-Za-z0-9]+)\) and record (?rec[A-Za-z0-9]+)/.exec(raw); - if (!record?.groups) { - return null; + const { tableName, tableId, recordId } = record?.groups ?? {}; + if (!tableName || !tableId || !recordId) { + return { message: raw, messages: withStack(raw), batchGroup: {} }; } - const { tableName, tableId, recordId } = record.groups; - - const field = /Failed to map field (?.+?) \((?fld[A-Za-z0-9]+)\)/.exec(raw); - const types = /from airtable type '(?[^']*)' to '(?[^']*)', as the Airtable API provided a '(?[^']*)'/.exec(raw); + const field = /Failed to map field (?.+?) \((?fld[A-Za-z0-9]+)\)/.exec(raw)?.groups; + const types = /from airtable type '(?[^']*)' to '(?[^']*)', as the Airtable API provided a '(?[^']*)'/.exec(raw)?.groups; - const location = field?.groups - ? `Field \`${field.groups.fieldName}\` on \`${tableName}\` (record ${recordId})` + const location = field + ? `Field \`${field.fieldName}\` on \`${tableName}\` (record ${recordId})` : `Record ${recordId} on \`${tableName}\``; - const reason = types?.groups - ? `can't map Airtable ${types.groups.fromType} → ${types.groups.tsType} (got ${types.groups.providedType})` + const reason = types + ? `can't map Airtable ${types.fromType} → ${types.tsType} (got ${types.providedType})` : raw; const message = `${location}: ${reason}. Set to undefined.`; - const annotations = [ - `Table: ${tableId}`, - ...(field?.groups ? [`Field: ${field.groups.fieldId}`] : []), - ]; - return { message, + messages: withStack(message), batchGroup: { - // Group by table/field/error, not by record: strip the one varying ID from the - // human-readable message so the same failure on different records collapses. - signature: message.replace(recordId!, 'REC'), - dedupeKeys: [recordId!], - itemNoun: 'record', - annotations, + // Group by table/field, not by record, so the same failure across many + // records collapses into one batched alert. + signature: `${tableId}/${field?.fieldId ?? 'record'}`, + dedupeKeys: [recordId], }, }; }; From 3338951237b37d590b71f09df142606dc178f57c Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:56:12 +0200 Subject: [PATCH 17/27] Fix outdated tests --- .../db/src/lib/formatAirtableWarning.test.ts | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.test.ts b/libraries/db/src/lib/formatAirtableWarning.test.ts index 2bf6305f5..6f92de87c 100644 --- a/libraries/db/src/lib/formatAirtableWarning.test.ts +++ b/libraries/db/src/lib/formatAirtableWarning.test.ts @@ -5,37 +5,54 @@ describe('formatAirtableWarning', () => { const fullMessage = 'Failed to map record from Airtable format for table \'self_serve_course_registration\' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type \'multipleLookupValues\' to \'string | null\', as the Airtable API provided a \'object\'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.'; test('parses a field-level warning into concise prose and batch metadata', () => { - const formatted = formatAirtableWarning(fullMessage); - - expect(formatted?.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); - expect(formatted?.batchGroup.dedupeKeys).toEqual(['rec1ArmjQ8wUmBwDC']); - expect(formatted?.batchGroup.itemNoun).toBe('record'); - expect(formatted?.batchGroup.annotations).toEqual(['Table: tbla338CpAd0FF96g', 'Field: fldsS2lCVlk1WDSDw']); - // Prose carries no raw table/field IDs; those live in annotations only. - expect(formatted?.message).not.toContain('tbla338CpAd0FF96g'); - expect(formatted?.message).not.toContain('fldsS2lCVlk1WDSDw'); - expect(formatted?.message).not.toContain('Suggestion'); + const formatted = formatAirtableWarning(new Error(fullMessage)); + + expect(formatted.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); + expect(formatted.batchGroup.signature).toBe('tbla338CpAd0FF96g/fldsS2lCVlk1WDSDw'); + expect(formatted.batchGroup.dedupeKeys).toEqual(['rec1ArmjQ8wUmBwDC']); + // Prose carries no raw table/field IDs and drops the verbose suggestion. + expect(formatted.message).not.toContain('tbla338CpAd0FF96g'); + expect(formatted.message).not.toContain('fldsS2lCVlk1WDSDw'); + expect(formatted.message).not.toContain('Suggestion'); + }); + + test('includes the stack trace as a reply message', () => { + const err = new Error(fullMessage); + const formatted = formatAirtableWarning(err); + + expect(formatted.messages[0]).toBe(formatted.message); + expect(formatted.messages[1]).toBe(`Stack:\n\`\`\`${err.stack}\`\`\``); }); test('signature ignores the record so same failure on different records groups together', () => { const other = fullMessage.replace('rec1ArmjQ8wUmBwDC', 'rec2XCb0ZcK3urHOY'); - const a = formatAirtableWarning(fullMessage); - const b = formatAirtableWarning(other); + const a = formatAirtableWarning(new Error(fullMessage)); + const b = formatAirtableWarning(new Error(other)); - expect(a?.batchGroup.signature).toBe(b?.batchGroup.signature); - expect(a?.batchGroup.signature).not.toContain('rec1ArmjQ8wUmBwDC'); + expect(a.batchGroup.signature).toBe(b.batchGroup.signature); + expect(a.batchGroup.signature).not.toContain('rec1ArmjQ8wUmBwDC'); }); test('degrades gracefully for a record-level warning with no field/type detail', () => { const message = 'Failed to map record from Airtable format for table \'exercise\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened'; - const formatted = formatAirtableWarning(message); + const formatted = formatAirtableWarning(new Error(message)); - expect(formatted?.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']); - expect(formatted?.batchGroup.annotations).toEqual(['Table: tblXXXXXXXXXXXXXX']); - expect(formatted?.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); + expect(formatted.batchGroup.signature).toBe('tblXXXXXXXXXXXXXX/record'); + expect(formatted.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']); + expect(formatted.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); }); - test('returns null when the table/record shape is absent (raw fallback)', () => { - expect(formatAirtableWarning('Some unrelated error with no airtable ids')).toBeNull(); + test('falls back to the raw message with no batch metadata when the shape is absent', () => { + const formatted = formatAirtableWarning(new Error('Some unrelated error with no airtable ids')); + + expect(formatted.message).toBe('Some unrelated error with no airtable ids'); + expect(formatted.batchGroup).toEqual({}); + }); + + test('coerces non-Error warnings', () => { + const formatted = formatAirtableWarning('string warning'); + + expect(formatted.message).toBe('string warning'); + expect(formatted.batchGroup).toEqual({}); }); }); From ab63c69c29db280203d1dce832a8e3a4f1263d13 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:56:19 +0200 Subject: [PATCH 18/27] Simplify calling of `formatAirtableWarning` --- apps/pg-sync-service/src/lib/db.ts | 11 +++-------- apps/website/src/lib/api/db/index.ts | 12 ++---------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/apps/pg-sync-service/src/lib/db.ts b/apps/pg-sync-service/src/lib/db.ts index 8cf6a7b41..972bf8992 100644 --- a/apps/pg-sync-service/src/lib/db.ts +++ b/apps/pg-sync-service/src/lib/db.ts @@ -17,22 +17,17 @@ export const db = new PgAirtableDb({ airtableApiKey: env.AIRTABLE_PERSONAL_ACCESS_TOKEN, ...(isTest ? createTestDbClients() : {}), onWarning: async (warning: unknown) => { - const err = warning instanceof Error ? warning : new Error(String(warning)); - const formatted = formatAirtableWarning(err.message); - const message = formatted?.message ?? err.message; + const { message, messages, batchGroup } = formatAirtableWarning(warning); // eslint-disable-next-line no-console console.warn(message); - await slackAlert(env, [ - message, - ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : []), - ], { + await slackAlert(env, messages, { channelId: env.PG_SYNC_SLACK_CHANNEL_ID, batchKey: 'airtable-validation', spikeThreshold: VALIDATION_WARNING_SPIKE_THRESHOLD, escalationChannelId: env.ALERTS_SLACK_CHANNEL_ID, - ...(formatted ? { batchGroup: formatted.batchGroup } : {}), + batchGroup, }); }, }); diff --git a/apps/website/src/lib/api/db/index.ts b/apps/website/src/lib/api/db/index.ts index dc5d4ece5..3b86b7cba 100644 --- a/apps/website/src/lib/api/db/index.ts +++ b/apps/website/src/lib/api/db/index.ts @@ -9,15 +9,7 @@ export default new PgAirtableDb({ airtableApiKey: env.AIRTABLE_PERSONAL_ACCESS_TOKEN, ...(isTest ? createTestDbClients() : {}), async onWarning(warning: unknown) { - const err = warning instanceof Error ? warning : new Error(String(warning)); - const formatted = formatAirtableWarning(err.message); - const message = formatted?.message ?? err.message; - await slackAlert(env, [ - message, - ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : []), - ], { - batchKey: 'airtable-validation', - ...(formatted ? { batchGroup: formatted.batchGroup } : {}), - }); + const { messages, batchGroup } = formatAirtableWarning(warning); + await slackAlert(env, messages, { batchKey: 'airtable-validation', batchGroup }); }, }); From dc2d344ff41dcc0d0bd043edc01369e27cd6d3d2 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 20 Jul 2026 16:56:35 +0200 Subject: [PATCH 19/27] Fix outdated Slack tests --- .../utils/src/slackNotifications.test.ts | 37 ++++--------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index d7f0e4964..c68ba8930 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -124,13 +124,11 @@ describe('slackNotifications', () => { }); describe('batching mode', () => { - test('should batch by shared signature and render dedupeKeys and annotations', async () => { + test('should batch by shared signature and render dedupeKeys', async () => { const message = 'Field `unitNumber` on `exercise`: cannot map value. Set to undefined.'; const batchGroupFor = (recordId: string) => ({ signature: 'exercise/unitNumber', dedupeKeys: [recordId], - itemNoun: 'record', - annotations: ['Table: tbla7lc2MtSSbWVvS', 'Field: fldL42M2hgchJYIdD'], }); slackAlert(mockEnv, [message], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: batchGroupFor('rec3BGObwkLPSskvb') }); @@ -154,12 +152,10 @@ describe('slackNotifications', () => { const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); expect(callBody.text).toContain('test-app:'); - expect(callBody.text).toContain('This error occurred 3 times affecting 3 records'); + expect(callBody.text).toContain('This error occurred 3 times affecting 3 item(s)'); expect(callBody.text).toContain('rec3BGObwkLPSskvb'); expect(callBody.text).toContain('rec3qTvZcFGYccFcl'); expect(callBody.text).toContain('rec3060zKybkbm9UH'); - expect(callBody.text).toContain('Table: tbla7lc2MtSSbWVvS'); - expect(callBody.text).toContain('Field: fldL42M2hgchJYIdD'); }); test('should send single occurrence without batching summary', async () => { @@ -223,26 +219,7 @@ describe('slackNotifications', () => { const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); // First message wins as the batch's main message. expect(callBody.text).toContain('Error affecting Alice'); - expect(callBody.text).toContain('This error occurred 2 times affecting 2 items'); - }); - - test('should default itemNoun to a pluralised "item"', async () => { - slackAlert(mockEnv, ['boom'], { - batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: ['x'] }, - }); - slackAlert(mockEnv, ['boom'], { - batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: ['y'] }, - }); - - fetchMock.mockResolvedValueOnce({ - ok: true, - json: async () => ({ ok: true, ts: '1.0' }), - }); - - await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); - - const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 2 items'); + expect(callBody.text).toContain('This error occurred 2 times affecting 2 item(s)'); }); test('should preserve first reply in batched messages', async () => { @@ -310,7 +287,7 @@ describe('slackNotifications', () => { }); test('should deduplicate items across batches', async () => { - const dedupe = (key: string) => ({ batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [key], itemNoun: 'record' } }); + const dedupe = (key: string) => ({ batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [key] } }); slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); slackAlert(mockEnv, ['Error'], dedupe('rec1AbCdEfGhIjKl')); // Same key @@ -324,7 +301,7 @@ describe('slackNotifications', () => { await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 2 records'); // Only unique keys + expect(callBody.text).toContain('affecting 2 item(s)'); // Only unique keys expect(callBody.text).toContain('rec1AbCdEfGhIjKl'); expect(callBody.text).toContain('rec2MnOpQrStUvWx'); }); @@ -333,7 +310,7 @@ describe('slackNotifications', () => { const recordIds = Array.from({ length: 15 }, (_, i) => `rec${i}AbCdEfGhIjKl`); for (const recordId of recordIds) { - slackAlert(mockEnv, ['Error'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [recordId], itemNoun: 'record' } }); + slackAlert(mockEnv, ['Error'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: [recordId] } }); } fetchMock.mockResolvedValueOnce({ @@ -344,7 +321,7 @@ describe('slackNotifications', () => { await vi.advanceTimersByTimeAsync(DEFAULT_FLUSH_INTERVAL_MS); const callBody = JSON.parse(fetchMock.mock.calls[0]?.[1].body); - expect(callBody.text).toContain('affecting 15 records'); + expect(callBody.text).toContain('affecting 15 item(s)'); expect(callBody.text).toContain('and 5 more'); // 15 - 10 = 5 }); From ea633e61b95f953bb13da25e5c6cb87ed9f4a04a Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 27 Jul 2026 15:50:45 +0200 Subject: [PATCH 20/27] `getPgAirtableFromTableId` helper --- libraries/db/src/lib/db-core.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libraries/db/src/lib/db-core.ts b/libraries/db/src/lib/db-core.ts index 7ec72a74b..c034280fb 100644 --- a/libraries/db/src/lib/db-core.ts +++ b/libraries/db/src/lib/db-core.ts @@ -224,6 +224,15 @@ export function getPgAirtableFromIds({ baseId, tableId }: { baseId: string; tabl return pgAirtableTableRegistry[key]; } +/** + * Airtable table ids are unique across bases, so a table id alone is enough to recover + * its base id. Useful when all we have is an error message from airtable-ts, which + * mentions the table id but not the base. + */ +export function getPgAirtableFromTableId(tableId: string): PgAirtableTable | undefined { + return Object.values(pgAirtableTableRegistry).find((table) => table.airtable.tableId === tableId); +} + export function pgAirtable< TTableName extends string, TColumnsMap extends Record, From cb8242eb8036c0c0631e371bfe27ff93f032ec9b Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Mon, 27 Jul 2026 15:51:20 +0200 Subject: [PATCH 21/27] Better message building with URLs --- libraries/db/src/lib/formatAirtableWarning.ts | 86 ++++++++++++++----- 1 file changed, 63 insertions(+), 23 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts index ad9266b74..892aac5ba 100644 --- a/libraries/db/src/lib/formatAirtableWarning.ts +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -1,3 +1,5 @@ +import { getPgAirtableFromTableId } from './db-core'; + /** * airtable-ts surfaces read-validation failures as one deeply-nested error string, e.g.: * Failed to map record from Airtable format for table 'self_serve_course_registration' @@ -5,14 +7,16 @@ * convert value from airtable type 'multipleLookupValues' to 'string | null', as the * Airtable API provided a 'object'. Suggestion: Update the types... * - * `formatAirtableWarning` turns an `onWarning` value into concise prose plus batching - * metadata (`slackAlert`'s `batchGroup`), so alerts read cleanly and group by - * table/field rather than by record. When the shape can't be parsed, it falls back to - * the raw message with no batching metadata. + * Rather than reformat that string, we scrape the Airtable ids out of it (ids are a stable + * contract; the prose around them is not) and rebuild the alert from our own table + * definitions. That gives us the column name, its expected type, and the base id needed to + * link to the offending record. The prose is only consulted for detail Airtable knows and + * we don't: the field's actual Airtable type and the shape the API returned. */ export type FormattedAirtableWarning = { + /** Plain text, for logs. */ message: string; - // message plus a stack-trace reply, ready to pass straight to slackAlert + /** Slack-flavoured (record ids become links), plus a stack-trace reply. Pass straight to slackAlert. */ messages: string[]; batchGroup: { signature?: string; @@ -20,38 +24,74 @@ export type FormattedAirtableWarning = { }; }; +const TABLE_ID = /\btbl[A-Za-z0-9]{10,}/; +const FIELD_ID = /\bfld[A-Za-z0-9]{10,}/; +const RECORD_ID = /\brec[A-Za-z0-9]{10,}/; + +// Prefixes airtable-ts prepends as an error bubbles up, plus the suggestion it appends. +// Stripping them leaves the innermost reason, which we can quote without repeating the +// location we've already stated. +const RECORD_PREFIX = /^Failed to map record from Airtable format for table '[^']*' \(tbl[A-Za-z0-9]{10,}\) and record rec[A-Za-z0-9]{10,}: /; +const FIELD_PREFIX = /^Failed to map field .+? from Airtable: /; +const SUGGESTION = / Suggestion: [\s\S]*$/; + +const scrubIds = (message: string) => message.replace(/\b(tbl|fld|rec)[A-Za-z0-9]{10,}/g, '$1***'); + export const formatAirtableWarning = (warning: unknown): FormattedAirtableWarning => { const err = warning instanceof Error ? warning : new Error(String(warning)); const raw = err.message; const withStack = (message: string) => [message, ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : [])]; - const record = /for table '(?[^']*)' \((?tbl[A-Za-z0-9]+)\) and record (?rec[A-Za-z0-9]+)/.exec(raw); - const { tableName, tableId, recordId } = record?.groups ?? {}; - if (!tableName || !tableId || !recordId) { + const tableId = TABLE_ID.exec(raw)?.[0]; + const recordId = RECORD_ID.exec(raw)?.[0]; + if (!tableId || !recordId) { return { message: raw, messages: withStack(raw), batchGroup: {} }; } - const field = /Failed to map field (?.+?) \((?fld[A-Za-z0-9]+)\)/.exec(raw)?.groups; - const types = /from airtable type '(?[^']*)' to '(?[^']*)', as the Airtable API provided a '(?[^']*)'/.exec(raw)?.groups; + const fieldId = FIELD_ID.exec(raw)?.[0]; + const table = getPgAirtableFromTableId(tableId); + const airtable = table?.airtable; - const location = field - ? `Field \`${field.fieldName}\` on \`${tableName}\` (record ${recordId})` - : `Record ${recordId} on \`${tableName}\``; + const columnName = fieldId && table + ? Array.from(table.airtableFieldMap).find(([, airtableId]) => airtableId === fieldId)?.[0] + : undefined; - const reason = types - ? `can't map Airtable ${types.fromType} → ${types.tsType} (got ${types.providedType})` - : raw; + // Prefer our own definitions, falling back to the message for tables we don't own. + const tableName = airtable?.name ?? /for table '([^']*)'/.exec(raw)?.[1] ?? tableId; + const fieldName = columnName ?? /Failed to map field (.+?) \(fld/.exec(raw)?.[1] ?? fieldId; + const expectedType = (columnName ? airtable?.schema[columnName] : undefined) ?? /to '([^']*)'/.exec(raw)?.[1]; + + const airtableType = /from airtable type '([^']*)'/.exec(raw)?.[1]; + const providedType = /provided a '([^']*)'/.exec(raw)?.[1]; + + const innermost = raw.replace(RECORD_PREFIX, '').replace(FIELD_PREFIX, '').replace(SUGGESTION, ''); + const reason = airtableType && expectedType + ? `can't map Airtable ${airtableType} → ${expectedType}${providedType ? ` (got ${providedType})` : ''}` + : innermost; + + const buildMessage = (recordRef: string) => { + const location = fieldId + ? `Field \`${fieldName}\` on \`${tableName}\` (record ${recordRef})` + : `Record ${recordRef} on \`${tableName}\``; + // airtable-ts substitutes a type-appropriate default (null, '', 0, false or []), so + // don't promise a specific value here. + const outcome = fieldId ? ' Value defaulted.' : ''; + return `${location}: ${reason.replace(/\.$/, '')}.${outcome}`; + }; - const message = `${location}: ${reason}. Set to undefined.`; + const recordLink = airtable + ? `` + : recordId; return { - message, - messages: withStack(message), + message: buildMessage(recordId), + messages: withStack(buildMessage(recordLink)), batchGroup: { - // Group by table/field, not by record, so the same failure across many - // records collapses into one batched alert. - signature: `${tableId}/${field?.fieldId ?? 'record'}`, - dedupeKeys: [recordId], + // Group by table/field, not by record, so the same failure across many records + // collapses into one batched alert. With no field to key on, fall back to the + // reason with ids scrubbed, so unrelated failures on one table stay separate. + signature: `${tableId}/${fieldId ?? scrubIds(innermost)}`, + dedupeKeys: [recordLink], }, }; }; From cde706597b7c085e8f4870175339dfa7606c6dae Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:02:41 +0200 Subject: [PATCH 22/27] Re-use `BatchGroup` type --- libraries/db/src/lib/formatAirtableWarning.ts | 6 ++---- libraries/utils/src/index.ts | 2 +- libraries/utils/src/slackNotifications.ts | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts index 892aac5ba..b62cd4faf 100644 --- a/libraries/db/src/lib/formatAirtableWarning.ts +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -1,3 +1,4 @@ +import { type BatchGroup } from '@bluedot/utils'; import { getPgAirtableFromTableId } from './db-core'; /** @@ -18,10 +19,7 @@ export type FormattedAirtableWarning = { message: string; /** Slack-flavoured (record ids become links), plus a stack-trace reply. Pass straight to slackAlert. */ messages: string[]; - batchGroup: { - signature?: string; - dedupeKeys?: string[]; - }; + batchGroup: BatchGroup; }; const TABLE_ID = /\btbl[A-Za-z0-9]{10,}/; diff --git a/libraries/utils/src/index.ts b/libraries/utils/src/index.ts index f43322ab3..4d57410dc 100644 --- a/libraries/utils/src/index.ts +++ b/libraries/utils/src/index.ts @@ -1,5 +1,5 @@ export { validateEnv } from './validateEnv'; -export { slackAlert } from './slackNotifications'; +export { slackAlert, type BatchGroup } from './slackNotifications'; export { chunk } from './array'; export { diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index e3838cfa1..854878552 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -6,7 +6,7 @@ type SlackAlertEnv = { // Domain-neutral batching metadata. Callers describe how to group and summarise their // messages without the util needing to know anything about the message contents. -type BatchGroup = { +export type BatchGroup = { // Grouping key: messages sharing a signature collapse into one batch. Defaults to the // message text, so callers wanting to group by e.g. error type (ignoring embedded IDs) // must supply a stable signature themselves. From c686039e99a1b25285ef74fd81a6471a12b2d7a9 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:04:30 +0200 Subject: [PATCH 23/27] Prefer `logger.warn`, not `console.warn` --- apps/pg-sync-service/src/lib/db.ts | 4 ++-- apps/website/src/lib/api/db/index.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/pg-sync-service/src/lib/db.ts b/apps/pg-sync-service/src/lib/db.ts index 972bf8992..536b8cc86 100644 --- a/apps/pg-sync-service/src/lib/db.ts +++ b/apps/pg-sync-service/src/lib/db.ts @@ -1,5 +1,6 @@ import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db'; import { slackAlert } from '@bluedot/utils'; +import { logger } from '@bluedot/ui/src/api'; import env from '../env'; const isTest = env.VITEST === 'true'; @@ -19,8 +20,7 @@ export const db = new PgAirtableDb({ onWarning: async (warning: unknown) => { const { message, messages, batchGroup } = formatAirtableWarning(warning); - // eslint-disable-next-line no-console - console.warn(message); + logger.warn(message); await slackAlert(env, messages, { channelId: env.PG_SYNC_SLACK_CHANNEL_ID, diff --git a/apps/website/src/lib/api/db/index.ts b/apps/website/src/lib/api/db/index.ts index 3b86b7cba..784d91764 100644 --- a/apps/website/src/lib/api/db/index.ts +++ b/apps/website/src/lib/api/db/index.ts @@ -1,5 +1,6 @@ import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db'; import { slackAlert } from '@bluedot/utils'; +import { logger } from '@bluedot/ui/src/api'; import env from '../env'; const isTest = env.VITEST === 'true'; @@ -9,7 +10,9 @@ export default new PgAirtableDb({ airtableApiKey: env.AIRTABLE_PERSONAL_ACCESS_TOKEN, ...(isTest ? createTestDbClients() : {}), async onWarning(warning: unknown) { - const { messages, batchGroup } = formatAirtableWarning(warning); + const { message, messages, batchGroup } = formatAirtableWarning(warning); + + logger.warn(message); await slackAlert(env, messages, { batchKey: 'airtable-validation', batchGroup }); }, }); From e0e8ef89751e8f973dc3c0730112748f5ab856d5 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:21:08 +0200 Subject: [PATCH 24/27] fix stick spike option semantics --- libraries/utils/src/slackNotifications.test.ts | 14 +++++++++----- libraries/utils/src/slackNotifications.ts | 8 ++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libraries/utils/src/slackNotifications.test.ts b/libraries/utils/src/slackNotifications.test.ts index c68ba8930..2bd2c6bee 100644 --- a/libraries/utils/src/slackNotifications.test.ts +++ b/libraries/utils/src/slackNotifications.test.ts @@ -507,20 +507,24 @@ describe('slackNotifications', () => { expect(escalationBody.text).toContain('3 affected'); }); - test('uses spike options from a later call in the same window', async () => { - // First call omits spike options; a later call in the same window supplies them. - // The later options should take effect rather than being silently ignored. + // Spike options are per-call but apply to the whole batcher, so whichever call in the + // window carries them must win: a later call must not be ignored, and a later call + // that omits them must not clear them. + test.each([ + ['a later call supplies them', [{}, { spikeThreshold: 2, escalationChannelId }]], + ['a later call omits them', [{ spikeThreshold: 2, escalationChannelId }, {}]], + ] as const)('escalates when %s', async (_name, [firstOptions, secondOptions]) => { slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, batchGroup: { signature: 'shared-batch', dedupeKeys: ['rec1AbCdEfGhIjKl'] }, + ...firstOptions, }); slackAlert(mockEnv, ['Validation warning'], { batchKey: 'test', flushIntervalMs: DEFAULT_FLUSH_INTERVAL_MS, - spikeThreshold: 2, - escalationChannelId, batchGroup: { signature: 'shared-batch', dedupeKeys: ['rec2MnOpQrStUvWx'] }, + ...secondOptions, }); fetchMock diff --git a/libraries/utils/src/slackNotifications.ts b/libraries/utils/src/slackNotifications.ts index 854878552..d8a3f9ed7 100644 --- a/libraries/utils/src/slackNotifications.ts +++ b/libraries/utils/src/slackNotifications.ts @@ -133,10 +133,10 @@ const addToBatch = ( }; batchers.set(batchKey, batcher); } else { - // Keep spike settings current: a later call in the same window should not be - // silently ignored just because the batcher already exists. - batcher.spikeThreshold = batchOptions.spikeThreshold; - batcher.escalationChannelId = batchOptions.escalationChannelId; + // Keep spike settings current without letting a call that omits them clear what an + // earlier call in the same window established. + batcher.spikeThreshold = batchOptions.spikeThreshold ?? batcher.spikeThreshold; + batcher.escalationChannelId = batchOptions.escalationChannelId ?? batcher.escalationChannelId; } const existing = batcher.batches.get(signature); From 08daa3cb6bd720e9e8e6f2c9dcaade7130a5200f Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:23:14 +0200 Subject: [PATCH 25/27] Simplifications to `formatAirtableWarning` --- libraries/db/src/lib/formatAirtableWarning.ts | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.ts b/libraries/db/src/lib/formatAirtableWarning.ts index b62cd4faf..09588a696 100644 --- a/libraries/db/src/lib/formatAirtableWarning.ts +++ b/libraries/db/src/lib/formatAirtableWarning.ts @@ -8,11 +8,10 @@ import { getPgAirtableFromTableId } from './db-core'; * convert value from airtable type 'multipleLookupValues' to 'string | null', as the * Airtable API provided a 'object'. Suggestion: Update the types... * - * Rather than reformat that string, we scrape the Airtable ids out of it (ids are a stable - * contract; the prose around them is not) and rebuild the alert from our own table - * definitions. That gives us the column name, its expected type, and the base id needed to - * link to the offending record. The prose is only consulted for detail Airtable knows and - * we don't: the field's actual Airtable type and the shape the API returned. + * We peel off the prefixes airtable-ts prepends as the error bubbles up, keeping the innermost + * reason plus the ids. Our own table definitions are consulted only for what the message can't + * tell us: the base id needed to link the offending record, and the column name in the cases + * where airtable-ts reports a field by id alone. */ export type FormattedAirtableWarning = { /** Plain text, for logs. */ @@ -29,9 +28,10 @@ const RECORD_ID = /\brec[A-Za-z0-9]{10,}/; // Prefixes airtable-ts prepends as an error bubbles up, plus the suggestion it appends. // Stripping them leaves the innermost reason, which we can quote without repeating the // location we've already stated. -const RECORD_PREFIX = /^Failed to map record from Airtable format for table '[^']*' \(tbl[A-Za-z0-9]{10,}\) and record rec[A-Za-z0-9]{10,}: /; -const FIELD_PREFIX = /^Failed to map field .+? from Airtable: /; +const RECORD_PREFIX = /^Failed to map record from Airtable format for table '([^']*)' \(tbl[A-Za-z0-9]{10,}\) and record rec[A-Za-z0-9]{10,}: /; +const FIELD_PREFIX = /^Failed to map field (.+?)(?: \(fld[A-Za-z0-9]{10,}\))? from Airtable: /; const SUGGESTION = / Suggestion: [\s\S]*$/; +const TYPE_MISMATCH = /from airtable type '([^']*)' to '([^']*)', as the Airtable API provided a '([^']*)'/; const scrubIds = (message: string) => message.replace(/\b(tbl|fld|rec)[A-Za-z0-9]{10,}/g, '$1***'); @@ -46,23 +46,25 @@ export const formatAirtableWarning = (warning: unknown): FormattedAirtableWarnin return { message: raw, messages: withStack(raw), batchGroup: {} }; } + // Peel the prefixes off one at a time, capturing what each one carries as we go. + const recordPrefix = RECORD_PREFIX.exec(raw); + const afterRecord = recordPrefix ? raw.slice(recordPrefix[0].length) : raw; + const fieldPrefix = FIELD_PREFIX.exec(afterRecord); + const afterField = fieldPrefix ? afterRecord.slice(fieldPrefix[0].length) : afterRecord; + const innermost = afterField.replace(SUGGESTION, ''); + const fieldId = FIELD_ID.exec(raw)?.[0]; const table = getPgAirtableFromTableId(tableId); - const airtable = table?.airtable; - - const columnName = fieldId && table - ? Array.from(table.airtableFieldMap).find(([, airtableId]) => airtableId === fieldId)?.[0] + const columnName = fieldId + ? Array.from(table?.airtableFieldMap ?? []).find(([, airtableId]) => airtableId === fieldId)?.[0] : undefined; - // Prefer our own definitions, falling back to the message for tables we don't own. - const tableName = airtable?.name ?? /for table '([^']*)'/.exec(raw)?.[1] ?? tableId; - const fieldName = columnName ?? /Failed to map field (.+?) \(fld/.exec(raw)?.[1] ?? fieldId; - const expectedType = (columnName ? airtable?.schema[columnName] : undefined) ?? /to '([^']*)'/.exec(raw)?.[1]; - - const airtableType = /from airtable type '([^']*)'/.exec(raw)?.[1]; - const providedType = /provided a '([^']*)'/.exec(raw)?.[1]; + const tableName = recordPrefix?.[1] ?? tableId; + // airtable-ts usually names the field in its prefix. When it doesn't — e.g. a field deleted + // from Airtable, reported by id — recover the column name rather than print a bare id. + const fieldName = fieldPrefix?.[1] ?? columnName ?? fieldId; - const innermost = raw.replace(RECORD_PREFIX, '').replace(FIELD_PREFIX, '').replace(SUGGESTION, ''); + const [, airtableType, expectedType, providedType] = TYPE_MISMATCH.exec(innermost) ?? []; const reason = airtableType && expectedType ? `can't map Airtable ${airtableType} → ${expectedType}${providedType ? ` (got ${providedType})` : ''}` : innermost; @@ -77,8 +79,9 @@ export const formatAirtableWarning = (warning: unknown): FormattedAirtableWarnin return `${location}: ${reason.replace(/\.$/, '')}.${outcome}`; }; - const recordLink = airtable - ? `` + const baseId = table?.airtable.baseId; + const recordLink = baseId + ? `` : recordId; return { From 68721800cee4a31461b73a24c5a2dfa830ac517f Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:23:53 +0200 Subject: [PATCH 26/27] Update format airtable tests --- .../db/src/lib/formatAirtableWarning.test.ts | 93 ++++++++++++++----- 1 file changed, 70 insertions(+), 23 deletions(-) diff --git a/libraries/db/src/lib/formatAirtableWarning.test.ts b/libraries/db/src/lib/formatAirtableWarning.test.ts index 6f92de87c..6506101aa 100644 --- a/libraries/db/src/lib/formatAirtableWarning.test.ts +++ b/libraries/db/src/lib/formatAirtableWarning.test.ts @@ -1,45 +1,65 @@ import { describe, expect, test } from 'vitest'; +import { mapRecordFromAirtable } from 'airtable-ts/dist/mapping/recordMapper'; +import type { AirtableRecord } from 'airtable-ts/dist/types'; import { formatAirtableWarning } from './formatAirtableWarning'; +import { selfServeCourseRegistrationTable } from '../schema'; + +// A real schema entry, so the table id resolves to a base id and a column name. +const TABLE_ID = 'tbla338CpAd0FF96g'; +const BASE_ID = 'appnJbsG1eWbAdEvf'; +const FIELD_ID = 'fldsS2lCVlk1WDSDw'; +const RECORD_ID = 'rec1ArmjQ8wUmBwDC'; +const RECORD_LINK = ``; describe('formatAirtableWarning', () => { - const fullMessage = 'Failed to map record from Airtable format for table \'self_serve_course_registration\' (tbla338CpAd0FF96g) and record rec1ArmjQ8wUmBwDC: Failed to map field fullName (fldsS2lCVlk1WDSDw) from Airtable: Cannot convert value from airtable type \'multipleLookupValues\' to \'string | null\', as the Airtable API provided a \'object\'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.'; + const typeMismatch = `Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: Failed to map field fullName (${FIELD_ID}) from Airtable: Cannot convert value from airtable type 'multipleLookupValues' to 'string | null', as the Airtable API provided a 'object'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.`; + + test('reduces a field-level warning to concise prose and batch metadata', () => { + const formatted = formatAirtableWarning(new Error(typeMismatch)); + + expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): can't map Airtable multipleLookupValues → string | null (got object). Value defaulted.`); + expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`); + }); - test('parses a field-level warning into concise prose and batch metadata', () => { - const formatted = formatAirtableWarning(new Error(fullMessage)); + test('links affected records into Airtable, using the base id from our schema', () => { + const formatted = formatAirtableWarning(new Error(typeMismatch)); - expect(formatted.message).toBe('Field `fullName` on `self_serve_course_registration` (record rec1ArmjQ8wUmBwDC): can\'t map Airtable multipleLookupValues → string | null (got object). Set to undefined.'); - expect(formatted.batchGroup.signature).toBe('tbla338CpAd0FF96g/fldsS2lCVlk1WDSDw'); - expect(formatted.batchGroup.dedupeKeys).toEqual(['rec1ArmjQ8wUmBwDC']); - // Prose carries no raw table/field IDs and drops the verbose suggestion. - expect(formatted.message).not.toContain('tbla338CpAd0FF96g'); - expect(formatted.message).not.toContain('fldsS2lCVlk1WDSDw'); - expect(formatted.message).not.toContain('Suggestion'); + // Only the Slack-bound copy carries mrkdwn; `message` stays plain for logs. + expect(formatted.messages[0]).toContain(RECORD_LINK); + expect(formatted.batchGroup.dedupeKeys).toEqual([RECORD_LINK]); }); test('includes the stack trace as a reply message', () => { - const err = new Error(fullMessage); + const err = new Error(typeMismatch); const formatted = formatAirtableWarning(err); - expect(formatted.messages[0]).toBe(formatted.message); + expect(formatted.messages).toHaveLength(2); expect(formatted.messages[1]).toBe(`Stack:\n\`\`\`${err.stack}\`\`\``); }); - test('signature ignores the record so same failure on different records groups together', () => { - const other = fullMessage.replace('rec1ArmjQ8wUmBwDC', 'rec2XCb0ZcK3urHOY'); - const a = formatAirtableWarning(new Error(fullMessage)); - const b = formatAirtableWarning(new Error(other)); + test('names the field from our schema when the warning carries no field prefix', () => { + // airtable-ts reports a field deleted from Airtable without its usual + // "Failed to map field ..." prefix, so the field name has to come from our schema. + const formatted = formatAirtableWarning(new Error(`Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: Field '${FIELD_ID}' does not exist in the table definition. This error should not happen in normal operation.`)); - expect(a.batchGroup.signature).toBe(b.batchGroup.signature); - expect(a.batchGroup.signature).not.toContain('rec1ArmjQ8wUmBwDC'); + expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): Field '${FIELD_ID}' does not exist in the table definition. This error should not happen in normal operation. Value defaulted.`); + expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`); }); - test('degrades gracefully for a record-level warning with no field/type detail', () => { - const message = 'Failed to map record from Airtable format for table \'exercise\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: something unexpected happened'; - const formatted = formatAirtableWarning(new Error(message)); + test('formats a table we do not own, minus the record link', () => { + const formatted = formatAirtableWarning(new Error('Failed to map record from Airtable format for table \'someone_elses_table\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: Failed to map field someField (fldZZZZZZZZZZZZZZ) from Airtable: Cannot convert value from airtable type \'formula\' to \'number\', as the Airtable API provided a \'string\'.')); - expect(formatted.batchGroup.signature).toBe('tblXXXXXXXXXXXXXX/record'); + expect(formatted.message).toBe('Field `someField` on `someone_elses_table` (record recYYYYYYYYYYYYYY): can\'t map Airtable formula → number (got string). Value defaulted.'); + // No base id available, so no link. expect(formatted.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']); - expect(formatted.message).toContain('Record recYYYYYYYYYYYYYY on `exercise`'); + }); + + test('keys record-level failures on the reason, with ids scrubbed', () => { + // Groups the same failure across records, without merging unrelated failures on one table. + const formatted = formatAirtableWarning(new Error(`Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: something unexpected happened about ${RECORD_ID}`)); + + expect(formatted.message).toBe(`Record ${RECORD_ID} on \`self_serve_course_registration\`: something unexpected happened about ${RECORD_ID}.`); + expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/something unexpected happened about rec***`); }); test('falls back to the raw message with no batch metadata when the shape is absent', () => { @@ -55,4 +75,31 @@ describe('formatAirtableWarning', () => { expect(formatted.message).toBe('string warning'); expect(formatted.batchGroup).toEqual({}); }); + + test('parses what airtable-ts actually emits, not just our fixture strings', async () => { + // Guards the prose regexes against an airtable-ts wording change: without this, a + // version bump could silently degrade us to the ungrouped fallback with CI still green. + const record = { + id: RECORD_ID, + fields: { fullName: {} }, + _table: { fields: [{ id: FIELD_ID, name: 'fullName', type: 'multipleLookupValues' }] }, + } as unknown as AirtableRecord; + + const warnings: unknown[] = []; + mapRecordFromAirtable(selfServeCourseRegistrationTable.airtable, record, { + readValidation: 'warning', + onWarning: (warning) => { + warnings.push(warning); + }, + }); + // onWarning is dispatched in a floating promise, so let the microtask queue drain. + await Promise.resolve(); + + const warning = warnings.find((w): w is Error => w instanceof Error && w.message.includes(FIELD_ID)); + expect(warning).toBeDefined(); + + const formatted = formatAirtableWarning(warning); + expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): can't map Airtable multipleLookupValues → string | null (got object). Value defaulted.`); + expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`); + }); }); From f4e7f1d88640d09f4a55d273a260b32a36767094 Mon Sep 17 00:00:00 2001 From: Josh Stein Date: Tue, 28 Jul 2026 10:29:13 +0200 Subject: [PATCH 27/27] Remove explicit return type --- libraries/db/src/lib/db-core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/db/src/lib/db-core.ts b/libraries/db/src/lib/db-core.ts index c034280fb..f82d8c3c7 100644 --- a/libraries/db/src/lib/db-core.ts +++ b/libraries/db/src/lib/db-core.ts @@ -229,7 +229,7 @@ export function getPgAirtableFromIds({ baseId, tableId }: { baseId: string; tabl * its base id. Useful when all we have is an error message from airtable-ts, which * mentions the table id but not the base. */ -export function getPgAirtableFromTableId(tableId: string): PgAirtableTable | undefined { +export function getPgAirtableFromTableId(tableId: string) { return Object.values(pgAirtableTableRegistry).find((table) => table.airtable.tableId === tableId); }