Skip to content

Fix the NZ bridge doc and tests the #1440 review flagged - #1447

Merged
MaxGhenis merged 3 commits into
mainfrom
nzhub/g8w3b-review-nits
Nov 2, 2026
Merged

MaxGhenis merged 3 commits into
mainfrom
nzhub/g8w3b-review-nits

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Status

Draft. This PR fixes three findings from the cloud review of #1440. It changes text and tests only: one doc paragraph, one test file, the generator's docstring, the reason on two overrides in as_rate_bridge.json, one column description in benefit_unit_rule.json, the golden that pins both resources' hashes, and a changelog fragment. No kernel, node declaration or override changes, and all three builders' --check pass. No output value changes, but the PR re-keys the whole NZ graph. as_rate_bridge.json's new hash alone re-keys the 22 nodes that bind it, and with their descendants 92 of 253. benefit_unit_rule.json's new text and hash change the params of the 11 nodes that select it. nz.create is one of them and every node descends from it, so all 253 node keys move. The first real NZ run comes at G8-x, so these keys invalidate no stored NZ run.

What changes

  • docs/transport-entitlement.md and the generator's docstring: both placed the masks, gross rate and cutout "on each population that runs" the Accommodation Supplement engine. The merged graph never did that: the shared nz.as.bridge population runs them once for S0, S1, S2, S3 and S5, and each variant repeats them on its own housing population. Both now say so, and one test's name drops the same wording.
  • Childless-family tests: test_unmasked_youngest_child_override_carries_a_missing_age_to_the_engine claimed the adapter's int64 conversion raises. It doesn't on these frames. A childless family's null reads as NaN and turns the override column into float64, so _batch_from_table takes its float branch. With the mask, the engine receives 0.0 for that adult; without it, NaN. Both tests cast the column themselves, so neither checked what the adapter does. They now read _batch_from_table and pin the float64 dtype.
  • as_rate_bridge.json: two reason strings gave the int64 read as why the override takes a mask. They now give the real reason: a null age would reach the engine input as NaN.
  • benefit_unit_rule.json: the column description said the youngest-child age holds 0 for a childless family. unit_construction.py writes a null, so it now says null.
  • Mixed couple through S5: as_rate_bridge.json says a partner on a main benefit takes the NZ Superannuation amount, through max over members. No test in test_nz_as_bridge.py ran that. A new test runs it in both person orders, using a new partner_age field on the fixture's Unit. It covers the max half of that statement. The fixture flags both adults receives_nz_super, which changes nothing here: these nodes read receipt_group and no receives_* column; test_receipt_any_member_priority_and_mixed_count pins how the classifier groups a mixed couple.

Invariant

S5 mixed couple. With no dependent child, a couple's S5 base rate equals twice the superannuitant's amount, whichever partner comes first in person order.

Results

The hub ran these with no pytest, because the host sits under its disk floor. CI runs the file as usual.

Run Result
Control: six test cases all pass
Mutant: S5 base rate aggregates with first the (50, 70) case fails; (70, 50) passes, so the test needs both orders
Mutant: S5 base rate aggregates with sum both mixed cases pass; test_a_partnered_superannuitant_unit_takes_coefficient_two_in_s5 fails, so the existing test covers sum

Left out

  • The review's finding on cents at an exact tie goes to G8-x, because it depends on how the pinned engine computes nets. Declare the NZ transport and entitlement graphs #1440's body records it.
  • The review's finding on an empty region string needs no change. _bind lets a layer read only code columns, and decode_atomic_support refuses empty codes, so geography.derive@1 can't write "".

🤖 Generated with Claude Code

MaxGhenis and others added 3 commits November 1, 2026 23:21
The bridge paragraph predated H29 and never named nz.as.bridge. The childless-family
tests cast the column themselves and claimed the adapter raises, but the masked
override returns float64, so _batch_from_table takes its float branch. Both tests
now read that function. A new test runs a mixed couple through S5's base rate in
both person orders, because a `first` aggregation passes one order and fails the other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generator's docstring and one test name repeated the wording the doc paragraph
lost, and the graph never matched it. Two as_rate_bridge.json reasons gave the
adapter's int64 read as why the youngest-child override takes a mask, which the
childless-family tests now show doesn't happen on this path: a null age would reach
the engine input as NaN. The golden moves only for that resource's hash.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The changelog credited the generator docstring with a change this PR did
not make: the docstring already named the shared population, and the PR
only dropped the sentence that contradicted it. The docstring's "every
coefficient" holds for the bridge alone, so it now says so.

benefit_unit_rule.json said the youngest-child age column holds 0 for a
childless family. unit_construction.py writes a null there, which is why
the bridge override takes a mask, so the description now says null. The
golden picks up that resource's new hash and the fingerprint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis marked this pull request as ready for review November 2, 2026 05:21
@MaxGhenis
MaxGhenis merged commit 25c8cc1 into main Nov 2, 2026
10 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged by the NZ hub (session local_e407a527) at 082f60f with --squash. Gates: gh pr checks exit 0; MERGEABLE; not draft; no CHANGES_REQUESTED review. Reviewed head: 082f60f. Independent review: an Opus 5.5 reviewer read d901f0d, 1a58826 and 082f60f, because the Codex lanes are closed while the host sits under its disk floor. Final verdict APPROVE_WITH_NITS, Needs Max no. The hub applied every finding; the last one corrected the PR body's re-key sentence and changed no file. Direct run at this head: six cases pass, and the first and sum mutants fail as the PR body states. All three NZ builders pass --check. The golden moves only the two resource hashes and the fingerprint.

@MaxGhenis
MaxGhenis deleted the nzhub/g8w3b-review-nits branch November 2, 2026 06:39
MaxGhenis added a commit that referenced this pull request Nov 2, 2026
Sol agreed at 989ce9b with six non-blocking findings. This commit makes
the two guard fixes and the four text fixes, and merges #1447.

Guards:
- The holdout-measure check now keys on the measure alone. A holdout
  column reused on another entity passed before.
- A pin check ties each aggregate key a row or factor selector pins to
  its own resolved-read entry. The digest binds an entry to its selectors
  but not to the keys it lists, so a rewritten selector with a recomputed
  digest kept its old keys. Roster selectors name no key, and only the
  feed audit proves what they resolve to.
- Four new mutants cover these, and each goes red with its guard removed.

Text:
- The overlap notes say ONSS scales with wages, not "a fixed share", and
  drop the "line 3" label on gross wages, which sits on an unnumbered row
  under that line.
- grammar_gaps names the second tolerance change: the ONSS row moves from
  B1's standard_admin_release 10% to 5%.
- Correction to c700edd's message: the generator needs the
  lexical_floats opt-in for the RSZ factor operands that
  resolve_reference_factor reads, not for its FPB pairs, whose 24 operands
  are JSON integers. The set membership stands.

The inventory text moves, so the golden repins its fingerprint and the
inventory hash. No row or number changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Nov 2, 2026
An Opus delta review at 40601e4 ran a holdout rewritten to the ONSS
calibration fact with a null aggregate key and a null measure. The resolver
treats a null field as no filter, so the row compiled, and every
artifact-free check passed. Other fact keys and mapping-form dimensions did
the same.

- Each selector now keeps to the committed fields, leaves none null or
  empty, and pins an aggregate key or, for a base selector, a roster.
- Each resolved-read entry must carry its own document's reference set, so
  relabelling an entry can't hide a holdout's keys.
- The operand mutant now rewrites the whole operand, so it models a read.

Ten spellings on the base and operand selectors and two relabelling mutants
cover this, and each goes red with its guard removed. No committed selector
trips the new checks, and no document, row or number changes.

Corrections to 40601e4's message: ba2590a merged #1447, not that commit;
it made three text fixes plus the PR-body fix for finding 6; and it also
rewrote the inventory's "Separation is checked" line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant