Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,58 @@ jobs:
needs: changes
runs-on: ubuntu-24.04
timeout-minutes: 20
permissions:
contents: read
# Read-only Actions access for the eval-canary liveness probe below.
actions: read
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false

# GitHub cron fires are best-effort, and the canary's own failure-issue step only reacts
# to runs that HAPPEN and fail — since #923 moved the canary cadence from daily to
# weekly (Sunday 18:00 UTC), a silently dropped fire would go unnoticed for a week or
# more. PR traffic runs many times a day, so a warn-only staleness probe here surfaces a
# dropped weekly canary within hours instead of weeks. Never fails the job — API hiccups
# and empty histories degrade to a warning at most.
- name: Eval-canary liveness (warn if stale)
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
try {
const { data } = await github.rest.actions.listWorkflowRuns({
owner: context.repo.owner,
repo: context.repo.repo,
workflow_id: "eval-canary.yml",
status: "completed",
per_page: 1,
});
const latest = data.workflow_runs?.[0];
// updated_at ~= completion time for a completed run (the REST payload carries no
// completed_at); run_started_at is the fallback for older/partial payloads.
const finishedAt = Date.parse(latest?.updated_at ?? latest?.run_started_at ?? "");
if (!latest) {
core.warning("Eval-canary liveness: no completed canary runs found.");
} else if (!Number.isFinite(finishedAt)) {
core.info("Eval-canary liveness: latest completed run has no parseable timestamp; skipping staleness check.");
} else {
const ageDays = (Date.now() - finishedAt) / 86_400_000;
if (ageDays > 8) {
Comment thread
BigSimmo marked this conversation as resolved.
core.warning(
`Eval-canary liveness: last completed canary run was ${ageDays.toFixed(1)} days ago (${latest.html_url}). ` +
"The weekly Sunday 18:00 UTC schedule may have been dropped by GitHub - dispatch one manually " +
"(provider-backed; needs explicit approval).",
);
} else {
core.info(`Eval-canary liveness: last completed run ${ageDays.toFixed(1)} days ago.`);
}
}
} catch (error) {
core.warning(`Eval-canary liveness check skipped: ${error.message}`);
}

- name: Setup Node and dependencies
uses: ./.github/actions/setup-node-cached

