Skip to content

Generalize the ACA-branded labels in the shared binary assignment ops#338

Merged
MaxGhenis merged 1 commit into
mainfrom
claude/nostalgic-hertz-e26657
Jul 9, 2026
Merged

Generalize the ACA-branded labels in the shared binary assignment ops#338
MaxGhenis merged 1 commit into
mainfrom
claude/nostalgic-hertz-e26657

Conversation

@daphnehanse11

Copy link
Copy Markdown
Collaborator

Summary

The generic assign_binary_from_rate / calibrate_binary_assignment / calibrate_binary_assignment_joint_targets handlers in us_runtime/source_runtime.py hard-coded "ACA binary assignment" / "ACA binary calibration" into every error message, even though they are shared manifest primitives. A failure in any non-ACA stage driving them — empirically verified with #334's person-grain medicaid_take_up stage, where dropping person_weight yielded "ACA binary calibration frame is missing required column(s)" — reported itself as an ACA failure. Closes the follow-up from the #334 review.

  • Labels are now keyed to the operation kind: "US binary assignment", "US binary calibration", "US joint binary calibration", matching the file's existing "US ratio computation" / "US support clip" naming. The shared helpers (_binary_calibration_value_state, _preserved_true_mask, _first_available_target_table, _group_columns, _domain_mask) take a label keyword so each handler reports its own name. The _ACA_ prefix is dropped from the corresponding parameter-key frozensets. aggregate_person_to_tax_unit keeps its ACA labels — it is genuinely ACA-specific (Marketplace coverage aggregation) and only the ACA stage uses it.
  • Draw salts are deliberately unchanged. aca:{output}, calibrate:{variable}, and joint-calibrate:{variable} are byte-identical, and the aca: prefix is now commented as frozen: existing ACA releases are bit-reproducible against it. The exact-seeded-outcome assertions in the existing ACA tests pass unchanged, which locks this in.
  • The calibrate handlers' silent stable_tax_unit_draw default is removed: draw is now a required parameter (the assign handler already required it), and the ACA manifest's three calibrate ops state the previous default explicitly — bit-identical behavior. A future person-grain stage that omits the param now fails loudly instead of silently minting positionally-keyed (not identity-stable) draws under a tax-unit column name. _stable_draws gained a docstring stating its tax_unit_id-vs-positional-index keying and that stages at other grains should mint their own identity-keyed draw column.

Interaction with open PR #334 (medicaid_take_up)

Tests

  • New: a person-grain reproduction of the mislabeled error (asserts the message names "US binary calibration" and contains no "ACA"), required-draw errors for both calibrate handlers, and a person-grain calibration happy path with a declared draw column.
  • Full packages/populace-build suite passes (884 tests); ruff check/format clean on the changed files.

🤖 Generated with Claude Code

The generic assign_binary_from_rate / calibrate_binary_assignment /
calibrate_binary_assignment_joint_targets handlers hard-coded "ACA binary
assignment" / "ACA binary calibration" in every error message, so a failure
in any non-ACA stage driving them (e.g. PR #334's person-grain
medicaid_take_up stage) reported itself as an ACA failure. Labels are now
keyed to the operation kind ("US binary assignment" / "US binary
calibration" / "US joint binary calibration", matching the file's existing
"US ratio computation" style) and threaded through the shared helpers so
the joint handler reports its own name.

The "aca:{output}" draw salt is deliberately unchanged (now commented as
frozen): existing ACA releases are bit-reproducible against it, and
label-only changes leave every seeded outcome byte-identical.

The calibrate handlers' silent 'stable_tax_unit_draw' draw-column default
is removed: 'draw' is now a required parameter (the assign handler already
required it), and the ACA manifest states the previous default explicitly,
so behavior is unchanged while a future person-grain stage that omits the
param fails loudly instead of minting positionally-keyed draws under a
tax-unit column name. _stable_draws documents its tax_unit_id-vs-positional
keying fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@daphnehanse11 daphnehanse11 requested a review from MaxGhenis July 7, 2026 14:42

@MaxGhenis MaxGhenis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve

Not purely mechanical — the rename rides alongside a real behavioral change, and both are complete for the PR's scope.

Behavioral change (correct and safe): the draw parameter is now required in both calibrate handlers (previously defaulted to stable_tax_unit_draw). That default was a tax-unit-grain assumption that would silently misfire if a person-grain stage reused the handler, so requiring it is the right hardening. Completeness of the migration checks out: all three calibrate_binary_assignment* operations in source_stages.json are ACA (takes_up_aca_if_eligible ×2, selected_marketplace_plan_benchmark_ratio < 1.0), and this PR adds an explicit "draw": "stable_tax_unit_draw" to each — behavior-preserving. assign_binary_from_rate already required draw pre-PR, so no op regresses. The US manifest is the only consumer of these us_-prefixed handlers, so there's no other call site to update.

Rename is complete for the targeted family. All three shared binary-assignment handlers are fully de-branded: constants (_ASSIGN_BINARY_FROM_RATE_PARAMETER_KEYS, _CALIBRATE_BINARY_ASSIGNMENT_PARAMETER_KEYS, _CALIBRATE_BINARY_JOINT_TARGET_PARAMETER_KEYS) and every error label (US binary assignment / US binary calibration / US joint binary calibration). The test_us_binary_calibration_errors_are_not_aca_labeled test pins the de-branding.

The two handlers left ACA-branded are in-scope-correct, not a miss. _ACA_COMPUTE_RATIO_PARAMETER_KEYS and _ACA_AGGREGATE_PERSON_TO_TAX_UNIT_PARAMETER_KEYS remain, but each has exactly one consumer (the ACA stage) and neither is a binary-assignment op. I confirmed #334's medicaid_take_up stage uses only assign_binary_from_rate and calibrate_binary_assignment (both draw: stable_person_draw) — it never touches compute_ratio or aggregate_person_to_tax_unit, so those two staying ACA-branded blocks no reuse. Worth a one-line note only: they're generic operations that would need the same treatment the day a non-ACA stage reuses them.

Reproducibility care is right: the "aca:" salt prefix in assign_binary_from_rate's draw-fallback branch is deliberately frozen (comment: existing ACA releases stay bit-reproducible), and _stable_draws gains a docstring warning that its positional-index fallback isn't identity-stable and other-grain stages should mint their own keyed draw — exactly what the Medicaid person stage does.

Relationship to #334: this PR generalizes the handlers #334 reuses at person grain. #334 passes draw explicitly so it's safe with or without this PR, but this PR is the hardening that makes a silent grain mismatch impossible. Both edit source_stages.json (ACA region here, medicaid region there), so the second to merge may need a trivial rebase.

CI green (test + wheels, 3.13/3.14), MERGEABLE/CLEAN. Not merging (review-only per request).

@MaxGhenis MaxGhenis merged commit abb85ed into main Jul 9, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants