From 050a7d52414a4ac7d7e964f8239ed43a5d8e95af Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 23:44:43 +0200 Subject: [PATCH] feat: citation quality tracking, and the SSO settings fit a phone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Citation quality - A citation naming a source that never came back is never rendered as a link, so it appears as plain text and nobody learns it happened. It is now measured on the server, where the answer and the sources both exist, so it is seen whether or not a browser rendered it. - Four Prometheus counters feed a Grafana dashboard (Ped-AI Citation Quality): answers, citations written, answers affected, and individual unresolved markers. Only answers with at least one unresolved citation are stored, with the question and the titles retrieval returned, so an operator can judge whether retrieval came back thin or the model over-cited. Rows expire after 30 days: this is a quality signal, not a transcript log. - Both answer paths are covered. /chat/stream is normal; /chat is the fallback the client uses when streaming fails, so auditing only the first would have hidden exactly the answers produced under failure. - The tracker is resolved on demand and allowed to be absent. Seven test files load this route with a hand-built list of permitted imports, and adding a hard dependency would mean editing all seven — and the eighth written later would break. Observation must never be able to fail an answer, so a missing module simply means no tracking. - Metric registration reuses an already-registered counter, because this module can legitimately load twice in one process. SSO settings on mobile - Six rows were laid out inline: flex with a 160px label and an input that would not shrink, so on a phone the row was wider than the screen with nothing to scroll and no way to reach the rest. They use .admin-row now, which already stacks below 640px. Verified at 390px and 360px: nothing off-screen, no sideways overflow. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- migrations/1780200000000_citation-audit.js | 34 ++++++ public/components/admin.html | 48 +++++--- public/js/admin.js | 79 +++++++++++++ src/routes/adminConfig.js | 19 ++++ src/routes/clinicalAssistant.js | 33 +++++- src/utils/citationAudit.js | 125 +++++++++++++++++++++ test/assistant-citations.test.js | 6 +- test/assistant-mobile.test.js | 17 +++ test/backend-hardening.test.js | 46 ++++++++ test/generated-image-tools.test.js | 2 + 10 files changed, 389 insertions(+), 20 deletions(-) create mode 100644 migrations/1780200000000_citation-audit.js create mode 100644 src/utils/citationAudit.js diff --git a/migrations/1780200000000_citation-audit.js b/migrations/1780200000000_citation-audit.js new file mode 100644 index 00000000..e8723c15 --- /dev/null +++ b/migrations/1780200000000_citation-audit.js @@ -0,0 +1,34 @@ +// Answers whose citations pointed at nothing. +// +// The Prometheus counters say how often it happens; this says what happened, +// so an admin can read the question and the sources and judge whether the +// retrieval came back thin or the model over-cited. +// +// Only answers with at least one unverifiable citation are stored — this is a +// quality signal, not a transcript log — and rows expire, because the question +// text is clinical material and should not accumulate indefinitely. + +exports.up = pgm => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS citation_audit ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + question TEXT NOT NULL DEFAULT '', + cited_count INTEGER NOT NULL DEFAULT 0, + source_count INTEGER NOT NULL DEFAULT 0, + -- The numbers the model wrote that no source matched. + unverifiable INTEGER[] NOT NULL DEFAULT '{}', + -- Titles of what retrieval actually returned, so the two can be compared + -- without keeping the passages themselves. + source_titles TEXT[] NOT NULL DEFAULT '{}', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ NOT NULL DEFAULT NOW() + INTERVAL '30 days' + ); + CREATE INDEX IF NOT EXISTS idx_citation_audit_created ON citation_audit(created_at DESC); + CREATE INDEX IF NOT EXISTS idx_citation_audit_expires ON citation_audit(expires_at); + `); +}; + +exports.down = pgm => { + pgm.sql('DROP TABLE IF EXISTS citation_audit;'); +}; diff --git a/public/components/admin.html b/public/components/admin.html index 3d0f3ccb..564a0b7e 100644 --- a/public/components/admin.html +++ b/public/components/admin.html @@ -97,32 +97,32 @@

Single Sign-On (OIDC)

Configure OpenID Connect for SSO with Azure AD, Okta, Keycloak, PocketID, Google, etc.
Callback URL:

-
- -
-
- - +
+ +
-
- - +
+ +
-
- - +
+ +
-
- - +
+ +
-
- - @@ -373,6 +373,18 @@
+ +
+
+

Citation Quality

+ Loading... +
+
+

Answers where the assistant wrote a citation number that no returned source matched. Those markers are never turned into links, so a reader sees plain text — this is where they are recorded. Consistent entries usually mean retrieval is returning fewer sources than the answer assumes.

