[MaxGhenis]

Follow-ups from the independent review of #650 (landed as 6925606fcb): #650 (comment), items P3 1 and P3 2. The PR changes only tests/core/test_release_tagging.py and adds changelog.d/core650-line-break-pins.changed.md. Nothing under policyengine_core/ or .github/ changes.

impact: tests only; no change to the package or to any workflow.

What's changed

• Each line break make_rules refuses (P3 1). test_make_rules_rejects_each_line_break_make_ignores is the review's test, placed after test_make_rules_rejects_syntax_it_cannot_follow. For each of \v, \x1c, \x1d, \x1e, \x85, U+2028 and U+2029 it writes the root Makefile to tmp_path as bytes, with $(if ,<char>)|| true added to the coverage xml -i recipe line. It expects make_rules to fail with "make_rules cannot read a line break". Writing bytes keeps \x85, U+2028 and U+2029 independent of the locale encoding that write_text uses on Windows. Each character is written as an escape, so the file stays pure ASCII. Together with the existing form-feed-inside-recipe-line and carriage-return-inside-recipe-line parameters, this pins all nine characters in MAKE_LINE_BREAK_MAKE_IGNORES. It closes the review's mutants c6 (drop \v) and c7 (drop \x1c-\x1e\x85 and U+2028/U+2029), which passed all 102 tests.
• CRLF allowance on an LF checkout (P3 2). test_make_rules_reads_crlf_makefile_as_lf turns the root Makefile's \r\n into \n, in case the checkout is already CRLF. It writes an LF copy and a CRLF copy to tmp_path, checks that the LF rules include coverage xml -i in test's recipe so the comparison isn't vacuous, and asserts make_rules returns the same rules for both. It closes mutant c4 (\r(?!\n) → \r), which before failed only on a CRLF checkout.

Tests

One file per process, uv run pytest -q -p no:cacheprovider <file>:

File	master (cc0157f)	this PR
tests/core/test_release_tagging.py	102 passed	110 passed
tests/core/test_docs_deployment.py	5 passed	5 passed

With the root Makefile converted to CRLF (bytes \n → \r\n, as a CRLF checkout gives), test_release_tagging.py at this head gives 110 passed. ruff format --check and ruff check pass on the test file.

Mutants

Each mutant edits MAKE_LINE_BREAK_MAKE_IGNORES in a scratch copy, runs test_release_tagging.py in its own process on the LF Makefile, and restores the file. The file matched the original after each one.

Mutant	Edit	Result	Failing tests
c4	\r(?!\n) → \r	1 failed, 109 passed	test_make_rules_reads_crlf_makefile_as_lf
c6	drop \v	1 failed, 109 passed	test_make_rules_rejects_each_line_break_make_ignores[\x0b]
c7	drop \x1c-\x1e\x85 and U+2028/U+2029	6 failed, 104 passed	...[\x1c], [\x1d], [\x1e], [\x85], [\u2028], [\u2029]

CI

Pull request run 41965682770 at 339c7f7 passes all 19 jobs: Check changelog fragment, Lint, Lint workflows, Test on ubuntu-latest and windows-latest for Python 3.11–3.14, and Test Core and country packages on both OSes for Python 3.11–3.14. Each Windows Test job ran tests\core\test_release_tagging.py as 4 skipped and 106 passed (110 tests), so both new tests ran on Windows.

---

🤖 Generated with Claude Code

[MaxGhenis]

Ready for hub review

Head: 339c7f79e61978fa8601a56dcf5e2fc335e016b1 (one commit on cc0157f, master at the time). Files: tests/core/test_release_tagging.py (+31) and changelog.d/core650-line-break-pins.changed.md. Nothing under policyengine_core/ or .github/ changes, and uv.lock is unchanged (I restored it after uv sync touched it).

Items

