Repository navigation
Fix the NZ bridge doc and tests the #1440 review flagged - #1447
Merged
Merged
Conversation
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
marked this pull request as ready for review
November 2, 2026 05:21
Contributor
Author
|
Merged by the NZ hub (session local_e407a527) at 082f60f with --squash. Gates: |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
reasonon two overrides inas_rate_bridge.json, one column description inbenefit_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'--checkpass. 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.createis 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.mdand 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 sharednz.as.bridgepopulation 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.test_unmasked_youngest_child_override_carries_a_missing_age_to_the_engineclaimed 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_tabletakes 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_tableand pin the float64 dtype.as_rate_bridge.json: tworeasonstrings 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.pywrites a null, so it now says null.as_rate_bridge.jsonsays a partner on a main benefit takes the NZ Superannuation amount, throughmaxover members. No test intest_nz_as_bridge.pyran that. A new test runs it in both person orders, using a newpartner_agefield on the fixture'sUnit. It covers themaxhalf of that statement. The fixture flags both adultsreceives_nz_super, which changes nothing here: these nodes readreceipt_groupand noreceives_*column;test_receipt_any_member_priority_and_mixed_countpins 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.
first(50, 70)case fails;(70, 50)passes, so the test needs both orderssumtest_a_partnered_superannuitant_unit_takes_coefficient_two_in_s5fails, so the existing test coverssumLeft out
_bindlets a layer read onlycodecolumns, anddecode_atomic_supportrefuses empty codes, sogeography.derive@1can't write"".🤖 Generated with Claude Code