Phase 0.2.6 — Swallow-catch audit

Phase 0.2.6 — Swallow-catch audit

Audit date: 2026-05-06 Branch: content-items-investigation Tools: ast-grep 0.40.0 + grep + manual read Scope: app/, lib/, scripts/ (test files excluded) Investigator: Claude (sub-agent)

---

Seed pattern

Path 7 (file upload) at app/api/upload/route.ts:408-447:

// Create source_documents row for lineage tracking
let sourceDocumentId: string | null = null;
try {
  const newVersion = reuploadInfo ? reuploadInfo.existing_version + 1 : 1;
  const parentId = reuploadInfo?.match_type === 'new_version'
    ? reuploadInfo.existing_document_id
    : null;

  const { data: sourceDoc } = await serviceClient
    .from('source_documents')
    .insert({
      filename, original_filename: filename, mime_type: mimeType,
      file_size: file.size, content_hash: contentHash,
      version: newVersion, parent_id: parentId,
      storage_path: storagePath, status: 'processing' as const,
      uploaded_by: user.id,
      ...(workspaceId ? { workspace_id: workspaceId } : {}),
      ...(pipelineRunId ? { pipeline_run_id: pipelineRunId } : {}),
    })
    .select('id')
    .single();

  if (sourceDoc) {
    sourceDocumentId = sourceDoc.id;
    // Link content_item to source_document
    await serviceClient
      .from('content_items')
      .update({ source_document_id: sourceDoc.id })
      .eq('id', itemId);
  }
} catch (srcDocErr) {
  logger.error({ err: srcDocErr }, 'Source document tracking failed');
  // Non-fatal — upload continues without lineage tracking
}

Note on the seed. The catch block at L444-447 does call logger.error, which under §1’s strict definition is structured logging — meaning this specific code does NOT meet the “swallow-catch with no observability” pattern. The Path 7 audit (docs/plans/phase-0-investigation/0.1-ts-file-upload.md) flagged this site primarily because the await ... insert + update chain has an internal failure mode (the inner UPDATE to content_items.source_document_id on L439-442 runs without its own error check inside the same try). If the outer INSERT succeeds and the inner UPDATE fails, sourceDoc.id is captured, but content_items.source_document_id is left NULL — and the catch arm then logs the inner UPDATE failure under the same generic message. The result is a content_item orphaned from its source_document at the data-integrity level even though the catch did log.

The audit therefore expanded from “literal seed shape” to “any try/catch or async-catch that silently masks data-integrity-relevant failures across the codebase.” The pattern variants below capture that wider class.

---

ast-grep patterns used

All scoped to app lib scripts excluding __tests__/ and *.test.ts.

#	Pattern	Lang	Total matches	app+lib (filtered)
1	try { $$$ } catch { $$$ } (no-param catch)	ts	136	~70
2	try { $$$ } catch ($_) { $$$ } (with-param catch)	ts	541	460
3	try { $$$ } catch { } (no-param empty catch)	ts	0	0
4	try { $$$ } catch ($_) { } (with-param empty catch)	ts	0	0
5	$X.catch(() => {}) (chained empty arrow)	ts	2	0 (both in scripts/mcp-eval/)
6	$X.catch($Y => $A) (chained single-arg single-expr)	ts	69	9
7	try: ... except: pass (bare except)	py	0	0
8	try: ... except Exception: pass	py	3	3
9	try: ... except $E: pass (any typed except: pass)	py	8	8
10	try: ... except $E: $$$ (any typed except, all bodies)	py	(large)	20 candidates

Filter passes applied to results. Patterns 2 and 1 yielded 460 + ~70 catches in app+lib; a regex post-filter looking for the absence of logger.{error,warn,fatal,info}, logBestEffortWarn, console.error, recordPipelineRun, Sentry.captureException, throw, NextResponse.json, isError: true, ctx.addIssue reduced this to 84 (with-param) + 65 (no-param) candidates. Of those, manual review classified each as one of P0 / P1 / P2 / EXCLUDED.

A second tighter filter intersected (1) “try body contains a Supabase write verb (.insert/.update/.upsert/.delete)” with (2) “catch body has no logger/throw/error-reference” — yielded 5 sites (no-param) and 0 sites (with-param ignoring the captured error). Those 5 are listed as P1 (data write to pipeline_runs is an observability concern, not a content-data concern).

---

P0 findings — silent data gap

Definition: swallow lets a content-data write fail without retry or observability. After inspecting every candidate’s surrounding context, zero P0 sites in app/ or lib/ match the strict definition that source_documents → content_items.source_document_id linkage drift in Path 7 exemplifies (where the inner write was the silent failure, masked by the outer logged catch). The closest analogue is the Path 7 site itself, which is in 0.1 scope and out of this audit. The intelligence-pipeline P1 entries below are the next-most-severe.

File:line	Pattern	Risk	Suggested fix
(none)	—	—	—

Caveat / 80% confidence: the inner-write-masked-by-outer-catch pattern that Path 7 actually represents is invisible to the ast-grep patterns I ran (both writes share one catch). A fuller audit would require structurally matching “try { await write1; await write2; } catch (…)” pairs — which ast-grep can in principle do, but I did not script the variant. Listed as open question §1.

---

P1 findings — silent degraded behaviour

Definition: non-content-data swallow that masks degraded-mode signals (telemetry/lineage/state-sync failures) without observability. Eight sites identified.

File:line	Pattern	Risk	Suggested fix
app/api/items/batch/route.ts:478	no-param catch {}, comment “Non-fatal — progress tracking failure should not disrupt creation”. try body: serviceClient.from('pipeline_runs').update({ items_created, items_processed, progress }).	Loss of pipeline_runs progress-update visibility during a 100-item batch. The user sees stale progress; ops cannot trace why.	Wrap serviceClient.update(...) in sb() from @/lib/supabase/safe, then logBestEffortWarn('items.batch.progress.update', 'Pipeline progress write failed', { pipelineRunId, err }) from @/lib/supabase/telemetry.
app/api/items/batch/route.ts:504	no-param catch {}, comment “Non-fatal”. try body: serviceClient.from('pipeline_runs').update({ status: 'failed'/'completed', items_created, items_processed, completed_at, progress, error_message? }).	Pipeline-run terminal-state write loss. UI keeps “in-progress” forever; cron sweeps it as orphan. High because this is the user-visible “is my batch done?” signal.	Same: sb() + logBestEffortWarn('items.batch.completion.update', ...). Consider hard-fail (rethrow) since this is the terminal write.
app/api/items/batch/route.ts:547	no-param catch {}, comment “Double-fault — nothing more we can do”. try body in the OUTER catch: serviceClient.from('pipeline_runs').update({ status: 'failed', error_message, completed_at }).	Double-fault scenario; arguably acceptable but completely silent — Sentry should still see this.	logBestEffortWarn('items.batch.outer.catch', 'Batch failure-record write failed', { err: outerErr, originalErr: err }).
app/api/intelligence/workspaces/route.ts:231	no-param catch {}, comment “Guide creation failed — workspace still succeeds”. try body: createIntelligenceGuide(supabase, workspace.id, ...) followed by supabase.from('workspaces').update({ domain_metadata: { ..., guide_id }}).	Guide-creation failure silently leaves workspace.domain_metadata.guide_id null; admin sees no guide and no signal.	logBestEffortWarn('intelligence.workspace.guide_create', ...).
lib/intelligence/pipeline.ts:455	no-param catch {}, comment “Summary generation failure is non-fatal — store without summary”. try body: generateArticleSummary(...). The next statement INSERTs feed_articles with ai_summary: null whenever this fires.	Articles with failing AI summaries are stored with ai_summary=null and no record of the failure mode (cost-tracking, model-error trend).	logBestEffortWarn('intelligence.feed.summarise', 'AI summary failed; storing article without summary', { articleUrl: normalisedUrl, err }).
lib/intelligence/pipeline.ts:309 (handler arm)	with-param catch (err) { ... } — surfaces to result.errors[] only.	Errors flow to runPipeline caller via result.errors[], but no Sentry / logger.error trail for trend analysis.	Add logger.error({ err, sourceId: source.id }, '[Pipeline] poll failed') alongside the result.errors.push(...) — the structured trail belongs in the logger, the user-facing line belongs in result.errors.
lib/intelligence/pipeline.ts:514 (and 371, 505)	with-param catch (err) { ... } — result.errors.push(...) only.	storeAsContentItem failures during feed ingest are returned to caller via array, but never reach Sentry. Each failure leaks a feed_articles row that never becomes a content_items row — the inverse of Path 7’s gap (article seen, item missing).	Add logger.error({ err, normalisedUrl, sourceId: source.id }, ...) to give ops a structured trail. The result.errors array stays as the user-facing response surface.
lib/content/chunk-store.ts:108, 132, 170	with-param catch (err) { errors.push(...); return ... } (similar pattern).	Chunk INSERT/UPDATE/DELETE failures bubble up via errors[] array, but several callers (e.g. app/api/items/[id]/route.ts:625) await regenerateChunks(...) without inspecting chunkResult.errors.	Audit all regenerateChunks/storeChunks callsites; ensure chunkResult.errors.length > 0 is logged via logger.warn at minimum.

---

P2 findings — best-effort acceptable

Definition: explicitly best-effort with documented intent (cleanup, optional features, fallbacks). 50+ sites; representative sample listed.

File:line	Pattern	Documented intent?
lib/format.ts:14, 25, 36, 96, 160, 171, 182	try { parseISO } catch { return ''; }	Yes — pure formatter; date-parse failure → empty string
lib/extraction/url-validation.ts:67	try { new URL } catch { return invalid format }	Yes — URL validity check
lib/extraction/content-type-detect.ts:46	try { new URL } catch { return 'article' }	Yes — URL fallback
lib/intelligence/rate-limiter.ts:44	try { new URL } catch { return url }	Yes — hostname extraction fallback
lib/intelligence/content-extractor.ts:47, 63, 97	try { new URL/redirect } catch { return false/url }	Yes — Google News redirect resolution; documented
lib/validation/jsonb.ts:93	try { JSON.parse } catch {/* fall through */}	Yes — JSONB-or-stringly-JSON dual handling
lib/dashboard.ts:253 and lib/reorient.ts:93	try { supabase.auth.getUser() } catch { /* fall back to defaults */ }	Yes — auth.getUser is itself idempotent best-effort
lib/search-history.ts:19	try { localStorage.getItem } catch { return [] }	Yes — browser localStorage may throw in private mode
lib/client-config.ts:289	try { ... } catch { return false }	Yes — feature-flag default-deny
lib/digest/digest-export.ts:17	try { ... } catch { return 'unknown' }	Yes — display fallback
lib/quality/qa-detection.ts:749	try { parseHTML } catch { return [] }	Yes — malformed HTML; empty array is functionally correct
app/api/health/route.ts:34	try { count check } catch { supabaseOk = false }	Yes — health probe must not crash
app/api/mcp/[transport]/route.ts:37	try { auth+role } catch { return undefined }	Yes — MCP auth failure must reject; comment is detailed
lib/supabase/server.ts:29	try { setAll cookies } catch { /* server-component */ }	Yes — Next.js server-component cookie shim
app/api/items/[id]/files/route.ts:58, 199	try { Anthropic file API } catch { /* proceed */ }	Yes — external-service idempotency
app/api/items/[id]/vision/route.ts:32	try { req.json } catch { /* default prompt */ }	Yes — body-optional
app/api/items/[id]/images/route.ts:153, 184	try { extractImages / sharp } catch { continue }	Yes — per-image best-effort during PDF extract
app/api/ingest/url/route.ts:118, 130, 153	various catches — /* ignore */, warnings.push, /* non-fatal */	Mixed — L130 surfaces to warnings; L118 is URL parse fallback; L153 is dedup fallback
app/api/ingest/markdown/route.ts:119, 141, 253	request-body parse fallbacks	Yes — body-validation fallthrough
app/api/ingest/url/route.ts:291, 305	classification/summary catch → warnings.push(...)	Yes — surfaces to user response
app/api/admin/batch-reclassify/route.ts:103	request-body parse fallback	Yes
app/api/jobs/[id]/cancel/route.ts:77	request-body parse fallback	Yes
app/api/bids/[id]/export/{docx,xlsx}/route.ts:28	empty body acceptable	Yes
app/api/admin/provenance/export/verification-history/route.ts:87, 95	display-name fallback (OQ-5 RLS)	Yes — documented
lib/logger/sentry-bridge.ts:44, 83	Sentry-write failure swallow	Yes — by design; “Sentry must never break the request”
lib/logger/sentry-bridge.ts:106	safeStringify fallback	Yes
lib/intelligence/feed-poller.ts:140, 377	feed-XML parse + HEAD pre-flight fallback	Yes — documented
lib/intelligence/relevance-scorer.ts:113	LLM JSON parse failure → score: 0, category: 'irrelevant'	Yes — documented; defensible
lib/intelligence/pipeline.ts:145	embedding-cache parse fallback (“regenerate”)	Yes
lib/queue/handlers/markdown-batch.ts:176 and lib/queue/handlers/batch-reclassify.ts:532	isJobCancelled SELECT fallback (returns false)	Yes — best-effort cancel poll
lib/ingest/markdown-orchestrator.ts:404	cancel-poll fallback (matches update-progress contract)	Yes — explicit reference to contract
lib/integrations/github-dispatch.ts:78, 97	retry-then-error-result pattern	Yes — error string returned; not silent
lib/validation/schemas.ts:1092	refine-time URL pre-flight; ctx.addIssue records	Yes
lib/supabase/safe.ts:164	network-fail wrap into SupabaseError	Yes — wraps and rethrows via pgError
lib/mcp/resources.ts (8 sites) and lib/mcp/tools/*.ts (~40 sites)	catch (err) { return MCP error response with isError: true }	Yes — MCP response surface IS the error surface
lib/extraction/markdown-front-matter.ts:96, 105	yaml/toml parse → { value: {}, error: message }	Yes — error returned to caller
app/api/intelligence/workspaces/[id]/seed-starter-pack/route.ts:83	per-feed seed fail → push to result.failed + warnings	Yes
app/api/intelligence/workspaces/[id]/prompts/preview/route.ts:192	rate-limit detect → warnings	Yes
lib/mcp/tools/review.ts:456, 552	notification fail → notificationError field	Yes — error surfaced to MCP response
lib/mcp/tools/governance.ts:417	embedding fail → items[].error	Yes
lib/ai/draft.ts:172, 214, 219, 347, 352	loadSkill fallback (skill optional)	Yes — documented
lib/mcp/tools/dashboard.ts:123	”Non-critical — ownership context is supplementary”	Yes
lib/client-telemetry.ts:33	Sentry.withScope fallback	Yes — documented “Sentry not initialised”
lib/ai/quality-check.ts:161	structured-outputs JSON edge cases	Yes — documented
Python scripts/extract_pdf_images.py (10 sites)	per-image extract fallback (continue/return None)	Yes — best-effort image extraction
Python scripts/kb_pipeline/quality_checks.py:103, 129, 143	date-parse fallback (except ValueError: pass)	Yes — multi-pattern date matching
Python scripts/import_bid_library.py:292	layer-inference fallback (“Non-blocking”)	Yes — documented
Python scripts/extract_tender_questions.py:342	float() parse fallback	Yes
Python scripts/ingest_markdown.py:339	os.path.getmtime fallback to datetime.now	Yes — file-stat fallback
Python scripts/audit-cross-arm-contamination.py:132, scripts/extract-agent-usage.py:48	data-extraction loop continue	Audit/dev tooling — out of production scope
Python scripts/wf-export.py:147	export script	Audit/dev tooling — out of production scope

---

ESLint gap

The existing local/no-silent-promise-catch rule (eslint-rules/no-silent-promise-catch.js) flags .catch(() => ...) ONLY when the handler has zero parameters. It misses:

1. .catch((_err) => null/{}) — single-arg single-expression with underscore-prefixed param. This is the dominant pattern in this codebase (9 app+lib sites listed in pattern 6). All current uses are intentional fallbacks for request.json() or fire-and-forget triggers, but the rule cannot distinguish intent. Either (a) ban underscore-prefixed catch with no body content, requiring the author to use logBestEffortWarn even for “intentional” swallows, or (b) leave as-is and document via convention.

2. try { ... } catch { ... } (no-param, non-empty body). 136 total sites; ~70 in app+lib; 5 of those wrap a Supabase write with no logger call. The existing rule is .catch-method-only; it does not match try/catch syntax. A complementary rule local/no-silent-try-catch-on-write could lint: try { await $X.from(...).insert/update/upsert/delete(...) } catch { /* anything except logger.* / throw / logBestEffortWarn / NextResponse.json */ } to catch app/api/items/batch/route.ts:478, 504, 547 and app/api/intelligence/workspaces/route.ts:231.