1. Pin each line break the check refuses (Close Makefile masks and limit PyPI credentials in the release tests #650 review P3 1). Done at tests/core/test_release_tagging.py:1124-1137, right after test_make_rules_rejects_syntax_it_cannot_follow. It is the review's test verbatim, with every character written as an escape (:1125). The file holds no code point above 127. Mutant c6 now fails [\x0b], and c7 fails [\x1c], [\x1d], [\x1e], [\x85], [\u2028] and [\u2029] (table below).
2. Pin the CRLF allowance on an LF checkout (P3 2). Done with test_make_rules_reads_crlf_makefile_as_lf at :1140-1152. Mutant c4 now fails it on an LF checkout: 1 failed, 109 passed.

Changelog: changelog.d/core650-line-break-pins.changed.md, type changed, one sentence.

Tests

One file per process, uv run pytest -q -p no:cacheprovider <file>:

File	before (cc0157f)	after (339c7f7)
tests/core/test_release_tagging.py	102 passed	110 passed (102 + 7 + 1)
tests/core/test_docs_deployment.py	5 passed	5 passed

With the root Makefile turned into CRLF (bytes \n → \r\n), test_release_tagging.py at head gives 110 passed. ruff format --check and ruff check pass on the test file.

Mutants

Each mutant edits MAKE_LINE_BREAK_MAKE_IGNORES (:422) as bytes and runs the file in its own process on the LF Makefile. After each run the script restores the file and asserts its bytes match the original. git status was clean afterwards apart from the intended change.

Mutant	Edit	Result	Failing test IDs
c4	\r(?!\n) → \r	1 failed, 109 passed	test_make_rules_reads_crlf_makefile_as_lf
c6	drop \v	1 failed, 109 passed	test_make_rules_rejects_each_line_break_make_ignores[\x0b]
c7	drop \x1c-\x1e\x85\u2028\u2029 (class becomes [\v\f])	6 failed, 104 passed	test_make_rules_rejects_each_line_break_make_ignores[\x1c], [\x1d], [\x1e], [\x85], [\u2028], [\u2029]

CI

The PR is a draft, and CI skips drafts, so no run exists at 339c7f7 (actions/runs?head_sha= returns none). I couldn't produce the Windows run the brief asks for without marking the PR ready, which is the hub's call.

Judgment calls

• Branch. I pushed to claude/core650-line-break-pins as the brief names it. The session also had a default branch name, which I didn't use.
• Fragment name. A / can't appear in a file name, so the fragment drops the claude/ prefix, matching core649-release-test-followups.changed.md.
• The CRLF test normalizes first. The repository has no .gitattributes, so a Windows runner with core.autocrlf may already check out a CRLF Makefile. A plain replace(b"\n", b"\r\n") would then give \r\r\n, the check would refuse the lone \r, and the test would fail on Windows. So the test turns \r\n into \n first and writes both the LF and CRLF copies to tmp_path from those bytes. It compares against that LF copy, not against make_rules(MAKEFILE). That keeps it the same test on either checkout.
• Non-vacuity guard. The CRLF test also asserts that coverage xml -i is in test's LF recipe (:1151), so an empty result on both sides can't pass.
• The CRLF check row needs bytes, not sed. The root Makefile has no final newline, so sed 's/$/\r/' leaves a lone \r at the end of the file. On that copy the check refuses it, and 3 tests fail. Git's conversion touches only real \n, and the bytes conversion gives 110 passed.
• I didn't rerun c4, c6 and c7 against master's file. The "before" numbers for those mutants (102 passed) come from the review.

Proposed follow-up

None.

---

Generated by Claude Code

[MaxGhenis]

Hub review in progress

Head 339c7f79e61978fa8601a56dcf5e2fc335e016b1. Findings so far. CI is still running.

• The PR changes only tests/core/test_release_tagging.py (+31) and changelog.d/core650-line-break-pins.changed.md. The test file has 0 bytes above 127.
• Counts, one file per process (Python 3.14.6, pytest 9.2.0): at head, test_release_tagging.py gives 110 passed and test_docs_deployment.py gives 5 passed. With master's test file, they give 102 and 5. With the root Makefile converted to CRLF as bytes, head gives 110 passed.
• Every one of the 7 parameters fails with make_rules cannot read a line break other than \n or \r\n in the Makefile.
• Mutants on the LF Makefile: c4 fails only test_make_rules_reads_crlf_makefile_as_lf, c6 fails [\x0b], and c7 fails the six others. That matches the hand-off. On master's test file, c4, c6 and c7 each give 102 passed.
• No P1 or P2 so far.

---

Generated by Claude Code

[MaxGhenis]

Ready for hub review
VERDICT: APPROVE
339c7f7
METHODOLOGY_CHOICE: no

I reviewed head 339c7f79e61978fa8601a56dcf5e2fc335e016b1 on base master cc0157f5ed. The PR is open and that commit is still its head. It does what the hub's two items asked. Every parameter reaches the new assertion. Mutants c4, c6 and c7 now fail the tests the hand-off names, on an LF and on a CRLF Makefile. CI passes all 19 jobs, including the four Windows Test jobs, which ran the 8 new tests. I found no P1 or P2. The one false claim is the body's CI sentence, which the hub edits anyway, so I approve over it.

Assignment

• pulls/651/files and git diff --stat cc0157f HEAD list only tests/core/test_release_tagging.py (+31) and changelog.d/core650-line-break-pins.changed.md (+1). No file under policyengine_core/ or .github/ changes.
• The test file has 0 bytes above 127 (sum(b > 127 for b in open(..., 'rb').read())). Every character is written as an escape.
• Item 1: test_make_rules_rejects_each_line_break_make_ignores (:1124-1137) sits right after test_make_rules_rejects_syntax_it_cannot_follow. It matches the Close Makefile masks and limit PyPI credentials in the release tests #650 review's text, with U+2028 and U+2029 written as and .
• Item 2: test_make_rules_reads_crlf_makefile_as_lf (:1140-1152). It normalizes \r\n to \n before converting, which the brief didn't ask for. I cover that under Checked and sound.
• Changelog: the fragment is named core650-line-break-pins.changed.md, not claude/core650-line-break-pins.changed.md. A / would put the fragment in a subdirectory, so dropping the prefix follows core649-release-test-followups.changed.md. The type changed matches docs/engineering/skills/github-prs.md ("changed for behavior, documentation, tooling, or refactors").
• What the items asked for that the PR doesn't do: nothing. The hand-off couldn't report a Windows run, and that run exists (P3 1).

P3

1. The body's CI section is false: CI ran on the draft (PR body, "## CI"). The hand-off comment repeats the claim: "CI skips drafts, so no run exists at 339c7f7".

Failure scenario: a reader trusts the body and thinks this head has no CI result, or that pr.yaml skips drafts. It doesn't. .github/workflows/pr.yaml:1-4 triggers on pull_request for master, with no types filter, and no workflow file contains draft. The PR opened at 04:06:06Z. Pull request run 41965682770 started at 04:06:12Z (event: pull_request). ready_for_review came later, at 04:12:20Z (issues/651/events). That run is the only run at this SHA (total_count: 1). The hand-off may have queried actions/runs?head_sha= before the run was registered. I can't tell from here.

Command:

gh api repos/PolicyEngine/policyengine-core/issues/651/events --jq '.[]|[.event,.created_at]|@tsv'
ready_for_review	2026-11-05T04:12:20Z
gh api repos/PolicyEngine/policyengine-core/actions/runs/41965682770 --jq '[.created_at,.event,.head_sha]|@tsv'
2026-11-05T04:06:12Z	pull_request	339c7f79e61978fa8601a56dcf5e2fc335e016b1

Replacement for "This is a draft, so CI hasn't run.":

Pull request run 41965682770 at 339c7f7 passes all 19 jobs: Check changelog fragment, Lint, Lint workflows, Test on ubuntu-latest and windows-latest for Python 3.11–3.14, and Test Core and country packages on both OSes for py3.11–3.14. Each Windows Test job ran tests\core\test_release_tagging.py as 4 skipped and 106 passed (110 tests), so both new tests ran on Windows.

Checked and sound

All local runs used python -m pytest -q -p no:cacheprovider <file>, one file per process, with Python 3.14.6 and pytest 9.2.0, in a uv venv with uv pip install -e ".[dev]". uv was already installed.

• Counts. At head, test_release_tagging.py gives 110 passed and test_docs_deployment.py gives 5 passed. With master's test_release_tagging.py copied in, they give 102 passed and 5 passed. With the root Makefile converted to CRLF as bytes (\n → \r\n), head gives 110 passed. These match the body's table and its CRLF sentence. ruff format --check reports "1 file already formatted" and ruff check reports "All checks passed!" (ruff 0.17.3).

• Each line-break parameter reaches the new assertion. \tcoverage xml -i occurs once in the root Makefile, so the replacement always lands. I called make_rules directly on each of the 7 edited files. Each one raised make_rules cannot read a line break other than \n or \r\n in the Makefile. The test's match="make_rules cannot read a line break" doesn't match the other assertion's message ("make_rules cannot read a conditional, ..."), so no parameter can pass through the syntax check. The check decodes bytes with bytes.decode(), whose default is UTF-8, and the test writes UTF-8 bytes. So \x85, U+2028 and U+2029 don't depend on the Windows locale.

• The CRLF test holds on both checkouts. It gives 110 passed with the LF Makefile and with the CRLF copy. The normalization makes both copies come from the same LF bytes either way. Without it, a CRLF checkout would give \r\r\n and trip the lone-\r check. The non-vacuity assertion can fail. I set MAKEFILE to a copy without the \tcoverage xml -i line and called the test function, and it raised AssertionError at :1151. The check is list membership, so it needs a recipe line exactly equal to coverage xml -i. That holds at head (make_rules(Path('Makefile'))['test'] = (['test-country-template'], ['coverage run -a --branch -m pytest tests ...', 'coverage xml -i'])). If someone later changes that line, the test fails loudly, which is the safe direction.

• Mutants, each as a byte edit on the test file, one process each, with the file restored and its bytes compared to the original after every run. git status --porcelain was empty at the end.

  Mutant	LF Makefile	CRLF Makefile	Failing tests (LF)
  unmodified	110 passed	110 passed	none
  c4 \r(?!\n) → \r	1 failed, 109 passed	3 failed, 107 passed	test_make_rules_reads_crlf_makefile_as_lf (CRLF also fails test_only_push_publish_job_uploads_to_pypi and test_test_job_cannot_pass_with_failing_tests)
  c6 drop \v	1 failed, 109 passed	1 failed, 109 passed	test_make_rules_rejects_each_line_break_make_ignores[\x0b]
  c7 class → [\v\f]	6 failed, 104 passed	6 failed, 104 passed	...[\x1c], [\x1d], [\x1e], [\x85], `[

], [
]| | m1 (mine)read_bytes().decode()→.decode("latin-1")| 2 failed, 108 passed | 2 failed, 108 passed |...[
], [
]| | m2 (mine)read_text()→read_text(newline="")andsplitlines()→split("\n")| 1 failed, 109 passed | 1 failed, 109 passed |test_make_rules_reads_crlf_makefile_as_lf` |

The c4, c6 and c7 rows match the hand-off's table. m2 makes make_rules keep a trailing \r on each CRLF line. Only the new CRLF test catches it, so the test pins the whole CRLF read, not only the regex lookahead. On master's test file, c4, c6 and c7 each give 102 passed, which confirms the review's "passed all 102 tests".

• "Together with the existing parameters, this pins all nine characters." form-feed-inside-recipe-line (:1105-1109) and carriage-return-inside-recipe-line (:1110-1114) cover \f and a lone \r. The 7 new parameters cover the other seven characters in the class at :422.
• The hand-off's sed note. sed 's/$/\r/' on the root Makefile leaves a lone \r at the end of the file, which has no final newline. On that copy, head gives 3 failed, 107 passed, as the hand-off says. The byte conversion gives 110 passed.
• The fragment. It names the five line breaks the PR newly pins and the CRLF check, in one sentence. Both claims hold (rows above).
• CI (run 41965682770, completed / success): all 19 jobs are success, with none cancelled or skipped. The jobs are Check changelog fragment, Lint, Lint workflows, Test (ubuntu-latest, 3.11–3.14), Test (windows-latest, 3.11), (3.12), (3.13), (3.14), and Test Core and country packages on ubuntu-latest and windows-latest for py3.11–3.14. In each Windows Test job log, tests\core\test_release_tagging.py shows ssss and 106 dots, so 110 tests ran and none failed.

What I couldn't check

• docs.github.com isn't reachable from this session. So my statement that CI ran on the draft rests on the run record and pr.yaml, not on GitHub's documentation of pull_request's default activity types.
• Whether actions/checkout on windows-latest writes the Makefile with CRLF. The job logs don't show line endings. The CRLF test gives the same result either way, so this no longer affects coverage.
• I didn't read the GNU make manual this round. Nothing in this PR depends on how make reads a line beyond the Close Makefile masks and limit PyPI credentials in the release tests #650 review's probes.

---

Generated by Claude Code

[MaxGhenis]

US + core hub merge audit, core#651 at 339c7f79e61978fa8601a56dcf5e2fc335e016b1 (squash): Pin each line break the Makefile parser refuses, and its CRLF allowance.

• Review: an independent review in a separate Claude Code session approved this head at round 1, with one P3 and no P1 or P2: Pin each line break the Makefile parser refuses, and its CRLF allowance #651 (comment)
  • It called make_rules on each of the 7 edited Makefiles, and each one raised the new assertion.
  • Mutants c4, c6 and c7 each fail the tests the description names, on an LF and on a CRLF Makefile.
• Change after the review: none to the code. The hub replaced the description's CI sentence with the review's text (its P3) and restored the closing line.
• Classification: CORE_SEMANTICS no. The PR changes release tests and the changelog, not package code.
• PE-US core gate: the hub skipped it, since no package code changes.
• Gates the lander checked at merge:
  • every check passes at the head;
  • MERGEABLE;
  • not a draft;
  • no CHANGES_REQUESTED review;
  • the merge pins the head (--match-head-commit).
• Axiom: n/a; the PR changes no policy.

🤖 Generated with Claude Code