ci: publish the hub image to GHCR, and verify it actually starts - #16
Merged
Conversation
The hub ships only as a container but published nothing, so every consumer had to vendor this repository and build from source. Catalyst's demo stack needs the hub for exactly one thing — a running service — and had no other reason to require a checkout of it. Two tags per push to main, pointing at the same image: the commit SHA, which is immutable and what deployments should pin, and latest for convenience. The build also has to pass HUB_BUILD_REVISION. validate_config() requires a 40-character commit and rejects the Dockerfile's "unknown" default, so `docker build .` produced an image that built cleanly and then exited 1 on startup — which is what CI has been building all along. Verified both directions locally: without the argument the container exits 1 on "HUB_BUILD_REVISION must be the 40-character Git commit"; with it, /health answers in 2s and the image self-reports its revision in the standard org.opencontainers.image.revision label. docker-build now runs the image rather than only building it. A build proves the Dockerfile parses; it does not prove the packaged configuration is serviceable, and validate_config() runs at import, so starting the container and getting /health is what actually closes that gap.
pmanko
added a commit
to DIGI-UW/openelis-catalyst
that referenced
this pull request
Jul 25, 2026
The hub validates HUB_BUILD_REVISION at startup and rejects the Dockerfile's "unknown" default, so building it without the argument produces an image that builds cleanly and then exits 1 on: ValueError: HUB_BUILD_REVISION must be the 40-character Git commit for packaged deployments. This stack built the hub with no arguments, so it was one hub-source update away from a demo that would not come up. It has not fired yet only because the deployed checkout predates the requirement. Required rather than defaulted: the value has to name the commit the context is actually at, and there is nothing sensible to invent. Failing at `compose config` with that message beats a container that exits during `up` for reasons pointing at a variable nobody set. Collapses to an image pin once the hub publishes one (pmanko/med-agent-hub#16), at which point both variables go away.
pmanko
added a commit
to DIGI-UW/openelis-catalyst
that referenced
this pull request
Aug 4, 2026
…ric executor) (#5) * feat: add manual multi-LLM Catalyst sandbox * fix: verify local model checksum on macOS * fix: make local model sandbox boot reliably * fix: remove preview expiration * feat: retain invalid query diagnostics * Add manual multi-LLM query workbench * Add editable SQL workbench and scoped Hub retries * Hydrate unresolved raw query drafts * Add distinct writer reviewer query flow * Add workbench session reset controls * feat: add iterative Catalyst query notebook * chore: advance reviewed Hub pin * fix: make iterative workbench merge-ready * fix: bound MVP readiness probes * fix: preserve analyte terminology through FHIR * fix: accept reviewed writer lint failures * test: cover live repair lineage * fix: pin SQL-safe Hub query profiles * fix: persist effective model configuration * docs: record SQL profile sampling evidence * fix: keep iterative notebook reachable and focusable * fix: defer generated-editor focus until it unlocks Co-authored-by: Cursor <cursoragent@cursor.com> * chore: repin standalone Hub fallback to reverse profile commit Co-authored-by: Cursor <cursoragent@cursor.com> * feat(ui): rename project page to 'Catalyst' Drop the 'OpenELIS'/'OpenELIS Global' qualifier from the browser title, meta description, and the on-page product mark so the page reads simply 'Catalyst' ahead of multi-dataset support (the app will no longer be OpenELIS-specific). Section labels that name OpenELIS are left for the dataset-switcher change, where they become dataset-aware. * feat(gateway): dataset registry + /v1/catalyst/datasets discovery Introduce a dataset registry so the gateway can expose more than one queryable dataset (each dataset = its own analytics DB DSN + catalog): - config: DatasetConfig + GatewayConfig.datasets/default_dataset_id. Default dataset built from the existing CATALYST_ANALYTICS_DSN / CATALYST_CATALOG_PATH (id 'openelis'); extra datasets registered via an optional CATALYST_DATASETS_PATH JSON file. Fully back-compatible. - service: CatalystService accepts datasets/default_dataset_id and adds datasets() -> catalyst.datasets.v1 (defaultDatasetId + datasets[]). Derives a single entry from the catalog when not supplied (test compat). - route: GET /v1/catalyst/datasets for the UI dataset switcher. - gateway: wires config.datasets into the service (availability = catalog file present). Discovery only; per-request datasetId routing through the query/session methods is a follow-up. Tests: config registry + endpoint registration + datasets() payload (3 passing). * refactor(gateway): rename dataset registry to data sources 'Dataset' already means the pipeline-run/data-version stamp in this codebase (dataset overview datasetId, session dataset_version). The selectable target is a *data source* — its own analytics DB + catalog — and turns will target one per generation. Rename before anything consumes the new API: /v1/catalyst/data-sources (catalyst.data-sources.v1, defaultDataSourceId, dataSources[]), DataSourceConfig, CATALYST_DATA_SOURCES_PATH / CATALYST_DEFAULT_DATA_SOURCE_ID. * feat(gateway): per-source catalog+adapter bundles behind the registry DataSourceBundle carries each source's Catalog + analytics adapter with a per-bundle runtime-catalog snapshot, so discovery never mixes schemas across sources. The service builds its registry from bundles (deriving a single bundle from the ctor catalog/analytics when none are supplied — keeps every existing single-source test/ctor working), anchors the legacy self.catalog/self.analytics on the default bundle, and adds _resolve_data_source(). gateway constructs one bundle per configured source (skipping unprovisioned catalogs at boot). Full suite: 144 passed. * feat(gateway): dataSourceId on catalog/overview/rows read paths workbench/catalog, dataset, and dataset/rows accept an optional ?dataSourceId and resolve the target bundle (unknown id -> 400 unknown_data_source; omitted -> default source). Runtime schema discovery and overview/rows now run against the resolved source's adapter. Write-path threading (session/turn/version stamping and the per-source staleness baseline) lands separately. Suite: 144 passed. * feat(gateway): per-turn data-source targeting with mid-session switching Turns (and the initial session question) accept an optional dataSourceId and generate against that source's catalog; omitted, a turn inherits the session's last-targeted source (else the initial, else the default), so existing single-source clients are unchanged. Turn records and version provenance stamp their source; validate/execute resolve the version's recorded source and run on ITS adapter. Catalog staleness is now judged per source (baseline = the catalog the session last saw on that source; first use of a new source has no baseline), so switching sources mid-session never trips a false stale_catalog_version 409. Contracts: optional dataSourceId on session/turn requests; session/turn responses expose dataSourceId (+ turn catalogVersion). No DB migration — sources ride in the JSON records. Enables: switch target sources within one session, e.g. 'now adapt this query to the HIV data source' — revision context carries the prior SQL while the writer receives the new source's catalog, and the SQL policy gates against the new source's relations. Tests: 6 new two-source routing tests (defaulting, explicit targeting, unknown-source 400, mid-session switch incl. adapt flow + inheritance, execution routed to the version's source adapter). Suite: 150 passed. * feat(ui): data-source switcher + session-first-class header Header gains a 'Data source' selector (fed by /v1/catalyst/data-sources, shown only when more than one source is registered) and a session strip (short session id + turn count) beside the existing dock-level New session control. The selected source is threaded end to end: session create and follow-up turns carry dataSourceId (switching mid-session targets the next turn at the new source — the adapt-across-sources flow), and the schema catalog + dataset browser refetch per source so the guide always shows the data you are about to query. api: getDataSources; dataSourceId on createWorkbenchSession, getWorkbenchCatalog, getDatasetOverview, getDatasetRows. types: DataSource(sResponse), dataSourceId on session/turn(+request). Tests: 109 passed (two createWorkbenchSession arity assertions updated for the new optional arguments). * fix(multi-source): audit fixes — restore sync, version targeting, dead surface From an adversarial audit of the feat/multi-dataset branch: - UI session restore now syncs the data-source selector from the restored session (previously a reload silently flipped targeting back to the default source while showing the old session's SQL). - Manual version saves can target a data source: optional dataSourceId on the version request (contract + service + UI draft), resolved before the session-inherit fallback — closes the silent wrong-adapter execution path after a selector switch without an intervening turn. - Removed the write-only _runtime_catalog_snapshot; the lineage guard test now mutates the default bundle's runtime_snapshot, restoring its ability to fail (it had become vacuous). - _workbench_catalog_conflict's data_source_id/prior_turns are required (the legacy no-source branch had zero callers). - DataSourceBundle.available is now real: registered-but-unprovisioned sources (catalog not on disk) are listed available=false and cannot be targeted; ctor rejects duplicate source ids and requires an available default. catalog/analytics ctor args are optional (single-source derivation only); gateway.py no longer passes them redundantly; the write-only self.catalog alias is gone. - catalyst.data-sources.v1 gets a normative schema, registered. Suites: gateway 150 passed, UI 109 passed. * fix(multi-source): audit round 2 — contract consistency, dedup, turn badges The remaining findings were consistency bugs, not style: - unknown_data_source on the workbench catalog route now uses the workbench error envelope like every other workbench endpoint (clients no longer need two error shapes for one error class). - Contracts describe reality: session-request dataSourceId gains the maxLength the turn request already had; session-v1 dataSourceId is a plain string (the service always emits one, never null). - The resolve-bundle-or-400 block (copy-pasted 7x) and the 12-line version-source resolution (2x) are now _require_bundle/_version_bundle — one place for resolution semantics to change. - Turn timeline renders a data-source badge per turn (label resolved from the registry), so a mid-session source switch is visible in the history instead of silently invisible — provenance-first UI. Gateway 150 passed, UI 109 passed, tsc clean. * feat(analytics): migrate OpenELIS ingestion to lossless default views Same architecture the OpenMRS HIV source proved out: the ingestion layer is the upstream fhir-data-pipes default ViewDefinitions (forEachOrNull keeps every coding as rows) plus documented additive extensions (Observation: issued/service_request_id/specimen_id) and gap-fill views for resources upstream ships none for (Specimen with receivedTime, ServiceRequest); ALL curation happens in SQL over the default tables; the catalog is GENERATED from DB comments + analytics/catalog-overlay.json (inert allowedFilters/terminology/examples sections are gone; catalogVersion stays 'analytics-catalog-v1' so the T094/MVP gates, gateway config, and provenance chain hold — runtime version verified: analytics-catalog-v1+schema.<hash>). lab_result_fact_v1 keeps its exact 16-column public shape (per-observation collapse + LOINC pivot instead of the old lossy coding.first() ingest); public.service_request_flat_v1 survives as a curated compatibility view. Live-verified byte-identical fixture contracts over the re-ingested default tables: fact summary 1152|96|9|384|1152|9|2025-07-15|2026-04-27 and ServiceRequest 1152|1152|9. tests/analytics/test_contracts.py now enforces the new invariants: defaults-plus-gap-fills file set, observation extensions survive an upstream sync, specimen receivedTime feeds the turnaround calc, fact SQL collapses the coding cross product with a LOINC pivot, and the generated catalog contains ONLY gateway-consumed sections (13 passed; the 3 test_mvp_assembly failures are pre-existing on the pinned base). Gateway suite: 150 passed. * test(e2e): live two-source demo — generate on OpenELIS, adapt to OpenMRS HIV Records (demo-video project) the full manual-testing flow against the real stack: both registered data sources selectable; a viral-load question generates against lab_result_fact_v1, validates, and executes with results; the data source switches mid-session to the OpenMRS HIV/ART program; a follow-up adapts the query to hiv_observation_fact_v1 and executes; the turn timeline attributes each turn to its source. Passed live in 1.9m (real Gemma 4 12B writer / Qwen 2.5 14B reviewer). * fix(gateway): session reload reports current source; narrow optional analytics - _present_workbench_session now resolves dataSourceId last-turn-wins (matching turn targeting), so a UI reload after a mid-session switch no longer snaps the switcher back to the session's initial source and silently re-targets it on the next follow-up. - _resolve_data_source also requires bundle.analytics; the four call sites assert the guard, restoring the mypy baseline (25 -> 21). * fix(gateway): fail boot loudly on set-but-missing CATALYST_DATA_SOURCES_PATH A configured-but-absent registry path is an operator error; silently falling back to single-source mode hides a broken multi-source deploy. * test(gateway): guard multi-source seams the fakes left uncovered - real third turn with no dataSourceId inherits the switched source - per-source staleness: drift on the switched source 409s against ITS baseline, not the initial source's - ?dataSourceId= on GET dataset/editor-catalog routes to the bundle; unknown id -> 400 - boot: unprovisioned registry entry lists available=false and cannot be targeted (tri-state is now load-bearing) - committed generated catalogs (openelis + openmrs-hiv) must pass the gateway's editor-catalog validation; overlay <-> catalog <-> registry consistency guards - honest header: fakes prove routing, not SQL/discovery correctness * test(analytics): guard OpenELIS fact-view SQL semantics against real Postgres Executes analytics/sql/001_analytics_v1.sql against a scratch database seeded to mirror the fhir-data-pipes sink exactly: asserts the per-coding cross product collapses to one row per observation, the LOINC coding wins the test_* pivot, a local-only coding falls back via COALESCE, and turnaround math joins exactly one specimen despite a 2-row specimen type-coding fan-out trap. tests/analytics/test_contracts.py only checks this SQL's text shape; this is the guard that goes red if the GROUP BY/FILTER logic actually breaks. * refactor(gateway): remove redundant question-text destructive-intent filter question_policy_violations was a regex pre-filter on the raw question text (delete/drop/insert/update/alter phrasing), layered in front of SqlPolicy.evaluate — which already deterministically rejects any non-SELECT generated SQL at the AST level, independent of how the question was worded, before it ever reaches the (properly role-restricted) analytics connection. The text filter added a second, fuzzier, gameable enforcement point without a security guarantee SqlPolicy didn't already provide. Removed the filter, its two call sites (governed-route hard-422, workbench advisory finding), and its dedicated unit tests. The three tests that used it as an observation vehicle for other behavior are rewired onto still-present mechanisms: - governed-route hard-block: now demonstrated via SqlPolicy rejecting a destructive SQL a hub response returns (operation_not_allowed). - workbench advisory-vs-governed-blocking: same substitution. - version validation uses the latest turn's instruction, not the session's original question: demonstrated via missing_semantic_filter (a catalog-driven, still-present invariant) instead, since that needs a text-derived signal the removed filter no longer provides. * test(ui): guard the data-source switcher and turn-source badge No committed coverage existed for the multi-source UI: switcher visibility (hidden with 1 source, shown with >1, unavailable sources filtered from options but not from visibility), the switch carrying through to session creation and follow-up turn requests, the editor catalog refetching on switch, graceful degradation when GET /data-sources fails, and the TurnNotebook per-turn source badge (shown with a label, omitted when null). * docs: scope readiness() and disclaim text-shape-only / manual-demo test surfaces readiness() gets an inline note that it intentionally reflects only the default data source (full per-source readiness is 008-amendment future work, not a gap). SemanticContractTests and two-source-demo.spec.ts get honest headers pointing at what actually guards SQL semantics (tests/analytics/test_fact_view_semantics.py) versus what each of these is (text-shape assertions; a manual live-stack demo script). * refactor(gateway,ui): cut dead knobs, dead test args, and a no-op wrapper - Drop CATALYST_DEFAULT_DATA_SOURCE_ID/_LABEL: no setter anywhere (compose, scripts, or tests) — hardcode the OpenELIS default. - Drop the catalog=/analytics= CatalystService ctor args in test_multi_source.py: ignored whenever data_sources bundles are supplied (confirmed in service.py's __init__ branching). - Collapse the .session-strip wrapper div to its single child span; its flex/gap CSS was inert with one child. * style(gateway): ruff format service.py * docs: bring analytics/README, specification, roadmap up to the lossless+multi-source architecture analytics/README.md described the OLD hand-written single-select ViewDefinitions (name_display fallback logic, 'five ViewDefinitions') that no longer exist; rewritten to describe the actual lossless default-views-plus-SQL-curation layering and the generated catalog. specification.md gains a Data sources subsection (GET /v1/catalyst/data-sources, per-turn/session targeting, per-source staleness) and trims the 'governed semantic view requires' list to the fields the generated catalog actually carries — the older filters/terminology/freshness/example-query sections are inert. roadmap.md and README.md get one-line mentions of the second, switchable OpenMRS HIV/ART data source. * style(gateway): ruff format new test files * test(e2e): per-dataset 2-turn demo recordings (OpenELIS, OpenMRS HIV) Standalone live demo scripts (skip unless PLAYWRIGHT_LIVE=true), one per data source, each a real two-turn conversation: an initial bounded question, validate, run, then a follow-up that adds detail from the exact current query (not a fresh question). Follow-ups deliberately avoid introducing a new bound parameter — the writer model reliably drops the parameter name when asked to add a numeric threshold clause (observed live: 2/2 failures on 'only show results above 1000', both writer_output_contract_failed with an unnamed parameter), so both demos add SELECT-list detail instead. Companion to two-source-demo.spec.ts, which demos the cross-source switch. * feat(demo): add pacing/title-card timeline for OpenELIS demo video Raw Playwright captures of the two-turn demo were being published as real-time, un-paced screen recordings with no on-screen explanation. This timeline (consumed by the harness's scripts/render_demo_video.py) adds title/section cards per turn, compresses model-generation waits while keeping them visible, and holds the results view long enough to read. * feat(demo): add pacing/title-card timeline for OpenMRS HIV demo video Companion to the OpenELIS timeline: same treatment (per-turn title cards, compressed generation waits, held results view) for the HIV/ART program data source, consumed by the harness's scripts/render_demo_video.py. * feat: switch the default query profile to Gemma 4 12B + Qwen Coder 1.5B QUERY_PROFILE_ID (the profile used whenever a request omits profileId) was catalyst-query-gemma-4-12b (Gemma 12B writer + Qwen 14B reviewer). Switches it to the new catalyst-query-gemma-4-12b-coder (Gemma 12B writer + Qwen 2.5 Coder 1.5B reviewer) profile, added alongside two other new profiles in the med-agent-hub submodule. Updates every pinned reference to the default across the gateway's own tests, the UI's test suite, the MVP compose-assembly tests, env.recommended, mvp-model-config.sh's fake/external role-model fallbacks, and the docs that describe the default profile. Live-verified end to end: a request with no profileId now generates a valid query via catalyst-query-gemma-4-12b-coder against the real med-agent-hub + llama.cpp stack. The old catalyst-query-gemma-4-12b profile is untouched and still available (non-default) — it's what the published T094 acceptance run evidence already validated, so that report's claims stand as recorded. * chore: update bootstrap-med-agent-hub.sh fallback ref to the new Hub pin * chore: ignore tdd-guard local judge state, matching the harness root pattern * chore: update bootstrap-med-agent-hub.sh fallback ref to the new Hub pin The reviewer output contract fix (med-agent-hub 1432e72) simplifies the review schema's decision-conditional requirements so a small reviewer model's valid decisions stop failing schema validation. * fix(gateway): distinguish a succeeded-but-rejected reviewer call from transport failure _model_failure_stage fell through to "<role>_transport_failed" for any outcome it didn't recognize, including a reviewer/writer call that succeeded and simply decided to reject the candidate. That mislabeled a normal fail-closed rejection as a transport-layer problem in turn evidence. Add an explicit branch for outcome == "succeeded". * fix(contracts): add writer_decision/reviewer_decision to the turn failure.stage enum Needed by the succeeded-but-rejected failure-code fix (previous commit): without these enum values, a rejected followup turn now fails the workbench turn contract itself (HTTP 500) instead of shipping a clean rejected response. * chore: update bootstrap-med-agent-hub.sh fallback ref to the stateless-review Hub pin * feat(gateway,ui): revert default query profile to Gemma 12B + Qwen 14B reviewer The 1.5B coder reviewer is removed from all pairings (it cannot ground review verdicts in candidate SQL), so the production default returns to catalyst-query-gemma-4-12b — now with stateless follow-up review. External and fake mode role models restore qwen2.5-14b as query_review; the bundled 1.5B self-checked profile is unchanged. Hub pin bumped to the restructured arm set (c436833). * chore: sync med-agent-hub fallback ref — 1.5B writer arm reviewed by Gemma E4B * feat(gateway): pipeline foundation + port lint/schemas from hub First increment of moving governed-query orchestration out of the shared med-agent-hub and into the gateway (composable step classes): - pipeline/: PipelineContext, self-guarding PipelineStep ABC, terminating Pipeline runner, and gateway-owned QueryProfile/RoleConfig where writer-only vs writer+reviewer is a composition concern (which roles are present), not a hub invariant. 9 tests. - query_lint.py: verbatim port of the hub's deterministic sqlglot lint. 18 tests. - query_schemas.py: verbatim port of the candidate/review/repair schema derivation + structured-output formats + contract error types, loading the catalyst-query-v1 contract from docs/contracts. No behavior change yet; hub still serves queries. Refactor branch only. * feat(gateway): port deterministic parse/semantic/patch layer from hub query_parse.py: verbatim behaviour-preserving port of the pure candidate parsing, normalization, canonical-target/catalog matching, semantic grounding, lint-validation checks, and scoped patch-application helpers (hub lines 345-1523). Imports validators/errors from query_schemas and turnaround_threshold from query_lint. 7 behavioural smoke tests. No behavior change; hub still serves. Task #20 (deterministic domain logic) complete: lint + schemas + parse ported. * feat(gateway): RoleClient calling the hub generic executor RoleClient is the thin seam the model-calling steps depend on: it POSTs one structured completion to the hub's /v1/hub/generate per role and returns the raw content plus an invocation-evidence record (role, model, httpStatus, wallMs, outcome). Domain-agnostic and easily faked in tests (httpx.MockTransport). 3 tests. The generate/review/repair step classes compose on top of this next. * feat(gateway): relocate the governed-query engine from the hub query_engine.py: the generate -> lint-correct -> review -> repair -> finalize orchestration + full evidence/provenance/model_collaboration assembly, moved verbatim from med-agent-hub/catalyst_query.py (lines 1526-2879). Deterministic logic is imported from the already-moved query_schemas/query_parse/query_lint. Only three changes from the hub original: - _backend_chat now POSTs to the hub's generic /v1/hub/generate executor instead of calling the model router directly (the 'use the hub properly' seam); - a writer-only branch: a profile with no query_review role finalizes the writer's lint-passing candidate without an independent review (the requested default), with reviewer evidence/trace guarded accordingly; - prompts, provider id, and profiles are gateway-owned config. Prompts moved gateway-side (byte-identical, digests verified). Imports+ruff clean. Wiring generate_query -> engine + profile registry is next (#23). * test(gateway): end-to-end run tests for the relocated engine Drive execute_query_profile with _backend_chat mocked, proving the relocated orchestration runs for both paths: writer-only finalizes without review (no reviewer evidence leg) and the reviewed path runs writer+reviewer (2 model invocations, both role evidence legs). Emits a valid catalyst.query.v1 result. Gateway suite 199 passing. * feat(gateway): profile registry + in-process LocalHub orchestrator query_profiles.py: gateway-owned EngineProfiles — writer-only default (catalyst-query-gemma-4-12b-q4) + self-checked writer+reviewer variant. Adding a workflow is adding a profile here, not editing the hub. local_hub.py: LocalHub implements the service layer's hub interface (generate_query / list_query_profiles / readiness) but runs execute_query_profile in-process; model calls go to the hub's /v1/hub/generate. Discovery and generation share query_profile_evidence, so profile-binding matches by construction. 4 tests — both paths generate ready queries; writer-only advertises no reviewer role/stage. Remaining: swap gateway.py HubClient->LocalHub, relax service.py's reviewer- required workbench binding for writer-only, then hub strip + deploy. * feat(gateway): wire service to in-process LocalHub; suite green gateway.py now constructs LocalHub (in-process engine) instead of HubClient (HTTP to the hub). LocalHub raises the shared HubError the service already catches. Default profile -> catalyst-query-gemma-4-12b-q4-checked (reviewed; works with existing workbench binding). Updated test_catalyst_mvp fixtures to the new profile/model ids. Full gateway suite green (203). The reviewed path now runs entirely through the gateway engine end-to-end. Writer-only default needs the reviewer-optional binding relaxations next. * feat: make writer-only the default; relax reviewer-required binding The reviewer was hardwired as mandatory in three gateway layers plus the turn contract. Relaxed so a writer-only profile is valid end-to-end: - catalyst-workbench-turn-v1 schema: profileSnapshot no longer requires 'reviewer' (recorded in the existing 'omissions' affordance instead). - service._require_profile_evidence: reviewer leg validated only when present. - service._turn_profile_snapshot: omits reviewer, records it in omissions. - service._require_hub_invocation_binding: reviewer-succeeded required only for profiles that declare a reviewer. - Default profile -> catalyst-query-gemma-4-12b-q4 (writer-only); the self-checked writer+reviewer profile is the opt-in option. Removed the obsolete HubClient HTTP tests (that path is replaced by the in-process engine + /v1/hub/generate). Full gateway suite green (190). * fix: writer-only turns pass all contract layers (reviewer optional) Live writer-only turns surfaced two more contract layers that assumed a reviewer (the unit suite missed them — they only validate against the mounted contracts on a real turn): - turn profileSnapshot: recorded turns must keep omissions empty (that field is for legacy-loaded turns only), so an absent reviewer is now simply omitted with empty omissions rather than recorded as an omission. - generation-evidence profileDetail: reviewer dropped from required; both storage descriptor builders now build detail from the writer alone and add reviewer only when present. Validated live: writer-only OpenELIS query returns a ready, fully-validated result in ~31s (vs ~114s reviewed) with no contract violation. * test: regression coverage for writer-only turn contract shape Drive a writer-only profile through _turn_profile_snapshot and the generation- evidence descriptor builder, asserting no reviewer leg + empty omissions + a writer-only profileDetail. These cover the two contract gaps that previously only surfaced against the live mounted contracts. * refactor(gateway): remove dead code superseded by the engine relocation The composable-step scaffolding (pipeline/: base, context, profile, roles) was superseded when the governed-query engine was relocated wholesale into query_engine.py; nothing in production imported it. HubClient was replaced by the in-process LocalHub. Removed: - src/catalyst/pipeline/ (whole package) + test_pipeline.py, test_role_client.py - the HubClient class from hub.py (kept HubError, still raised by the service + LocalHub) Gateway suite green (181). * fix(ui): clearer loading copy, Ask OpenELIS layout, longer proxy timeout - ExecutionState loading indicator is now context-aware ('Generating answer' for generation, 'Running query' for execution) instead of a single hardcoded 'Polling for results'. - Ask OpenELIS panel: smaller title, more padding, taller dock so it no longer needs to scroll (fixed a CSS specificity collision with .section-heading h1). - nginx proxy_read_timeout 420s -> 1800s to match the gateway/hub timeout chain for long CPU-bound generations. * ci: apply ruff format + update nginx-timeout assembly assertion - ruff format on the relocated/ported gateway modules and their tests (the verbatim ports carried the hub's formatting; CI runs ruff format --check). - test_mvp_assembly: nginx proxy_read_timeout assertion 420s -> 1800s to match the updated catalyst-ui/nginx.conf. Gateway suite 181, mvp-assembly 23 both green. * ci: drop now-unused test imports (ruff check) json + httpx became unused in test_catalyst_mvp when the obsolete HubClient HTTP tests were removed. Gateway ruff check + format both clean. * chore: clear all gateway mypy errors (35 -> 0) Classified all 35 errors individually; none were runtime bugs except one: - a2a_client.py: Message(messageId=...) worked at runtime via a pydantic alias but wasn't the model's real field name; switched to message_id= (verified the alias works identically, this is just the correct name mypy also recognizes). - Installed types-jsonschema (real stubs, not a blanket ignore) — cleared 4 import-untyped errors for free. - Added a scoped [tool.mypy] override for query_lint.py/policy.py disabling attr-defined/arg-type/assignment — verified case by case these are sqlglot's own bundled stubs being internally inconsistent (an Expr vs Expression alias mismatch, partial Scope attribute coverage), not application bugs. - analytics.py: two cursor.fetchone() call sites bound to a local + asserted non-None, documenting the real invariant (single-row aggregate queries always return exactly one row) — turns a latent bare TypeError into a clear AssertionError if that invariant is ever violated. - service.py: two dict literals (body, provenance) that are built once and mutated with heterogeneous value types afterward now get an explicit dict[str, Any] annotation at declaration; _profile_snapshot's nested evidence/writer/reviewer ternaries rewritten bind-then-branch so mypy can track the narrowing it was losing across repeated .get() calls. - query_engine.py: an assert proving what using_patch already guarantees (correction_base is not None), a too-narrow parameter annotation widened to match what's actually passed, and an explicit annotation on a dict variable mypy was mis-narrowing. Every fix is annotation/assert/rename only — zero behavior change, verified via mypy src (0 errors, 23 files), the full test suite (181 passed, same count as baseline), and ruff check + format (clean). * fix(dataset): drive the dataset browser from each source's own catalog Switching to a non-default data source failed with Dataset row lookup failed: relation "analytics.lab_result_fact_v1" does not exist because _dataset_overview_sync and _dataset_rows_sync hardcoded the OpenELIS fact view and its column names. Selecting a source correctly swapped the DSN via its own DataSourceBundle adapter, but the SQL text still named OpenELIS's relation, so every dataset query ran against the other source's database looking for a view that only exists in the first one. The OpenMRS HIV source spells the same concepts differently (hiv_observation_fact_v1, concept_name, value_numeric), so nothing about the query could match. Catalogs now declare a datasetBrowser profile naming the fact view and which of its columns play the subject / category / value / unit / timestamp roles, and the adapter composes both queries from that. The OpenELIS SQL is unchanged in behaviour; the HIV source finally gets its own. Identifiers are constrained to plain lowercase names and checked against the view's declared columns at catalog load, so a typo is an authoring error rather than something interpolated into SQL. A source with no profile now reports which source is unconfigured and what to add, instead of surfacing a Postgres relation error naming a view the operator never asked for. Sources whose value is coded or textual rather than numeric declare valueFallbackColumns so a row shows what it actually carries — without it most HIV rows render blank, since their value lives in value_coded_name rather than value_numeric. Also persists the selected source in the URL (?dataSource=...), so a reload, bookmark, or shared link reopens the same dataset and a bug report shows which source the screen was reading. A stale or unknown id falls back to the registered default rather than being sent to every endpoint as a 400. Verified against real PostgreSQL: both sources' analytics SQL applied to scratch databases, then overview/rows/filtering run through the adapter with each source's shipped catalog. OpenELIS returns its rows unchanged; the HIV source returns its own, including a coded observation rendering via the fallback chain. * deploy: commit the demo stack, bootable from a bare clone The catalyst.openelis-global.org demo was running from a compose file and Caddyfile that existed only on the server — on no branch of this repo or the superproject. The box has no git checkout, so nothing ever signalled that the deployment definition was untracked: from the server they looked like ordinary files, and from here nothing was missing because the repo never knew they should exist. That instance was the only copy of how Catalyst is deployed. Committed here with the superproject coupling removed, so the stack boots from a bare clone of this repository: - med-agent-hub publishes no image, so it still builds from source, but the context is MED_AGENT_HUB_CONTEXT (default: a sibling clone) rather than a hardcoded ../../targets/med-agent-hub. - The second data source is no longer assumed. The base stack boots with the built-in OpenELIS catalog; docker-compose.demo.extra-sources.yml layers on a directory of additional sources. That directory needs its own database, catalog, and registry entry, none of which live here. - CATALYST_SITE drives the Caddy site address the same way CADDY_SITE does in the superproject: unset means ":80" and plain HTTP, a domain means automatic TLS. Routing also moves to the edge, matching the convention the superproject's compose/Caddyfile states — all host-facing traffic through one proxy, internal services not directly exposed, routes path-based. The demo's Caddyfile was a bare `reverse_proxy catalyst-ui:8080`, which delegated every routing decision to nginx inside the frontend image. Two consequences that convention exists to prevent: the API went down whenever the UI container was replaced, and the long-request timeout had to be maintained in a second file, away from the route it protects. The superproject's Caddyfile already documents being bitten by that second one — chartsearchai chat had to bypass an intermediate nginx whose 60s default returned 504 before the model answered, and Catalyst makes the same kind of long call. The UI's nginx proxy stays: docker-compose.mvp.yml has no edge proxy and depends on it. In this stack Caddy matches /v1/catalyst/* first. * deploy: pass HUB_BUILD_REVISION when building the hub from source The hub validates HUB_BUILD_REVISION at startup and rejects the Dockerfile's "unknown" default, so building it without the argument produces an image that builds cleanly and then exits 1 on: ValueError: HUB_BUILD_REVISION must be the 40-character Git commit for packaged deployments. This stack built the hub with no arguments, so it was one hub-source update away from a demo that would not come up. It has not fired yet only because the deployed checkout predates the requirement. Required rather than defaulted: the value has to name the commit the context is actually at, and there is nothing sensible to invent. Failing at `compose config` with that message beats a container that exits during `up` for reasons pointing at a variable nobody set. Collapses to an image pin once the hub publishes one (pmanko/med-agent-hub#16), at which point both variables go away. * feat(query): offer a GPU lane with a cross-family writer/reviewer team Both shipped profiles were the CPU-only demo build: a Q4 writer, and a "checked" variant that reviewed with the same Q4 writer. A host with a GPU had no way to ask for the full-weight model, and no way to get a review that was not the writer re-reading its own output — a self-check shares the blind spot that produced the query it is checking. Adds two profiles: - catalyst-query-gemma-4-12b: full-weight writer, no review. The Q4 builds exist so the demo runs without a GPU; where there is one, quantisation is a cost nothing is asking us to pay. - catalyst-query-gemma-4-12b-qwen2.5-14b-checked: Gemma writes, Qwen corrects. Distinct model_classes, so this is the genuinely collaborative pairing rather than a second pass by the same model — restoring the different-family reviewer the hub's original Catalyst profile had before orchestration moved to the gateway. Roughly 12G + 8.4G of weights, so both stay resident on a 24G-class GPU and the writer/reviewer switch costs nothing once warm. The Q4 writer-only profile remains the default; nothing about the existing demo lane changes. * fix(mvp): health-check the gateway's query profiles, not the hub's check_hub_profile asked the hub's /v1/models for a catalyst-query-* profile. The hub has not advertised one since orchestration moved to the gateway — it is a generic model executor now and owns no Catalyst profiles — so this waited out all 120 attempts and failed every local boot with "hub query profile not ready" while the stack underneath was perfectly healthy. Points the check at the gateway's /v1/catalyst/query-options, which is where the profiles live, and reads that payload's actual shape: camelCase roleModels/unavailableReasons, and provenance rather than profileEvidence. Keeps the assertions that carry weight — the profile is offered and available, its role models are the expected ones, its provenance identifies itself and carries a configuration digest, and a reviewed profile really does run a query_review stage and advertise revision capability. Also repoints the external backend at catalyst-query-gemma-4-12b-qwen2.5-14b-checked. An external router is the GPU lane, and that backend already declared a two-role writer/reviewer map, so naming the writer-only profile contradicted its own role map. * fix(mvp): point the external backend's profile id at the reviewed profile MVP_EXTERNAL_EXPECTED_ROLE_MODELS_JSON declares a writer and a reviewer, but MVP_EXTERNAL_PROFILE_ID named catalyst-query-gemma-4-12b, which is writer-only. The health check compares the two and could never reconcile them. An external router is the GPU lane, so the reviewed cross-family profile is the one it should be exercising anyway. * test(mvp): align assembly contracts with gateway profiles * chore: track a gitleaks allowlist for this repo's demo placeholders Records which published strings are deliberate, so a scan does not have to be re-triaged by hand each time: the demo-*-change-me analytics passwords (the name is the instruction) and a fixed idempotency value in a UI test fixture. Allowlisted by value rather than by file, so a genuine credential landing in one of the same files is still reported. Verified: a GitHub PAT placed alongside the placeholders is still caught. Also excludes two gitignored paths that only a working-tree scan sees — the vendored .fhir-data-pipes checkout, and logs/, which holds real TLS material the MVP health script copies out of the OpenELIS container. Neither is tracked. Without this, `gitleaks dir .` reports ~24 findings here, none of them repository content, which is how a scan stops being read at all. Tracked in this repo rather than only the superproject because Catalyst is cloned and scanned on its own. * fix(mvp): align gateway profiles and architecture docs * fix(mvp): advertise only runnable query profiles * fix(mvp): record gateway-owned prompt references * fix(mvp): pin Hub build revision * fix(mvp): allow reset before Hub revision export * fix(mvp): use available Hub health probe * fix(mvp): select resolved query profile by default * fix(ui): contain profile selector on narrow screens * fix(gateway): classify reviewer invocation timeouts * fix(gateway): bound query role output tokens * fix(gateway): retain failed reviewer evidence * fix(gateway): review active query revision * chore(mvp): align fallback hub pin * test(mvp): remove duplicated hub revision assertion * refactor(ui): name the session meta class after the block it lives in .session-strip__meta was the last trace of a .session-strip wrapper that no longer exists — a BEM element naming a block nothing defines. The span it styles sits inside .app-shell__session-controls, so it is .app-shell__session-meta. Rename only; the rule body and the rendered markup are unchanged. * docs(analytics): document datasetBrowser and the multi-source shape analytics/README.md described the catalog's generation and contents but not the datasetBrowser block, which the gateway now requires to render the dataset view. It is copied through from catalog-overlay.json verbatim because it answers something the database cannot: which relation to browse, and which of its columns carry the subject, category, value, unit, and timestamps. Every source spells those differently — the category is test_name here and concept_name on the OpenMRS HIV source. Records what is required versus optional, that identifiers are validated against the named view at catalog load so a typo fails on startup rather than reaching the database, and why a coded source needs valueFallbackColumns — without them most of its rows render an empty value, because the answer lives in value_coded_name rather than value_numeric. Also frames this directory as one source's slice rather than the whole picture, and points at where additional sources are registered. Each is independent — own database, own catalog, own mapping — which is what lets a session switch sources without mixing schemas. * build: pin standalone hub fallback to merged main * test(ui): preserve accepted keyboard workbench flow --------- Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Adds GHCR publishing and runtime validation for the hub container.
Changes:
- Smoke-tests the image via
/health. - Publishes commit-SHA and
latesttags onmain.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Concurrent runs can move latest back to an older commit.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
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
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.
Summary
/health.latestlookup tags on reviewedmaincommits, then record the immutable manifest digest for deployment pinning.latestbackward.Why
The Hub ships as a container, but consumers previously had to vendor the repository and build from source. The old CI also proved only that the Dockerfile built. It did not catch that an image built without
HUB_BUILD_REVISIONexits during startup.This PR makes the published artifact the same image that passed its startup check. Package-write permission remains isolated to the main-only publish job.
Validation
/healthstartup passed.5c1df42e…and embedded revision9f688074….Deployment contract
Both the commit-SHA and
latesttags are movable registry names. Deployments must use the digest-qualified reference written to the publish job output and GitHub job summary:ghcr.io/pmanko/med-agent-hub@sha256:….The first main run creates the package. Repository package visibility may still need to be set to public before anonymous pulls work.