3. try { ... } catch (err) { /* err never referenced, no logger */ }. The post-filter found 1 such site in app+lib (lib/integrations/github-dispatch.ts:78, already documented as P2 retry pattern). A lint rule that flags “named-catch-param never referenced AND no logger call AND no throw” would give zero false positives here, since the existing site is intentional and has _err.

4. Python except E: pass with no logger. 3 sites in scripts/ (all in image-extract / multi-pattern date parse; acceptable). No equivalent linter to ESLint runs on Python — flake8 has B902 (“blind except”) but it is not currently enforced. A pre-commit ruff rule (B902/E722) on scripts/kb_pipeline/ (production) would close this gap; intentional uses can opt in via # noqa: B902 — best-effort.

---

Re-ingest readiness gate

Findings that MUST be fixed before re-ingest:

Severity	File:line	Reason
GATE	app/api/upload/route.ts:417-443 (the seed itself)	Wrap inner content_items.source_document_id UPDATE in its own error check; if the UPDATE fails after the source_documents INSERT succeeds, EITHER hard-fail the request OR explicitly delete the just-created source_documents row before re-throwing — currently leaves the row orphaned. (This is part of 0.1 work, not 0.2.6, but listed as a gate because re-ingest will exercise it again.)
GATE	lib/intelligence/pipeline.ts:514 (also 371, 505)	The storeAsContentItem catch arm pushes to result.errors[] but does not log via Sentry/logger. After re-ingest, ops will not see why feed_articles rows lack content_items rows. Add logger.error({err, sourceId, normalisedUrl}, '[Pipeline] storeAsContentItem failed').
GATE	lib/content/chunk-store.ts:108, 132, 170	Chunk write helpers return errors via errors[] but at least one caller (app/api/items/[id]/route.ts:625) does not inspect them. Add a guard at every callsite OR change regenerateChunks to throw on non-empty errors and let the caller decide.
RECOMMENDED	app/api/items/batch/route.ts:478, 504, 547	Pipeline-runs progress + completion writes can silently fail. Recommend sb() + logBestEffortWarn rather than no-op.
RECOMMENDED	app/api/intelligence/workspaces/route.ts:231	Guide-creation outer catch swallows; admins see workspace without guide and no signal.
RECOMMENDED	lib/intelligence/pipeline.ts:455	AI summary failure → feed_articles.ai_summary=null with no observability into model error trends.

