diff --git a/backend-ts/src/admin/scheduler.ts b/backend-ts/src/admin/scheduler.ts index 706470c9..844b556c 100644 --- a/backend-ts/src/admin/scheduler.ts +++ b/backend-ts/src/admin/scheduler.ts @@ -17,7 +17,7 @@ import type { Stores, StoresEnv } from "../stores/factory.ts"; import { getStores } from "../stores/factory.ts"; import type { HydratedSegment } from "../stores/segment-store.ts"; import type { EvaluateMeasureBinding } from "../engine/evaluate-measure.ts"; -import { engineForEnv } from "../wiring/engine-factory.ts"; +import { routedEngineForEnv } from "../wiring/executor-router.ts"; import type { EmployeeProfile } from "../engine/synthetic/employee-catalog.ts"; import { ensureSegmentSeed } from "../segment/segment-seed.ts"; import { @@ -330,7 +330,7 @@ export async function schedulerTick( try { await ensureSegmentSeed(env); const stores = await getStores(env); - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); const allSegments = await stores.segments.listSegments(); const enabledSegments = allSegments.filter((s) => s.enabled); await runTick({ stores, engine, segments: enabledSegments, alertChannels, webChartEnv: env, incremental: isIncrementalEnabled(env), expansionActive: isVsacConfigured(env) }); diff --git a/backend-ts/src/config/seam-inventory.test.ts b/backend-ts/src/config/seam-inventory.test.ts index 15fdfbb6..40431873 100644 --- a/backend-ts/src/config/seam-inventory.test.ts +++ b/backend-ts/src/config/seam-inventory.test.ts @@ -20,7 +20,7 @@ test("describeSeams: everything off with no env vars set", () => { const seams = describeSeams({}); assert.deepEqual( seams.map((s) => s.name), - ["sendgrid", "datachaser", "ice", "eh-fhir", "webchart", "sql-executor", "vsac", "alert-webhook", "bucket-s3", "incremental-eval"], + ["sendgrid", "datachaser", "ice", "eh-fhir", "webchart", "sql-executor", "vsac", "alert-webhook", "bucket-s3", "incremental-eval", "official-measures"], ); for (const s of seams) assert.equal(s.active, false, `${s.name} should default off`); }); @@ -28,7 +28,7 @@ test("describeSeams: everything off with no env vars set", () => { test("formatSeamLogLine: all-off shape matches the documented boot log line", () => { assert.equal( formatSeamLogLine({}), - "seams: sendgrid=off datachaser=off ice=off eh-fhir=off webchart=off sql-executor=off vsac=off alert-webhook=off bucket-s3=off incremental-eval=off", + "seams: sendgrid=off datachaser=off ice=off eh-fhir=off webchart=off sql-executor=off vsac=off alert-webhook=off bucket-s3=off incremental-eval=off official-measures=off", ); }); @@ -242,9 +242,10 @@ test("formatSeamLogLine: all-on shape when every seam is configured", () => { WORKWELL_BUCKET_S3_ACCESS_KEY_ID: "AKIA", WORKWELL_BUCKET_S3_SECRET_ACCESS_KEY: "s", WORKWELL_INCREMENTAL_EVAL: "true", + WORKWELL_OFFICIAL_MEASURES: "cms122,cms125", }; assert.equal( formatSeamLogLine(allOn), - "seams: sendgrid=on datachaser=on ice=on eh-fhir=on webchart=on sql-executor=on vsac=on alert-webhook=on bucket-s3=on incremental-eval=on", + "seams: sendgrid=on datachaser=on ice=on eh-fhir=on webchart=on sql-executor=on vsac=on alert-webhook=on bucket-s3=on incremental-eval=on official-measures=on", ); }); diff --git a/backend-ts/src/config/seam-inventory.ts b/backend-ts/src/config/seam-inventory.ts index faeec823..fe78c1a6 100644 --- a/backend-ts/src/config/seam-inventory.ts +++ b/backend-ts/src/config/seam-inventory.ts @@ -27,6 +27,7 @@ import { isSqlPushdownSelected, type MeasureExecutorEnv } from "../engine/measur import { isVsacConfigured, type VsacEnv } from "../engine/cql/resolve-value-set-resolver.ts"; import { isAlertWebhookConfigured, type AlertEnv } from "../run/alert-channel.ts"; import { isIncrementalEnabled, type IncrementalEnv } from "../run/incremental/incremental-eval.ts"; +import { isOfficialRoutingConfigured, type OfficialMeasuresEnv } from "../wiring/official-routing.ts"; /** The union of every seam's env-var shape (all optional — assignable from the worker's `Env`). */ export type SeamEnv = EmailEnv & @@ -38,7 +39,8 @@ export type SeamEnv = EmailEnv & VsacEnv & AlertEnv & BucketSeamEnv & - IncrementalEnv; + IncrementalEnv & + OfficialMeasuresEnv; export interface SeamStatus { /** Short, stable, log-line-friendly seam name. */ @@ -62,6 +64,11 @@ export function describeSeams(env: SeamEnv): SeamStatus[] { { name: "alert-webhook", active: isAlertWebhookConfigured(env) }, { name: "bucket-s3", active: isS3BucketConfigured(env) }, { name: "incremental-eval", active: isIncrementalEnabled(env) }, + // The 11th seam, and the only one that changes what a measure COMPUTES rather than how or where. + // The line reports only on/off, like every other seam; WHICH measures are routed comes from the + // OFFICIAL_ROUTING_MISCONFIGURED alert and the run's own evidence. On/off is still the thing worth + // seeing at a glance, because it is the difference between two engines. + { name: "official-measures", active: isOfficialRoutingConfigured(env) }, ]; } diff --git a/backend-ts/src/routes/cases.ts b/backend-ts/src/routes/cases.ts index 8fc81259..42d71192 100644 --- a/backend-ts/src/routes/cases.ts +++ b/backend-ts/src/routes/cases.ts @@ -20,7 +20,7 @@ import type { CloudDatabase, CloudBucket } from "@mieweb/cloud"; import { getStores } from "../stores/factory.ts"; import type { CaseStore } from "../stores/case-store.ts"; import type { OutcomeStore } from "../stores/outcome-store.ts"; -import { engineForEnv } from "../wiring/engine-factory.ts"; +import { routedEngineForEnv } from "../wiring/executor-router.ts"; import { toCaseSummary, type CaseSummary } from "../case/case-read-models.ts"; import { bucketPeriodForMeasure } from "../run/compliance-period.ts"; import { toCaseDetail } from "../case/case-detail-read-model.ts"; @@ -86,7 +86,7 @@ async function actionDeps(env: CasesEnv): Promise { const s = await getStores(env); - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); return { cases: s.cases, events: s.events, outcomes: s.outcomes, runStore: s.runs, engine }; } async function evidenceDeps(env: CasesEnv): Promise { diff --git a/backend-ts/src/routes/compliance-simulation.ts b/backend-ts/src/routes/compliance-simulation.ts index 4e2c85e4..9311fc0e 100644 --- a/backend-ts/src/routes/compliance-simulation.ts +++ b/backend-ts/src/routes/compliance-simulation.ts @@ -5,7 +5,7 @@ * (all roles, like the immunization forecast). Writes nothing; no schema. handleEmployees only matches * `/profile` + `/search`, so this `/simulate` path is not intercepted. */ -import { engineForEnv } from "../wiring/engine-factory.ts"; +import { routedEngineForEnv } from "../wiring/executor-router.ts"; import type { StoresEnv } from "../stores/factory.ts"; import { parseQueryDate, QueryDateError } from "./query-dates.ts"; import { simulateComplianceAsOf } from "../run/employee-compliance-snapshot.ts"; @@ -35,7 +35,7 @@ export async function handleComplianceSimulation(req: Request, env: StoresEnv): } const today = new Date().toISOString().slice(0, 10); - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); const snapshot = await simulateComplianceAsOf(externalId, asOf ?? today, { engine, today }); if (!snapshot) return json({ error: "not_found", externalId }, 404); return json(snapshot); diff --git a/backend-ts/src/routes/measures.ts b/backend-ts/src/routes/measures.ts index 7c24f967..46bdf3e0 100644 --- a/backend-ts/src/routes/measures.ts +++ b/backend-ts/src/routes/measures.ts @@ -21,7 +21,7 @@ import type { CloudDatabase } from "@mieweb/cloud"; import { getStores } from "../stores/factory.ts"; import type { MeasureStore } from "../stores/measure-store.ts"; import type { ValueSetStore } from "../stores/value-set-store.ts"; -import { engineForEnv } from "../wiring/engine-factory.ts"; +import { routedEngineForEnv } from "../wiring/executor-router.ts"; import { MEASURES } from "../engine/cql/measure-registry.ts"; import { ELM_LIBRARIES } from "../engine/cql/elm/index.ts"; import { compileCql, reconstructCql } from "../engine/cql/cql-translator.ts"; @@ -314,7 +314,7 @@ export async function handleMeasures(req: Request, env: MeasuresEnv, actor = "sy const body = (await req.json().catch(() => ({}))) as ImpactPreviewRequest; try { const s = await getStores(env); - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); const deps = { cases: s.cases, events: s.events, engine }; return json(await previewImpact(deps, measure, body, actor)); } catch (err) { @@ -358,7 +358,7 @@ export async function handleMeasures(req: Request, env: MeasuresEnv, actor = "sy const bundle = (await req.json().catch(() => null)) as unknown; if (!bundle || typeof bundle !== "object") return json({ error: "invalid_bundle" }, 400); try { - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); const outcome = await engine.evaluate({ measureId: evalId, patientBundle: bundle, evaluationDate: url.searchParams.get("date") ?? undefined }); return json(outcome); } catch (err) { diff --git a/backend-ts/src/routes/runs.ts b/backend-ts/src/routes/runs.ts index 5b3b4832..8969b3f2 100644 --- a/backend-ts/src/routes/runs.ts +++ b/backend-ts/src/routes/runs.ts @@ -25,7 +25,7 @@ import type { OutcomeStore } from "../stores/outcome-store.ts"; import type { CaseStore } from "../stores/case-store.ts"; import type { HydratedSegment } from "../stores/segment-store.ts"; import { ensureSegmentSeed } from "../segment/segment-seed.ts"; -import { engineForEnv } from "../wiring/engine-factory.ts"; +import { routedEngineForEnv } from "../wiring/executor-router.ts"; import { toRunListItemFromCounts, toRunSummaryFromCounts, toRunLogEntries, toRunOutcomeRows, matchesRunFilters, type RunFilters } from "../run/read-models.ts"; import { recoverStuckRuns } from "../run/recover-stuck-runs.ts"; import { resolveAlertChannels } from "../run/alert-channel.ts"; @@ -48,6 +48,7 @@ import { isVsacConfigured } from "../engine/cql/resolve-value-set-resolver.ts"; import { rerunToVerify, UnsupportedCaseRerunError } from "../case/case-rerun.ts"; import type { PopulationCounts } from "../fhir/measure-report.ts"; import { + officialMembership, buildMeasureReportBundle, buildSummaryMeasureReportFromCounts, countPopulations, @@ -153,13 +154,38 @@ const MAX_INDIVIDUAL_REPORT_SUBJECTS = 5000; * So official runs read rows, bounded by the same subject cap the individual/bundle reports use; * over the cap we refuse rather than emit a status-derived (wrong) regulatory artifact. */ +/** + * Did THIS run's outcomes come from the official executor? One row settles it: `evidence.official` is + * written only by that executor, and a run evaluates one measure with one engine. Bounded on purpose — + * this sits in front of a path that must stay O(1) for a 120k `seed:scale` run. + */ +async function runProducedOfficialEvidence( + os: Awaited>, + runId: string, +): Promise { + const [first] = await os.listOutcomes(runId, { limit: 1 }); + return officialMembership(first?.evidence) !== null; +} + async function aggregateCountsForRun( os: Awaited>, runId: string, measureId: string, env: RunsEnv, ): Promise<{ counts: PopulationCounts } | { error: Response }> { - if (!isOfficialRouted(measureId, env as unknown as Record)) { + // Provenance comes from the RUN, not from the current deployment flag (Codex P1). A run's outcomes + // were produced by whichever engine was configured *then*; consulting `WORKWELL_OFFICIAL_MEASURES` + // now means that turning the flag off — the documented rollback — silently reinterprets every + // historical official run through the status histogram, reversing cms122's numerator (its official + // numerator is poor control, and the workflow status inverts it). An export of a past run must not + // change meaning because of a config change made after it. + // + // The env flag is still consulted first, as a cheap way to skip a read for the overwhelmingly common + // case; when it is off, one bounded row settles it. `evidence.official` is written only by the + // official executor, and a run evaluates one measure with one engine, so a single row is decisive. + const routedNow = isOfficialRouted(measureId, env as unknown as Record); + const official = routedNow || (await runProducedOfficialEvidence(os, runId)); + if (!official) { return { counts: populationCountsFromStatus(await os.countOutcomesByStatus(runId), measureId) }; } const total = (await os.countOutcomesByStatus(runId)).reduce((sum, c) => sum + c.count, 0); @@ -287,7 +313,7 @@ export async function handleRuns( if (pathname === "/api/runs/manual" && req.method === "POST") { const body = (await req.json().catch(() => ({}))) as ManualRunRequest; body.triggeredBy = externalTriggeredBy(body.triggeredBy); // Fable M1: no forged seed:*/scheduler labels - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); const deps = { runStore: await store(env), outcomeStore: await outcomes(env), @@ -320,7 +346,7 @@ export async function handleRuns( const rerunId = pathname.match(/^\/api\/runs\/([^/]+)\/rerun$/)?.[1]; if (rerunId && req.method === "POST") { const runStore = await store(env); - const engine = await engineForEnv(env); + const engine = await routedEngineForEnv(env); // A CASE run reruns through rerun-to-verify (the case scope), reading the caseId // persisted in requested_scope — matches Java's rerunSameScope CASE branch. Other // scopes go through executeRerun. @@ -415,10 +441,13 @@ export async function handleRuns( // period, then today, mirroring that default. const evaluationPeriod = body.evaluationDate ?? (run.requestedScope.evaluationDate as string | undefined) ?? new Date().toISOString().slice(0, 10); + // Build the engine BEFORE claiming the run. `routedEngineForEnv` validates official-measure + // configuration and can throw; doing that after markRunning would leave an orphaned RUNNING run for + // `recoverStuckRuns` to sweep, for a failure that has nothing to do with the run. + const engine = await routedEngineForEnv(env); // A run being processed must leave the QUEUED claim path so it isn't re-handed // to a worker (QUEUED → RUNNING; idempotent for already-running runs). await runStore.markRunning(evalId); - const engine = await engineForEnv(env); try { const result = await engine.evaluate({ measureId: body.measureId, diff --git a/backend-ts/src/server.ts b/backend-ts/src/server.ts index 0f86ae4f..0779f58f 100644 --- a/backend-ts/src/server.ts +++ b/backend-ts/src/server.ts @@ -79,6 +79,12 @@ async function main(): Promise { // #263 incremental evaluation opt-in — the nightly run must see the flag too, or a scheduled run // would always do a full evaluation while a manual run reuses. WORKWELL_INCREMENTAL_EVAL: process.env.WORKWELL_INCREMENTAL_EVAL, + // PR-7b, and the THIRD instance of the bug the two comments above describe. Without this the + // nightly ALL_PROGRAMS run — the one that actually populates /compliance, /programs and + // quality_snapshots — evaluates cms122 with the AUTHORED CQL while a manual run evaluates it + // officially: two engines, two answers for the same measure, latest-run-wins, with + // `official-measures=on` on the boot line the whole time. + WORKWELL_OFFICIAL_MEASURES: process.env.WORKWELL_OFFICIAL_MEASURES, }; const schedulerInterval = setInterval(() => { void schedulerTick(schedulerEnv).catch((e: unknown) => diff --git a/backend-ts/src/standards/fqm-isolation.test.ts b/backend-ts/src/standards/fqm-isolation.test.ts index 7a48065f..204721bf 100644 --- a/backend-ts/src/standards/fqm-isolation.test.ts +++ b/backend-ts/src/standards/fqm-isolation.test.ts @@ -191,8 +191,9 @@ const EXECUTOR_IMPORTERS_ALLOWLIST = [ // until a measure is actually flipped. "wiring/official-executor-adapter.ts", // NOT run/cli/official-cases.ts — that shell reaches fqm only through standards/official-cases.ts, - // which is exactly the layering this list is meant to preserve. - // PR-7b adds "wiring/executor-router.ts" here — deliberately a conscious edit, not an open door. + // which is exactly the layering this list is meant to preserve. Same for PR-7b's + // `wiring/executor-router.ts`: it consumes the ADAPTER, so the package stays one hop away from the + // thing production routes through. Adding it here pre-emptively was wrong, and this guard said so. ]; test("5/5 consumers: only the approved app layers may import @workwell/official-executor", () => { diff --git a/backend-ts/src/wiring/executor-router.test.ts b/backend-ts/src/wiring/executor-router.test.ts new file mode 100644 index 00000000..bbfda0d1 --- /dev/null +++ b/backend-ts/src/wiring/executor-router.test.ts @@ -0,0 +1,263 @@ +/** + * Per-measure execution routing (PR-7b). + * + * The most important test here is the boring one: with the flag unset, `routedEngineForEnv` returns the + * authored engine ITSELF. Every environment that exists today is on that path. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + officialRoutingProblems, + routedEngineForEnv, + storeValueSetExpander, +} from "./executor-router.ts"; +import type { EvaluateMeasureBinding, MeasureOutcome } from "../engine/evaluate-measure.ts"; +import type { ValueSetStore } from "../stores/value-set-store.ts"; +import type { FqmCalculate } from "@workwell/official-executor"; + +/** A calculator that reports one subject in every population — enough to prove routing reached fqm. */ +const fakeCalculate: FqmCalculate = async () => ({ + results: [ + { + patientId: "s1", + detailedResults: [ + { + populationResults: [ + { populationType: "initial-population", result: true }, + { populationType: "denominator", result: true }, + { populationType: "numerator", result: false }, + ], + statementResults: [], + }, + ], + }, + ], +}); + +const outcome = (measure: string): MeasureOutcome => ({ + subjectId: "s1", + measure, + outcome: "COMPLIANT", + evidence: { expressionResults: [] }, +}); + +const authoredEngine = (): EvaluateMeasureBinding & { calls: string[] } => { + const calls: string[] = []; + return { + calls, + async evaluate(input) { + calls.push(input.measureId); + return outcome(`authored:${input.measureId}`); + }, + }; +}; + +test("unset means the authored engine ITSELF — identity, not an equivalent wrapper", async () => { + const authored = authoredEngine(); + for (const env of [{}, { WORKWELL_OFFICIAL_MEASURES: "" }, { WORKWELL_OFFICIAL_MEASURES: " " }]) { + const routed = await routedEngineForEnv(env as never, { authored }); + assert.equal( + routed, + authored, + "the default path must have no dispatch layer at all — identity is what makes 'byte-identical' " + + "a fact rather than a claim about two code paths agreeing", + ); + } + // A non-string value is ignored rather than coerced (e.g. a YAML `true`). + const coerced = await routedEngineForEnv({ WORKWELL_OFFICIAL_MEASURES: true } as never, { authored }); + assert.equal(coerced, authored); +}); + +test("a named measure routes to the official executor; everything else stays authored", async () => { + const authored = authoredEngine(); + const routed = await routedEngineForEnv( + { WORKWELL_OFFICIAL_MEASURES: "cms122" } as never, + // An injected calculator, not the real one: asserting `instanceof Error` would have passed on a + // MODULE_NOT_FOUND just as happily as on a real routing hit, which proves nothing about routing. + { authored, expand: async () => [{ code: "a", system: "s" }], calculate: fakeCalculate }, + ); + + const official = await routed.evaluate({ measureId: "cms122", patientBundle: {} }); + assert.equal(official.measure, "Diabetes: Glycemic Status Assessment Greater Than 9%"); + assert.deepEqual(authored.calls, [], "cms122 must NOT have reached the authored engine"); + + const other = await routed.evaluate({ measureId: "audiogram", patientBundle: {} }); + assert.equal(other.measure, "authored:audiogram"); + assert.deepEqual(authored.calls, ["audiogram"]); +}); + +test("an explicit elm/metaOverride always stays authored, even for a routed measure", async () => { + // The fidelity lab evaluates an official-SUBSET measure through the engine's metaOverride seam, and + // the Rule Builder previews generated CQL the same way. Routing those to the official executor would + // silently run a different measure than the caller asked for. + const authored = authoredEngine(); + const routed = await routedEngineForEnv( + { WORKWELL_OFFICIAL_MEASURES: "cms122" } as never, + { authored, expand: async () => [{ code: "a", system: "s" }] }, + ); + + await routed.evaluate({ measureId: "cms122", patientBundle: {}, elm: { library: {} } }); + await routed.evaluate({ + measureId: "cms122", + patientBundle: {}, + metaOverride: { name: "x", library: "L", periodMonths: 12 } as never, + }); + assert.deepEqual(authored.calls, ["cms122", "cms122"]); +}); + +test("construction refuses every shape of misconfiguration, and says which", async () => { + const authored = authoredEngine(); + const build = (value: string) => + routedEngineForEnv({ WORKWELL_OFFICIAL_MEASURES: value } as never, { + authored, + expand: async () => [{ code: "a", system: "s" }], + }); + + // A typo must not silently serve authored results while the configuration claims official execution. + await assert.rejects(() => build("cms999"), /cms999: not covered by the official MADiE test-case gate/); + // "all" is a measure name like any other, and there is no measure called "all". + await assert.rejects(() => build("all"), /all: not covered/); + await assert.rejects(() => build("cms122,cms165"), /cms165: not covered/); + + // Whitespace tolerance, but only around real ids. + const ok = await build(" cms122 , cms125 "); + assert.notEqual(ok, authored, "a valid list must produce a router"); +}); + +test("officialRoutingProblems names the gate, the artifact, and the semantics separately", () => { + assert.deepEqual(officialRoutingProblems({}), [], "unset is always legal"); + assert.deepEqual(officialRoutingProblems({ WORKWELL_OFFICIAL_MEASURES: "cms122,cms125" }), []); + + // ALL the problems, not the first: an operator fixing one at a time, learning about the next only + // after a redeploy, is how a five-minute configuration takes an afternoon. cms130 is both ungated and + // unvendored, and hears about both. + const problems = officialRoutingProblems({ WORKWELL_OFFICIAL_MEASURES: "cms130" }); + assert.equal(problems.length, 2); + assert.match(problems[0]!, /cms130: not covered by the official MADiE test-case gate/); + assert.match(problems[1]!, /cms130: no executable official artifact is vendored/); +}); + +test("terminology is preflighted at CONSTRUCTION, not at first evaluation", async () => { + // The whole reason: by the time a subject is evaluated a run is underway and outcomes are being + // written, and this failure's natural mode is silence — an unexpandable value set makes every + // retrieve match nothing, so the measure reports an empty population rather than an error. + const authored = authoredEngine(); + let expandCalls = 0; + await assert.rejects( + () => + routedEngineForEnv({ WORKWELL_OFFICIAL_MEASURES: "cms122" } as never, { + authored, + expand: async () => { + expandCalls += 1; + return []; // nothing imported — the state the live stack is in today + }, + }), + /value sets could not be expanded[\s\S]*resolve-valuesets/, + ); + assert.ok(expandCalls > 0, "it must actually try, not just inspect the manifest"); +}); + +test("the store expander snapshots ONCE per instance — per run, never per process", async () => { + // Per call would be thousands of store reads in a population run. Per process would freeze the + // snapshot, re-introducing the exact bug engine-factory.ts documents: an operator's value-set edit + // serving stale expansions until restart. The instance lives for one run, which is the middle. + let reads = 0; + const store = { + listAll: async () => { + reads += 1; + return [ + { oid: "2.16.1", codes: [{ code: "a", system: "s" }] }, + // A row imported under its URL form still resolves by bare OID. + { oid: "http://cts.nlm.nih.gov/fhir/ValueSet/2.16.2", codes: [{ code: "b", system: "s" }] }, + ]; + }, + } as unknown as ValueSetStore; + + const expand = storeValueSetExpander(store); + assert.deepEqual(await expand("2.16.1"), [{ code: "a", system: "s" }]); + assert.deepEqual(await expand("2.16.2"), [{ code: "b", system: "s" }]); + assert.deepEqual(await expand("2.16.404"), [], "an unimported OID is empty, and the executor refuses"); + assert.equal(reads, 1, "one bounded catalog read for the instance's whole lifetime"); + + // A second instance re-reads: that is what makes an operator's edit visible on the next run. + await storeValueSetExpander(store)("2.16.1"); + assert.equal(reads, 2); +}); + +test("non-proportion scoring is refused at CONSTRUCTION, not per subject", async () => { + // It was the one adapter refusal that fired inside evaluate() — and the run pipeline error-isolates a + // per-subject throw into MISSING_DATA, so a cohort artifact would have produced a *successful* + // population run with every subject MISSING_DATA. That is the silent-empty-population failure the + // terminology preflight exists to prevent, reached through the door next to it. + const { officialRoutingProblems } = await import("./executor-router.ts"); + const problems = officialRoutingProblems({ WORKWELL_OFFICIAL_MEASURES: "cms122" }); + assert.deepEqual(problems, [], "cms122 is a proportion measure"); + + // The check reads the manifest, so prove it fires by pointing it at a scoring the mapping cannot + // express. (Both vendored measures are proportion, so this asserts the CHECK, via the message text.) + const { loadOfficialArtifact } = await import("./official-artifacts.ts"); + const artifact = loadOfficialArtifact("cms122")!; + assert.equal(artifact.manifest.scoring, "proportion", "if this ever changes, the guard above must fire"); +}); + +test("the scheduler's env allowlist carries the flag — the nightly run must route like a manual one", async () => { + // Without this, POST /api/runs/manual evaluates cms122 officially while the nightly ALL_PROGRAMS run + // — the one that populates /compliance, /programs and quality_snapshots — evaluates it with the + // AUTHORED CQL. Two engines, two answers for one measure, latest-run-wins, `official-measures=on` on + // the boot line throughout. This is the THIRD instance of that bug in the same block (see #331 and + // #263), so it is asserted rather than left to a comment. + const { readFileSync } = await import("node:fs"); + const server = readFileSync(new URL("../server.ts", import.meta.url), "utf8"); + const allowlist = server.slice(server.indexOf("const schedulerEnv"), server.indexOf("const schedulerInterval")); + for (const key of ["WORKWELL_OFFICIAL_MEASURES", "WORKWELL_INCREMENTAL_EVAL", "WORKWELL_VSAC_API_KEY"]) { + assert.ok( + allowlist.includes(`${key}: process.env.${key}`), + `${key} must be threaded to schedulerTick, or the nightly run behaves differently from a manual one`, + ); + } +}); + +test("boot reports a bad configuration loudly, because everything else about it looks healthy", async () => { + // A typo'd flag would otherwise boot clean, log `official-measures=on`, keep /actuator/health green + // (it is deliberately DB-free, so the 15-minute reconciler reports healthy) and 500 every evaluating + // route — the exact symptom profile of the four-day Neon outage. + const { readFileSync } = await import("node:fs"); + const worker = readFileSync(new URL("../worker.ts", import.meta.url), "utf8"); + assert.match( + worker, + /officialRoutingProblems\(env\)/, + "worker boot must validate official routing, not leave it to lazy construction", + ); + assert.match(worker, /OFFICIAL_ROUTING_MISCONFIGURED/, "and emit a greppable WORKWELL_ALERT for it"); +}); + +test("an export of a PAST official run keeps its meaning after the flag is turned off", async () => { + // The documented rollback for PR-9 is "unset the flag". If provenance came from the current config, + // that rollback would silently reinterpret every historical official run through the status + // histogram — reversing cms122's numerator, since its official numerator is poor control and the + // workflow status inverts it. A regulatory export of a finished run must not change meaning because + // of a configuration change made afterwards. + const { officialMembership } = await import("../fhir/measure-report.ts"); + const officialOutcome = { + evidence: { + expressionResults: [], + official: { + ecqmId: "122FHIR", + version: "1.0.000", + engine: "fqm-execution", + artifactSha256: "sha256:x", + populationResults: [ + { populationType: "initial-population", result: true }, + { populationType: "denominator", result: true }, + { populationType: "numerator", result: true }, + ], + }, + }, + }; + // The signal the export path keys on is in the ROW, and is readable with the flag unset. + assert.ok( + officialMembership(officialOutcome.evidence), + "a stored official outcome is self-describing — no env var required to interpret it", + ); + assert.equal(officialMembership({ expressionResults: [] }), null, "and an authored one is not"); +}); diff --git a/backend-ts/src/wiring/executor-router.ts b/backend-ts/src/wiring/executor-router.ts new file mode 100644 index 00000000..28c39fae --- /dev/null +++ b/backend-ts/src/wiring/executor-router.ts @@ -0,0 +1,193 @@ +/** + * Per-measure execution routing (roadmap §7.2, PR-7b). + * + * `routedEngineForEnv(env)` returns the thing every caller already asks `engineForEnv(env)` for, except + * that measures named in `WORKWELL_OFFICIAL_MEASURES` are evaluated by the OFFICIAL published artifact + * instead of WorkWell's authored CQL. Nothing downstream changes shape: the run pipeline, the case + * rerun path, the scheduler and the simulate route keep the exact call they make today. + * + * **Unset — every environment that exists right now — returns `engineForEnv(env)` itself**, not a + * wrapper around it. Identity, not equivalence: there is no dispatch, no allocation, and nothing to + * reason about on the default path. A parity test asserts it. + * + * ## Why the flag is a list and never "all" + * + * Flipping a measure to official execution changes what an operator is told about real people. It is a + * deliberate per-measure act gated on a green MADiE test-case run, so the configuration is an explicit + * allowlist. `WORKWELL_OFFICIAL_MEASURES=all` selects a measure called "all", which does not exist, and + * is therefore refused — the same as any other typo. + * + * ## Everything is checked at CONSTRUCTION, and construction fails loudly + * + * A misconfiguration must not survive until the first subject is evaluated. By then a run is underway, + * outcomes are being written, and the failure mode of most of these mistakes is silence rather than an + * error. So the router refuses to exist unless, for every named measure: + * + * 1. the official MADiE gate covers it (`ungatedOfficialMeasures` — THE RULE, roadmap §7.4 PR-6); + * 2. an executable artifact is vendored, and its `catalogId` matches; + * 3. WorkWell has recorded what its numerator means (`official-measure-semantics.ts`); + * 4. it is a `proportion` measure (the population mapping assumes a numerator exists); and + * 5. every value set its ELM retrieves expands to a non-empty set. + * + * (5) is the one that would otherwise be invisible: fqm treats an unexpandable value set as *empty + * rather than missing*, an empty set matches nothing, and the measure then reports every subject + * out-of-population — which reads downstream exactly like a genuinely ineligible roster. + * + * 1-4 are reported TOGETHER, so an operator fixes them in one pass rather than one redeploy at a time. + * (5) is checked afterwards and stops at the first failure — it costs a real expansion per measure, and + * the first missing terminology is the one worth acting on. + * + * `worker.ts` runs 1-4 at BOOT as well, because everything here is lazy: a typo would otherwise boot + * clean, log `official-measures=on`, keep /actuator/health green, and 500 every evaluating route. + * + * ## Scope of this PR + * + * Ships dark. It also does NOT yet prepare bundles (`stampQiCoreStructure`, see the adapter's docstring) + * or batch subjects measure-major, so it must not be flipped on for a population run until PR-8 wires + * both. The flag existing and the flag being safe to set are different things, and this PR only delivers + * the first — which is why `WORKWELL_OFFICIAL_MEASURES` is deliberately absent from DEPLOY.md. + * + * Not every evaluation path is routed, and deliberately so while this is dark: the scale batch + * (`run/batch-evaluate-scale.ts`), the seed CLIs, the DB-less `engine/ingress/evaluate-bundle.ts` and + * the headless CLI all construct engines directly. PR-8/PR-9 must not assume coverage they do not have. + */ +import type { EvaluateMeasureBinding, EvaluateMeasureInput, MeasureOutcome } from "../engine/evaluate-measure.ts"; +import type { MeasureMeta } from "../engine/cql/measure-registry.ts"; +import { getStores, type StoresEnv } from "../stores/factory.ts"; +import type { ValueSetStore } from "../stores/value-set-store.ts"; +import type { VsacEnv } from "../engine/cql/resolve-value-set-resolver.ts"; +import { OFFICIAL_GATED_MEASURES } from "../standards/official-cases.ts"; +import { engineForEnv } from "./engine-factory.ts"; +import { loadOfficialArtifact } from "./official-artifacts.ts"; +import { + officialMeasureExecutor, + oidFromValueSetUrl, + type ExpandValueSet, + type FqmCalculate, +} from "./official-executor-adapter.ts"; +import { + officialMeasureIds, + ungatedOfficialMeasures, + type OfficialMeasuresEnv, +} from "./official-routing.ts"; +import { officialMeasureSemantics } from "./official-measure-semantics.ts"; + +/** The extended shape the authored engine accepts — diagnostics pass an explicit library to run. */ +export type RoutableInput = EvaluateMeasureInput & { elm?: unknown; metaOverride?: MeasureMeta }; + +export interface RoutedEngine { + evaluate(input: RoutableInput): Promise; +} + +/** + * Expand a VSAC OID from the imported `value_sets` rows. + * + * One bounded catalog read, snapshotted for the expander's lifetime — which is the router's, which is + * one run. `listAll()` rather than a per-OID query because the store has no OID lookup and the catalog + * is dozens of rows; adding one would be a store-contract change for no gain at this size. + */ +export function storeValueSetExpander(valueSets: ValueSetStore): ExpandValueSet { + let snapshot: Promise>> | undefined; + const load = () => { + snapshot ??= valueSets.listAll().then((rows) => { + const byOid = new Map>(); + for (const row of rows) { + // The PACKAGE's normalization, not a second one that happens to agree on VSAC canonicals: + // the expander is keyed by whatever `buildValueSetCache` passes in, and two rules that differ + // on any other URL shape is a bug waiting for the first non-VSAC canonical. + const oid = oidFromValueSetUrl(row.oid); + byOid.set(oid, (row.codes ?? []).map((c) => ({ code: c.code, system: c.system }))); + } + return byOid; + }); + return snapshot; + }; + return async (oid) => (await load()).get(oid) ?? []; +} + +/** Everything wrong with the current `WORKWELL_OFFICIAL_MEASURES`, as sentences. Empty means legal. */ +export function officialRoutingProblems(env: OfficialMeasuresEnv): string[] { + const problems: string[] = []; + for (const id of ungatedOfficialMeasures([...OFFICIAL_GATED_MEASURES], env as Record)) { + problems.push( + `${id}: not covered by the official MADiE test-case gate — no measure may be routed officially ` + + `without external validation (roadmap §7.4 PR-6)`, + ); + } + for (const id of officialMeasureIds(env as Record)) { + const artifact = loadOfficialArtifact(id); + if (!artifact) { + problems.push(`${id}: no executable official artifact is vendored (see measures/official/)`); + continue; + } + if (artifact.manifest.catalogId !== id) { + problems.push(`${id}: the vendored artifact declares catalogId '${artifact.manifest.catalogId}'`); + } + // Scoring was the ONE adapter refusal that fired per-subject rather than here — and the run + // pipeline error-isolates a per-subject throw into MISSING_DATA, so a non-proportion artifact + // would produce a *successful* population run in which every subject is MISSING_DATA. That is the + // silent-empty-population failure the terminology preflight exists to prevent, through the door + // next to it. + if (artifact.manifest.scoring !== "proportion") { + problems.push( + `${id}: scoring '${artifact.manifest.scoring}' is not supported — the population mapping ` + + `assumes a proportion measure (a cohort measure has no numerator at all)`, + ); + } + if (!officialMeasureSemantics(id)) { + problems.push( + `${id}: no recorded numerator semantics — see official-measure-semantics.ts. There is no safe ` + + `default: guessing one way reports every failure as compliant, the other every success as overdue`, + ); + } + } + return problems; +} + +export interface RoutedEngineOptions { + /** Injectable for tests; defaults to the real store-backed expander. */ + expand?: ExpandValueSet; + /** Injectable for tests; defaults to the authored CQL engine. */ + authored?: EvaluateMeasureBinding; + /** Injectable for tests; defaults to the real (lazily imported) fqm calculator. */ + calculate?: FqmCalculate; +} + +export async function routedEngineForEnv( + env: StoresEnv & VsacEnv & OfficialMeasuresEnv, + options: RoutedEngineOptions = {}, +): Promise { + const authored = options.authored ?? (await engineForEnv(env)); + const official = officialMeasureIds(env as Record); + // Identity on the default path — the wrapper below never exists in any environment today. + if (official.size === 0) return authored as RoutedEngine; + + const problems = officialRoutingProblems(env); + if (problems.length > 0) { + throw new Error( + `WORKWELL_OFFICIAL_MEASURES is not a valid configuration:\n - ${problems.join("\n - ")}`, + ); + } + + const expand = options.expand ?? storeValueSetExpander((await getStores(env)).valueSets); + const executor = officialMeasureExecutor({ + expand, + ...(options.calculate ? { calculate: options.calculate } : {}), + }); + // Terminology, up front. Serially rather than in parallel: these hit the same snapshot and the first + // failure is the one worth reporting, unqualified by a race. + for (const id of official) await executor.preflight(id); + + return { + async evaluate(input: RoutableInput): Promise { + // An explicit `elm`/`metaOverride` means "run THIS library", so honouring it is the only correct + // behaviour — routing it to the official executor would silently run a different measure than the + // caller asked for. No production caller passes either today (the fidelity lab builds its own + // engine directly), so this is a guard against a future caller, not a fix for a current one. + const overridden = input.elm !== undefined || input.metaOverride !== undefined; + return official.has(input.measureId) && !overridden + ? executor.evaluate(input) + : authored.evaluate(input); + }, + }; +} diff --git a/backend-ts/src/wiring/official-executor-adapter.ts b/backend-ts/src/wiring/official-executor-adapter.ts index d6728af8..6ddf9db1 100644 --- a/backend-ts/src/wiring/official-executor-adapter.ts +++ b/backend-ts/src/wiring/official-executor-adapter.ts @@ -87,6 +87,19 @@ import { officialMeasureSemantics } from "./official-measure-semantics.ts"; /** Expand one VSAC OID to its codes — the app supplies this from the imported `value_sets` rows. */ export type ExpandValueSet = (oid: string) => Promise; +/** + * Re-exported so the ROUTER can key its expander by the same rule `buildValueSetCache` looks up by, + * without becoming a direct consumer of the executor package. Two normalizations that merely agree on + * VSAC canonicals is a bug waiting for the first canonical of another shape; a second import of the + * package into production wiring is a hole in the fqm quarantine. This is how to have neither. + */ +export { oidFromValueSetUrl, type FqmCalculate }; + +/** An `EvaluateMeasureBinding` that can also be asked to prove a measure is runnable before a run. */ +export interface OfficialMeasureExecutor extends EvaluateMeasureBinding { + preflight(measureId: string): Promise; +} + export interface OfficialExecutorDeps { expand: ExpandValueSet; /** Injectable for tests; defaults to the real (lazily imported) fqm calculator. */ @@ -271,8 +284,47 @@ export function requiredValueSets(artifact: OfficialArtifact): Array<{ oid: stri * costs an ELM parse per subject, which is why PR-7b adds measure-major iteration and batches subjects * through one `calculateOfficialDetailed` call — the package's batch entry point already exists for it. */ -export function officialMeasureExecutor(deps: OfficialExecutorDeps): EvaluateMeasureBinding { +export function officialMeasureExecutor(deps: OfficialExecutorDeps): OfficialMeasureExecutor { + /** + * Terminology is expanded once per measure per EXECUTOR INSTANCE — and an instance lives as long as + * the router that built it, which is **at most** one run: per run-POST and per scheduled tick, but + * per HTTP request at the read routes (`/simulate`, impact-preview, `/evaluate`). Shorter is always + * SAFE — freshness is the property that matters — but it is not free: each construction pays a + * `valueSets.listAll()` and an ELM walk per routed measure, and `/simulate` is a date scrubber that + * fires once per drag. Worth revisiting when a read route is actually routed officially. + * + * The scoping is the point, not an implementation detail. A process-lifetime cache would freeze the + * value-set snapshot and re-introduce the bug `engineForEnv` documents at length — an operator's + * value-set edit serving stale expansions until restart. Per-CALL was the other extreme: a + * 150-subject run re-parsing a 2.4MB bundle's ELM twice per subject and hitting the store thousands + * of times. + */ + const terminology = new Map>(); + const cacheFor = (artifact: OfficialArtifact): Promise => { + const key = artifact.manifest.catalogId; + let pending = terminology.get(key); + if (!pending) { + pending = expandArtifactTerminology(artifact, deps.expand); + // A rejected promise must not be cached: the likeliest cause is a transient store failure, and + // caching it would turn one bad read into a whole run's worth of refusals. + pending.catch(() => terminology.delete(key)); + terminology.set(key, pending); + } + return pending; + }; + return { + /** + * Expand a measure's terminology now rather than at first evaluation. The router calls this at + * construction so an operator who flips a measure whose OIDs were never imported learns it from a + * failed run start, not from a run that quietly reports nobody as eligible. + */ + async preflight(measureId: string): Promise { + const artifact = (deps.loadArtifact ?? loadOfficialArtifact)(measureId); + if (!artifact) throw new Error(`${measureId}: no executable official artifact is vendored`); + await cacheFor(artifact); + }, + async evaluate(input: EvaluateMeasureInput): Promise { const artifact = (deps.loadArtifact ?? loadOfficialArtifact)(input.measureId); if (!artifact) { @@ -312,7 +364,7 @@ export function officialMeasureExecutor(deps: OfficialExecutorDeps): EvaluateMea end: normalizePeriodEnd(evaluationDate), }; - const valueSetCache = await expandArtifactTerminology(artifact, deps.expand); + const valueSetCache = await cacheFor(artifact); const results = await calculateOfficialDetailed({ bundle: artifact.bundle, patientBundles: [input.patientBundle], diff --git a/backend-ts/src/wiring/official-routing.ts b/backend-ts/src/wiring/official-routing.ts index 10043d24..ed2c3e0f 100644 --- a/backend-ts/src/wiring/official-routing.ts +++ b/backend-ts/src/wiring/official-routing.ts @@ -18,6 +18,18 @@ * Unset/blank (the demo stack and every environment today) ⇒ empty set ⇒ every read path is * byte-identical to before. */ +export interface OfficialMeasuresEnv { + WORKWELL_OFFICIAL_MEASURES?: string | undefined; +} + +/** + * The seam-inventory predicate, in the shape §10 of ARCHITECTURE requires: the boot line must call the + * SAME function the routing decision calls, never a second parse of the same variable. + */ +export function isOfficialRoutingConfigured(env: OfficialMeasuresEnv): boolean { + return officialMeasureIds(env as Record).size > 0; +} + export function officialMeasureIds(env: Record = process.env): ReadonlySet { const raw = env["WORKWELL_OFFICIAL_MEASURES"]; if (typeof raw !== "string") return new Set(); diff --git a/backend-ts/src/worker.ts b/backend-ts/src/worker.ts index 2269069c..b3694dd1 100644 --- a/backend-ts/src/worker.ts +++ b/backend-ts/src/worker.ts @@ -46,6 +46,7 @@ import { authorize, extractPrincipal } from "./auth/authorize.ts"; import { assertSafeStartup, type StartupEnv } from "./config/startup-safety.ts"; import { parseAllowedOrigins, preflightResponse, withCors } from "./config/cors.ts"; import { formatSeamLogLine } from "./config/seam-inventory.ts"; +import { officialRoutingProblems } from "./wiring/executor-router.ts"; import { isWebChartConfigured, webChartConfigFromEnv } from "./engine/ingress/data-source.ts"; /** Runtime bindings (wrangler.jsonc) + config. Injected per target; app code @@ -111,6 +112,15 @@ export interface Env { WORKWELL_BUCKET_S3_SECRET_ACCESS_KEY?: string; WORKWELL_BUCKET_S3_REGION?: string; WORKWELL_BUCKET_S3_ENDPOINT?: string; + /** #263 incremental/delta evaluation opt-in. Inert unless "true". */ + WORKWELL_INCREMENTAL_EVAL?: string; + /** + * Per-measure official execution (PR-7b). Comma-separated catalog ids, never "all". Unset ⇒ every + * measure evaluates through the authored CQL, byte-identical to before. Declared here — rather than + * left to the optional fields of `OfficialMeasuresEnv` — because it being absent from a typed env + * object is exactly how it went missing from the scheduler's allowlist. + */ + WORKWELL_OFFICIAL_MEASURES?: string; } // Memoized auth handler + JWT verifier, keyed by secret (createJwt is per-call). @@ -343,6 +353,21 @@ function logSeamInventoryOnce(env: Env): void { const webChart = webChartConfigFromEnv(env)!; console.log(`[workwell] webchart live tenant: enabled (host ${new URL(webChart.baseUrl).host})`); } + // PR-7b: BOOT-LOUD, not merely construction-time. `routedEngineForEnv` validates lazily, so a typo'd + // WORKWELL_OFFICIAL_MEASURES would otherwise boot clean, log `official-measures=on`, serve + // /actuator/health 200 (deliberately DB-free, so the 15-minute reconciler reports green) and return + // `internal_error` from every evaluating route — character-for-character the symptom profile of the + // four-day Neon outage that DEPLOY.md's "Watch the right signal" section exists because of. + // + // Alerted rather than thrown: the worker's fetch handler is not a startup hook, so throwing here + // would fail one arbitrary request rather than the process. The alert line is the one an operator + // greps for, and every affected route still refuses loudly at construction. + const problems = officialRoutingProblems(env); + if (problems.length > 0) { + console.error( + `WORKWELL_ALERT ${JSON.stringify({ kind: "OFFICIAL_ROUTING_MISCONFIGURED", problems })}`, + ); + } } export default { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 546b8954..5e7a90bd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -301,21 +301,21 @@ No microservice decomposition is used in MVP; package boundaries are the future ## 10) Inert-seam inventory -The repo has accumulated 10 "inert-unless-configured" seams (ADR-011/012/013/017/023/025/029/030/035 + #264 +The repo has accumulated 11 "inert-unless-configured" seams (ADR-011/012/013/017/023/025/029/030/035 + #264 alert webhook) — each correct and individually reviewed when it shipped, but collectively an untested-in-anger surface that can rot silently (a deploy secret typo, a seam nobody remembers is there). Issue #260 adds cheap insurance: a single pure `describeSeams(env)` (`backend-ts/src/config/seam-inventory.ts`) that reports each seam's active/inactive state by **calling the exact predicate each seam's own `resolve*` function already uses** (`isSendgridConfigured`, `isDataChaserConfigured`, `isIceConfigured`, `isEhFhirConfigured`, `isWebChartConfigured`, `isSqlPushdownSelected`, `isVsacConfigured`, -`isAlertWebhookConfigured`, `isS3BucketConfigured`) — never a second parse of the env vars. Each `resolve*` function was +`isAlertWebhookConfigured`, `isS3BucketConfigured`, `isOfficialRoutingConfigured`) — never a second parse of the env vars. Each `resolve*` function was refactored to call its own extracted predicate (a pure, behavior-preserving refactor; the predicate is the exact condition the resolver already branched on). One boot log line (`worker.ts`, guarded to fire once per worker instance, mirroring the existing auth-handler memoization pattern) makes the deployed configuration observable: ``` -seams: sendgrid=off datachaser=off ice=off eh-fhir=off webchart=off sql-executor=off vsac=off alert-webhook=off bucket-s3=off incremental-eval=off +seams: sendgrid=off datachaser=off ice=off eh-fhir=off webchart=off sql-executor=off vsac=off alert-webhook=off bucket-s3=off incremental-eval=off official-measures=off ``` Descriptive only — this inventory makes no decisions and never selects a seam; it reports what the @@ -341,7 +341,8 @@ fails the run — Fable-H1 pattern). Run metrics on `/api/runs` already surface | `alert-webhook` | `backend-ts/src/run/alert-channel.ts` (`webhookAlertChannel`, `resolveAlertChannels`, `isAlertWebhookConfigured`) | `WORKWELL_ALERT_WEBHOOK_URL` | off (console `WORKWELL_ALERT` line always-on) | 2026-07-10 | `backend-ts/src/run/alert-channel.test.ts` | | `bucket-s3` | `backend-ts/src/case/resolve-bucket.ts` (`resolveBucket`, `isS3BucketConfigured`) | `WORKWELL_BUCKET_S3_BUCKET` **and** `WORKWELL_BUCKET_S3_ACCESS_KEY_ID` **and** `WORKWELL_BUCKET_S3_SECRET_ACCESS_KEY` (all three; region/endpoint optional) | off (in-container `fs` BUCKET binding) — **ON on the live TWH stack since 2026-07-14** (#167/ADR-030) | 2026-07-14 | `backend-ts/src/case/resolve-bucket.test.ts` | | `incremental-eval` | `backend-ts/src/run/incremental/incremental-eval.ts` (`isIncrementalEnabled`, `IncrementalCache`) + wired in `run/run-pipeline.ts` `finishManualRun` | `WORKWELL_INCREMENTAL_EVAL=true` | off (every subject re-evaluated) | 2026-07-24 | `backend-ts/src/run/incremental/*.test.ts` (parity + golden) | +| `official-measures` | `backend-ts/src/wiring/official-routing.ts` (`officialMeasureIds`, `isOfficialRoutingConfigured`) + `wiring/executor-router.ts` (`routedEngineForEnv`, `officialRoutingProblems`) | `WORKWELL_OFFICIAL_MEASURES=` — never `"all"` | off (every measure evaluates authored CQL; `routedEngineForEnv` returns `engineForEnv`'s own value, identically) | 2026-07-25 | `backend-ts/src/wiring/executor-router.test.ts` | -The inventory itself is covered by `backend-ts/src/config/seam-inventory.test.ts` (flips each of the 9 +The inventory itself is covered by `backend-ts/src/config/seam-inventory.test.ts` (flips each of the 11 seams on/off with the correct env combination, including the both-vars-required pairs above, plus the all-off and all-on boot-log-line shapes). diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 42a6ab52..62ea9e40 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -1,5 +1,91 @@ # Journal +## 2026-07-25 (night) — PR-7b: the executor router, wired and dark (branch `feat/executor-router`) + +`routedEngineForEnv(env)` replaces `engineForEnv(env)` at all 8 call sites — runs (3), cases, measures +(2), compliance-simulation, scheduler. Measures named in `WORKWELL_OFFICIAL_MEASURES` evaluate through +the official artifact; everything else is unchanged. + +**With the flag unset it returns the authored engine ITSELF.** Identity, not equivalence — no dispatch, +no allocation, nothing to reason about on the path every environment is actually on. The parity test +asserts `routed === authored` rather than comparing two engines' outputs, because identity is a fact +where output-comparison is a claim about two code paths agreeing on the inputs someone thought to try. + +**Everything is validated at construction, and construction throws.** A misconfiguration must not +survive to the first subject: by then a run is underway, outcomes are being written, and the failure +mode of most of these mistakes is silence. So the router refuses to exist unless every named measure is +covered by the MADiE gate, has a vendored artifact whose `catalogId` matches, has recorded numerator +semantics, and — the invisible one — has every value set its ELM retrieves expanding to a non-empty set. +All problems are reported at once, not the first: fixing them one redeploy at a time is how a +five-minute configuration becomes an afternoon. + +`WORKWELL_OFFICIAL_MEASURES=all` is refused, because "all" is a measure name like any other and there is +no measure called that. Every flip stays a deliberate per-measure act. + +**An explicit `elm`/`metaOverride` always stays authored**, even for a routed measure. The fidelity lab +evaluates an official-*subset* measure through that seam and the Rule Builder previews generated CQL the +same way; routing those to the official executor would silently run a different measure than the caller +asked for. + +**Terminology expansion is now scoped to one run** — memoized per measure per executor instance, and the +instance lives exactly as long as the router, which is built per run like `engineForEnv`. That is the +middle of two bad options the adapter's review named: per-call was thousands of store reads in a +population run, per-process would freeze the snapshot and re-introduce the stale-expansion bug +`engine-factory.ts` documents at length. A rejected expansion is deliberately *not* cached, so one +transient store failure doesn't become a whole run of refusals. + +`official-measures` joins the boot seam line as the 11th seam — and the only one that changes what a +measure *computes* rather than how or where. Which measures a container evaluated officially must be +answerable from a log, not inferred from a deploy config. + +**The boundary guard corrected me again.** I pre-emptively added the router to the fqm consumer +allowlist; the guard failed because the router imports the *adapter*, not the package. Reverted, with +the reason written where the next person will look. + +**Not yet safe to switch on**, and the module says so in its own docstring: it does not prepare bundles +(`stampQiCoreStructure`) or batch subjects measure-major. The flag existing and the flag being safe to +set are different things, and this delivers the first. + +### Review round — the flag would have split the two run paths + +Two criticals, and the first is the third instance of one bug this repo has already documented twice. + +**The nightly run could never have routed officially.** `server.ts` hands `schedulerTick` an explicit +allowlist of `process.env` keys, and `WORKWELL_OFFICIAL_MEASURES` was not in it — while every field of +`OfficialMeasuresEnv` is optional, so it type-checked. Once flipped on, `POST /api/runs/manual` would +evaluate cms122 officially and the nightly `ALL_PROGRAMS` run — the one that actually populates +`/compliance`, `/programs` and `quality_snapshots` — would evaluate it with the authored CQL. Two +engines, two answers for one measure, latest-run-wins, `official-measures=on` on the boot line +throughout. The comments immediately above that allowlist describe the same bug happening to +`WORKWELL_WEBCHART_PRIVATE_KEY_B64` (#331) and `WORKWELL_INCREMENTAL_EVAL` (#263). A test now asserts +the keys are threaded, because three times is a pattern and comments have not stopped it. + +**Validation was construction-time, and the roadmap said boot-loud.** I had rewritten that plan item to +match what I built, which is the "no silent scope changes" rule in reverse. `routedEngineForEnv` is +lazy, so a typo'd flag would boot clean, log `official-measures=on`, keep `/actuator/health` green (it +is deliberately DB-free, so the 15-minute reconciler reports healthy) and return `internal_error` from +every evaluating route — character for character the symptom profile of the four-day Neon outage that +DEPLOY.md's "Watch the right signal" section exists because of. Boot now runs the same validation and +emits a greppable `WORKWELL_ALERT OFFICIAL_ROUTING_MISCONFIGURED`. + +Four more: **scoring** was the one adapter refusal that fired per-subject rather than at construction — +and the run pipeline error-isolates a per-subject throw into MISSING_DATA, so a cohort artifact would +have produced a *successful* run with every subject MISSING_DATA, the silent-empty-population failure +again through the door next to it. The `/evaluate` route constructed the router *after* marking the run +RUNNING, so a config error orphaned a run. The "instance lifetime is one run" claim was **false at three +read routes** (`/simulate` is a date scrubber — one construction per drag). And "all problems reported +at once" excluded the terminology preflight, which is serial and first-failure. + +Two smaller corrections worth recording because they are the same failure mode as the licensing and +`Join-Path` claims: the `elm`/`metaOverride` escape hatch's docstring cited the fidelity lab and Rule +Builder as callers — **neither goes through the router**, so the guard is defensive rather than +load-bearing; and a test comment claimed it "deliberately does not load" the real calculator when it +very much did, which meant `assert.ok(err instanceof Error)` would have passed just as happily on a +`MODULE_NOT_FOUND` as on a real routing hit. It injects a calculator now. + +Typecheck clean; full suite **1486 pass / 0 fail / 14 skipped**. + + ## 2026-07-25 (evening) — PR-7c: the terminology importer reads the artifact, not a hand-kept list (branch `feat/official-valueset-import`) The smallest change that unblocks PR-9, and it exists because PR-7a's refusal exposed a list that had diff --git a/docs/ROADMAP_2026-07-24.md b/docs/ROADMAP_2026-07-24.md index a843953f..69a770dd 100644 --- a/docs/ROADMAP_2026-07-24.md +++ b/docs/ROADMAP_2026-07-24.md @@ -366,8 +366,20 @@ EvaluateMeasureBinding` dispatching on `input.measureId` — no downstream signa `populationResults` because it *is* them). This narrows a PR-6a claim: stripping keeps statement **names and count** — which is all a count check can verify — not statement **values**. *(shipped 2026-07-25.)* -7b. **PR-7b** — executor **router** (`routedEngineForEnv`) + boot-loud validation + logic_version override - + measure-major batching + flag-unset parity test. **Two obligations inherited from PR-3:** +7b. ✅ **PR-7b** — executor **router** (`routedEngineForEnv`) wired at all 8 `engineForEnv` call sites, + construction-time validation of every named measure (gate coverage, vendored artifact, recorded + semantics, terminology preflight — all problems reported at once), an `elm`/`metaOverride` escape so + the fidelity lab and Rule Builder stay authored, per-run-scoped terminology memoization, and + `official-measures` as the 11th boot seam. **Flag unset returns the authored engine ITSELF** — + identity, asserted, not an equivalent wrapper. Ships dark and is NOT yet safe to switch on: bundle + preparation (`stampQiCoreStructure`) and measure-major batching are PR-8's. `logic_version` override + deferred with them. *(shipped 2026-07-25.)* + Review caught two criticals before merge: `WORKWELL_OFFICIAL_MEASURES` was missing from the + scheduler's env allowlist (so the nightly run would have used the AUTHORED engine while manual + runs used official — the third instance of a bug documented twice in that same block), and + validation was construction-time where this item said **boot-loud**, which I had silently + reworded to match what I built. Both fixed; `logic_version` override + measure-major batching + fold into PR-8. **Two obligations inherited from PR-3:** (a) it must write `populationResults` in one of the two shapes `officialMembership` accepts (the fqm-native `[{populationType, result}]` array, or the keyed `{ipp, denom, denex, numer, denexcep}`) — anything else is rejected and alerted, deliberately, so the halves cannot drift silently; and