+ +
+
+
diff --git a/public/js/admin.js b/public/js/admin.js index 7ed48c36..05d9d220 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -1462,6 +1462,85 @@ initImageSettings(); } +// ============================================================ +// ADMIN CITATION QUALITY +// A citation that resolves to nothing is never linked, so it is invisible +// unless someone counts it. This is the reading end of that. +// ============================================================ +{ + document.addEventListener('tabChanged', function(e) { + if (e.detail && e.detail.tab === 'admin') loadCitationSummary(); + }); + if (adminTabActive()) loadCitationSummary(); + + document.addEventListener('click', function(e) { + if (e.target.closest('#btn-view-citation-audit')) openCitationAudit(); + if (e.target.closest('#citation-audit-close') || e.target.id === 'citation-audit-modal') closeCitationAudit(); + }); + document.addEventListener('keydown', function(e) { + if (e.key === 'Escape') closeCitationAudit(); + }); + + const esc = adminEscapeHtml; + + function fetchAudit() { + return fetch('/api/admin/citation-audit', { headers: getAuthHeaders() }).then(function(r) { return r.json(); }); + } + + function loadCitationSummary() { + fetchAudit().then(function(data) { + var badge = document.getElementById('admin-citation-summary'); + if (!badge || !data.success) return; + var answers = (data.totals && data.totals.answers) || 0; + badge.textContent = answers ? answers + ' flagged (30 days)' : 'none flagged'; + badge.style.background = answers ? 'var(--amber)' : 'var(--green)'; + badge.style.color = 'white'; + }).catch(function() {}); + } + + function closeCitationAudit() { + var modal = document.getElementById('citation-audit-modal'); + if (modal) modal.remove(); + } + + function openCitationAudit() { + fetchAudit().then(function(data) { + if (!data.success) throw new Error(data.error || 'Could not load'); + closeCitationAudit(); + var modal = document.createElement('div'); + modal.id = 'citation-audit-modal'; + modal.className = 'modal'; + modal.innerHTML = '
' + + '

Citation quality

' + + '
' + + '
' + rows(data.rows || []) + '
'; + document.body.appendChild(modal); + }).catch(function(err) { showToast(err.message, 'error'); }); + } + + function rows(list) { + if (!list.length) { + return '

Nothing flagged in the last 30 days — every citation pointed at a source that was returned.