Expand Down
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -659,3 +659,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD
| 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (Phase C live verdict — no code change; main stays reverted at 0a498e6) | canary run 29773198933 (#55, on f96217c) | ADDENDUM 4 Phase C LIVE-REFUTED: canary on the merged tail code FAILED 3/36 (doc_recall 1.0→0.9167, mrr@10 0.8921→0.8138) — the user-ordered revert (#1005) was correct and STANDS | Failures: patient-safety-plan-include (PtSafetyPlan out of top-5, rr 0.33→0.14), patient-property-visual-table (rr 1.00→0.11) and schizophrenia-overview (rr 1.00→0.14) — two previously rank-1 cases destroyed. Root cause (post-hoc): the S1 lexical-chunk lift spreads hybrid_score, which is the PRIMARY release sort key — inside the dead (0.48,0.5) band it still PREEMPTS every downstream key, so raw ts_rank order overrode the boost/title/subject-aware relevance order that previously decided all-tied-at-0.48 pools. Lexically-loud chunks leapfrogged title-boosted correct documents = the #118 mechanism, reproduced live. The offline S1 test used identical-content candidates (coverage tie) and could not see it. The S2 similarity tail (tie-break-position key) remains reviewer-verified safe in isolation — retrieval reviewer verdict on the full diff: APPROVE-WITH-NITS, P3 only (0.49-lowering proven unreachable via the SQL 0.48 cap; no gate crossings in (0.92,0.94); hardening nits recorded). DISPOSITION: no re-land as-is. Any retry = S2-only + S1 redesigned to a key BELOW relevance in the comparator chain, new design + fresh pair. The staged-rollback discipline (canary pair + instant revert) worked exactly as designed. Spend ≈$4-8 of ≤$20. | Canary #55 read from job log (3 FAIL lines + summary); reviewer verdict from subagent report; main verified reverted (0 saturationTailUnit refs at 0a498e6) |
| 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (PR: RAG memory + safeguards) | see PR head | User-directed close-out: durable RAG behaviour memory (docs/rag-behaviour/, 4 files) + AGENTS.md standing protection rules + ENFORCED safeguards — pr-policy blocking `RAG impact:` gate on protected surfaces (self-tested: undeclared/vague blocked, no-change/canary declarations pass) + source-pin contract test on imputation formulas and release comparator key order (red-proven vs a mutated constant). Confirmation canary #56 (29774459706, reverted main 0a498e6): SUCCESS — 36/36 restored, closing the #55-regression→revert→restore arc live. Phase D complete (D-1 ref input #1003, D-2 eval:trend #1006, D-3 policy §3.1 #1006, D-4 latency in trend rows); remaining documented plans: word-boundary matcher (own PR), irrelevant@10 labeling audit, Phase E (separate approval). Spend ≈$5-10 of ≤$20. | pr-policy self-test + workflow guard PASS; contract test 4/4 + red-proof; npm run test 3025 passed / 1 known container pdf-budget artifact; lint+typecheck+prettier clean; docs:check-links 1030 refs PASS; check:github-actions PASS |
| 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (PR: matcher + artifact follow-ups) | ab145f6 | Remaining documented improvements implemented: word-boundary textContainsClinicalTerm + top-10 canary artifact rows | Matcher: boundaries + internal separators widened to any non-alphanumeric run — PROVEN strict superset by artifact replay on canary #53 (1,126 term×alias×result comparisons, 0 lost matches, 7 gained = exactly the previously-documented punctuation-joined occurrences: treatment,/mood,/(opioid/ptsd.[35]/ciwa-ar ×3). More-tolerant measurement cannot fail a passing case → weekly scheduled canary = free live confirmation. Exported + 3 direct unit-test groups (superset preservation, audit classes incl. line-broken 'ciwa- ar' and 'full-blood-count', substring-inside-word rejections). Artifact: topResultSummary 5→10 rows so rr@10/irrelevant@10 metrics' actual inputs are captured — unblocks the offline irrelevant@10 labeling audit next artifact. docs/rag-behaviour updated to implemented state. Phase E remains gated on separate approval. | Targeted vitest 59/59; npm run test 3028 passed / 1 known container pdf-budget artifact; lint+typecheck+prettier clean; audit script run recorded above; no provider calls |
| 2026-07-20 | claude/clinical-kb-pwa-review-asi3wb (PR: DB easy wins) | see PR head | User-requested database + process easy-wins pass: read-only Supabase advisor sweep (user-authorized) → canary-liveness probe + advisor-disposition docs; live mutations withheld for per-item confirmation | Advisors (live, read-only): security = 1 INFO (document_title_words RLS-no-policy = the deliberate fail-closed pattern — now comment-documented at the schema block so it is never 'fixed'); performance = ~33 unused-index INFOs + auth connection-strategy note → docs/db-maintenance.md TRIAGE list with retrieval-surface trgm indexes flagged RAG-protected (dropping = full canary protocol), owner-scoped indexes retained for multi-tenant design, operational candidates deferred (negligible benefit at corpus size). Implemented: ci.yml static-pr warn-only eval-canary staleness probe (actions:read, github-script pinned, >8 days → warning; never fails) — needed because #923 (2026-07-19) moved the canary cadence from daily to weekly Sunday 18:00 UTC, where a dropped fire would go unnoticed for a week (the failure-issue step only reacts to runs that happen). CORRECTION (CodeRabbit review on this PR): the initially recorded "2026-07-20 dropped Sunday fire" incident did not occur — 2026-07-20 is a Monday; the Sunday 2026-07-19 slot fired as scheduled run #48 (19:03 UTC, success), and under the weekly cron no 2026-07-20 slot existed. Probe stands as proactive hardening, staleness now measured from updated_at with a finite-timestamp guard. Presented for confirmation (NOT implemented): scheduled telemetry retention (purge:query-logs is owner-scoped + unscheduled; needs owner/window/policy decision), auth percentage connection strategy (dashboard config), any index drops. | check:github-actions PASS; check:ci-scope PASS; check:function-grants 28/28 PASS; docs:check-links 1034 PASS; supabase-schema vitest 66/66; lint+typecheck+prettier clean; Supabase access read-only only |
58 changes: 58 additions & 0 deletions docs/db-maintenance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Database maintenance notes (Supabase `Clinical KB Database`)

Live project `sjrfecxgysukkwxsowpy`. This file records advisor snapshots and the standing
disposition for each finding class, so routine advisor output does not get re-triaged from
scratch (or "fixed" against design intent). All live mutations remain confirmation-required
(`AGENTS.md` — API and provider confirmation boundary).

## Advisor snapshot — 2026-07-20 (read-only, user-authorized)

### Security: 1 INFO — by design, do not fix

- `rls_enabled_no_policy` on `public.document_title_words`: **deliberate fail-closed
pattern** — RLS enabled, zero policies, all client roles revoked, service-role-only grants
(`supabase/schema.sql`, the `enable row level security` block for this table; a schema
comment was deliberately skipped to avoid drift-manifest churn — this entry is the record).
Adding policies would be a regression. Expected to reappear in every future advisor run.

Everything else clean: no RLS-disabled tables, no exposed SECURITY DEFINER functions (the
`check:function-grants` guard enforces this offline on every PR), no auth misconfigurations
beyond the note below.

### Performance: ~33 INFO "unused index" + 1 Auth note — TRIAGE LIST, not a delete list

`unused_index` findings need interpretation before any action, and several are protected:

| Class | Examples | Disposition |
| ---------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **Retrieval-surface indexes** | `document_chunks_content_trgm_idx`, `documents_title_trgm_idx`, `document_labels_label_trgm_idx`, `document_summaries_summary_trgm_idx`, `document_index_units_heading_path_idx` | **RAG-protected — do not drop without the full protocol** (`docs/rag-behaviour/safeguards.md`). "Unused" can mean the planner satisfies fast-path queries another way today; dropping changes the retrieval search space and needs a canary pair. |
| **Owner-scoped indexes on the single-tenant corpus** | `*_owner_id_idx`, `*_owner_*_idx` on registry/audit/eval/summary tables | Live corpus is all-public (`owner_id` NULL), so owner-scoped paths never execute — but the multi-tenant design intends them. Keep unless multi-tenancy is formally abandoned. |
| **Operational/rare-path indexes** | rate-limit buckets, cleanup jobs, ingestion stages, `rag_query_misses_aliases_idx`, feedback/approval tables | Genuinely droppable candidates, but the benefit is negligible at this corpus size (write amplification on low-write tables). Revisit only if write latency or storage becomes a measured problem. |

Standing rule: unused-index removal is a considered maintenance task with its own migration +
replay + (for retrieval tables) canary pair — never a bulk advisor-driven sweep.

- `auth_db_connections_absolute`: Auth server pinned at 10 connections instead of a
percentage strategy. Only matters when resizing the instance; auth load here is tiny
(magic-link/OAuth only). Revisit at the next instance-size change (dashboard config —
confirmation-required).

## Telemetry retention — open decision (2026-07-20)

`rag_queries` / `rag_retrieval_logs` telemetry grows with live and eval traffic.
`npm run purge:query-logs` exists (owner-scoped, `--older-than-days`, `--dry-run`,
fails loudly on unknown flags) but is **scheduled nowhere** — retention is currently manual
and unbounded. Wiring a schedule is deliberately NOT done unilaterally because it is a
recurring live-deletion job needing an explicit policy: which owner scope, what retention
window (script default 90 days), and where it runs (a small weekly workflow with the
service-role secret is the natural home). Decision recorded here when made.

## Cross-references

- Weekly eval canary + liveness probe: `docs/observability-slos.md` §3 (the `static-pr` job
warns when the last completed canary is > 8 days old — added when #923 moved the canary
from daily to weekly cadence, where a dropped Sunday fire would otherwise go unnoticed for
a week; a suspected 2026-07-20 drop turned out to be a mis-read — the Sunday 2026-07-19
slot fired and succeeded, and Monday 2026-07-20 has no slot under the weekly cron).
- RAG-protected surfaces and change protocol: `docs/rag-behaviour/safeguards.md`.
- Hybrid RPC health: `search_schema_health()` execution smoke + `npm run check:indexing`.
Loading