---

Open questions for parent session

1. Outer-catch-masks-inner-write pattern (Path 7’s actual seed). ast-grep patterns 1-9 do not detect the case where two writes share one try block and the inner write fails silently because the outer catch logs only the generic outer message. Audit confidence is ~80% for app/lib without a structural-AST sweep specifically for “two awaited writes inside one try”. Recommend a follow-up scan with a pattern like try { $$$ await $X.from(...).insert(...) $$$ await $Y.from(...).update(...) $$$ } catch ($_) { $$$ } plus manual review.

2. logger.error versus logBestEffortWarn for non-fatal swallows. Several P1 sites already log via logger.error (e.g. the seed itself). The CLAUDE.md guidance and lib/supabase/telemetry.ts:38 JSDoc mark logBestEffortWarn as “the ONLY sanctioned way to swallow a non-fatal error in a route handler” — but Path 7 uses logger.error. Are these two patterns equivalent for the spec, or should Path 7 be migrated to logBestEffortWarn (which adds Sentry breadcrumb + warn-level routing) versus the current logger.error (warn-level routing only via captureForLevel)? Pattern selection deserves a written decision.

3. chunk-store.ts errors-array contract. regenerateChunks(...) returns { stored, errors } but several callers (e.g. app/api/items/[id]/route.ts:625, app/api/upload/route.ts:694) use it in two different ways: some inspect chunkResult.errors, some just call and discard. Should regenerateChunks THROW on non-empty errors (forcing every caller to handle), or keep the array contract and add an ESLint rule against unused-result?

4. MCP-tool catch arms returning isError: true. ~40 sites in lib/mcp/tools/*.ts use the pattern catch (err) { return { content: [...err message...], isError: true }; }. This is the MCP response surface, so it IS observability — but it does NOT call Sentry. Should MCP tool catches additionally log via Sentry? Trade-off is duplicated noise (every MCP error becomes a Sentry event) versus loss-of-trend-visibility today. Recommend Liam pick.

5. Python pipeline ESLint-equivalent. The TS ESLint rule blocks empty catches; Python has none. Should we add ruff with B902 (blind except) and E722 (bare except) to the Python pre-commit hook in scripts/kb_pipeline/? Two sites (extract_pdf_images.py, extract_tender_questions.py) would need # noqa markers but otherwise the production-path kb_pipeline/ is already clean.

---

Confidence summary

Aspect	Confidence	Reason
TS no-param try/catch pattern coverage	95%	Sweeping pattern 1; manual review of 65 candidates
TS with-param try/catch pattern coverage	92%	460 candidates; regex-filtered to 84; spot-checked all unusual ones
TS chained .catch(...) coverage	95%	Patterns 5+6
Python except coverage	90%	Patterns 7-10; only 20 candidates in production scripts
Outer-catch-masks-inner-write detection	80%	Not directly scanned; see Open Question §1
MCP-tools catch arm classification	88%	Pattern 2 hit them all; classified P2 because of MCP isError surface, but see Open Question §4