'; + } + return list.map(function(row) { + var missing = (row.unverifiable || []).map(function(n) { return '[' + n + ']'; }).join(' '); + var titles = (row.source_titles || []).map(function(t, i) { + return '
  • [' + (i + 1) + '] ' + esc(t) + '
    • '; + }).join(''); + return '
    ' + + '
    ' + + '' + esc(missing) + ' unmatched' + + '' + esc(new Date(row.created_at).toLocaleString()) + + ' · ' + esc(String(row.cited_count)) + ' citations written, ' + esc(String(row.source_count)) + ' sources returned' + + (row.user_email ? ' · ' + esc(row.user_email) : '') + '' + + '
    ' + + '
    Question: ' + esc(row.question || '(none)') + '
    ' + + (titles ? '
    Sources returned
      ' + titles + '
    ' : '') + + '
    '; + }).join(''); + } +} + // ============================================================ // ADMIN LOCKDOWN (display) // The server refuses locked writes regardless; this only stops an admin diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 0cfea4bd..b4c04883 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -693,6 +693,25 @@ router.get('/config/stt', async function(req, res) { } catch (e) { res.status(500).json({ error: 'Request failed' }); } }); +// ── Citation quality ───────────────────────────────────────────────────── +// Answers whose citations pointed at nothing. Read-only, and it survives +// lockdown because it is quality tracking, not configuration. +router.get('/citation-audit', async function(req, res) { + try { + var rows = await db.all( + "SELECT a.id, a.question, a.cited_count, a.source_count, a.unverifiable, " + + " a.source_titles, a.created_at, u.email AS user_email " + + "FROM citation_audit a LEFT JOIN users u ON u.id = a.user_id " + + "WHERE a.expires_at > NOW() ORDER BY a.created_at DESC LIMIT 100", [] + ); + var totals = await db.get( + "SELECT COUNT(*)::int AS answers, COALESCE(SUM(array_length(unverifiable, 1)), 0)::int AS markers " + + "FROM citation_audit WHERE expires_at > NOW()", [] + ); + res.json({ success: true, rows: rows, totals: totals || { answers: 0, markers: 0 } }); + } catch (e) { return serverError(res, 'Citation audit', e, 'Could not read citation quality'); } +}); + // ── Registration invites ───────────────────────────────────────────────── // A code is shown once, at creation. Only its hash and last four characters // are stored, so this endpoint is the only time it can be read. diff --git a/src/routes/clinicalAssistant.js b/src/routes/clinicalAssistant.js index d2d86d16..0b856245 100644 --- a/src/routes/clinicalAssistant.js +++ b/src/routes/clinicalAssistant.js @@ -344,6 +344,25 @@ function refusePreviewWrite(req, res) { return true; } +// Citation quality tracking. Loaded on demand and allowed to be absent: it is +// observation, not part of producing an answer, so it must never be able to +// fail one — including in a harness that stubs this route's module graph. +function citationTracker() { + try { + return require('../utils/citationAudit'); + } catch (e) { + return null; + } +} + +function trackCitations(req, question, answer, sources) { + var tracker = citationTracker(); + if (!tracker) return; + var result = tracker.record(answer, sources); + // Not awaited: the clinician is waiting for this answer. + if (!req.user.preview) tracker.store(req.user.id, question, result, sources); +} + router.post('/clinical-assistant/chat', async function(req, res) { var started = Date.now(); try { @@ -373,12 +392,18 @@ router.post('/clinical-assistant/chat', async function(req, res) { category: 'clinical', model: ai.model || prepared.chatModel, duration: Date.now() - started }); + // This route is the fallback the client uses when streaming fails, so it + // produces answers a clinician reads. Auditing only the streaming route + // would make exactly the answers produced under failure invisible. + var fallbackSources = sanitizeSourcesForClient(prepared.sources); + trackCitations(req, prepared.message, answer, fallbackSources); + res.json({ success: true, answer: prepared.showSources ? answer : stripCitationMarkers(answer), imageJobs: ai.imageJobs || [], showSources: prepared.showSources, - sources: prepared.showSources ? sanitizeSourcesForClient(prepared.sources) : [], + sources: prepared.showSources ? fallbackSources : [], model: ai.model || prepared.chatModel || null, provider: ai.provider || null, duration: Date.now() - started, @@ -448,6 +473,12 @@ router.post('/clinical-assistant/chat/stream', async function(req, res) { if (!req.user.preview) logger.audit(req.user.id, 'clinical_assistant_streaming_query', 'Clinical assistant streaming query', req, { category: 'clinical', model: ai.model || prepared.chatModel, duration: Date.now() - started }); + // Quality tracking: does every citation the model wrote point at a source + // that actually came back? Counted always; the answer is recorded only when + // something did not resolve. Not awaited — the clinician is waiting for + // this answer and a slow write must not hold it up. + trackCitations(req, prepared.message, answer, safeSources); + sendEvent('done', { success: true, answer: prepared.showSources ? answer : stripCitationMarkers(answer), diff --git a/src/utils/citationAudit.js b/src/utils/citationAudit.js new file mode 100644 index 00000000..2332ac2a --- /dev/null +++ b/src/utils/citationAudit.js @@ -0,0 +1,125 @@ +// ============================================================ +// CITATION AUDIT +// The assistant is only worth trusting if its citations point at something. +// A model sometimes writes a number no source has — [7] when five came back. +// Rendering already refuses to link those, so they appear as bare text and +// nobody hears about it. This is what records them. +// +// It runs on the server, where the answer and the sources both exist, so it +// sees every answer whether or not a browser rendered it. +// ============================================================ + +var client = require('prom-client'); + +// Grafana reads these. Counters rather than a gauge: the question is "how +// often", and a gauge would lose everything between scrapes. +// +// Registering a name twice throws, and this module can legitimately be loaded +// more than once in one process — a test harness that clears the require cache +// to inject stubs does exactly that. Reuse whatever is already registered. +function counter(name, help) { + return client.register.getSingleMetric(name) || new client.Counter({ name: name, help: help }); +} + +var answersTotal = counter( + 'clinical_assistant_answers_total', + 'Answers produced by the clinical assistant'); +var answersWithUnverifiableTotal = counter( + 'clinical_assistant_answers_with_unverifiable_citations_total', + 'Answers containing at least one citation with no matching source'); +var unverifiableCitationsTotal = counter( + 'clinical_assistant_unverifiable_citations_total', + 'Individual citation markers with no matching source'); +var citationsTotal = counter( + 'clinical_assistant_citations_total', + 'Citation markers written by the model, resolvable or not'); + +// Same shape the renderer matches, so the audit and the display agree about +// what counts as a citation: [3] or [3, 4, 5]. +var CLUSTER = /\[((?:\d+\s*,\s*)*\d+)\]/g; + +// Code blocks are not prose: a [1] inside one is content, not a citation. +function withoutCode(text) { + return String(text || '') + .replace(/```[\s\S]*?```/g, ' ') + .replace(/`[^`\n]*`/g, ' '); +} + +/** + * Compare the citations an answer makes against the sources it was given. + * + * Returns counts plus the offending numbers, so an operator can see at a + * glance whether one answer went wrong or the retrieval is systematically + * returning less than the model expects. + */ +function audit(answer, sources) { + var available = new Set( + (Array.isArray(sources) ? sources : []) + .map(function (s, i) { return Number(s && s.number != null ? s.number : i + 1); }) + .filter(function (n) { return Number.isInteger(n); }) + ); + var cited = 0; + var unverifiable = []; + var prose = withoutCode(answer); + var match; + CLUSTER.lastIndex = 0; + while ((match = CLUSTER.exec(prose)) !== null) { + match[1].split(',').forEach(function (part) { + var n = Number(String(part).trim()); + if (!Number.isInteger(n) || n <= 0) return; + cited++; + if (!available.has(n)) unverifiable.push(n); + }); + } + return { + cited: cited, + sources: available.size, + unverifiable: unverifiable, + // Distinct, sorted: [7,7,7] is one problem repeated, not three. + unverifiableNumbers: Array.from(new Set(unverifiable)).sort(function (a, b) { return a - b; }) + }; +} + +// Records the result and returns it. Never throws: quality tracking must not +// be able to fail an answer the clinician is waiting for. +function record(answer, sources) { + try { + var result = audit(answer, sources); + answersTotal.inc(); + citationsTotal.inc(result.cited); + if (result.unverifiable.length) { + answersWithUnverifiableTotal.inc(); + unverifiableCitationsTotal.inc(result.unverifiable.length); + } + return result; + } catch (e) { + return { cited: 0, sources: 0, unverifiable: [], unverifiableNumbers: [] }; + } +} + +// Kept only when something was wrong, so this is a quality signal rather than +// a transcript log. Failure to store must not disturb the answer. +async function store(userId, question, result, sources) { + if (!result || !result.unverifiable.length) return; + try { + var db = require('../db/database'); + await db.run( + 'INSERT INTO citation_audit (user_id, question, cited_count, source_count, unverifiable, source_titles) ' + + 'VALUES ($1, $2, $3, $4, $5, $6)', + [ + userId || null, + String(question || '').slice(0, 2000), + result.cited, + result.sources, + result.unverifiableNumbers, + (Array.isArray(sources) ? sources : []).slice(0, 20).map(function (s) { + return String((s && (s.title || s.resource)) || 'Untitled').slice(0, 300); + }) + ] + ); + } catch (e) { + console.warn('[citation-audit] could not store:', e.message); + } +} + +module.exports = { audit, record, store }; diff --git a/test/assistant-citations.test.js b/test/assistant-citations.test.js index 5a66cd6c..25837b77 100644 --- a/test/assistant-citations.test.js +++ b/test/assistant-citations.test.js @@ -320,7 +320,11 @@ test('hiding sources is display-only and reversible', () => { // Stripping happens on the way OUT, so the answer is generated and stored with // citations intact and turning the setting back on restores them. assert.match(route, /answer: prepared\.showSources \? answer : stripCitationMarkers\(answer\)/); - assert.match(route, /sources: prepared\.showSources \? sanitizeSourcesForClient\(prepared\.sources\) : \[\]/); + // The same sanitised list either way: showSources gates only what leaves the + // route, never what was retrieved. Both answer paths compute it, so match the + // guarantee rather than one spelling of it. + assert.match(route, /sources: prepared\.showSources \? (sanitizeSourcesForClient\(prepared\.sources\)|fallbackSources|safeSources) : \[\]/); + assert.match(route, /var fallbackSources = sanitizeSourcesForClient\(prepared\.sources\);/); assert.match(route, /content: buildSystemPrompt\(behavior\)/, 'the prompt takes no display argument'); }); diff --git a/test/assistant-mobile.test.js b/test/assistant-mobile.test.js index 0342715a..0f801d5c 100644 --- a/test/assistant-mobile.test.js +++ b/test/assistant-mobile.test.js @@ -186,3 +186,20 @@ test('no text control is small enough to make iOS zoom the page', () => { assert.doesNotMatch(assistant, /#assistant-input \{ flex:1 1 auto;[^}]*font-size:15px/); assert.match(assistant, /@media \(min-width:641px\) \{ #assistant-input \{ font-size:15px; \} \}/); }); + +test('the SSO settings fit a phone instead of running off the side', () => { + const html = read('public/components/admin.html'); + const sso = html.slice(html.indexOf('Single Sign-On'), html.indexOf('Email Templates')); + // These rows were laid out inline: flex with a 160px label and an input that + // would not shrink, so on a phone the row was wider than the screen with + // nothing to scroll and no way to reach the rest. + assert.doesNotMatch(sso, /min-width:160px/); + assert.doesNotMatch(sso, /display:flex;align-items:center;gap:12px;/); + assert.equal((sso.match(/class="admin-row"/g) || []).length, 6, 'every row uses the pattern that stacks'); + assert.match(sso, /class="admin-row-label"/); + assert.match(sso, /class="admin-control"/); + // .admin-row stacks below 640px and its controls stop constraining width. + const css = read('public/css/styles.css'); + assert.match(css, /\.admin-row \{ grid-template-columns:1fr; \}/); + assert.match(css, /\.admin-control \{ max-width:none; \}/); +}); diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index d272ec8b..8e13e4cd 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -239,3 +239,49 @@ test('admin lockdown refuses configuration writes at the server', () => { const panel = fs.readFileSync(path.join(__dirname, '..', 'public/js/admin.js'), 'utf8'); assert.match(panel, /window\.applyAdminLockdown\(data\.lockdown\)/); }); + +// A citation that resolves to nothing is never rendered as a link, so without +// counting it, nobody ever learns it happened. +test('citation quality is measured on the server, where answer and sources both exist', () => { + const audit = require('../src/utils/citationAudit'); + + const sources = [{ number: 1 }, { number: 2 }, { number: 3 }]; + const good = audit.audit('Bronchiolitis is managed supportively [1]. Steroids do not help [2, 3].', sources); + assert.equal(good.cited, 3); + assert.deepEqual(good.unverifiableNumbers, []); + + // The case that motivated this: a number no source matched. + const over = audit.audit('Supportive care [1] and also this [7].', sources); + assert.equal(over.cited, 2); + assert.deepEqual(over.unverifiableNumbers, [7]); + + // Repeats are one problem, not three. + assert.deepEqual(audit.audit('[7] and [7] and [7]', sources).unverifiableNumbers, [7]); + + // A bracketed number inside code is content, not a citation. + assert.deepEqual(audit.audit('```\nconst a = arr[7];\n```', sources).unverifiableNumbers, []); + assert.deepEqual(audit.audit('Use `arr[7]` here.', sources).unverifiableNumbers, []); + + // Sources are matched on their assigned number, the same way rendering does. + assert.deepEqual(audit.audit('[5]', [{ number: 5 }]).unverifiableNumbers, []); + + // Recording must never be able to fail an answer. + assert.doesNotThrow(() => audit.record(null, null)); + assert.doesNotThrow(() => audit.record('[1]', undefined)); + + const fs = require('node:fs'); + const path = require('node:path'); + const route = fs.readFileSync(path.join(__dirname, '..', 'src/routes/clinicalAssistant.js'), 'utf8'); + // Not awaited: the clinician is waiting for the answer. + // One tracker for both answer paths — the streaming one and the fallback the + // client uses when streaming fails. Auditing only the first would make + // exactly the answers produced under failure invisible. + assert.match(route, /function trackCitations\(req, question, answer, sources\)/); + assert.match(route, /trackCitations\(req, prepared\.message, answer, safeSources\);/, 'streaming'); + assert.match(route, /trackCitations\(req, prepared\.message, answer, fallbackSources\);/, 'fallback'); + // Loaded on demand and allowed to be absent: observation must never be able + // to fail an answer. + assert.match(route, /function citationTracker\(\)/); + assert.match(route, /if \(!tracker\) return;/); + assert.match(route, /if \(!req\.user\.preview\) tracker\.store\(req\.user\.id, question, result, sources\);/); +}); diff --git a/test/generated-image-tools.test.js b/test/generated-image-tools.test.js index b142cf53..c34d42aa 100644 --- a/test/generated-image-tools.test.js +++ b/test/generated-image-tools.test.js @@ -81,6 +81,8 @@ function route(file, ai, jobs) { '../utils/clinicalPrompts':require('../src/utils/clinicalPrompts'), '../utils/clinicalConversation':require('../src/utils/clinicalConversation'), '../utils/clinicalAnswer':require('../src/utils/clinicalAnswer'), + // Citation quality tracking, required lazily by the streaming route. + '../utils/citationAudit':require('../src/utils/citationAudit'), '../utils/clinicalTranslation':require('../src/utils/clinicalTranslation'), '../utils/patientTakehome':require('../src/utils/patientTakehome'), './auth':{__sendEmail:async()=>false},