Skip to content

Tighten five G tests that checked less than their names claim - #1501

Merged
MaxGhenis merged 4 commits into
mainfrom
nzhub/g-review213-nits
Nov 4, 2026
Merged

MaxGhenis merged 4 commits into
mainfrom
nzhub/g-review213-nits

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Draft. Tightens five G tests that review 213 v4 (#1183 comment 6615417145) found checking less than their names claim. Tests only: no runtime, golden or resource file changes. It targets main since G (#1478) landed as f8f543a4. After the retarget, GitHub kept the PR's merge ref on G's old head f43e8fbb, so the hub merged main into the branch (121b5c02). That merge is clean over the single base f43e8fbb, and against main it changes only this PR's two files.

What changes

  1. test_each_coarse_row_selects_its_native_rows_committed_labels (test_nz_target_references.py, review nit 1). Each finite coarse band's first native label now starts $1 above the band's lower cut, and its last label ends at the band's upper_cent (reference_intervals.json, table ird_taxable_income_standin_ty2024). The three native rows whose label repeats the previous row's upper endpoint (row_60, row_90, row_230: $50,000, $80,000 and $300,000) start at the cut instead, and the test pins exactly those three, in the style of _WAGE_SHARED_ENDPOINT_ROWS. The open tail's one label reads "Over $1,000,000", and the cut below it is $1,000,000. Before, moving row_59 from b01 to b02 in the labels, in metadata.native_rows and in the two bands' expected_member_count passed every feed-free test.
  2. test_measurement_prefit_fails_the_toy_only_on_its_unwired_evidence (test_nz_transport_toy_graph.py, nit 2). The toy entry's roster_total_bounds, band_bounds and maximum_discrepant_bands must equal N/gates.json measurement_prefit's, its parameter keys must equal the packaged ones, and its two support floors must equal TOY_PREFIT_SUPPORT. The reviewer's mutant R2 (bounds [0, 100], 99 discrepant bands) passed before.
  3. test_property_a_positive_standin_above_the_first_cut_lands_in_one_band (nit 3). An explicit @example puts wages at 0 and on each aged cut (16,500, 44,000 and 66,000), each one either side, with every other component 0. In 5,000 random examples the reviewer found no record on or within one of an edge, and closure mutant R1 passed every toy test.
  4. New test_the_toy_standin_measure_keeps_the_packaged_central_rules (nit 4). The toy declaration's entity, add-backs, floor and scale equal N/income_measurements.json measures.central's, it names the same three outputs, and both give every component sign +1. Mutant R3 (no rental add-back) passed every toy test before.
  5. test_target_profile_coverage_passes_on_the_toy_with_income_required (nit 5). The gate's matches_by_requirement["income"] must name exactly the six toy stand-in rows, so this test now pins the gate's own match list. The binding hands the gate the calibration problem's row names, which read name@period (gate_bindings.py:803-823, target_kernels.py:552-554), so the test drops the period before it compares. test_each_toy_coarse_row_enters_the_problem_matrix_once_with_support already catches a row that leaves the income family; this test adds the gate's view.

Invariants the tests now hold

  • Each coarse income band covers exactly the native rows between its own cuts, read from committed files alone.
  • The toy pre-fit gate carries R2C-3's frozen ratio numbers; only its support floors differ, as TOY_PREFIT_SUPPORT declares.
  • The toy measure follows the packaged measure's rules: same entity, add-backs, floor, scale and output roles.

Checks so far

  • ruff check and ruff format --check pass on both files.
  • The hub replicated item 1's assertions on the committed JSON in plain Python: they pass at f43e8fbb, and the row_59 move turns them red (b01's last label ends at $49,000).
  • Review 221 (Activate the NZ graph against pinned Chronicle facts (NZ graph G8) #1183 comment 6616218228, at 92ff931b) ran both files and five mutants. test_nz_target_references.py gave 20 passed and 41 skipped (each skip: NZ_CHRONICLE_ARTIFACT unset). Fixes 1–4 each turned red on their mutant: row_59 moved (M1), the R2C-3 bounds widened (M2), right-closed cuts made inclusive (M3, on the new explicit example) and the rental add-back dropped (M4). It owed two fixes. Item 5's assertion failed at the head, since the gate's names carry @2002 and the test's didn't: 8edd52a3 drops the period before comparing. Item 5's text overstated the gap the test closes: this body rewords it. The reviewer ran the same comparison without the outer sorted: it passed at 92ff931b and turned red when one stand-in row left the income family (M5). The outer sorted changes neither result, since the gate already sorts its matches.
  • Delta check of 92ff931b..8edd52a3 (an in-session Opus agent; read-only, plus a plain-Python replay of both comparisons on the six names): APPROVE_WITH_NITS. The fix holds and both cites support the comment; the hub applied its body nits here. Brief 222 (Activate the NZ graph against pinned Chronicle facts (NZ graph G8) #1183 comment 6616766311) ran it at 8edd52a3: APPROVE. test_nz_transport_toy_graph.py gave 32 passed, 0 skipped; under M5 (toy_income_standin_amount__b2 moved to family toy), the coverage test passed its gate-status assertion and went red on the new one.
  • Not run locally (the hub's machine has no slot for these files tonight). CI run 41745653899 runs both files against main.

🤖 Generated with Claude Code

Review 213 v4 (#1183 comment 6615417145) found five places where a G test
let a mutant through. Each test now pins what it names:

- test_each_coarse_row_selects_its_native_rows_committed_labels ties each
  coarse band's first and last native labels to the band's cuts, so moving
  row_59 from b01 to b02 in both lists turns it red without the feed.
- The toy pre-fit test pins R2C-3's roster bounds, band bounds and maximum
  discrepant bands to N/gates.json, and the support floors to the toy's.
- The toy property adds an explicit example with wages at zero and on each
  aged cut, one either side.
- A new test ties the toy declaration's entity, add-backs, floor, scale and
  output names to measures.central.
- The coverage test pins the income requirement's matches to the six toy
  stand-in rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The coverage binding hands the gate the calibration problem's row names,
which read name@period, so the nit-5 assertion compared
toy_income_standin_amount__b0@2002 with toy_income_standin_amount__b0 and
failed at 92ff931 (review 221). Drop the period before comparing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis and others added 2 commits November 3, 2026 19:16
After the retarget to main, GitHub kept this PR's merge ref on G's old
head f43e8fb. Merging main starts CI on a fresh merge ref. The merge
is clean over the single base f43e8fb, and against main it changes
only this PR's two test files, line for line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This PR's CI passed at 01:5xZ against main f8f543a. Main has since
taken six PRs (#1345, #1493, #1467, #1426, #1465, #1391), 178 files,
none of them this PR's. #1493 changes how the target-compilation
kernels hash their code, so CI reruns here on today's main before the
PR lands. The merge is clean over the single base f8f543a, and
against main it changes only this PR's own lines, file for file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis marked this pull request as ready for review November 4, 2026 18:22
@MaxGhenis
MaxGhenis merged commit e2918f4 into main Nov 4, 2026
11 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged by the NZ hub (session local_e407a527) at 3ab4860 with --merge. Gates: gh pr checks exit 0; MERGEABLE; not draft; no CHANGES_REQUESTED review. Reviewed head: 8edd52a; the merged head differs only by a base update, and the PR's own changed lines are identical to the reviewed head's. Review 222 (#1183 comment 6616766311): APPROVE at 8edd52a. CI run 41851099404 tested 3ab4860 merged into main 875506e. Main since then changes 23 files from #1473 and #1506, none of them NZ's or #1501's, plus H2 (#1485, merged 18:21:54Z as 358b1c1). H2 and #1501 share only test_nz_transport_toy_graph.py, where #1501 edits three existing tests and adds one in the middle and H2 appends at the end; the merge is clean, with no duplicate names or imports (the hub's 11/03 check).

MaxGhenis added a commit that referenced this pull request Nov 4, 2026
H2 and #1501 landed on main, and both change test_nz_transport_toy_graph.py, as this PR does. git merges every other file without a conflict. The one conflict sits at the toy file's end, where H2's section and this PR's section both append. The resolution keeps main's file whole, #1501's edits included, then appends this PR's section unchanged: blob 5cb8c87. This PR's diff against main still adds the same 158 toy-file lines in the same order, and its other three files keep their patch-ids. No top-level name in the resolved file repeats.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Nov 4, 2026
Three PRs reached main after this PR's last CI run, which tested 17e2643 on main 0f0e777: #1506, #1485 (H2) and #1501. None of them changes a file this PR changes, and git merge-tree writes tree 79ee630 with no conflict. H2 adds an NZ coresident binding and teaches test_support's reference concepts the coresident_adult transform, which the concept-mapping differential beside this PR's encode change reads, so this merge has CI test the two together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis deleted the nzhub/g-review213-nits branch November 4, 2026 18:39
MaxGhenis added a commit that referenced this pull request Nov 4, 2026
…1488)

* Prove extension calibration nodes' measurement producers in compose

The extension pass of compose_transport_graph compiled the extended graph
and reran the ancestry guard, but never ran _check_measurement_proofs. So a
calibration-tier scenario's calibrate.* node could calibrate against a
money-scaled measurement that no proof covered: measurement_proofs reads
only the declared graph's rows and never names those nodes.

The extension pass now runs the check after the ancestry guard, as the
skeleton pass does. For a calibration node that an extension adds, the
compiled graph proves the producer under R2C-1's rule: exactly one compiled
ancestor declares the measurement, and that ancestor outputs every proof
column. R2C-15's only-writer rule applies as before. A skeleton calibration
node keeps the declared comparison, which the pass now also runs against
the extended graph.

Tests: unit cases on the compiled toy; a differential property that the
declared proof and the compiled extension proof agree, refusal for refusal,
on chains of declarers and writers; and NZ's synthetic graph with four V3
mutants (M1, M2, M4 and the NZ hub's G7), each of which composes without
this change.

* Move the NZ-toy measurement-proof tests into NZ's toy-graph file

The five tests that run the extension proof on NZ's synthetic graph name the
toy's V3 nodes, and the NZ hub owns the toy and its node ids. They move, with
their helpers, from test_transport_measurement_proof.py into
test_nz_transport_toy_graph.py and use that file's module `toy` fixture, so a
change to the toy edits only NZ's files. The shared file keeps the unit tests
on the compiled toy and the differential property.

The move adds G7 in the NZ hub's placement. V3's calibration keeps its base on
V3, while preparation, the compiled targets and the problem move into the
review version that rewrites both measured columns. The problem reads the
rewritten columns, so only the calibration's artifact edge from the problem
puts the rewrite among its compiled ancestors. The earlier G7 opens the
calibration from the review version, where the rewrite joins its base, and
doesn't need that edge.

* Refuse a malformed measurement_id in the compiled proof check

Review r2 found that the compiled check skipped a node whose
measurement_id was neither None nor a nonempty string, while the declared
check refuses it. With the string producer still in place, an extra
upstream node declaring measurement_id 7 composed, though the same param on
a declared row parks every measurement. r2 showed it on NZ's toy graph,
where V3's earnings.cents with id 7 composed.

_check_measurement_proofs now refuses such a node with the declared
check's reason before it proves any producer, and reads an explicit None as
no declaration, as measurement_proofs does.

The new test gives a node upstream of calibration a measurement_id of 7, ""
or None, and runs the declared check and the compiled check on a
calibration node that an extension adds and on a skeleton one. Both checks
refuse 7 and "" with the same reason, and both ignore None. Without the
fix, the compiled check accepts the four 7 and "" cases, and those cases
fail.

The commit also fixes three docstrings that r2 flagged: the test module's
account of the compiled tests, which patch in a proof; _chain's, whose
nodes chain by step columns; and compose's module docstring, which now
names gate_inputs, the other explicit way an extension may add a skeleton
predecessor.

* Say the compiled proof check scans malformed ids only under a proof

_check_measurement_proofs returns before its malformed-id scan when the declaration proves no measurement, and its docstring now says so. The independent review of b64eb6c noted that the sentence read as unconditional.

Two merges of main bring G (#1478) and then H2 (#1485) and #1501. The second
keeps main's test_nz_transport_toy_graph.py whole and appends this PR's
section.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Nov 4, 2026
Since the merge base 875506e, main gained #1473, #1506, H2 (#1485),
#1501, #1480 and #1488. The merge is clean. Two files changed on both
sides, in separate hunks: compose.py (#1488 on main's side) and
test_transport_evidence_kernels.py (H2 on main's side).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Nov 5, 2026
…losure (S2b PR 1) (#1482)

* Add binding-local views to the Axiom adapter

AxiomEngine takes an optional views sequence of AxiomView declarations:
a logical engine entity no frame entity maps to (rulespec-be's Child), one
bool person selector column, and an outside value per output. The adapter
gathers the selected person rows in table order, runs the module's program
on the logical entity, checks the engine's array against the selected count,
and scatters it back through the same mask; every other row holds the
outside value, cast through the output's graph dtype. An empty selection
makes no engine call, and a null or non-bool selector refuses.

View outputs resolve to the person entity, so callers that key on
(binding entity, variable) read them unedited. Each view's program compiles
under its own cache key, re-checks the pinned RuleSpec tree first, refuses
dense relations, and contributes its derived metadata, so a Child-only
module has metadata. The selector joins the person entity's
relation_dependencies; view_root_inputs() reports each view's roots.
axiom_engine_ref writes views only when present, so every existing
reference is unchanged. The view code stays in axiom.py, because
source_hash hashes defining files and does not follow imports.

* Partition binding-local view roots in the input closure

closure_partition no longer refuses a binding that declares views, and
BINDING_LOCAL_VIEW_MESSAGE is gone. Per binding, each view's logical entity
(Child) joins the live entities and maps to the person table, so a view's
roots partition against the closure's Child entries and the existing frame
check reads them on the person slice. Roots on that entity stay unmapped on
every other binding, and unreached_roots still reads only the binding's
engine entity, so it cannot excuse a view root.

engine_root_inputs adds each view's roots under its logical entity through
AxiomEngine.view_root_inputs(), and refuses a view binding on an injected
adapter. binding_view_entities reads a binding's logical entities for the
closure and compose.

* Parse binding-local views in the registry and check them in compose

The registry parses a binding's views block into AxiomView declarations
(logical_entity, selector {column}, outside {name: value}) and builds the
adapter with them. It refuses an empty list, views beside relations, a
binding off the person entity, a logical entity that names a mapped engine
entity, a repeated logical entity, a selector with any field but column, a
structural selector, a selector that names a binding output, and an outside
key that names no binding variable. Once the adapter is pinned, it checks
each view's program for relations in place of the binding entity's program,
requires each view's outside keys to equal the binding's variables on its
logical entity, and refuses a view input named like a root input of the
binding entity's program. _check_engine compares an injected adapter's views
with the binding's and refuses views on an injected Python adapter.

binding_view_dependencies reports each selector as a whole-table person
slice, the same columns AxiomEngine.relation_dependencies reports to the
rules kernels. Compose unions them with the relation dependencies for every
rules node, refuses a view node whose person slice omits an input the
closure encodes (other than through a graph node) or defaults on the view's
logical entity, and requires each selector on the closure producer's person
slice.

* Pin NZ-shaped and relation engine references to goldens from the base

No NZ CI test builds an axiom_engine_ref for NZ's rows: NZ's engine-free
graph injects Python toy engines. The new golden holds references generated
at main 4a36edc for NZ's six binding rows (unreached_roots included) over a
synthetic tree, and for the registry and adapter relation fixtures, all
built engine-free through build_transport_registry and axiom_engine_ref.
The test compares them byte for byte, so a reference that gains
"views": [] or "views": null without a declared view fails.

A second test, gated on POPULACE_RULESPEC_NZ, builds NZ's six real
references from the tree alone and compares them with a frozen copy the NZ
hub writes at the base.

* Add the changelog fragment for binding-local views

* Test that every rules kernel requires a view's whole selector slice

SimulateRulesKernel._require_relation_inputs reads the adapter's
relation_dependencies, which now carry each view's selector, so a node
whose person slice omits or masks the selector refuses before the engine
runs. The test pins that path for a view adapter.

* Pin view code to axiom.py and the no-view outputs to the base path

The placement lint now walks every import node in axiom.py, relative and
function-body imports included, and a second test requires AxiomView and
every view helper to carry axiom.py's module name. A helper moved into a
module axiom.py already imports (materialize, concept_mapping) would leave
the simulate.* hashes unmoved by an edit to it; the module-name test
refuses that move, which the import pin alone passes.

P4 gains an output clause: with views=None, materialize returns the same
names, order, dtypes and values, through the same engine calls, as a
reference copy of main 4a36edc's no-view path kept in the test. NZ's CI
graph injects toy engines and never runs AxiomEngine.materialize, so this
is the executable guard on view-free outputs.

* Class a view's defaulted roots as person columns in the support roster

_support_roster mapped each defaulted closure entry to a frame entity
through entity_names alone, so a defaulted Child root, which PR 2 writes on
the person table, mapped to no entity and missed the closure_default class.
_closure_defaults now maps a binding-local view's logical entity to the
person entity for entries of that binding's module. Without views the
mapping is unchanged.

* Freeze NZ's real-tree engine references at the base

test_nz_real_refs_match_their_frozen_copy compares NZ's six real
references with a frozen copy, and wrote none without a rulespec-nz
tree. This adds the copy, generated at PR 1's base (main 4a36edc) from
a git archive of rulespec-nz 8dc2507d, the commit NZ's bindings pin.

The base run froze the file, and a second base run passed against it.
At this branch's head the test passes too, so the six references keep
their bytes with views in place. The CI golden test passed at the base
as well, which checks that axiom_engine_refs.json came from the base.

* Document binding-local views in the declared-units guide

The guide said binding-local views remained separate work. It now
describes the declaration, the gather and scatter, the outside values,
the registry's refusals, the selector slice, the closure's treatment of
view roots, and the row-locality assumption that the skip-gated
real-engine differential tests.

* Union shared-path closure roots to remove binding-order dependence

* Cover both logical views to catch dropped input roots

* Validate complete outside declarations before pinning bindings

* Preserve outside value types in view equality and hashing

* Exercise view property edges across every graph dtype

* Type and stabilize real Child differential inputs

* Freeze CI reference bindings to isolate NZ edits

* Clarify view evidence, serialization and selector refusals

* Accept overrides of a view selector that a program reads

The first review of the fix round (P2) found that 0d6201a's selector
refusal also refused an override of a selector that a Person program
reads as a root input. The registry accepts that shape, variables()
lists the selector, and before 0d6201a the counterfactual bridge
applied such an override. validate_input_override now names a selector
only when no program reads it, which is exactly when the bridge refuses
the override as outside the engine's inputs, so the check changes no
override the bridge applies.

test_a_selector_that_a_program_reads_stays_overridable drives the
counterfactual bridge with a Person program that reads is_child: the
override reaches that program's input and the rows the view selects. It
fails on 0d6201a's check. The refusal test now also runs through the
bridge, which calls no program. The guide states the rule.

* Correct the two docstrings that review B flagged

`_real_inputs` said the pinned engine refuses a wrong-typed input by name.
At axiom-rules-engine 753ce7d, `src/dense.rs` reports a bool, text or date
column as a type mismatch when a program needs a number from it ("expected
decimal-compatible dense column" for an arithmetic operand, "{side} side of
comparison is not numeric" for a comparison), and neither message names the
input. The docstring now says that.

`axiom_ref_goldens.py`'s module docstring wrote `"\n"` in a non-raw string,
so it rendered a line break where the regeneration recipe needs the two
characters. It now escapes the backslash.

Two merges of main bring this branch up to date: `02265bb5` merges main `875506e8`, and `36800b14` merges main `3e7a7045` (#1473, #1506, H2 #1485, #1501, #1480 and #1488). git merged both without conflicts.

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