Repository navigation
Conversation
Follow-up to #1309's review (GPT-6.1 Sol, approve with minors): - calibrate records its code identity in consumer_export.json; package refuses to name a release after any other commit, a dirty writer without --allow-dirty, or a checkpoint that records no writer. - The interim WIC limitation states whom the assumption covers (the people without a stored decision, all on the ACS spine), and the receipt records how many people were allowed to lack one. - _booleans accepts only bools and int/float 0/1, as policyengine.py's reader does (a Fraction is refused). - consumer_ready's gate summary carries the live-input receipt and the writer's code. - Tests: every stored Boolean dtype (pandas BooleanDtype with NA, numpy bool, float 0/1/NaN, object), an unreadable or absent spine tag refuses rather than fills, the writer binding, a mask built from person_spine; the spine-blindness entry is in order with an exact comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The abbreviated sha's length depends on the checkout: Modal's shallow clone abbreviates to 7 characters, a full clone to 9 or more, so comparing short shas could refuse an artifact written by the same commit. Record and compare the full commit; the release id keeps the abbreviated sha. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #1311 (GPT-6.1 Sol r1, approve): - F1: build_dirty is true when either the packaging tree or the tree that wrote the artifact was dirty; _require_artifact_written_by returns the writer's flag. - F2: do_package refuses another writer before any release directory exists, and a dirty writer marks a clean-packaged release dirty. - F3: the spine-tag test builds its frame with the tag instead of editing a Frame's stored table. - F4: nullable Int64/Float64 with NA and int8/uint8 read alike; Decimal and complex are refused, and the docstring no longer overstates parity with policyengine.py (this rule is stricter about complex). - F5: the Modal runbook says package runs at the commit calibrate ran at. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #1311 round 2 (approve): the two new do_package tests write H5 and need the requires_pytables marker, and the dirty-writer test now patches a clean packaging identity so it cannot pass on a dirty checkout without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
1 participant
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.
Part of #1263. Follow-up to #1309's review (GPT-6.1 Sol,
20261016-220039-mc1309-review-r1, approve with minors).Change
consumer_export.json(code). Package (_require_artifact_written_by) refuses to name a release after any commit other than the writer's. It also refuses a dirty writer without--allow-dirty, and a checkpoint that records no writer at all. Before this, a release id could name packaging code that never wrote the artifact.acs_wic_takeup_assumed_interimnow says the assumption covers "every person without one (N, all on the ACS spine, of M ACS-spine people)". The receipt recordspeople_allowed_without_decision, so N = M can be checked.BooleanDtypewithpd.NA, numpy bool, float 0/1/NaN, and object.person_spineis absent, NaN, None, bytes or another label.person_spine, as the tool does._booleansaccepts only bool and int/float 0/1, matching policyengine.py's reader, soFraction(1)is refused.consumer_ready's gate summary carrieslive_stored_inputsandcode.The optional nit (check the live input's shape in the tool) is left to the engine-contract test, which already pins it on CI.
The #1263 release (
populace-us-2024-buildo-acs-local-766b7c7-20261017T030317Z) was packaged at #1309's merged head766b7c792, before this binding existed. Calibrate and package ran at that same commit, so it satisfies the rule this PR adds.Tests
Local, at most 4 workers: the module, release-tool, sparse-CD and spine-blindness suites all pass (631 passed, 2 skipped). The package-stage fixtures now record the checkout's real code identity.
🤖 Generated with Claude Code