diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 8b2b6a2b..9886cf18 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -27,31 +27,35 @@ Durable, cross-session memory of everything still outstanding for this repo: ope - Resolving an item moves its row to **Resolved / archive** with the date and a one-line outcome — rows are archived, not deleted, so the history stays auditable. - + ## Open items -| ID | Pri | Type | Summary | Detail / next action | Source | Added | -| ---- | --- | ----- | ------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ---------- | -| #001 | P2 | task | Semantic reranking still gated off | `RAG_SEMANTIC_RERANK_ENABLED=false` from PR #901. Do not enable until the provider-backed 36/36 retrieval-quality gate **and** an ambiguity-focused canary are explicitly approved and recorded. | `docs/process-hardening.md` (Semantic reranking rollout debt); PR #901 | 2026-07-21 | -| #005 | P3 | rec | `finalScore` saturates at clamp ceiling | Base + ~40 stacked boosts routinely exceed 1.0, so strong matches tie at 1.0 and order by an arbitrary `document_id` tiebreak. If ranking is ever revisited, break ties by the **pre-clamp** score rather than raising the `[0,1]` ceiling (downstream gates assume `[0,1]`). Ordering already sorts by the unbounded pre-clamp `rankScore` (`clinical-search.ts:1735,1927,1950-1955`), so the clamp confines only the reported confidence value, not result order. Not a defect on the current golden set; any change here is a protected RAG surface (canary required). | `docs/rag-hybrid-findings-and-todo.md` P1 item 4; `src/lib/clinical-search.ts:1735` | 2026-07-21 | -| #006 | P2 | issue | Globe "Language & region" button had no handler | Fixed in this PR to the disabled "Coming soon" placeholder convention (`aria-disabled` + sr-only note). Wire to a real language/region settings screen when one exists, then drop the placeholder state. | `src/components/clinical-dashboard/master-search-header.tsx:1829`; session 2026-07-21 | 2026-07-21 | -| #007 | P3 | rec | `/tools` vs `/?mode=tools` parallel Tools entry points | `/tools` (standalone `ApplicationsLauncherPage`) has no inbound in-app link; the sidebar Tools item uses `/?mode=tools`. Decide the canonical entry point and wire nav consistently, or drop the standalone `/tools` page + `/applications` redirect. Currently allowlisted in `tests/route-reachability.test.ts`. | `src/app/tools/page.tsx`; `src/app/applications/route.ts` | 2026-07-21 | -| #009 | P3 | rec | Confirm `/api/jobs` is intentionally server/ops-only | No client `fetch()` reaches `/api/jobs` (only tests import it). Confirm it is a deliberate ops/manual surface; if abandoned, remove it. | `src/app/api/jobs/route.ts` | 2026-07-21 | -| #010 | P3 | task | Un-built "Coming soon" controls across forms/favourites | ~10 disabled placeholders (forms refine/reset, favourites sort/add/new-set, move-to-set, remove-favourite). Correctly flagged (`aria-disabled` + "Coming soon"), not defects — wire when the underlying features land. | `forms-search-results-page.tsx`; `favourites-hub.tsx`; `favourites-command-library-page.tsx` | 2026-07-21 | -| #011 | P3 | task | Auth DB-connection allocation is operator-only | Supabase Auth (GoTrue) is capped at ~10 absolute DB connections (Supabase perf advisor). Switch to **percentage-based** allocation in the Supabase **dashboard** before the first compute scale-up — **not settable via SQL/MCP** (operator-owned). Verify via a staging soak + an approval-gated read-only advisor re-check. | `docs/auth-connection-cap-runbook.md`; `docs/process-hardening.md` (Known follow-up debts) | 2026-07-21 | -| #012 | P3 | rec | Slim the lazy cross-mode differentials chunk | `cross-mode-differentials.ts` is dynamically imported (correctly code-split **out** of the initial/dashboard bundle — verified), but it pulls the full ~860 KB differentials snapshot (~125 KB gzip lazy chunk) just to build a tiny `{slug,title,clinicalHinge}` + presentations + aliases catalog. A precomputed lightweight index (generator + drift check, like the `specifiers-content` split / medications `fields=index`) would cut that lazy chunk ~5–10×. Not a bundle leak — an M-effort slim. | `src/lib/cross-mode-differentials.ts`; `src/components/clinical-dashboard/cross-mode-links.tsx:150`; session 2026-07-21 (build:analyze) | 2026-07-21 | -| #013 | P3 | rec | Route-chunk + mockup catalogue JSON weight | `build:analyze`: `/specifiers` ships `specifiers-search-index.json` (~180 KB parsed), `/forms` ships `forms-catalog.json` (~132 KB), `/formulation` ships `formulation-content.json` (~52 KB, client-side local search — needs index/full split or a search endpoint, architectural). All route-scoped (not initial bundle). Also `*-mockups.tsx` (~100 KB across chunks) build though `/mockups` 404s in prod — exclude from the prod artifact. | session 2026-07-21 (build:analyze) | 2026-07-21 | -| #014 | P3 | rec | Realize the `next/image` win on signed previews | `next.config` `images` (AVIF + `*.supabase.co` `remotePatterns` pinned to the project host, from #1024) is currently inert — signed document/image previews still render as raw ``. Route them through `next/image` to actually get AVIF + lazy optimization. | #1024; `src/components/clinical-dashboard/signed-image.tsx`; session 2026-07-21 | 2026-07-21 | -| #016 | P3 | rec | "Big but not easy" structural + motion perf | Deferred larger levers: (a) nonce-CSP forces every product route to `ƒ Dynamic` (zero static generation) — evaluate Partial Prerendering / static shells for the static clinical catalogues (DSM/differentials/therapy/specifiers/formulation); (b) sidebar expand/collapse animates `grid-template-columns` (biggest smoothness cost, motion-gated — needs a transform-overlay rethink); (c) Therapy Compass fetches 692 KB / 2.5 MB JSON client-side (defer until interaction + confirm brotli); (d) settings/setup/admin dialogs static-imported into the home chunk (`next/dynamic` them). | session 2026-07-21 (build route table + design audit) | 2026-07-21 | -| #017 | P3 | task | Field Web-Vitals baseline via live Lighthouse | In-sandbox runtime vitals were blocked (prod server hard-requires Supabase secrets; dev-mode CLS measured excellent at 0.00–0.04, content-first pages 0.000). Run Lighthouse against `psychiatry.tools` for real LCP/INP/CLS to prioritize #012–#016 by measured impact rather than reasoning. | session 2026-07-21 (measurement pass) | 2026-07-21 | -| #018 | P2 | task | Monitoring retrieval-depth trio misses schedules | quality-lithium-monitoring-range, quality-antipsychotic-metabolic-monitoring, quality-adhd-medication-monitoring still fail targeting ("no schedule/interval") after the #1040 gate parity — their live chunk sets apparently surface no admissible schedule sentence. Diagnose via read-only chunk queries + run #61 dump artifact (30d retention) to split corpus-content vs retrieval-depth vs fact-extraction; fix shape TBD after diagnosis. A retrieval-side fix is a protected surface (canary pair, fresh ~$2-4 authorization). | run #61 targeting misses; branch-review-ledger 2026-07-21 | 2026-07-21 | -| #019 | P2 | task | Comparison-class coverage: admission doc never in top-5 | admission-discharge-comparison misses its expected admission-side doc on every canary (#57-#61) — top-5 carries discharge-planning + unrelated docs only. Same "why doesn't retrieval surface X" species as #018; diagnose jointly. Ground-truth already widened (#1020); the residual is genuine comparison-class retrieval coverage. | runs #57-#61; PR #1020 replay verdict | 2026-07-21 | -| #020 | P3 | task | Validate eval:quality cost readout post-fix | The estimator fix landed in PR #1050 (zero-usage cases count $0 with rates configured; provider-attempted cases with missing usage stay n/a). Remaining action: confirm the next canary run prints a real dollar figure in Answer Metrics, then archive this row. | PR #1050; run #60/#61 Answer Metrics tables | 2026-07-21 | -| #021 | P3 | rec | E-3d H2 residual: strong/comparison generation discards | approx. 6 generation attempts per full 44-case run still fail the final quality gate and fall to extractive on strong-route comparison/complex shapes (the designed-conservative outcome). PARKED: weakest cost/benefit on the queue — a wave (approx. $2-4 pair + reviewer cycle) to shave seconds off a few hard cases. Revisit only if latency/waste complaints or a cheaper lever appears. | E-3c design record; runs #59-#61 diagnostics | 2026-07-21 | -| #022 | P2 | task | Source-governance metadata refresh (operator) | Governance warning rate ~0.84 across canaries: stale/review-required/unknown source metadata on most top results. Operator work (document review-status attestation in the app), not a code defect. Next action: generate the prioritized refresh worklist ($0 read-only) and schedule a metadata pass. | runs #57-#61 Source Governance tables; docs/observability-slos.md | 2026-07-21 | -| #023 | P3 | task | Read Sunday 2026-07-26 scheduled-run artifacts | The 18:00 UTC scheduled runs deliver three free datapoints at once: first full-44 weekly canary (validates the #1044 ANSWER_CASE_LIMIT raise), browser-matrix flake second datapoint (webkit ui-route-coverage now reproduced + root-caused 2026-07-22 → see #024; firefox ui-formulation:91 still awaits a datapoint), and the irrelevant@10 labeling-audit artifact (§3.1 human-decision class). Read all three, then disposition. | sessions 2026-07-20/21; branch-review-ledger convergence notes | 2026-07-21 | -| #024 | P3 | issue | WebKit e2e `_rsc`-prefetch access-control-checks errors | verify:release:offline on `main` ce32fe170 (2026-07-22) reproduced #023's webkit clause: **6/6 deterministic** failures in `tests/ui-route-coverage.spec.ts` (Therapy Compass; DSM home/comparison; Specifier comparison/map; Differential stream), each a `pageerror … ?_rsc=… due to access control checks` on Next.js RSC prefetch — Chromium + Firefox clean. Not merge-blocking (required gate `test:e2e:pr` is chromium-only; the full webkit matrix is advisory/release-time). Most likely a Playwright route-interception × WebKit interaction, not a Safari user defect. Next: decide (a) allow/mock the `_rsc` routes for the `webkit` e2e project, or (b) confirm real Safari impact — before trusting the full-matrix webkit gate at release. NB the 2 other webkit fails (`ui-stress:412`, `ui-universal-search:210`) passed on isolated re-run = true flake. | session 2026-07-22 (verify:release:offline, `main` ce32fe170); refines #023 | 2026-07-22 | +| ID | Pri | Type | Summary | Detail / next action | Source | Added | +| ---- | --- | ----- | ------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #001 | P2 | task | Semantic reranking still gated off | `RAG_SEMANTIC_RERANK_ENABLED=false` from PR #901. Do not enable until the provider-backed 36/36 retrieval-quality gate **and** an ambiguity-focused canary are explicitly approved and recorded. | `docs/process-hardening.md` (Semantic reranking rollout debt); PR #901 | 2026-07-21 | +| #005 | P3 | rec | `finalScore` saturates at clamp ceiling | Base + ~40 stacked boosts routinely exceed 1.0, so strong matches tie at 1.0 and order by an arbitrary `document_id` tiebreak. If ranking is ever revisited, break ties by the **pre-clamp** score rather than raising the `[0,1]` ceiling (downstream gates assume `[0,1]`). Ordering already sorts by the unbounded pre-clamp `rankScore` (`clinical-search.ts:1735,1927,1950-1955`), so the clamp confines only the reported confidence value, not result order. Not a defect on the current golden set; any change here is a protected RAG surface (canary required). | `docs/rag-hybrid-findings-and-todo.md` P1 item 4; `src/lib/clinical-search.ts:1735` | 2026-07-21 | +| #006 | P2 | issue | Globe "Language & region" button had no handler | Fixed in this PR to the disabled "Coming soon" placeholder convention (`aria-disabled` + sr-only note). Wire to a real language/region settings screen when one exists, then drop the placeholder state. | `src/components/clinical-dashboard/master-search-header.tsx:1829`; session 2026-07-21 | 2026-07-21 | +| #007 | P3 | rec | `/tools` vs `/?mode=tools` parallel Tools entry points | `/tools` (standalone `ApplicationsLauncherPage`) has no inbound in-app link; the sidebar Tools item uses `/?mode=tools`. Decide the canonical entry point and wire nav consistently, or drop the standalone `/tools` page + `/applications` redirect. Currently allowlisted in `tests/route-reachability.test.ts`. | `src/app/tools/page.tsx`; `src/app/applications/route.ts` | 2026-07-21 | +| #009 | P3 | rec | Confirm `/api/jobs` is intentionally server/ops-only | No client `fetch()` reaches `/api/jobs` (only tests import it). Confirm it is a deliberate ops/manual surface; if abandoned, remove it. | `src/app/api/jobs/route.ts` | 2026-07-21 | +| #010 | P3 | task | Un-built "Coming soon" controls across forms/favourites | ~10 disabled placeholders (forms refine/reset, favourites sort/add/new-set, move-to-set, remove-favourite). Correctly flagged (`aria-disabled` + "Coming soon"), not defects — wire when the underlying features land. | `forms-search-results-page.tsx`; `favourites-hub.tsx`; `favourites-command-library-page.tsx` | 2026-07-21 | +| #011 | P3 | task | Auth DB-connection allocation is operator-only | Supabase Auth (GoTrue) is capped at ~10 absolute DB connections (Supabase perf advisor). Switch to **percentage-based** allocation in the Supabase **dashboard** before the first compute scale-up — **not settable via SQL/MCP** (operator-owned). Verify via a staging soak + an approval-gated read-only advisor re-check. | `docs/auth-connection-cap-runbook.md`; `docs/process-hardening.md` (Known follow-up debts) | 2026-07-21 | +| #012 | P3 | rec | Slim the lazy cross-mode differentials chunk | `cross-mode-differentials.ts` is dynamically imported (correctly code-split **out** of the initial/dashboard bundle — verified), but it pulls the full ~860 KB differentials snapshot (~125 KB gzip lazy chunk) just to build a tiny `{slug,title,clinicalHinge}` + presentations + aliases catalog. A precomputed lightweight index (generator + drift check, like the `specifiers-content` split / medications `fields=index`) would cut that lazy chunk ~5–10×. Not a bundle leak — an M-effort slim. | `src/lib/cross-mode-differentials.ts`; `src/components/clinical-dashboard/cross-mode-links.tsx:150`; session 2026-07-21 (build:analyze) | 2026-07-21 | +| #013 | P3 | rec | Route-chunk + mockup catalogue JSON weight | `build:analyze`: `/specifiers` ships `specifiers-search-index.json` (~180 KB parsed), `/forms` ships `forms-catalog.json` (~132 KB), `/formulation` ships `formulation-content.json` (~52 KB, client-side local search — needs index/full split or a search endpoint, architectural). All route-scoped (not initial bundle). Also `*-mockups.tsx` (~100 KB across chunks) build though `/mockups` 404s in prod — exclude from the prod artifact. | session 2026-07-21 (build:analyze) | 2026-07-21 | +| #014 | P3 | rec | Realize the `next/image` win on signed previews | `next.config` `images` (AVIF + `*.supabase.co` `remotePatterns` pinned to the project host, from #1024) is currently inert — signed document/image previews still render as raw ``. Route them through `next/image` to actually get AVIF + lazy optimization. | #1024; `src/components/clinical-dashboard/signed-image.tsx`; session 2026-07-21 | 2026-07-21 | +| #016 | P3 | rec | "Big but not easy" structural + motion perf | Deferred larger levers: (a) nonce-CSP forces every product route to `ƒ Dynamic` (zero static generation) — evaluate Partial Prerendering / static shells for the static clinical catalogues (DSM/differentials/therapy/specifiers/formulation); (b) sidebar expand/collapse animates `grid-template-columns` (biggest smoothness cost, motion-gated — needs a transform-overlay rethink); (c) Therapy Compass fetches 692 KB / 2.5 MB JSON client-side (defer until interaction + confirm brotli); (d) settings/setup/admin dialogs static-imported into the home chunk (`next/dynamic` them). | session 2026-07-21 (build route table + design audit) | 2026-07-21 | +| #017 | P3 | task | Field Web-Vitals baseline via live Lighthouse | In-sandbox runtime vitals were blocked (prod server hard-requires Supabase secrets; dev-mode CLS measured excellent at 0.00–0.04, content-first pages 0.000). Run Lighthouse against `psychiatry.tools` for real LCP/INP/CLS to prioritize #012–#016 by measured impact rather than reasoning. | session 2026-07-21 (measurement pass) | 2026-07-21 | +| #018 | P2 | task | Monitoring retrieval-depth trio misses schedules | quality-lithium-monitoring-range, quality-antipsychotic-metabolic-monitoring, quality-adhd-medication-monitoring still fail targeting ("no schedule/interval") after the #1040 gate parity — their live chunk sets apparently surface no admissible schedule sentence. Diagnose via read-only chunk queries + run #61 dump artifact (30d retention) to split corpus-content vs retrieval-depth vs fact-extraction; fix shape TBD after diagnosis. A retrieval-side fix is a protected surface (canary pair, fresh ~$2-4 authorization). | run #61 targeting misses; branch-review-ledger 2026-07-21 | 2026-07-21 | +| #019 | P2 | task | Comparison-class coverage: admission doc never in top-5 | admission-discharge-comparison misses its expected admission-side doc on every canary (#57-#61) — top-5 carries discharge-planning + unrelated docs only. Same "why doesn't retrieval surface X" species as #018; diagnose jointly. Ground-truth already widened (#1020); the residual is genuine comparison-class retrieval coverage. | runs #57-#61; PR #1020 replay verdict | 2026-07-21 | +| #020 | P3 | task | Validate eval:quality cost readout post-fix | The estimator fix landed in PR #1050 (zero-usage cases count $0 with rates configured; provider-attempted cases with missing usage stay n/a). Remaining action: confirm the next canary run prints a real dollar figure in Answer Metrics, then archive this row. | PR #1050; run #60/#61 Answer Metrics tables | 2026-07-21 | +| #021 | P3 | rec | E-3d H2 residual: strong/comparison generation discards | approx. 6 generation attempts per full 44-case run still fail the final quality gate and fall to extractive on strong-route comparison/complex shapes (the designed-conservative outcome). PARKED: weakest cost/benefit on the queue — a wave (approx. $2-4 pair + reviewer cycle) to shave seconds off a few hard cases. Revisit only if latency/waste complaints or a cheaper lever appears. | E-3c design record; runs #59-#61 diagnostics | 2026-07-21 | +| #022 | P2 | task | Source-governance metadata refresh (operator) | Governance warning rate ~0.84 across canaries: stale/review-required/unknown source metadata on most top results. Operator work (document review-status attestation in the app), not a code defect. Next action: generate the prioritized refresh worklist ($0 read-only) and schedule a metadata pass. | runs #57-#61 Source Governance tables; docs/observability-slos.md | 2026-07-21 | +| #023 | P3 | task | Read Sunday 2026-07-26 scheduled-run artifacts | The 18:00 UTC scheduled runs deliver three free datapoints at once: first full-44 weekly canary (validates the #1044 ANSWER_CASE_LIMIT raise), browser-matrix flake second datapoint (webkit ui-route-coverage now reproduced + root-caused 2026-07-22 → see #024; firefox ui-formulation:91 still awaits a datapoint), and the irrelevant@10 labeling-audit artifact (§3.1 human-decision class). Read all three, then disposition. | sessions 2026-07-20/21; branch-review-ledger convergence notes | 2026-07-21 | +| #024 | P3 | issue | WebKit e2e `_rsc`-prefetch access-control-checks errors | verify:release:offline on `main` ce32fe170 (2026-07-22) reproduced #023's webkit clause: **6/6 deterministic** failures in `tests/ui-route-coverage.spec.ts` (Therapy Compass; DSM home/comparison; Specifier comparison/map; Differential stream), each a `pageerror … ?_rsc=… due to access control checks` on Next.js RSC prefetch — Chromium + Firefox clean. Not merge-blocking (required gate `test:e2e:pr` is chromium-only; the full webkit matrix is advisory/release-time). Most likely a Playwright route-interception × WebKit interaction, not a Safari user defect. Next: decide (a) allow/mock the `_rsc` routes for the `webkit` e2e project, or (b) confirm real Safari impact — before trusting the full-matrix webkit gate at release. NB the 2 other webkit fails (`ui-stress:412`, `ui-universal-search:210`) passed on isolated re-run = true flake. | session 2026-07-22 (verify:release:offline, `main` ce32fe170); refines #023 | 2026-07-22 | +| #025 | P2 | task | Activate the three webhooks (operator secrets) | Merged (#968) + deployed but inert — verified live: `POST /api/webhooks/railway` returns `503 webhook_not_configured`. To turn on: (1) Railway → set `RAILWAY_WEBHOOK_SECRET` + add the `?token=…` webhook URL; (2) the chat URLs `SLACK_WEBHOOK_URL`/`DISCORD_WEBHOOK_URL` must be set in BOTH places — the Railway **app/server env** (the receiver forwards deploy alerts via `postChatNotification`, which reads server env, so repo-secret-only leaves the Railway webhook authenticated but returning `delivered:false`) AND as **GitHub repo secrets** (the CI-failure workflow reads `secrets.*`); (3) `SUPABASE_INGESTION_WEBHOOK_SECRET`. Each fails closed until set, so this is pure ops. See docs/webhooks.md. | session 2026-07-22; PR #968; docs/webhooks.md | 2026-07-22 | +| #026 | P2 | task | Wire the Supabase document-change trigger | The `/api/webhooks/supabase/document-change` receiver is live but nothing calls it. The trigger (`notify_document_change_ingestion_webhook()` + `documents_ingestion_webhook`, SQL in docs/webhooks.md §3) must land as a **committed migration** — drift covers functions/triggers, so applying raw SQL to live would fail `check:drift`: add the migration, reconcile `supabase/schema.sql`, run `npm run drift:manifest` (needs a local Supabase container), then deploy. Operator-applied live state is the Vault secret (`select vault.create_secret('','ingestion_webhook_secret')`) + the base-URL GUC `app.ingestion_webhook_base_url`, which is MANDATORY per environment (the trigger no-ops when it is unset, so `reindex_requested` flips never enqueue — there is no production default). Trigger is `AFTER UPDATE` on `reindex_requested` only — NOT `INSERT` (an INSERT-fired webhook can beat the upload route's own job insert, which then deletes its just-created doc → intermittent upload failures); external inserts set `reindex_requested=true` to be picked up. Note delivery is at-most-once (pg_net has no retry) and there is NO built-in recovery (automatic or manual) for a dropped insert — neither the autopilot nor `scripts/reindex.ts` sweeps `queued` docs with no job (`hasIncompleteDocumentsWithoutOpenJobs` requires `queuedDocuments === 0`). External imports must insert then issue a SEPARATE UPDATE flipping `reindex_requested=true` (an insert with the flag pre-set won't fire the AFTER UPDATE trigger). The app upload path is NOT a guarantee either: it inserts the `documents` row then the `ingestion_jobs` row as two separate non-atomic calls (rollback only on a job-insert error), so a crash/timeout between them strands the same queued-without-job row nothing sweeps. The only true guarantees are an atomic enqueue (document + job in one transaction/RPC) or a scheduled queued-without-job sweep. Until applied, webhook #5 never fires. | session 2026-07-22; docs/webhooks.md §3 | 2026-07-22 | +| #027 | P3 | rec | External uptime monitor independent of GitHub/Railway | `live-domain-monitor.yml` runs on GitHub's cron, so it won't run in exactly the outage it should catch (Actions or the deploy itself down). Add an off-platform synthetic monitor (UptimeRobot / Better Stack / Checkly) hitting `/api/health` with a webhook alert. Provider setup, not code. | session 2026-07-22 webhook review | 2026-07-22 | +| #028 | P3 | rec | Runtime error tracking (Sentry or similar) | No error tracking in the repo — production exceptions on `psychiatry.tools`, including how often `RAG_PROVIDER_MODE=auto` silently degrades to source-only, are invisible. Weigh adding `@sentry/nextjs` (dependency + DSN secret + instrumentation) vs cost; alert → chat/issue. Provider-backed; needs explicit sign-off before adding the dependency. | session 2026-07-22 webhook review | 2026-07-22 | ## Resolved / archive diff --git a/docs/webhooks.md b/docs/webhooks.md index 36a9ca2c..8e0065a7 100644 --- a/docs/webhooks.md +++ b/docs/webhooks.md @@ -43,6 +43,13 @@ https://psychiatry.tools/api/webhooks/railway?token= Transient phases (`BUILDING`, `DEPLOYING`, `QUEUED`, …) are dropped to keep the channel quiet; the receiver answers `200 { "skipped": true }` for them. +> Chat destination: this receiver forwards through `postChatNotification`, which +> reads `SLACK_WEBHOOK_URL`/`DISCORD_WEBHOOK_URL` from **server env** — set them on +> the Railway `Database` service, not only as GitHub repo secrets. With the token +> set but no chat URL in server env the receiver authenticates and returns +> `200 { "forwarded": false }`, so deploy alerts are silently undelivered. The +> repo secrets in §2 cover only the GitHub CI-failure workflow. + > Note: the receiver runs inside the app being deployed, so a notification about > a deploy that takes the app fully down may not be delivered. Pair it with an > external uptime monitor for hard-down detection. @@ -71,15 +78,22 @@ constant-time. Fails closed (`503`) when unset, `401` on a bad secret. **When it enqueues (idempotent + loop-safe):** -- **INSERT** of a not-yet-`indexed` document → enqueue. The app upload route also - enqueues, but the `ingestion_jobs` one-open-job-per-document unique index makes - the duplicate insert a benign no-op (`already_active`). +- **INSERT** of a not-yet-`indexed` document → enqueue. Note this is what the + _receiver_ does with an INSERT event; the **recommended trigger below does not + send INSERT events** because the app upload route deletes its own just-created + document if _its_ job insert loses the one-open-job race, so a webhook enqueue + winning that race would break the upload. Drive external inserts through a + post-insert `reindex_requested` UPDATE instead (see Setup). - **UPDATE** acts only when `record.metadata.reindex_requested === true`, then clears that flag via `apply_document_metadata_patch`. The worker's own completion writes (also UPDATEs) never carry the flag, so they cannot retrigger an endless loop. If the clear itself fails, the receiver responds `500` (not - `2xx`) so Supabase retries delivery until the flag is actually cleared — the - idempotent enqueue means a retry cannot double-queue. + `2xx`), so a caller that _retries_ would re-deliver until the flag is cleared + (the idempotent enqueue means a retry cannot double-queue). But note neither + documented path retries (the raw `net.http_post` trigger and the UPDATE-only + managed webhook are both at-most-once — see the note below), so a failed clear + can leave `reindex_requested` stuck `true`; recover it with the clear-then-flip + shown below. - `checkIngestionMutationSafety` refuses while a job is already active, and the enqueue reports `already_active` instead of erroring on a lost race. @@ -87,12 +101,242 @@ Every write is owner-scoped (`owner_id`) — the app's single tenancy layer, sin the service-role client bypasses RLS. Events without an `owner_id`, on other tables, or of type `DELETE` are skipped with `200`. -**Setup (Supabase).** Create a Database Webhook (or SQL trigger via -`supabase_functions.http_request`) on `public.documents` for INSERT/UPDATE with: +### Setup (operator-applied) + +The receiver is deployed but inert until a Supabase-side trigger actually calls +it. The trigger function/trigger below must ship as a **committed migration**, not +raw SQL applied to the live database — this repo's drift inventory +(`supabase/drift-manifest.json`) covers functions and triggers, so creating them +directly on live would diverge from `supabase/schema.sql` and **fail the next +`check:drift`**, and a schema restore would omit the webhook entirely. The only +operator-applied live state is the Vault secret (step 2) and the per-environment +base-URL GUC (step 3, required). + +**Landing the trigger (step 4) needs a local Supabase container** — the schema +mirror + drift manifest are regenerated from a live replay, which cannot be done +offline: + +- Add the SQL from step 4 as `supabase/migrations/_document_change_ingestion_webhook.sql`. +- Reconcile it into `supabase/schema.sql` (the checker `check:function-grants` + reads the snapshot, and the `revoke execute … from public` below must appear there). +- Regenerate the drift manifest with `npm run drift:manifest`, then apply the + migration to the live project through the normal deploy path. + +**1. Set the app env var** — `SUPABASE_INGESTION_WEBHOOK_SECRET` (min 16 chars) on +the Railway `Database` + `worker` services. + +**2. Store the same secret in Supabase Vault** so the trigger can read it without +hardcoding it: + +```sql +select vault.create_secret('', 'ingestion_webhook_secret'); +``` + +**3. Point the base URL at _this_ environment (REQUIRED, every environment).** +There is no production default — if this GUC is unset the trigger no-ops, so an +un-configured staging/local replay can never post to production. Set it explicitly +per environment (production included): + +```sql +-- production: 'https://psychiatry.tools' · staging/local: that environment's host +alter database postgres set app.ingestion_webhook_base_url = 'https://'; +``` + +**4. Create the fail-safe trigger.** It fires only on an explicit +`reindex_requested` flip (an `AFTER UPDATE` trigger — **not** `INSERT`, to avoid +racing the app upload route; see the comment in the function), reads the secret +from Vault, and — critically — never aborts a document write if the secret is +missing or the POST fails (it just does nothing, mirroring the receiver's inert +`503`). `net.http_post` is asynchronous, so it does not block the write. + +> **Delivery is at-most-once.** `net.http_post` is fire-and-forget — it returns a +> request id and does not retry on failure ([pg_net retries are still an open +> feature request](https://github.com/supabase/pg_net/issues/110)). So if the app +> is down, the secret is mismatched, or the receiver 500s _before_ enqueueing, +> that single event is lost. The receiver's "500 → provider retries" wording only +> holds against a caller that actually retries — **do not assume either the raw +> trigger _or_ a managed Supabase Database Webhook provides that**: managed +> webhooks are themselves a thin wrapper over pg_net, so they share the same +> no-retry limitation today. +> +> **There is no built-in recovery — automatic _or_ manual — for a dropped +> delivery.** A fresh insert lands as `status = 'queued'` with no `ingestion_jobs` +> row, and nothing sweeps that state: the scheduled Ingestion Autopilot's health +> check (`assessIngestionHealth`) only flags _failed_ or _stale-processing_ jobs, +> and the manual `scripts/reindex.ts` predicate `hasIncompleteDocumentsWithoutOpenJobs` +> explicitly requires `queuedDocuments === 0` (`src/lib/reindex-pipeline.ts`), so a +> stranded `queued`-without-job row matches neither — it stays unindexed. +> +> **To recover a stranded row, make the flag transition happen.** An `UPDATE` that +> sets `metadata.reindex_requested = true` fires this trigger only when the value +> actually _changes_ to true. A row stranded because the original flip's delivery +> was lost already has the flag `true`, so setting it `true` again is a no-op +> `true → true` that will **not** re-fire — you must **clear it, then set it**: +> +> ```sql +> update public.documents set metadata = metadata || '{"reindex_requested": false}'::jsonb where id = ''; +> update public.documents set metadata = metadata || '{"reindex_requested": true}'::jsonb where id = ''; +> ``` +> +> (A scheduled sweep should do the same clear-then-flip, or target `queued` rows +> with no open job directly.) **So for a document that must never be left +> unindexed, do not rely on webhook delivery alone** — ingest it through the app +> upload route and confirm the job appears, or use the insert-then-flag pattern +> below. Note the upload route is **not** fully transactional either: it inserts +> the `documents` row and then the `ingestion_jobs` row as two separate calls, +> deleting the document only if the job insert returns an error +> (`src/app/api/upload/route.ts`). Under normal operation that rollback prevents +> an orphan, but a process crash/timeout/kill _between_ the two inserts can still +> strand the same `queued`-without-job row nothing sweeps. The only true +> must-never-drop guarantees are an atomic enqueue (document + job in one +> transaction/RPC) or a scheduled queued-without-job sweep. This trigger is a +> low-latency optimisation, not a delivery guarantee. + +```sql +create or replace function public.notify_document_change_ingestion_webhook() +returns trigger +language plpgsql +security definer +set search_path = public, extensions, vault, pg_temp +as $$ +declare + v_secret text; + v_base_url text; +begin + -- Fire ONLY on an explicit reindex request (metadata.reindex_requested flips to + -- true). The INSERT case is deliberately NOT handled here: the app upload route + -- inserts the document and then its own ingestion job in the same request, and + -- it DELETES the document if that job insert loses a race against the one-open- + -- job-per-document unique index (src/app/api/upload/route.ts). A webhook firing + -- on INSERT could win that race and turn a normal upload into an intermittent + -- failure. Documents inserted outside the upload flow are enqueued by a SEPARATE + -- UPDATE that flips metadata.reindex_requested to true (an INSERT with the flag + -- already set does NOT fire this AFTER UPDATE trigger). See docs below. + -- + -- Compare the JSON value directly to the JSON boolean `true` — matches the + -- receiver's strict `=== true` (a JSON string "true" is NOT actionable) and, + -- unlike `->> ... ::boolean`, never raises on a malformed value. + if new.metadata->'reindex_requested' = 'true'::jsonb + and new.metadata->'reindex_requested' is distinct from old.metadata->'reindex_requested' + then + -- actionable + else + return new; + end if; + + -- Fail SAFE: with no Vault secret the trigger no-ops rather than break the write. + select decrypted_secret into v_secret + from vault.decrypted_secrets + where name = 'ingestion_webhook_secret' + limit 1; + + if nullif(v_secret, '') is null then + return new; + end if; + + -- REQUIRED, per environment. There is deliberately no production fallback: if + -- this GUC is unset the trigger no-ops (like a missing secret) rather than post + -- to a hardcoded host. A hardcoded prod default would make a staging/local + -- replay POST that environment's document row to the PRODUCTION receiver — + -- which is wired to a different Supabase project, so the row would fail to + -- enqueue and its metadata would cross environments. Each environment sets its + -- own base URL (step 3). + v_base_url := nullif(current_setting('app.ingestion_webhook_base_url', true), ''); + if v_base_url is null then + return new; + end if; + + perform net.http_post( + url := v_base_url || '/api/webhooks/supabase/document-change', + headers := jsonb_build_object( + 'Content-Type', 'application/json', + 'Authorization', 'Bearer ' || v_secret + ), + -- Minimal payload by design. The receiver reads only id / owner_id / status / + -- metadata.reindex_requested and then reloads the authoritative row by id, so + -- the full row is never needed. Sending `to_jsonb(new)` (plus `old_record`) + -- would ship file names, storage paths, and content hashes to whatever host + -- app.ingestion_webhook_base_url resolves to — needlessly widening what a + -- mistyped/wrong-environment GUC could leak. Send the four fields it uses and + -- omit old_record entirely. + body := jsonb_build_object( + 'type', tg_op, + 'table', tg_table_name, + 'schema', tg_table_schema, + 'record', jsonb_build_object( + 'id', new.id, + 'owner_id', new.owner_id, + 'status', new.status, + 'metadata', jsonb_build_object( + 'reindex_requested', new.metadata -> 'reindex_requested' + ) + ) + ), + timeout_milliseconds := 5000 + ); + + return new; +exception + when others then + -- A notification failure must never abort the document write. + raise warning 'notify_document_change_ingestion_webhook failed: %', sqlerrm; + return new; +end; +$$; + +-- SECURITY DEFINER hardening: never callable directly by clients. +revoke execute on function public.notify_document_change_ingestion_webhook() from public, anon, authenticated; + +drop trigger if exists documents_ingestion_webhook on public.documents; +-- UPDATE only, by design — see the INSERT-race comment in the function above. +create trigger documents_ingestion_webhook + after update on public.documents + for each row execute function public.notify_document_change_ingestion_webhook(); +``` + +To reindex an existing document — or to index one inserted outside the app upload +flow — issue an **`UPDATE` that flips `metadata.reindex_requested` to `true`**; the +trigger POSTs, the receiver enqueues the job and clears the flag. + +> **Insert-then-flag, as two statements.** Because this is an `AFTER UPDATE` +> trigger whose predicate requires the flag to _change_ from not-true to true, the +> two statements must be: (1) `INSERT` the row **with `reindex_requested` absent or +> `false`**, then (2) a separate `UPDATE` that flips it to `true`. An `INSERT` with +> the flag already `true` never fires the trigger (no UPDATE happened), and a later +> `UPDATE` that sets it `true` again is a no-op `true → true` (not _distinct from_ +> the old value), so it will not fire either — both leave a `queued` row with no +> job. Concretely: +> +> ```sql +> update public.documents +> set metadata = metadata || '{"reindex_requested": true}'::jsonb +> where id = ''; +> ``` +> +> Then confirm an `ingestion_jobs` row appears for that document. -- URL: `https://psychiatry.tools/api/webhooks/supabase/document-change` -- HTTP header: `Authorization: Bearer ` +**Managed alternative (dashboard convenience).** A Supabase **Database Webhook** +on `public.documents` scoped to **`UPDATE` events only** — do **not** tick +`INSERT` — pointed at the URL above with an `Authorization: Bearer ` +header avoids writing the trigger SQL yourself. An `INSERT` webhook re-introduces +exactly the upload-race the SQL trigger avoids (the upload route deletes its +document if a webhook enqueue beats its own job insert), so `UPDATE`-only is not +optional here. Its only real advantage over the committed migration is +convenience — it does **not** buy you at-least-once delivery (it wraps pg_net, +same no-retry limitation as the raw trigger; see the note above). Its other trade-offs: it fires on _every_ update (the receiver then skips +the non-actionable ones, versus the SQL gating above), and the dashboard-created +object is operator/live state that must be recorded in +`supabase/drift-allowlist.json` if it appears in the drift inventory. It adds +**no** delivery guarantee — see the at-most-once note above for why neither path +self-heals a dropped insert. -To request a reindex of an existing document, set -`metadata.reindex_requested = true` on its row; the receiver enqueues the job and -clears the flag. +> **Privacy: prefer the committed trigger for `public.documents`.** A managed +> Database Webhook builds the POST body automatically and includes the **full +> row** — both `record` and, on UPDATE, `old_record` +> ([Supabase docs](https://supabase.com/docs/guides/database/webhooks#payload)). +> Unlike the SQL trigger above (which sends only `id`/`owner_id`/`status`/the +> reindex flag), this ships filenames, storage paths, content hashes, and the +> prior row's metadata on every update. A mistyped or wrong-environment URL would +> leak all of it. Because `documents` holds private clinical rows, use the +> **committed minimal trigger** for this table; reserve the dashboard webhook for +> non-sensitive tables where the full-row payload is acceptable.