feat: Phase D5 scorecard uncertainty intervals, risk vs coverage, and manifest-backed subgroup analysis - #63
Conversation
… manifest-backed subgroup analysis
|
Droid finished @txmed82's task —— View job Uncertainty intervals and D5 scorecard expansions look solid, but a few edge cases can still mislead (missing head-covered world provenance) or break (clustered bootstrap with empty clusters) and the worst-case subgroup logic needs to respect continuous metric direction and unassessable subgroups. |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| "claim_footer": result.claim_footer, | ||
| "metrics_only": _metrics_only(result, world_engine), | ||
| "head": result.head, | ||
| "independent_cases": result.independent_cases, |
There was a problem hiding this comment.
[P0] Preserve head-covered world_engine provenance in scorecards
scorecard_data() currently sets world_engine only from the optional world_engine argument, so render_markdown()/render_html() can show backend/engine as unknown and skip the synthetic-stub banner even when JobResult.world_engine was recorded at run time and hashed into the artifact head. Prefer result.world_engine.model_dump(mode="json") when present, and fall back to the world_engine parameter only for legacy callers.
| sampled_values: list[float] = [] | ||
| for key in sampled_keys: | ||
| sampled_values.extend(clusters[key]) | ||
| if sampled_values: |
There was a problem hiding this comment.
[P1] clustered_bootstrap_mean_ci can crash when any cluster is empty
The loop only appends to means when sampled_values is non-empty, but the CI indices are computed from draws; if any cluster list is empty, some draws will be skipped and len(means) can be < draws, so indexing means[upper_idx] can go out of range. Fix by rejecting/ignoring empty clusters up front (so every draw produces at least one value) or by computing quantile indices from len(means) instead of draws.
| subgroups = [] | ||
| worst_case = None | ||
| if len(scenario_trials) > 1: | ||
| worst_rate = 1.1 |
There was a problem hiding this comment.
[P1] Worst-case subgroup selection can be None or inverted for continuous metrics
worst_rate is initialized to 1.1 and the selector always treats smaller s_rate as worse; for continuous headline means that can exceed 1.1, worst_case can incorrectly remain None, and for continuous metrics with direction == "minimize" the worst subgroup should be the maximum mean, not the minimum. Initialize with ±inf and compare based on headline_outcome.direction for the continuous branch.
| for t in s_trials | ||
| if t.vector.headline.value is not None | ||
| ] | ||
| s_rate = fmean(s_vals) if s_vals else 0.0 |
There was a problem hiding this comment.
[P1] Continuous subgroup stats report 0.0 with CI [0, 0] when nothing was assessed
In the continuous subgroup branch, an empty s_vals currently yields s_rate = 0.0 and ci = [0.0, 0.0], which fabricates a measured zero and a zero-width interval. This is inconsistent with the top-level continuous metric row (which uses None when unassessable) and it can wrongly dominate worst_case. Consider emitting rate=None/ci_95=None (and rendering n/a), and skip such subgroups when computing worst_case.
| ci = list(wilson_score_interval(s_pass, s_count)) if s_count > 0 else [0.0, 1.0] | ||
|
|
||
| entry = { | ||
| "subgroup": sc_id, |
There was a problem hiding this comment.
[P2] [security] Subgroup labels are emitted verbatim into scorecards
_trial_subgroup() returns scenario.subgroup/scenario.id verbatim and the value is written into scorecard.json and rendered into scorecard.md; if scenario IDs encode patient/site identifiers (a plausible footgun in this domain) or contain special characters, this can leak or inject confusing content into shared artifacts. Consider hashing/allowlisting subgroup identifiers (or explicitly documenting they must be non-sensitive and safe-to-render).
2 participants
What: Implements Wilson score intervals for binomial rates, percentile and clustered bootstrap for continuous metric means; adds risk-vs-coverage breakdown for abstaining models; adds manifest-backed subgroup analysis and worst-case identification; renders 95% CI column in Markdown, HTML, and JSON scorecards.
Why: Satisfy D4 and D5 requirements ensuring zero observed failures is not reported as zero risk, quantifying sampling noise honestly, and analyzing subgroup disparities without fabricating seed groups.
Verify: Full test suite green (1576 passed, 10 skipped); 6 new unit tests in tests/test_uncertainty.py; ruff + format + mypy strict clean.