diff --git a/README.md b/README.md index 4d7d5c4..a0c2a67 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,11 @@ npm run e2e Primary references: +- `docs/ARCHITECTURE.md` for the current system map and service boundaries. +- `docs/DEVELOPMENT.md` for day-to-day code-change workflow. +- `docs/SCALING.md` for scaling priorities and readiness work. +- `docs/CLINICAL_ASSISTANT.md` for MCP-backed assistant behavior and safety rules. +- `docs/MODULE_CONVENTIONS.md` for CommonJS, ESM, globals, and rendering rules. - `docs/architecture.md` for high-level architecture. - `docs/api-reference.md` for API routes. - `docs/authentication.md` for auth, OIDC, and security configuration. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md new file mode 100644 index 0000000..91588eb --- /dev/null +++ b/docs/ARCHITECTURE.md @@ -0,0 +1,90 @@ +# Architecture + +This document is the current high-level map for Ped-AI. It is intentionally shorter and more operational than the older deep-dive files under `docs/logic/`. + +## System Shape + +Ped-AI is a self-hosted Express application with a browser frontend, PostgreSQL storage, Redis operational state, LiteLLM model routing, and optional MCP-backed clinical retrieval. + +| Area | Owner | Notes | +|---|---|---| +| Web app | Ped-AI | Auth, UI, clinical workflows, admin settings, notes, Learning Hub, bedside tools | +| Database | PostgreSQL | Users, sessions, settings, saved app data, audit/API/access logs | +| Operational cache | Redis | Prompt suggestions, lightweight state, queue groundwork; not clinical answer caching | +| Model gateway | LiteLLM | Text, speech, image, embedding model discovery and routing | +| Clinical retrieval | MCP service | Nextcloud access, indexing, search, rerank, source metadata | +| Reverse proxy | Caddy or equivalent | TLS and public routing | + +## Request Flow + +Normal app request: + +```txt +browser + -> reverse proxy + -> Express middleware + -> auth/session check when protected + -> route handler + -> PostgreSQL/Redis/provider calls as needed + -> JSON or HTML fragment response +``` + +Clinical Assistant request: + +```txt +browser + -> Ped-AI clinical assistant route + -> MCP semantic search for indexed clinical sources + -> Ped-AI builds grounded answer prompt + -> LiteLLM chat model + -> Ped-AI returns answer plus source metadata + -> browser renders markdown, citations, and source cards +``` + +Ped-AI owns the user workflow and rendering. MCP owns retrieval and indexed source metadata. LiteLLM owns model routing. + +## Runtime Boundaries + +| Boundary | Main Risk | Current Direction | +|---|---|---| +| Browser to Ped-AI | XSS, stale shell, session handling | Sanitized rendering, httpOnly cookie for web, cache busting | +| Ped-AI to PostgreSQL | schema drift, slow queries | migrations, maintenance checks, indexes where needed | +| Ped-AI to Redis | unavailable operational state | Redis is useful but should not hold required clinical answers | +| Ped-AI to LiteLLM | provider downtime, wrong model mode | metadata-based model discovery and timeouts | +| Ped-AI to MCP | retrieval latency/failure | explicit MCP client layer and graceful fallback messages | +| MCP to Nextcloud | stale indexed metadata | scanner/indexer updates source metadata over time | + +## Source Of Truth + +| Data | Source Of Truth | +|---|---| +| User accounts and sessions | Ped-AI PostgreSQL | +| Admin app settings | Ped-AI PostgreSQL `app_settings` | +| Clinical source documents | Nextcloud and MCP index | +| Clinical source title/path shown to users | MCP result metadata, especially indexed `file_path` | +| Clinical answer text | Generated per request; intentionally not cached | +| Model availability | LiteLLM metadata and configured fallbacks | + +## Deployment Shape + +Production usually runs: + +```txt +Caddy/TLS + -> pediatric-ai-scribe container + -> pedscribe-db container + -> ped-ai-redis container + -> LiteLLM endpoint + -> MCP endpoint +``` + +The app should stay private behind the reverse proxy. Do not expose PostgreSQL, Redis, MCP internals, or provider keys publicly. + +## Design Principles + +- Keep Ped-AI stateless enough to run more than one app container. +- Keep clinical answer generation live and source-grounded; do not cache final clinical answers. +- Prefer model capability metadata over model-name regexes. +- Prefer indexed file names and paths over embedded PDF metadata for source titles. +- Keep renderer fixes narrow and tested because LLM markdown is messy. +- Keep old frontend globals working until the affected feature is intentionally converted to ESM. diff --git a/docs/CLINICAL_ASSISTANT.md b/docs/CLINICAL_ASSISTANT.md new file mode 100644 index 0000000..497e888 --- /dev/null +++ b/docs/CLINICAL_ASSISTANT.md @@ -0,0 +1,97 @@ +# Clinical Assistant + +The Clinical Assistant is a retrieval-grounded assistant for pediatric clinical reference questions. It is not the same as the app's note-generation/HPI workflow. + +## Responsibilities + +| Component | Responsibility | +|---|---| +| Browser UI | question input, source display, markdown/citation rendering, export | +| Ped-AI backend | settings, MCP search call, answer prompt construction, model call | +| MCP server | Nextcloud access, indexing, vector search, rerank, source metadata | +| LiteLLM | model routing and provider abstraction | + +## Request Flow + +```txt +User asks a question + -> browser posts to Ped-AI + -> Ped-AI calls MCP `nc_semantic_search` + -> MCP returns source excerpts and metadata + -> Ped-AI builds an answer prompt with source constraints + -> LiteLLM model returns answer text + -> browser renders answer and source cards +``` + +## Source Rules + +- Prefer MCP `file_path` basename for displayed source titles when present. +- Do not relabel one source as another requested source. +- If the user names a source and retrieval does not return it, say that before using other sources. +- Use citations only for returned source numbers. +- Unknown citation numbers should remain plain text instead of being guessed. + +## Table And Markdown Rendering + +LLM output is not guaranteed to be valid markdown. The browser renderer defensively handles common problems: + +- adjacent citation clusters, +- missing closing bracket in narrow citation cases, +- smashed bullet lists, +- inline headings, +- malformed pipe tables, +- bare source numbers in source/citation table columns, +- orphan markdown emphasis markers, +- code blocks that must not be modified. + +Renderer fixes must be narrow. Do not add broad repairs that turn arbitrary clinical numbers into citations. + +## Image Routing + +Table lookup requests should stay in retrieval flow. + +Examples that should use retrieval: + +```txt +show me the table +show me Table 13.1 +summarize the developmental table +``` + +Explicit visual creation/display requests can use image flow. + +Examples: + +```txt +create an infographic +generate a diagram +show me the image/figure +``` + +## Caching Policy + +Clinical answer response caching is intentionally disabled. Redis can support prompt suggestions and operational metadata, but final answers should be generated from current retrieval context. + +## Settings + +Important settings include: + +| Setting | Purpose | +|---|---| +| `clinical_assistant.chat_model` | Chat model used for answers | +| `clinical_assistant.image_model` | Image model used for explicit image generation | +| `clinical_assistant.search_limit` | Number of MCP results requested | +| `clinical_assistant.context_chars` | Context characters requested from MCP | +| `clinical_assistant.system_behavior` | Admin-editable assistant behavior guidance | + +## Testing Priorities + +Add or update tests when changing: + +- citation rendering, +- source title cleanup, +- named-source provenance behavior, +- table rendering, +- image intent routing, +- MCP result normalization, +- model discovery or settings behavior. diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md new file mode 100644 index 0000000..043601d --- /dev/null +++ b/docs/DEVELOPMENT.md @@ -0,0 +1,103 @@ +# Development + +This is the practical guide for changing Ped-AI safely. + +## Local Start + +```bash +cp .env.example .env +docker compose up -d --build +curl -fsS http://127.0.0.1:3552/api/health +``` + +Run tests from the repository root: + +```bash +npm test +``` + +Run a focused syntax check when touching backend entrypoints: + +```bash +node --check server.js +node --check src/routes/clinicalAssistant.js +``` + +## Code Map + +| Path | Purpose | +|---|---| +| `server.js` | Express entrypoint, middleware, static serving, route mounting | +| `src/routes/` | API route handlers | +| `src/utils/ai.js` | Text model routing through configured providers | +| `src/utils/clinicalAnswer.js` | Clinical Assistant answer prompt and source-grounding rules | +| `src/utils/clinicalRetrieval.js` | MCP result normalization and source title cleanup | +| `src/utils/clinicalMcpClient.js` | MCP streamable HTTP client/session handling | +| `src/utils/litellm.js` | LiteLLM API/admin header helpers | +| `src/db/database.js` | PostgreSQL pool and compatibility helpers | +| `public/js/app.js` | SPA shell, tab loading, shared browser actions | +| `public/js/admin.js` | Admin panel logic | +| `public/js/assistant/` | Clinical Assistant rendering, sources, images, export, API helpers | +| `public/js/learningHub/` | Newer modular Learning Hub frontend code | +| `test/` | Node test suite and frontend module regression tests | + +## Change Workflow + +1. Read the relevant route, utility, frontend module, and tests before editing. +2. Make the smallest correct change. +3. Add or update a regression test when changing clinical rendering, model routing, auth, settings, or source handling. +4. Run focused tests first if available. +5. Run `npm test` before deploy or commit. +6. Deploy with Docker only after tests pass. +7. Verify `/api/health` after deploy. + +## Clinical Assistant Changes + +Clinical Assistant changes should usually include tests because small rendering or prompt changes can affect clinical trust. + +High-risk areas: + +- citation linking, +- table rendering, +- source title cleanup, +- named-source provenance rules, +- image intent detection, +- MCP result normalization, +- provider/model selection. + +When a real answer renders badly, save a de-identified example as a fixture or direct test input. Do not make broad global repairs that convert arbitrary numbers into citation links. + +## Frontend Rendering Rules + +Use `textContent` for plain text. Use `innerHTML` only for static templates, sanitized markdown, or HTML built entirely from escaped values. + +Safe patterns: + +```js +el.textContent = userText; +el.innerHTML = escapeHtml(userText).replace(/\n/g, '
'); +el.innerHTML = sanitizeHtml(renderMarkdown(modelOutput)); +``` + +Unsafe pattern: + +```js +el.innerHTML = modelOutput; +``` + +If a dynamic value enters an HTML string, escape it at the point of insertion. If it is an attribute value, escape quotes too. + +## Deployment Checks + +After deployment: + +```bash +curl -fsS http://127.0.0.1:3552/api/health +docker compose ps pediatric-scribe +``` + +If the browser still shows old frontend behavior, force-refresh or check the injected `BUILD_ID` asset query string. + +## Documentation Expectations + +Keep docs close to operational truth. If a behavior changes, update the most specific doc in the same change. Prefer short, current docs over long historical explanations. diff --git a/docs/MODULE_CONVENTIONS.md b/docs/MODULE_CONVENTIONS.md new file mode 100644 index 0000000..caa1977 --- /dev/null +++ b/docs/MODULE_CONVENTIONS.md @@ -0,0 +1,88 @@ +# Module Conventions + +Ped-AI currently uses mixed JavaScript module styles. This is intentional during incremental modernization. + +## Current Convention + +| Area | Module Style | Notes | +|---|---|---| +| Backend `server.js`, `src/**` | CommonJS | Use `require` and `module.exports` for now | +| New frontend modules | ESM | Use `import` and `export` | +| Older frontend files | Classic browser globals | Convert only when touching the feature intentionally | +| Dual browser/test files | Case-by-case | Keep classic style only when tests or browser globals require it | + +Do not add root-level `"type": "module"` without a full backend migration plan. It would change how every `.js` file is interpreted by Node. + +## CommonJS Example + +```js +var express = require('express'); +var router = express.Router(); + +module.exports = router; +``` + +## ESM Example + +```js +import { escapeHtml } from './assistant/citations.js'; + +export function renderSourcesList(sources) { + return ''; +} +``` + +## Frontend Modernization Path + +1. New frontend code should be ESM where possible. +2. Existing globals can remain until that feature is refactored. +3. Keep browser script load order stable while refactoring. +4. Export pure helper functions so Node tests can import them. +5. Use `CustomEvent` or explicit imports instead of adding new global APIs when practical. + +## Acceptable Globals + +Globals are acceptable when they are part of the current shell contract. + +Examples: + +- `window.activateTab`, +- `window.getAuthHeaders`, +- shared UI helpers still consumed by legacy feature files. + +Do not add new globals when an import or event would be clearer. + +## Rendering And `innerHTML` + +`innerHTML` is allowed only when one of these is true: + +- the HTML is a static template controlled by the app, +- all dynamic values are escaped before insertion, +- the HTML has passed through the approved sanitizer, +- the content is a trusted app component fetched from `public/components/`. + +Prefer `textContent` for plain text. + +Unsafe: + +```js +el.innerHTML = userText; +el.innerHTML = modelOutput; +``` + +Safer: + +```js +el.textContent = userText; +el.innerHTML = escapeHtml(userText).replace(/\n/g, '
'); +el.innerHTML = sanitizeHtml(renderMarkdown(modelOutput)); +``` + +## Test Expectations + +When converting a frontend file to ESM, add or update tests for: + +- exported helper functions, +- expected globals still present if legacy code needs them, +- no browser-native `prompt`, `alert`, or `confirm`, +- no unescaped dynamic text inserted through `innerHTML`. diff --git a/docs/SCALING.md b/docs/SCALING.md new file mode 100644 index 0000000..28849d5 --- /dev/null +++ b/docs/SCALING.md @@ -0,0 +1,119 @@ +# Scaling + +This document describes how Ped-AI should scale without becoming harder to debug or maintain. + +## Current Scaling Model + +Ped-AI is currently a single app container backed by PostgreSQL and Redis. That is acceptable for self-hosted use, but the code should keep moving toward a shape where multiple app containers can run safely. + +```txt +reverse proxy + -> pediatric-ai-scribe replica 1 + -> pediatric-ai-scribe replica 2 + -> shared PostgreSQL + -> shared Redis + -> LiteLLM + -> MCP +``` + +## Horizontal Scaling Requirements + +| Requirement | Why It Matters | +|---|---| +| Session state in PostgreSQL/Redis | Any app replica can handle the next request | +| No clinical state only in memory | Restarting or scaling containers should not lose required state | +| Shared uploads/storage if files grow | Local container disk does not scale across replicas | +| Idempotent migrations | Deploying more than one app container should not corrupt schema state | +| Request timeouts | Slow providers should not exhaust Node workers | +| Queue for slow jobs | Long work should not block interactive requests | +| Readiness endpoint | Load balancer should only send traffic to ready replicas | + +## What Can Stay In Memory + +Small process-local caches are acceptable when they are optional and short-lived. + +Examples: + +- settings cache with short TTL, +- provider model metadata cache, +- static configuration derived at boot. + +Do not store required user workflow state only in memory if the action must survive restart or run across replicas. + +## Redis Use + +Redis is appropriate for: + +- prompt suggestion pools, +- rate-limit coordination if needed, +- queues and job status, +- short-lived provider metadata, +- operational locks. + +Redis should not be used for final clinical answer response caching. Clinical answers should be generated live from current retrieval context. + +## Queue Candidates + +Consider moving these to a queue when latency or concurrency becomes a problem: + +- long transcription jobs, +- file import/export, +- Learning Hub AI generation from large files, +- image generation, +- bulk document operations, +- provider metadata refresh, +- long-running admin maintenance actions. + +BullMQ with Redis is a natural fit if a queue is added. + +## Readiness And Health + +Keep `/api/health` fast and simple for liveness. + +Add a separate readiness endpoint when scaling: + +```txt +GET /api/ready +``` + +It should check: + +- PostgreSQL query works, +- Redis ping works if Redis is required for this deployment, +- core settings can be read, +- MCP health is reachable if Clinical Assistant is enabled, +- LiteLLM metadata or configured model endpoint is reachable if AI features are enabled. + +## Database Scaling + +Priorities: + +- confirm indexes on hot user/session/settings/log tables, +- keep migrations explicit and reversible where practical, +- monitor slow queries, +- cap admin log queries with safe limits, +- keep audit/log writes batched where possible, +- avoid long transactions around provider calls. + +## Provider Scaling + +LiteLLM and MCP can become the bottlenecks before Ped-AI does. + +Track: + +- LiteLLM request latency, +- LiteLLM error rate by model, +- MCP search latency, +- MCP timeout/error rate, +- queue depth if async jobs are added, +- Postgres connections, +- app container memory and event-loop delay. + +## Scaling Order + +1. Add request IDs across browser, Ped-AI, MCP, and LiteLLM calls. +2. Add `/api/ready` for dependency readiness. +3. Ensure sessions and settings are not process-local. +4. Add a queue for slow jobs if interactive requests block. +5. Run a second app replica behind the reverse proxy in a staging/test environment. +6. Add metrics and alerts around latency, errors, and resource saturation. diff --git a/public/js/admin.js b/public/js/admin.js index 8082128..a8ad23f 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -4,7 +4,7 @@ function adminEscapeHtml(str) { if (!str) return ''; - return String(str).replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"'); + return String(str).replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"').replace(/'/g, '''); } function adminTableMessage(colspan, color, text) { diff --git a/public/js/app.js b/public/js/app.js index ed92a67..f16f2e4 100644 --- a/public/js/app.js +++ b/public/js/app.js @@ -743,8 +743,8 @@ function suggestBillingCodes(outputElementId, noteText, noteType, patientAge, vi if (data.emLevel) { html += '
E/M Assessment
'; - html += 'Level ' + data.emLevel.level + ''; - html += 'MDM: ' + data.emLevel.complexity + ' | ' + data.emLevel.diagnosisCount + ' dx | ' + data.emLevel.rosCount + ' ROS | ' + data.emLevel.peCount + ' PE'; + html += 'Level ' + escHtml(data.emLevel.level) + ''; + html += 'MDM: ' + escHtml(data.emLevel.complexity) + ' | ' + escHtml(data.emLevel.diagnosisCount) + ' dx | ' + escHtml(data.emLevel.rosCount) + ' ROS | ' + escHtml(data.emLevel.peCount) + ' PE'; html += '
'; } diff --git a/test/frontend-rendering-safety.test.js b/test/frontend-rendering-safety.test.js new file mode 100644 index 0000000..53cd6da --- /dev/null +++ b/test/frontend-rendering-safety.test.js @@ -0,0 +1,20 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); + +const root = path.join(__dirname, '..'); + +test('admin HTML escape helper also escapes single quotes for attribute contexts', () => { + const source = fs.readFileSync(path.join(root, 'public', 'js', 'admin.js'), 'utf8'); + assert.match(source, /function adminEscapeHtml\(str\)[\s\S]+replace\(\/\'\/g, '''\)/); +}); + +test('billing E/M suggestion fields are escaped before innerHTML insertion', () => { + const source = fs.readFileSync(path.join(root, 'public', 'js', 'app.js'), 'utf8'); + assert.match(source, /Level ' \+ escHtml\(data\.emLevel\.level\)/); + assert.match(source, /MDM: ' \+ escHtml\(data\.emLevel\.complexity\)/); + assert.match(source, /escHtml\(data\.emLevel\.diagnosisCount\)/); + assert.match(source, /escHtml\(data\.emLevel\.rosCount\)/); + assert.match(source, /escHtml\(data\.emLevel\.peCount\)/); +});