feat(standards): per-measure executor router, wired and dark (PR-7b)#341
Conversation
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. Every named measure must be covered by the MADiE gate, have a vendored artifact whose catalogId matches, have recorded numerator semantics, and — the invisible one — have every referenced value set expand non-empty. All problems are reported at once, not the first. "all" is refused: it 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, the instance living exactly as long as the router. That is the middle of the two bad options 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. A rejected expansion is deliberately not cached. 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. The boundary guard corrected me again: I pre-emptively added the router to the fqm consumer allowlist and it failed, because the router imports the adapter, not the package. Reverted, with the reason written where the next person looks. Not yet safe to switch on, and the module says so: no bundle preparation (stampQiCoreStructure), no measure-major batching. Both are PR-8's. Typecheck clean; 1482 pass / 0 fail / 14 skipped. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Np757pAsNQSCPiq8S7qsxB
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11514bc381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await ensureSegmentSeed(env); | ||
| const stores = await getStores(env); | ||
| const engine = await engineForEnv(env); | ||
| const engine = await routedEngineForEnv(env); |
There was a problem hiding this comment.
When the Node container enables both the scheduler and WORKWELL_OFFICIAL_MEASURES, the production entrypoint's schedulerEnv in server.ts copies the VSAC, WebChart, and incremental variables but not the new official-measures variable. Because officialMeasureIds reads only the passed object, this call always builds the authored engine for nightly ALL_PROGRAMS runs while HTTP-triggered runs use the official executor, producing contradictory compliance outcomes and cases for the same deployment. Pass the variable through the scheduler environment and its declared type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 152d4cd. Correct, and it is the rollback path specifically that makes it bite: PR-9's documented rollback is "unset the flag", which would then 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.
aggregateCountsForRun now reads provenance from the run itself: one bounded row, checked for evidence.official, which only the official executor writes and which a run carries uniformly (one measure, one engine). The env flag is still consulted first purely to skip the read in the common case, so the O(1) property the 120k seed:scale path depends on is unchanged. A test asserts a stored official outcome is self-describing with the flag unset.
| return official.has(input.measureId) && !overridden | ||
| ? executor.evaluate(input) |
There was a problem hiding this comment.
When WORKWELL_OFFICIAL_MEASURES changes after a run—especially during the documented config rollback—this branch leaves official outcomes persisted in historical runs, but aggregateCountsForRun in routes/runs.ts chooses their interpretation from the current isOfficialRouted(measureId, env) value. After disabling the flag, a summary MeasureReport or QRDA III for an earlier official CMS122 run therefore falls back to the status histogram and reverses the official poor-control numerator. Persist or infer executor provenance from each run's outcomes rather than consulting the current deployment flag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 152d4cd. Correct, and it is the rollback path specifically that makes it bite: PR-9's documented rollback is "unset the flag", which would then 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.
aggregateCountsForRun now reads provenance from the run itself: one bounded row, checked for evidence.official, which only the official executor writes and which a run carries uniformly (one measure, one engine). The env flag is still consulted first purely to skip the read in the common case, so the O(1) property the 120k seed:scale path depends on is unchanged. A test asserts a stored official outcome is self-describing with the flag unset.
…n paths (PR-7b review)
Two criticals, and the first is the third instance of a bug this repo has
already documented twice.
C1. WORKWELL_OFFICIAL_MEASURES was missing from the scheduler's env allowlist
in server.ts, 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 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 directly
above that allowlist describe the same bug happening to
WORKWELL_WEBCHART_PRIVATE_KEY_B64 (#331) and WORKWELL_INCREMENTAL_EVAL
(#263), so a test now asserts the keys are threaded: three times is a
pattern, and comments have not stopped it.
C2. Validation was construction-time where the roadmap said boot-loud — and I
had reworded the 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 (deliberately DB-free, so the reconciler reports healthy) and 500
every evaluating route — the symptom profile of the four-day Neon outage.
Boot now runs the same validation and emits WORKWELL_ALERT
OFFICIAL_ROUTING_MISCONFIGURED.
Four more: scoring was the one adapter refusal firing 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; /evaluate built the router after markRunning, 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 at once" excluded the terminology preflight, which is serial
and first-failure.
Two corrections of the same kind as the earlier false claims: the
elm/metaOverride escape hatch cited the fidelity lab and Rule Builder as
callers — neither goes through the router, so the guard is defensive, not
load-bearing; and a test comment claimed it "deliberately does not load" the
real calculator when it did, meaning assert.ok(err instanceof Error) would have
passed on a MODULE_NOT_FOUND as happily as on a real routing hit.
Also: ARCHITECTURE §10 updated to 11 seams, and oidFromValueSetUrl re-exported
through the adapter so the router shares one normalization rule without
becoming a second direct consumer of the fqm-quarantined package.
Typecheck clean; 1486 pass / 0 fail / 14 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Np757pAsNQSCPiq8S7qsxB
…g (Codex P1) aggregateCountsForRun decided how to interpret a HISTORICAL run's outcomes by consulting the CURRENT WORKWELL_OFFICIAL_MEASURES. The documented PR-9 rollback is "unset the flag" — which would then silently reinterpret every past official run through the status histogram, reversing cms122's numerator (its official numerator is poor control; the workflow status inverts it). A regulatory export of a finished run must not change meaning because of a configuration change made afterwards. Provenance now comes from the run's own outcomes: one bounded row, checked for evidence.official, which only the official executor writes and which a run carries uniformly (one measure, one engine). The env flag is still consulted first as a cheap way to skip the read in the common case. Typecheck clean; 1487 pass / 0 fail / 14 skipped. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Np757pAsNQSCPiq8S7qsxB
1 participant
Roadmap §7.2, migration step PR-7b.
routedEngineForEnv(env)replacesengineForEnv(env)at all 8 call sites — runs (3), cases, measures (2), compliance-simulation, scheduler. Measures named inWORKWELL_OFFICIAL_MEASURESevaluate 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 === authoredrather than comparing two engines' outputs — 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. The router refuses to exist unless every named measure
catalogIdmatches,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=allis refused: "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/metaOverridealways stays authoredEven 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 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 the two bad options the PR-7a 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 bugengine-factory.tsdocuments at length. A rejected expansion is deliberately not cached, so one transient store failure doesn't become a whole run of refusals.official-measuresjoins the boot seam lineThe 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 — so the package stays one hop away from what production routes through. Reverted, with the reason written where the next person will look.
Not yet safe to switch on
The module's own docstring says so: no bundle preparation (
stampQiCoreStructure— whichliteral-diff.tsneeds before this same artifact reads our synthetic bundles) and no measure-major batching. Both are PR-8's, along with thelogic_versionoverride. The flag existing and the flag being safe to set are different things, and this PR delivers the first.Verification
🤖 Generated with Claude Code
https://claude.ai/code/session_01Np757pAsNQSCPiq8S7qsxB