feat(report-viewer): add embedded viewer service to chat#501
Open
andylassiter wants to merge 118 commits into
Open
feat(report-viewer): add embedded viewer service to chat#501andylassiter wants to merge 118 commits into
andylassiter wants to merge 118 commits into
Conversation
Direct write to OWUI's user.settings on signup so the in-chat iframe renders with allow-same-origin from the first search. OWUI has no admin API for per-user settings; the webhook is awaited in the OAuth callback so this lands before the browser redirect.
Half-finished split before, /searches/* and /reports/* lived at the root, which is why chat was 404ing. Rename /export.csv to /csv to mirror /rows. Chat-alias ingress proxies /api/searches and /api/reports so iframe fetches stay same-origin for now.
The chat-host same-origin alias existed so the SPA could resize via
window.frameElement. OWUI 0.9.5+ already listens for cross-origin
{type:'iframe:height'} postMessages, so the alias is dead weight.
Iframe now loads from report-viewer.<env>, fitParentIframe posts a
fixed 500px to the parent, and the sameOriginAlias ingress block plus
the chat_alias_enabled variable are gone.
Matches the Voila/Superset/OWUI MCP pattern in ADR 0022. Adds the report_viewer_svc Keycloak client, grants it ImpersonateUser in the OPA policy, swaps trino_client from JWT pass-through to client_credentials + X-Trino-User. Drops user.token from auth.py since the user JWT no longer goes outbound. Reverts oauth2-proxy back to its ADR 0003 shape: drops pass_access_token and cookie_refresh, removes X-Auth-Request-Access-Token from the forwardAuth middleware, drops trino-audience from the oauth2-proxy Keycloak client. The token broker pattern is gone, oauth2-proxy is the approval gate again and never participates in the Trino-trust chain.
Also ignore the whole vite build output dir, not just assets, so static/index.html stops dirtying on every rebuild.
Wire build, scan, publish, and smoke-test analytics for the image. Add pytest for the service and the OWUI scout_report_viewer tool. Multistage Dockerfile so Vite builds in node, runtime stays on python:3.12-slim. VERSION goes to latest. Python package scout_report_viewer, REPORT_VIEWER env prefix, and the report_viewer_svc keycloak client are already short and stay unchanged.
Scout standard middleware chain on the report-viewer subdomain per ADR 0003. New embeddable-security-headers Traefik middleware mirrors the global one but with frameDeny off and frame-ancestors parameterized so the chat iframe is not blocked. Two middleware definitions because browsers intersect CSP headers, so an app-layer override of frame-ancestors cannot loosen the global 'none'. Drop the FastAPI SecurityHeadersMiddleware. The browser identity path already reads X-Auth-Request-Preferred-Username from oauth2-proxy, no app code change needed. Add auth-curl smoke tests for the report-viewer subdomain.
Drops /webhooks, /metrics, /healthz, /docs from external reachability so the webhook is in cluster only.
scout_get_reports used to require a prior search_id, so reading one report by ID needed either an LLM-built SELECT or a two-tool find/get dance, and the model kept slipping on which column to use. New shape takes ids and id_column directly, hits /api/reports/read, defaults id_column to primary_report_identifier so the lake file path from the Discuss in Chat handoff just works. Report-scoped IDs hit reports_latest directly to keep the hot path off the epic_view JOIN. Patient-scoped IDs (epic_mrn, mpi, scout_patient_id) route through reports_latest_epic_view with mrn and mpi transparently mapping to the resolved_* columns. Discuss in Chat uses OWUI input:prompt (not :submit) because OWUI gates auto-submit on real same-origin, not on iframeSandboxAllowSameOrigin, and Scout's iframe is on a dedicated subdomain. User hits Enter to send. Other polish: row vs card visual contrast, spinner in a reserved slot, SQL becomes Explain Search, CSV becomes Download CSV.
Add report-viewer-audience client scope and attach it to the OWUI client, then flip verify_aud=True. Today any realm token validates, which is the gap. Drop trino-audience from the OWUI client since report-viewer no longer forwards user JWTs to Trino.
Trino emits timestamp-with-timezone as YYYY-MM-DDTHH:MM:SS+00:00 and the lake is UTC, so the offset is always noise. Card already trimmed it via a helper; the table didn't. Lifted the helper to module scope, swapped the regex for Date + toISOString, and routed the Date cell through it.
Drops the Helm release, openwebui_mcp_svc Keycloak client and password.db, and flips Trino to JWT-only.
Bumps search ID entropy from 6 to 16 base62 chars while here.
…ailable columns FiltersModal used to render all inputs unconditionally, so filtering on a column the search SQL did not SELECT would 502 from Trino. Each row is now gated on the returned columns, and the new MRN, accession, and facility inputs slot in the same way. Also show MRN by default and add an MPI column.
…nement
The OWUI tool used to parse CSVs and post a JSON list of IDs. Now it
forwards the file as multipart. Parsing, header inference, dedup, and
column resolution live in report-viewer.
Fixes the display-blank MRN bug: from-file filtered on raw epic_mrn
but selected resolved_epic_mrn AS epic_mrn, so v2.4 historical rows
came back with a blank MRN. Both validate and saved SQL now target
reports_latest_epic_view and translate patient-scoped inputs through
PATIENT_ID_COLUMNS. Also fixes the count, was len(matched_ids), now a
real SELECT COUNT(*).
Adds {{cohort}} placeholder so the LLM can refine a file-mode search
or run a cohort aggregate without ever seeing the ID list. Same
mechanic on scout_find_reports (optional custom sql) and
scout_query_sql (new /api/reports/query/from-file endpoint).
Placeholder must appear exactly once, checked before any Trino call.
Dependency ReviewThe following issues were found:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+101
to
+103
| _cols, vrows = await trino_client.execute( | ||
| validate_sql, user=user.sub, params=[cleaned] | ||
| ) |
Comment on lines
+177
to
+179
| columns, rows = await trino_client.execute( | ||
| sql, user=user.sub, params=[[str(i) for i in body.ids]] | ||
| ) |
| sample_sql = f"SELECT s.* FROM ({sql}) s LIMIT {_LLM_SAMPLE_ROWS}" | ||
| try: | ||
| with metrics.time_trino("create_sample_query"): | ||
| columns, sample_rows = await trino_client.execute(sample_sql, user=user.sub) |
| count_sql = f"SELECT COUNT(*) AS n FROM ({sql}) s" | ||
| try: | ||
| with metrics.time_trino("create_count_query"): | ||
| _cols, count_rows = await trino_client.execute(count_sql, user=user.sub) |
Comment on lines
+359
to
+361
| _cols, rows = await trino_client.execute( | ||
| validate_sql, user=user.sub, params=[chunk] | ||
| ) |
| .map((t) => t.trim()) | ||
| .filter((t) => t.length >= 2) | ||
| .map((t) => t.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); | ||
| const highlightRe = escaped.length ? new RegExp(`\\b(${escaped.join('|')})\\b`, 'gi') : null; |
| new_user_email = user_obj.get("email") or body.get("email") | ||
|
|
||
| if not new_user_id: | ||
| log.info("webhook fired with no user id", extra={"action": action}) |
| metrics.OWUI_WEBHOOK_EVENTS.labels(action="other", result="skipped").inc() | ||
| return None | ||
| if action and "signup" not in action.lower() and "new" not in action.lower(): | ||
| log.info("ignoring non-signup webhook", extra={"action": action}) |
| metrics.OWUI_WEBHOOK_EVENTS.labels(action="signup", result="enabled").inc() | ||
| log.info( | ||
| "iframe-defaults applied", | ||
| extra={"user_id": new_user_id, "email": new_user_email}, |
| except Exception: | ||
| log.exception( | ||
| "iframe-defaults apply failed", | ||
| extra={"user_id": new_user_id, "email": new_user_email}, |
| await cur.execute('SELECT settings FROM "user" WHERE id = %s', (user_id,)) | ||
| row = await cur.fetchone() | ||
| if row is None: | ||
| log.warning("user not in OWUI DB yet", extra={"user_id": user_id}) |
Comment on lines
+432
to
+441
| extra={ | ||
| "search_id": stored["id"], | ||
| "id_column": resolved_id_column, | ||
| "column_inferred": column_inferred, | ||
| "unique_ids": len(cleaned), | ||
| "matched_ids": len(final_ids), | ||
| "unmatched_ids": len(unmatched), | ||
| "report_count": row_count, | ||
| "custom_sql": bool(sql), | ||
| }, |
…ooldown to report-viewer blocks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Product
Adds a new report-viewer service that replaces mcp-trino as Scout Chat's data path. When a user asks Scout Chat to find reports, the LLM calls into report-viewer and renders results in an embedded iframe inside the chat, instead of pulling report rows into the LLM's context window. Aggregate questions stay inline. Single-report reads return the full report content in the chat.
The viewer is a small React SPA hosted alongside a Python FastAPI backend, framed by OWUI. Users can filter, sort, resize columns, expand rows to see the full report text, download the search as CSV, and pull selected reports into the chat context.
Technical
report-viewer/(FastAPI + React SPA), Helm chart athelm/report-viewer/, Ansible role atansible/roles/report_viewer/scout_report_viewer_tool.pyreport_viewer_svc+X-Trino-User)audpinned toreport-viewer/apiand/spahttp-server.authentication.type=JWT)Type of change
Impact
Security
Authorization
Per-user Trino AuthZ via
X-Trino-Userimpersonation (ADR 0022), so OPA row filters and PHI masks still apply. Search access is owner-scoped in Postgres. The OWUI tool only forwards the caller's access token, no refresh token, no client-set impersonation.Appsec
JWT validated (aud + iss). Trino params bound, identifiers allowlisted. External surface locked to
/apiand/spabehind oauth2-proxy. NetworkPolicy deny-default; pod runs non-root, RO rootfs, caps dropped. OWUI settings backfill role is column-scoped GRANT only.Performance
Not blazing fast. Searches run one sample + one count query, reads paginate the saved SQL as a subquery (no materialization). Queries through
reports_latest_epic_vieware on the slower side, and single-report reads pull throughreports_latest_epic_viewto get resolved patient IDs.Data
Flow: LLM authors SQL, report-viewer runs it through Trino (impersonating the caller) to validate + count + sample, then persists the search to its own Postgres. Report rows themselves live in Delta Lake and are re-queried through Trino on every read; nothing about the rows is cached or copied out. Report-viewer stores per search in its
searchestable: the SQL, the LLM's plain-language explanation, cached row count, the user, and optional match_terms/match_diagnoses. The OWUI signup webhook additionally writes iframe-sandbox UI defaults into OWUI's Postgres (would like to remove this when upgrading OWUI to 0.10.x if possible).Backward compatibility
mcp-trino is removed and Trino no longer accepts password auth (JWT-only).
Testing
Backend unit tests under
report-viewer/tests/, including JWT audience/iss regressions and Trino client cache single-flight. Frontend TypeScript typechecks viatsc --noEmit. New Playwright test attests/auth/tests/owui-iframe-seed.spec.tscovers the OWUI iframe-sandbox seeding path. Deployed and exercised on the dev cluster. QA issue #483 tracks the full acceptance criteria checklist.Note for reviewers
Sorry for the big PR!
#471 (Vega-Lite charts) is patched, not solved. Prompt tweaks nudge the model toward Vega-Lite but it still slips up. The real fix is probably a proper plotting feature but keeping Vega-Lite for now until we have a better solution.
Closes #463
Related to #483 (QA)
Related to #471 (partial)
Checklist
pre-commit run --all-files)