Skip to content

Share childcare calibration targets and make the test able to fail - #472

Open
vahid-ahmadi wants to merge 13 commits into
mainfrom
sync-childcare-calibration-targets
Open

Share childcare calibration targets and make the test able to fail#472
vahid-ahmadi wants to merge 13 commits into
mainfrom
sync-childcare-calibration-targets

Conversation

@vahid-ahmadi

@vahid-ahmadi vahid-ahmadi commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Refs #470.

Three problems

The targets were duplicated and had drifted. datasets/childcare/takeup_rate.py moved Tax-Free Childcare to the June 2025 figures. tests/test_childcare.py still asserted 0.6 / 660 from the September 2024 release. Same numbers, two places, silently out of step.

The TFC caseload target did not match its own cited source. It was recorded as 985 thousand children, cited to HMRC "Tax-Free Childcare statistics: June 2025" for 2024-25. That release reports 1,085,020 children with used accounts in 2024-25 (Table 2), unrevised in the March 2026 release, and 985,000 does not appear anywhere in it. Corrected to match the citation.

The test could not fail in any useful way. Its tolerance was:

passed = abs(ratio - 1) < 1

which admits any ratio in (0, 2). TFC spending was passing at 1.96× its target, with 0.04 to spare. A check that cannot detect a doubling is not doing much.

Changes

  • Targets move to datasets/childcare/targets.py, imported by both the optimisation and the test, so they cannot drift again.
  • TFC caseload target corrected from 985 to 1,085 thousand.
  • The test is parametrised per programme and metric, so each reports separately instead of one monolithic pass/fail.
  • Tolerance is ±40% — comfortably wider than the largest genuine deviation (universal caseload at 0.74×) and tight enough to catch a 2× miss.
  • TFC spending does not meet its target, so rather than widening the tolerance to hide that, it is recorded in KNOWN_MISSES and reported as xfail with the reason and the issue link.
  • A companion test fails if a known miss starts passing, so the exemption cannot outlive the problem it documents.

Where things stand at 2024 on enhanced_frs_2024_25

metric tfc extended targeted universal
spending 1.87× (known miss) 1.11× 0.76× 0.74×
caseload 1.02× 0.98× 0.74× 0.76×

On the TFC spending miss

Not fixed here, because it is not a take-up problem and this PR should not pretend otherwise.

Caseload is within 2% of its target once that target is corrected. Against HMRC's annual Table 1 and Table 2 for 2025-26 the built dataset is likewise 1.05× on families and 0.98× on children. The overshoot is entirely the average award: £1,353 against HMRC's £691.

That means no uniform take-up rate can fix it — about 0.836 would hit the caseload and about 0.427 the spending, from the present 0.88. The take_up/tax_free_childcare.yaml comment already anticipates this ("a full recalibration ... is planned; this manual bump closes most of the gap in the interim"); running that recalibration will land on the same wall.

The award overshoot has two sources, discussed in #470 and #471: the childcare fee base, and the fact that the model routes essentially all of a family's childcare spend through a TFC account (£5,412 per claiming family, against £2,764 implied by HMRC) where the scheme pays 25% only of what actually goes through the account.

Making the gap visible in CI is the useful step available now.

Testing

Dry-run against the built aggregates above:

x........                                                            [100%]
XFAIL test_childcare.py::test_childcare_hits_its_calibration_target[spending-tfc]
  - spending/tfc is a known miss (1.87x): ... See PolicyEngine/policyengine-uk-data#470.
8 passed, 1 xfailed

ruff format clean.

vahid-ahmadi and others added 2 commits August 27, 2026 17:15
The childcare targets were duplicated between the take-up optimisation and
the test that checks them, and had drifted. takeup_rate.py moved Tax-Free
Childcare to 0.63 / 985 against the HMRC June 2025 release; test_childcare.py
still asserted the 0.6 / 660 targets from the September 2024 release.

The test also could not fail in any useful way. Its tolerance was
abs(ratio - 1) < 1, which admits any ratio in (0, 2), so it cannot detect a
doubling. TFC spending was passing at 1.96x its target with 0.04 to spare.

Targets move to datasets/childcare/targets.py and are imported by both, so
they cannot drift again. The test is parametrised per programme and metric,
so each reports separately, and the tolerance is +/-40% — comfortably wider
than the largest genuine deviation (universal caseload at 0.74x) and tight
enough to catch a 2x miss.

TFC spending does not meet its target and is recorded as a known miss rather
than tolerated silently: CI reports it as xfail with the reason and the
issue. A companion test fails if a known miss starts passing, so the
exemption cannot outlive the problem.

Ratios at 2024 on enhanced_frs_2024_25, against the shared targets:

  spending  tfc 1.87 (known miss)  extended 1.11  targeted 0.76  universal 0.74
  caseload  tfc 1.12               extended 0.98  targeted 0.74  universal 0.76

Refs #470

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
The target was recorded as 985 thousand children, cited to HMRC "Tax-Free
Childcare statistics: June 2025" for 2024-25. That release reports 1,085,020
children with used accounts in 2024-25 (Table 2), unrevised in the March 2026
release, and 985,000 does not appear anywhere in it.

Correcting it strengthens the diagnosis in the PR rather than weakening it:
the built dataset is 1,107.7 thousand children, so caseload is 1.02x its
target rather than 1.12x, while spending stays at 1.87x. Caseload is right to
within 2% and the entire overshoot is the average award.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@juaristi22

Copy link
Copy Markdown
Collaborator

Adversarial review — request changes

The target correction and the move to a shared source of truth are good changes: HMRC's 2024–25 outturn is approximately £632.2 million of government top-up and 1,085,020 annual-unique children with used accounts. However, I found one current blocker, two issues to address, and a potential modeling improvement that may explain most of the remaining TFC spending miss.

Critical

  1. The required Test check is failing. The current PR has three successful checks and one failing required check (Pull request / Test). The branch should not merge until that job is green or the failure is diagnosed and shown to be unrelated.

Should address

  1. The global ±40% tolerance remains too permissive. targets.py allows every non-exempt programme/metric to miss its target by almost 40%. This is much better than the previous ±100%, but a 39% spending or caseload error still passes. I would use tighter, metric-specific tolerances and place each accepted material deviation in KNOWN_MISSES, rather than defining the general gate from the largest current miss.

  2. The old 985k figure remains in the take-up parameter's provenance. tax_free_childcare.yaml still says the 0.88 rate was based on “985k annual unique” children. That now contradicts both the cited HMRC release and this PR's corrected 1.085 million target. The rationale should be updated or removed. More fundamentally, the PR's own results show that a single take-up-rate adjustment cannot hit both 1.02× caseload and 1.87× spending.

Potential modeling improvement: separate annual participation from payment-active duration

HMRC defines a “used account” as one from which a payment was made to a childcare provider during the relevant month or year. Monthly figures count a child once in each month containing a provider payment; annual figures count that child once if there was any such payment during the year. HMRC explicitly explains that annual used-account counts do not equal the sum of monthly counts because children can appear in multiple months. See the HMRC TFC quality and methodology report and the March 2025 release commentary.

That provides an official-data estimate of mean payment-active duration:

mean active months
    = sum(monthly children with used accounts)
      / annual-unique children with used accounts

2024-25: 642,967 * 12 / 1,085,020 = 7.11 months
annual payment-active fraction: 7.11 / 12 = 0.5926

The inputs come from Table 2 of HMRC's 2024–25 accompanying spreadsheet. This is a payment-active fraction, not an eligibility fraction: HMRC also cautions that the payment date need not be the date on which childcare was received.

This matters because the model currently has annual-unique claimant selection and annualized childcare expenses, but no separate intensive margin for the portion of the year in which a selected child's TFC account makes provider payments. A child can be eligible throughout the year but only make TFC payments during part of it. As a diagnostic:

current spending ratio × payment-active fraction
1.87 × 0.5926 = 1.11

That is not a final calibration, but it shows that full-year treatment of annual-unique users can explain most of the spending excess while leaving the annual caseload target intact.

Potential implementation

  1. Add a versioned aggregate target/parameter derived from HMRC Table 2, for example:

    TFC_PAYMENT_ACTIVE_FRACTION = {2024: 0.5926}
  2. In policyengine-uk, introduce a distinct annual variable or parameter such as tax_free_childcare_payment_active_fraction. Do not overload tax_free_childcare_eligible_declaration_periods: eligibility duration and months containing provider payments measure different things.

  3. Apply the fraction to the annual qualifying expense routed through the TFC calculation, before applying the contribution formula and cap:

    tfc_routed_expense = (
        qualifying_childcare_expense
        * tax_free_childcare_payment_active_fraction
    )

    The statutory annual cap should remain unchanged in this expected-value correction. The separate gross-cost versus parent-deposit question in the contribution formula should still be resolved independently.

  4. Calibrate and test the two margins separately:

    • annual-unique participation against HMRC's annual used-child count;
    • payment intensity against the monthly-to-annual activity fraction and annual government top-up.
  5. Add a calibration test for the derived activity fraction, alongside the existing spending and caseload targets, and keep the remaining TFC spending deviation as an explicit known miss until all three margins are jointly within their metric-specific tolerances.

HMRC's more recent statistics also show why this should be versioned rather than treated as permanent: funded-hours expansions have reduced average TFC top-up and changed account usage over time. See the March 2026 commentary.

Validation summary

  • Reviewed head: ebb7c42aa0b63e8ce5db48870a3d285738a2466b
  • Target values independently checked against official HMRC releases.
  • Current GitHub status: one failing required Test check; changelog, lint, and bundle-manifest checks pass.
  • Local Ruff passed in the earlier review; the full dataset test could not be reproduced without the private enhanced FRS, and no private microdata was accessed.

Review severity: REQUEST_CHANGES

The target correction is sound, but the failing required check and contradictory take-up provenance should be resolved. The payment-active fraction is a non-blocking modeling recommendation for addressing the documented 1.87× spending miss without disturbing the nearly correct annual caseload.

Addresses review on #472.

The failing check was this PR's own guard, working as intended. I recorded
Tax-Free Childcare spending as a known miss at 1.87x, measured on the
published enhanced FRS artefact (v1.56.16). A dataset built from main meets
the target at 1.12x, so test_known_misses_are_still_missing failed and told
me the exemption was recorded from the wrong thing. Removed.

That difference matters beyond this entry: local figures taken from the
published artefact are not a safe basis for setting tolerances, which is why
this does not tighten them to the per-programme values I had measured. Added
report_ratios, which prints every programme's built value, target and ratio
on each run, so a follow-up can set bounds from the built dataset. The
tolerance helper and per-programme override table are in place for that.

Also corrects the take-up parameter's provenance comment, which still cited
985k children and so contradicted both the HMRC release and the corrected
target, and records that a single take-up rate cannot hit both targets when
they diverge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

Thanks — this was a good catch on all three, and the blocker turned out to be the most interesting of them.

1. The failing check — fixed, and it was this PR's own guard working

test_known_misses_are_still_missing failed with:

AssertionError: tfc spending now hits its target (1.12x).
Remove it from KNOWN_MISSES in datasets/childcare/targets.py.

I recorded the known miss at 1.87×, measured on the published enhanced FRS artefact (v1.56.16, the revision policyengine-uk downloads by default). A dataset built from main meets the target at 1.12×. The guard existed precisely so an exemption could not outlive the problem it records, and the problem was already gone. KNOWN_MISSES is now empty and the suite should be green.

I have not isolated why the published artefact and a fresh build diverge this much on TFC. Things I ruled out: it is not the benunit sort fix (#462) — is_married aligns with adult counts at 97.6% in the published data, matching the 2023-24 vintage — and both use take-up 0.88 (would_claim_tfc = 0.874). Worth someone looking at separately; the published artefact is what model users currently get.

This also corrects the premise of #470, and I have updated it there.

2. Tolerance — structure added, values deliberately not tightened

You are right that ±40% is too permissive, and I have put the machinery in (tolerance(metric, programme) with a per-programme override table). I have not set tighter values, because the point above says I cannot do it safely: the deviations I measured locally come from the published artefact, and it differs from a fresh build by enough to have caused this exact mistake once already. Setting 0.2 from those numbers risks breaking CI for reasons unrelated to the data being wrong.

Instead I added report_ratios, which prints every programme's built value, target and ratio on each run. Once one run records the built dataset's actual position, tightening is a two-line follow-up with real numbers behind it. I would rather do that than guess twice.

3. Take-up provenance — fixed

take_up/tax_free_childcare.yaml no longer cites 985k. It now records the correct 1,085,020, that the 0.88 bump was calibrated against enhanced_frs_2022_23 and not revisited when the build moved to the 2024-25 FRS five weeks later, and your point that a single rate cannot hit both targets when they diverge.

On the payment-active fraction

This is the most useful part of the review and I agree with it, including the part where it corrects me.

Your 7.11 months from sum(monthly children) / annual-unique children is a better construction than the 7.6 I got from families, and the distinction you draw is one I had blurred: eligibility duration and months containing provider payments are different things, and I should not have implied the second could be recovered from the first.

PolicyEngine/policyengine-uk#1830 fixes a genuine latent bug — eligible_fraction was applied to the cap but not to the spend, so a part-year family below the cap received the full-year top-up — but it is only the mechanism, and it is keyed on eligibility. I have said in that PR that a maintainer may want a calibrated parameter instead; your framing is sharper, and a separate tax_free_childcare_payment_active_fraction not overloading tax_free_childcare_eligible_declaration_periods is the right shape. I will follow up there rather than widen this PR.

Your diagnostic 1.87 × 0.5926 = 1.11 is also worth noting against the finding above: a fresh build is already at 1.12× without any payment-active adjustment. So on current main that adjustment would take TFC spending below target rather than to it, which is a further reason to calibrate it rather than derive it.

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

Correcting my own reasoning on the KNOWN_MISSES removal

I removed the Tax-Free Childcare entry saying a build from main meets the target at 1.12×, so the exemption was recorded from the wrong thing. That reasoning was wrong, and I want it on the record before this is reviewed.

CI builds with TESTING=1, which in datasets/create_datasets.py does:

is_testing = os.environ.get("TESTING", "0") == "1"
epochs    = 32 if is_testing else 512
oa_clones = 2  if is_testing else 10

So the 1.12× comes from a 32-epoch smoke build, not the full 512-epoch release calibration. The repo already knows this — test_vehicle_ownership.py says "under the reduced-epoch CI build (TESTING=1) the vehicle-ownership target under-converges. Loose CI smoke check only — the full-calibration release dataset matches NTS", and test_scotland_babies.py widened its tolerance for the same reason.

I also ruled out the model version: policyengine-uk 2.89.2 (what uv.lock pins) gives the identical £1.178bn on the published dataset as 2.92.0 does. It is purely the build.

So 1.87× on the published release dataset is the real figure, and the exemption was correctly recorded. What it could not survive is that the guard runs against a build that cannot see the miss.

The removal itself should still stand, for a different reason: PolicyEngine/policyengine-uk#1833 applies HMRC's payment-active fraction and takes TFC spending to 1.11× on the release dataset, so once that merges there is no miss to exempt. Keeping an entry would then fail test_known_misses_are_still_missing on the real build too.

This does leave a structural problem worth raising separately: a calibration check that runs only against a reduced-epoch build cannot detect calibration regressions. Every childcare tolerance in this PR is set against numbers the CI build cannot reproduce. I have not tried to solve that here — report_ratios at least records what the smoke build sees on each run, so the gap between the two is visible rather than assumed.

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 — re-requested review. Since your last pass I have addressed all three points, and then corrected my own reasoning on the blocker in the comment above: the 1.12× I removed the exemption for is CI's reduced-epoch TESTING=1 build, not the release dataset. Your original framing was right. The payment-active fraction you proposed is now implemented in PolicyEngine/policyengine-uk#1833 and takes the release dataset to 1.11×.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1] The green calibration check does not validate the artifact users receive. CI builds with TESTING=1, which reduces calibration from 512 to 32 epochs. The PR now removes the TFC spending entry from KNOWN_MISSES because that smoke build reports about 1.12x target, even though the code and PR discussion acknowledge the published release artifact at 1.87x. report_ratios observes the same smoke build, so it cannot expose the release miss either. A separate model PR cannot make this check truthful until that model is released, the data lock is updated, and the actual release-calibration artifact is validated. Keep the release miss explicit or add a release-artifact/full-calibration gate. — targets.py:43-70, pull_request.yaml:69-94
  2. [C2] The new “single source of truth” drops the sources for six calibration targets. targets.py defines the extended, targeted, and universal spending and caseload figures only as “prior DfE values.” The PR deletes the DfE and calculation-spreadsheet URLs that were previously adjacent to them. Those values now drive both optimization and CI but cannot be independently traced from the authoritative module. Restore authoritative, value-level references (and direct links for the HMRC targets too). — targets.py:9-40, takeup_rate.py:2-13

Should Address

  1. [A1] The default ±40% gate remains too permissive. The override structure exists, but TOLERANCES is empty, so every current target may miss by almost 40% and pass. Set evidence-based per-metric tolerances and use explicit known misses for accepted material deviations. — targets.py:43-61
  2. [A2] The target period mapping is implicit and inconsistent. HMRC reports financial-year 2024-25, while the test calculates annual period 2024. The ODS monthly figures sum to £638.2m for calendar 2024 versus the selected £632.2m fiscal-year target; the spending difference is only 0.95%, but annual-unique caseload cannot be converted by summing months. Name and document the mapping, and use one period convention across targets, dataset, and calculations. — targets.py:9-28, test_childcare.py:38-43
  3. [A3] The TFC source values are rounded unnecessarily. The prose cites £632.2m and 1,085,020, but the target dictionary stores £630m and 1,085,000. The differences are small, but an aggregate calibration target should retain published precision or state a rounding rule. — targets.py:9-18, targets.py:27-40
  4. [A4] The chronology in the target rationale is wrong. A September 2025 funded-hours expansion cannot explain a 2024-25 outturn. HMRC says it was still forthcoming in the June 2025 release; the April and September 2024 expansions are the changes inside the target year. — targets.py:20-22

Suggestions

  1. [S1] Add data-independent unit tests for target units, tolerance lookup/overrides, and the known-miss guard. The current focused file entirely skips when the private enhanced FRS is absent.

Source Audit Summary

Category Count
Confirmed correct official values 2
Confirmed value mismatches 0
Untraceable moved targets 6
Period-definition gaps 1

Validation Summary

Check Result
Regulatory/source definition 1 release-vs-smoke validation flaw; 1 period gap
Reference quality 6 moved targets lack usable source links
Code patterns Ruff passed
Test coverage GitHub green, but not release-representative; local 10 skipped without private FRS
Official values £632.2m and 1,085,020 confirmed from HMRC ODS
CI status 4/4 attached checks passing

Review Severity: REQUEST_CHANGES

Next Steps

Make the calibration gate representative of the released artifact, restore target-level sources, then tighten and document the target periods and tolerances.

…ision

Addresses review on #472.

The check validates a smoke build, not the release artefact. CI builds with
TESTING=1, which cuts calibration from 512 epochs to 32, and this repo's own
tests already say the reduced-epoch build under-converges while "the
full-calibration release dataset matches NTS". Tax-Free Childcare spending is
1.87x target on the published enhanced_frs_2024_25 against about 1.12x on a
CI build. That is now stated prominently in targets.py, with the pointer that
a green run here is not evidence the released dataset meets its targets and
that closing the gap needs a release-calibration gate which does not exist.

Restores the published precision the targets were rounded away from: £632.2m
rather than £630m, and 1,085,020 children rather than 1,085,000.

Corrects the chronology in the rationale. A September 2025 expansion cannot
explain a 2024-25 outturn; the April and September 2024 expansions of the
working-parent entitlement are the changes inside the target year.

Documents the period mapping, which was implicit: HMRC and DfE report
financial years while the checks evaluate the model at annual period 2024.
For spending the difference is under 1% (£638.2m for calendar 2024 against
£632.2m fiscal), but an annual-unique caseload cannot be converted by summing
months, so the fiscal-year count is used as-is.

Flags the extended targets as inherited unsourced, and points at the DfE
release that the universal and early-learning-for-2-year-olds targets are
corrected against in the follow-up change.

Adds data-independent tests for target shape and units, the tolerance lookup
and its overrides, and the known-miss guard — test_childcare.py skips
entirely without the private dataset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

All addressed except one, which I have documented rather than solved because I do not think this PR can solve it.

C1 — the check validates a smoke build, not the release artefact. You are right, and it is now stated prominently in targets.py: CI builds with TESTING=1 at 32 epochs instead of 512, this repo's own tests already say the reduced-epoch build under-converges while "the full-calibration release dataset matches NTS", and Tax-Free Childcare is 1.87× on the published artefact against about 1.12× on a CI build. The file now says explicitly that a green run here is not evidence the released dataset meets its targets, and that closing the gap needs a release-calibration gate which does not exist.

I have not added that gate. It needs a full 512-epoch build in CI or a published-artefact fetch, which is a change to the workflow and its runtime rather than to these targets. Worth its own issue; happy to file it.

On keeping the release miss explicit: with PolicyEngine/policyengine-uk#1830 and #1834 plus #473, the release artefact lands at 1.02×, so the miss disappears rather than needing an exemption. Until those ship it stands at 1.87×, and that figure is now written into the file.

C2 — moved targets lost their sources. Fair. The universal and early-learning-for-2-year-olds targets are corrected against DfE's published January 2024 figures in the stacked #474, with the derivation and link in the module. The extended targets are now flagged as inherited unsourced rather than presented as traceable — I could not find where they came from, and would rather say so than imply provenance.

A3 — precision. Restored: £632.2m rather than £630m, 1,085,020 rather than 1,085,000.

A4 — chronology. Corrected. The September 2025 expansion cannot affect a 2024-25 outturn; the April and September 2024 expansions are the in-year changes. My error, carried over from the note I was editing.

A2 — period mapping. Documented. HMRC and DfE report financial years, the checks evaluate at annual period 2024, and the spending difference is under 1% (£638.2m calendar against £632.2m fiscal) while an annual-unique caseload cannot be converted by summing months, so the fiscal-year count is used as-is.

A1 — tolerances still empty. Deliberate, for the reason in C1: I cannot set evidence-based per-metric bounds from numbers the CI build cannot reproduce, and setting them from the release artefact would make CI fail for reasons unrelated to the data being wrong. This is the same dependency as the release-calibration gate.

S1 — data-independent tests. Added test_childcare_targets.py: target shape and units, the tolerance lookup and its overrides, and a guard that every known miss names a real programme and carries a reason citing an issue. Six tests, no dataset needed.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1 STILL OPEN] The green test still validates only a 32-epoch smoke build. The new warning accurately documents that the published artifact can miss TFC spending by 1.87x while CI sees about 1.12x, but no release-calibration gate or known-miss protection was added. A passing check therefore still cannot validate the artifact users receive. — policyengine_uk_data/datasets/childcare/targets.py:68
  2. [C2 STILL OPEN] The registry still contains unsourced non-TFC targets. #474 proposes source corrections for universal/targeted, but #472 itself retains those values and explicitly says the extended targets are inherited unsourced; even after the stack, extended £2.5bn/740k remain untraceable. — policyengine_uk_data/datasets/childcare/targets.py:34
  3. [C3 NEW] Towncrier ignores the changelog fragment. towncrier build --draft renders “No significant changes” because changelog.d/childcare-calibration-targets.md has no configured type. — changelog.d/childcare-calibration-targets.md:1

Should Address

  1. [A1 STILL OPEN] The global ±40% tolerance remains too broad and no per-target overrides are populated. — policyengine_uk_data/datasets/childcare/targets.py:82

Resolved Prior Findings

  • [A2 RESOLVED] Financial-year/calendar-year mapping is now explicit and quantified for TFC spending.
  • [A3 RESOLVED] Exact £632.2m and 1,085,020 precision restored and tested.
  • [A4 RESOLVED] Chronology now cites the April/September 2024 expansions rather than September 2025.
  • [S1 RESOLVED] Six data-independent target/tolerance tests added.

Validation Summary

  • Focused tests: 6 passed; Ruff passed.
  • Towncrier: no significant changes.
  • CI: 4/4 checks passed on the exact head.
  • Full gated-data/release build not run; the PR itself documents why CI cannot substitute for it.

Review Severity: REQUEST_CHANGES

Next Steps

Add a release-artifact calibration gate or tracked known miss, source every remaining target, and fix the changelog type.

It had no configured type, so towncrier rendered "No significant changes".
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 re-review requested. Only C3 is addressed here: the fragment had no towncrier type and rendered "No significant changes"; renamed to .fixed.md and confirmed it renders. C1 (release-calibration gate) and C2 (unsourced extended £2.5bn/740k) are still open, and A1's tolerance is unchanged. Do you want those in this PR, or tracked separately so the sourced TFC corrections can land?

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1 STILL OPEN] The green calibration test still validates only a 32-epoch smoke build, not the artifact users receive. The new warning accurately records that the published Enhanced FRS artifact misses TFC spending by 1.87x while CI sees about 1.12x, but KNOWN_MISSES is empty and there is still no release-artifact/full-calibration gate. report_ratios observes the same smoke build, so a green run cannot validate the release. Keep the release miss separately tracked or add a release-calibration gate. — policyengine_uk_data/datasets/childcare/targets.py:68
  2. [C2 STILL OPEN] The target registry still contains untraceable non-TFC values. In #472, the extended, targeted and universal spending/caseload values are not backed by value-level authoritative references. Stacked #474 corrects and sources universal and targeted, but the extended £2.5bn and 740k targets remain explicitly described as inherited and unsourced even after that stack. A shared target registry used by optimization and CI needs traceable sources for every target. — policyengine_uk_data/datasets/childcare/targets.py:34

Should Address

  1. [A1 STILL OPEN] The global ±40% tolerance remains broad and no per-target overrides are populated. Every programme/metric may miss by almost 40% and still pass. Populate evidence-based overrides from release-representative measurements and use explicit tracked misses for material accepted deviations. — policyengine_uk_data/datasets/childcare/targets.py:82

Resolved Prior Findings

  • [C3 RESOLVED] changelog.d/childcare-calibration-targets.fixed.md is now consumed by Towncrier and renders under Fixed.
  • [A2 RESOLVED] Financial-year/calendar-year mapping is explicit and quantified for TFC spending.
  • [A3 RESOLVED] Published £632.2m and 1,085,020 precision is retained and tested.
  • [A4 RESOLVED] The chronology now identifies the April/September 2024 expansions.
  • [S1 RESOLVED] Six data-independent target/tolerance tests exist.

Source Audit Summary

Category Count
Confirmed official TFC values 2
Confirmed value mismatches 0
Untraceable targets in #472 6
Untraceable after stacked #474 2

Validation Summary

  • Focused local tests: 6 passed, 10 skipped without the private enhanced FRS.
  • Ruff: passed on all changed Python files.
  • Towncrier: fragment renders under Fixed.
  • Exact-head CI at review snapshot: changelog, lint and manifest checks passed; Test pending.
  • Full release-calibration build: not run; this absence is the substance of C1.

Review Severity: REQUEST_CHANGES

Next Steps

Add release-artifact calibration protection, source the remaining extended targets, and replace the global tolerance with release-informed per-target bounds.

740 thousand and GBP2.5bn were inherited from before this module existed and
could not be traced. DfE reports 379,000 3-4 year olds and 242,500 2-year olds
on the working parent entitlement in January 2025, giving 621.5 thousand and
GBP2.415bn at the statutory 570 additional hours.

January 2024 cannot serve this programme: the 2-year-old entitlement began in
April 2024, so that census counts only 3 and 4-year-olds. The resulting mixed
basis, the full-entitlement upper bound, and the model's lack of under-2s
against DfE's 224,300 are documented rather than smoothed over.
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 C2 addressed for extended — the last untraceable targets in the registry are now sourced.

I traced them rather than re-flagging them. DfE, "Funded early education and childcare", reporting year 2025, January 2025 census:

children
3 and 4-year-olds, working parent entitlement 379,000
2-year-olds, working parent entitlement 242,500
matching the ages the model places on this scheme 621,500

379,000 × 570 × 5.88 + 242,500 × 570 × 8.28 = £2.415bn. So 740 → 621.5 thousand and £2.5bn → £2.415bn. Neither prior figure appears in any release I could find.

Three things I have documented rather than smoothed over, because each is a real weakness in the result:

The basis is January 2025, not 2024. Every other target here is for 2024, and I would rather not mix bases. But January 2024 cannot serve this programme at all: the 2-year-old entitlement began in April 2024, so that census counts only 3 and 4-year-olds (361,800) and misses half the scheme the model implements. A documented mixed basis beats an untraceable number, but it is not clean.

The spending figure is a full-entitlement upper bound, assuming every registered child took all 570 additional hours. Unlike universal and targeted it is not the caseload times a constant in the model — extended_childcare_entitlement varies with maximum_extended_childcare_hours_usage — so it is not redundant and I kept it. But calibrating an hours distribution against an upper bound biases those hours upward, and no outturn exists to replace it.

Under-2s are not covered at all. DfE reports 195,100 one-year-olds and 29,200 aged 9 to 11 months registered in January 2025. The model places nobody under 2 on this scheme. Those 224,300 children are outside both the target and the model — a coverage gap in the model, not a calibration error, and no target change fixes it. Worth an issue if you agree.

Against the built dataset the new targets sit at 1.17× (caseload) and 1.15× (spending), both inside tolerance. Test added asserting the values and their derivation. #474 rebased on this.

C1 remains open and I have not attempted it — a release-calibration gate is a CI change well beyond this PR, and I would rather not bolt a half version onto it. Happy to take it as a follow-up issue if you would accept that.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1 STILL OPEN] Green CI still validates a 32-epoch smoke build, not the release artifact users receive. The exact head continues to state that the published enhanced FRS misses TFC spending at 1.87x while the reduced-epoch CI build is about 1.12x, and there is still no release-calibration gate. Therefore the test can be green while the released dataset materially misses its target. Documentation is useful but does not make the check validate the production artifact. — policyengine_uk_data/datasets/childcare/targets.py:108
  2. [C2 STILL OPEN, NARROWED] The new £2.415bn extended-childcare spending target is not the quantity the model calibrates. DfE corroborates the 379,000 and 242,500 headcounts and the £5.88/£8.28 rates, but not a £2.415bn outturn. The PR multiplies both age groups by 570 hours. On the exact model contract, extended_childcare_entitlement pays 15 weekly hours (570/year) for 2-year-olds but 30 weekly hours (1,140/year) for 3–4-year-olds, because extended-eligible children are excluded from the separate universal variable. Applying the PR's full-usage construction to the modeled output gives £3.685bn, not £2.415bn; the proposed target is only 65.5% of the comparable full-usage model value. It also knowingly calibrates an hours distribution to an upper bound rather than observed expenditure. Remove this spending constraint until an observed or usage-adjusted target exists, or redefine and test the model/output so target and quantity match. — policyengine_uk_data/datasets/childcare/targets.py:41; consumer at policyengine_uk_data/datasets/childcare/takeup_rate.py:94

Should Address

  1. [A1 STILL OPEN] The global ±40% tolerance remains without programme-specific overrides. A 39% deviation still passes every target despite very different source quality and expected precision. — policyengine_uk_data/datasets/childcare/targets.py:98
  2. [A2 NEW] The extended caseload uses a January 2025 snapshot against an annual-period-2024 model and otherwise 2024 targets. The code acknowledges the mixed basis, but does not quantify the bias from using an end-of-expansion stock rather than an annual-average or aligned-period count. Under-2s are also excluded because the model lacks them. Prefer an aligned model period or explicitly derive an annual 2024 comparator. — policyengine_uk_data/datasets/childcare/targets.py:54

Resolved Prior Findings

  • [C2 PARTIALLY RESOLVED] The extended caseload inputs are now traceable to DfE, and the arithmetic is tested. The spending construction remains non-corroborating and mismatched to the model as described above. Stacked #474 addresses the remaining universal/targeted source issues.
  • Towncrier, TFC precision, chronology, period documentation, target sharing, and data-independent test findings remain resolved.

Validation Summary

  • Source audit: DfE confirms the 379,000 and 242,500 registrations and 2024-25 average rates; it does not publish the derived £2.415bn as spending.
  • Exact model-path reproducer: ages 2/3/4 receive 15/30/30 weekly hours over 38 weeks; comparable full-usage value is £3.6850158bn versus target £2.415bn.
  • Focused target tests: 7 passed; the new test reproduces the PR arithmetic but does not test the model's 30-hour 3–4-year-old path.
  • Ruff and Towncrier: passed.
  • Exact-head GitHub CI: all 4 checks passed; PR is mergeable.

Review Severity: REQUEST_CHANGES

Next Steps

Remove or replace the mismatched extended spending target, and add a production-artifact calibration gate rather than relying on the reduced-epoch smoke build.

…the model pays 75% of

The earlier GBP2.415bn gave both age groups 570 hours, but the model gives
3 and 4-year-olds 1,140 because extended-eligible children are excluded from
the universal variable. Correctly constructed it is GBP3.685bn, a full-usage
ceiling against GBP2.778bn actually paid. Calibrating the hours distribution
to a ceiling biases it upward, and no outturn exists. The caseload stays.
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 C2 — you are right, and the £2.415bn is gone.

I had given both age groups 570 hours. The model pays 3 and 4-year-olds 30 weekly hours (1,140 a year) because extended-eligible children are excluded from the universal variable — confirmed in gov/dfe/extended_childcare_entitlement/hours.yaml, where the 3 and 4 brackets have been 30 since 2015. Correctly constructed the DfE figure is:

379,000 × 1,140 × 5.88 = £2.540bn
242,500 ×   570 × 8.28 = £1.145bn
                         £3.685bn

against £2.778bn actually paid on enhanced_frs_2024_25 — 75% of full usage. My figure was 65.5% of the comparable model quantity, exactly as you computed, and would have pulled the calibrated hours distribution against a number the model cannot reach.

But I have not replaced it with £3.685bn either. That is a ceiling — every registered child taking every funded hour — and calibrating maximum_extended_childcare_hours_usage to a ceiling biases hours upward. No outturn exists. So the constraint comes out, on the same reasoning #474 applied to universal and targeted, and the module now records the corrected arithmetic and why it is still not a target. The 621.5k caseload stays, since that part you corroborated.

Net effect on the registry: spending is now Tax-Free Childcare only, the one programme with a published expenditure outturn. Test added asserting extended has no spending entry. #474 rebased on this; its spending set is {tfc}.

A2 I have documented rather than resolved. An annual-average 2024 comparator would need monthly 2-year-old registrations through the April 2024 expansion, which DfE does not publish; I have said so in the module and left the bias unquantified rather than invent a number.

C1 remains open and unattempted, as before.

targets.py claimed no release-calibration gate existed. push.yaml has built
the release at 512 epochs, run make test, and only then uploaded since July
2025; the 1.87x artefact passed because the (0, 2) tolerance admitted it.

- test_release_gate_is_wired asserts the release build is not a TESTING
  smoke build, tests precede the upload, and a failing test stops the job.
- The check reports which build it validated.
- CASES iterates over the targets that exist, fixing a KeyError now that
  only Tax-Free Childcare has a spending target.
- Tax-Free Childcare tolerances tighten to 0.25 with stated provenance.

Consequence, stated in the module: main misses TFC at 1.87x, so #473 must
merge before this or the next release is blocked until it does.
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 C1 and A1 addressed — and C1 turned out to rest on a false statement in my own module, which I have corrected rather than built around.

The release gate already exists. push.yaml triggers on the pyproject.toml bump that versioning.yaml pushes to main — i.e. on every release — builds at the full 512 epochs with no TESTING flag, runs make test, and only then runs make upload. A failing test stops the upload. It has been wired that way since July 2025 (f085b40). The 1.87x artefact went through it because the (0, 2) tolerance admitted it with 0.13 to spare, not because nothing was checking. My targets.py said "a release-calibration gate does not exist yet", which was wrong, and your C1 was a fair reading of it.

So the fix is not to add a gate but to make the existing one legible and un-removable:

  • test_release_gate_is_wired parses push.yaml and asserts the three properties that make it a gate: the release build is not a TESTING smoke build, Run tests precedes Upload data, and the test step has no continue-on-error. Verified it passes on the real workflow and fails on each of those three mutations.
  • The check now prints which build it validated (smoke (TESTING=1, 32 epochs) vs release (512 epochs)), so a push log and a PR log cannot be confused for each other.
  • The module now says plainly: measure KNOWN_MISSES and overrides on the release build, never a PR run — which is exactly the mistake that removed the earlier TFC entry on a 1.12x PR build while the release sat at 1.87x.

A1 — TFC tightens to ±25% on both metrics. Provenance stated in the file: both HMRC figures are exact outturns, and the published artefact with the corrected inputs (policyengine-uk 2.93.0 plus #473's adjustment) measures 1.02x on both; 0.25 leaves room for a fresh calibration to move the weights. The other three stay at ±40% deliberately — the only release-build measurements I have predate #474's take-up corrections, so I would be guessing. Set from the first push log after that lands.

Also fixed a bug I introduced: CASES iterated every programme for both metrics, which would KeyError now that extended has no spending target.

Merge-order consequence, and it is the important part. main currently misses TFC spending at 1.87x. Once this merges, the next release's push build will fail that check and block the upload until #473 merges — which is the gate doing its job, but it is worth choosing rather than discovering. #473 is approved and green. Merge it first, and this PR lands onto a main that passes.

A2 (January 2025 stock vs annual 2024) is documented, not resolved, as before.

Re-review requested.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Prior Finding Status

  1. [C1 RESOLVED] The live release workflow does run a 512-epoch build and gate upload on the tests. The checked-in path is push.yaml build → test → upload; the build has no effective TESTING=1, create_datasets.py selects 512 epochs, the tests open the same enhanced-dataset path, and upload follows only after a successful test step. The new guard test is incomplete, but that is a new regression-protection issue rather than proof that the live gate is absent. — .github/workflows/push.yaml:29-77; policyengine_uk_data/datasets/create_datasets.py:69-75,245-289,342-345; policyengine_uk_data/tests/conftest.py:18-34
  2. [C2 RESOLVED] The unsupported extended-childcare spending target has been removed. The objective now iterates only over registered target keys. DfE supports 570 annual hours for eligible two-year-olds, 1,140 total hours for working-parent three-/four-year-olds, and the £8.28/£5.88 national rates. The old £2.415bn target was not a like-for-like outturn; the corrected £3.6850158bn calculation is a full-usage ceiling, not a calibration target. — policyengine_uk_data/datasets/childcare/targets.py:46-65,89-103; policyengine_uk_data/datasets/childcare/takeup_rate.py:121-126; DfE census; DfE operational guide
  3. [A1 STILL OPEN] The global ±40% fallback remains for non-TFC targets. Adding explicit ±25% TFC overrides is a useful narrowing, but a 39% miss still passes each remaining programme regardless of source precision. Keep this documented until release evidence supports programme-specific thresholds. — policyengine_uk_data/datasets/childcare/targets.py:138-159
  4. [A2 STILL OPEN] The extended caseload still compares an annual-period-2024 model with a January-2025 end-of-expansion stock. The new prose now discloses the mismatch, but disclosure does not align or quantify it. DfE also publishes summer/autumn headcount and PTE-hours observations, so the claim that unpublished monthly registrations are the only route to a better comparator is too absolute. Use the termly series for a documented annualisation/sensitivity check, or retain the target explicitly as a proxy. — policyengine_uk_data/datasets/childcare/targets.py:67-74,97-100; policyengine_uk_data/datasets/childcare/takeup_rate.py:42-69,89-103; DfE termly collection

Critical (Must Fix)

  1. [C3] The target-shape unit test deterministically contradicts the new sparse target registry. Production intentionally removes TARGETS["spending"]["extended"], while test_every_programme_has_both_metrics still requires every programme under every metric; the same test file later asserts that extended spending must be absent. The exact focused run fails at this invariant. Update it to validate all caseload programmes and the intended spending-key subset. — policyengine_uk_data/tests/test_childcare_targets.py:21-24,78-85; policyengine_uk_data/datasets/childcare/targets.py:89-103
  2. [C4] The tolerance test fails and corrupts real module state. Production now defines both TFC overrides as 0.25, but the test expects caseload to fall back to 0.4. Its finally block then deletes the pre-existing spending override rather than restoring 0.25, so later tests can silently see the global fallback. Assert the committed overrides directly and use monkeypatch or save/restore the original value for fallback testing. — policyengine_uk_data/tests/test_childcare_targets.py:47-53; policyengine_uk_data/datasets/childcare/targets.py:146-159
  3. [C5] Removing extended spending makes the fitted hours mean and standard deviation exactly underidentified. The optimiser still fits ext_hours_mean and ext_hours_sd, but after spending is removed the scored extended caseload observes hours only as positive/non-positive. For fixed latent draws, clip(mu + sigma*z, 0, 30) > 0 depends on mu/sigma; valid pairs (15,5) and (30,10) therefore give the same mask and the same entire objective. No prior, distributional moment, regularisation term, or retained source evidence selects 15.019/4.972, yet those values remain embedded in frs.py. extended_rate is not subject to this exact claim because it is also indirectly constrained by targeted/universal mutual-exclusion moments. Remove the two hours-distribution parameters from this optimisation and label/fix them as a modelling assumption, or add independent usage/distribution evidence before reporting them as optimised. — policyengine_uk_data/datasets/childcare/takeup_rate.py:16-31,60-69,109-126,148-177; policyengine_uk_data/datasets/frs.py:1475-1482

Should Address

  1. [A3] The structural release-gate test has confirmed blind spots and overstates “full fidelity.” The current workflow is valid, but the test still passes if TESTING=1 moves to workflow/job scope, if upload uses if: always() or if: failure(), or if build/upload commands are replaced. It also ignores PE_UK_DATA_OA_CLONES="1", although the non-testing code default is 10. Resolve effective environment across all scopes, pin normalised build/test/upload commands and safe upload conditions, and define the intended clone contract; otherwise describe the release accurately as 512-epoch, one-clone. — policyengine_uk_data/tests/test_childcare.py:119-143; .github/workflows/push.yaml:29-79; policyengine_uk_data/datasets/create_datasets.py:69-75
  2. [A4] “£2.778bn actually paid” mislabels a model output as an administrative outturn. enhanced_frs_2024_25 is an enhanced microdataset and the amount is the modelled weighted sum of extended_childcare_entitlement; DfE does not report it as paid expenditure. Relabel it “modelled on enhanced_frs_2024_25.” The derived 75.39% comparison is reasonable once clearly described as model output divided by a constructed full-usage ceiling. — policyengine_uk_data/datasets/childcare/targets.py:46-65; measurement path at policyengine_uk_data/tests/test_childcare.py:55-60
  3. [A5] The 1.02x TFC result used to justify ±25% is not reproducible from a cited artifact or log. HMRC independently corroborates the £632.2m and 1,085,020 targets, but repository/artifact metadata does not contain the stated 1.02x ratios. Attach the exact dataset revision, stacked code SHAs, command, and captured release-representative output, or describe the tolerance as a QA judgement rather than as a verified release measurement. — policyengine_uk_data/datasets/childcare/targets.py:147-154; changelog.d/childcare-calibration-targets.fixed.md:3; HMRC source

Suggestions

  1. [S1] Correct the sparse-target comment. The test says only TFC retains a spending target, but targeted and universal spending remain; say only extended spending was removed. — policyengine_uk_data/tests/test_childcare.py:35-42; policyengine_uk_data/datasets/childcare/targets.py:89-96
  2. [S2] Do not infer dataset provenance solely from the current TESTING environment. A non-testing local run can inspect a previously downloaded artifact, so label that case non-smoke/unknown unless build metadata proves it is a release build. — policyengine_uk_data/tests/test_childcare.py:44
  3. [S3] Narrow the historical gate claim. Upload-after-tests dates to May 2025, and the workflow briefly used TESTING=1 in December 2025. State the verified behavior for release 1.56.16 instead of saying the same configuration has applied continuously “since July 2025.” — policyengine_uk_data/datasets/childcare/targets.py:121-126; changelog.d/childcare-calibration-targets.fixed.md:3
  4. [S4] Add the direct HMRC publication/ODS URL beside the TFC constants. The values are corroborated, but the current module names the release without a clickable source. — policyengine_uk_data/datasets/childcare/targets.py:9-18

Source Audit Summary

Category Count
Quantitative/provenance claims independently checked 4
Regulatory or source-value mismatches 0
Evidence/provenance limits 2
PDFs / pages rendered 0 / 0
Prior blockers resolved 2

The checked claims were: the £3.6850158bn full-usage arithmetic; the non-outturn nature of £2.778bn/75%; the absence of a reproducible 1.02x result artifact; and the availability of official termly evidence. No PDF mismatch passed through the two-stage verifier because no PDFs were in scope.

Validation Summary

Check Result
Regulatory Accuracy No enacted-rule mismatch; prior C2 resolved; period proxy remains A2
Reference Quality Official TFC/caseload/rate values corroborated; two model-run claims need clearer provenance
Code Patterns 1 exact identification failure; 1 incomplete structural regression guard
Formatting Ruff and git diff --check passed
Test Coverage 2 deterministic failures; 7 focused tests passed
Source Value Audit 0 mismatches; 4 claims verified with stated qualifications
CI Status 3 checks passed; Test pending at consolidation

The focused command produced 2 failed, 7 passed. uv run --locked could not start because the snapshot lockfile required updating, so verification used a read-only isolated uv run --no-project --with-editable . environment. Both failures exercise only in-memory constants and dictionary lookup and do not depend on the private dataset or PolicyEngine model outputs.

Review Severity: REQUEST_CHANGES

Next Steps

Fix the two deterministic tests, stop presenting the unidentifiable hours mean/SD as calibrated values, and harden the release-gate regression test. Then correct the two model-output provenance claims and retain the January-2025 caseload target only with an explicit proxy/sensitivity rationale.

To auto-fix issues: run the fix-pr workflow for this PR.

vahid-ahmadi and others added 2 commits August 29, 2026 10:42
Addresses María's review of #472.

C3, C4: the shape test required every programme under both metrics, which
the sparse spending registry deliberately breaks, and the tolerance test
expected a default that the committed TFC overrides no longer allow — and
its cleanup deleted the real override rather than restoring it. Both now
assert what production actually defines, using monkeypatch for the override
case.

C5: with no extended spending target, the objective sees the hours
distribution only through whether a clipped draw is positive, so (15, 5) and
(30, 10) give the same loss. The mean and sd are removed from the
optimisation and held as a documented assumption, imported by frs.py so the
two cannot drift.

A3: the gate test now checks TESTING at workflow and job scope too, and that
the upload carries no `if:` that would run it despite a failing test.
A4: £2.778bn relabelled as modelled, not paid.
A5: the 0.25 tolerance described as a QA judgement, not a measured bound.
S1, S2: comment corrections.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses A5 of María's review of #472: the 1.02x is not reproducible from
a cited release log.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

Thanks — C3, C4 and C5 were all real. Pushed 39ec216 and 591f3af.

C3, C4 — both tests failed on the exact head; confirmed locally before fixing. The shape test required every programme under both metrics, which the sparse spending registry deliberately breaks, and the tolerance test expected caseload/tfc to fall back to 0.4 when the committed override is 0.25. Its finally deleted the real override rather than restoring it, exactly as you describe. They now assert what production defines — test_the_registry_covers_the_programmes_it_claims splits caseload from the intended spending subset, and the override case uses monkeypatch.setitem on a key with no committed override.

C5 — you are right, and it is worse than underidentified in principle: the mask is identical. With extended spending gone the objective sees the hours distribution only through clip(mu + sigma*z, 0, 30) > 0, and at the committed draws both (15, 5) and (30, 10) put every unit above zero, so the loss is bit-identical. ext_hours_mean and ext_hours_sd are out of the optimisation — it is four take-up rates now — and 15.019 / 4.972 live in one place as EXTENDED_HOURS_MEAN / EXTENDED_HOURS_SD, documented as a modelling assumption and imported by frs.py so the two copies cannot drift. Replacing them needs published usage evidence, not a re-run.

A3 — the gate test now resolves TESTING across workflow, job and step scope, and asserts the upload step carries no if: at all, so always() and failure() both fail it. I have not built the full command-normalisation machinery; the clone contract is still unpinned.

A4 — £2.778bn is relabelled as the modelled weighted sum of extended_childcare_entitlement on enhanced_frs_2024_25, not an outturn.

A5 — the ±25% is now described as a QA judgement resting on the exact HMRC outturns, noting the 1.02x came from a local build and not a citable release log, in both the module and the changelog.

A1 stays open and I think deliberately so: the ±40% fallback should be replaced by measurement from the first push.yaml log after this stack lands, not by a guess now. A2 (extended caseload period basis) also stays a documented proxy — your point about the termly series is fair and I would rather do that as its own change than fold an annualisation into this one.

S1, S2 — comments corrected; a non-testing run is now labelled "release or previously built" rather than asserting a release build.

S3 of María's review: the note read as if #473 were still pending.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

  • HMRC TFC evidence: Tax-Free Childcare statistics, June 2025 and the official ODS tables. Table 1 reports £632.2m government top-up in 2024–25; Table 2 reports 1,085,020 children with used accounts. The March 2026 tables independently repeat both values.
  • DfE caseload evidence: Funded early education and childcare, reporting year 2025, including its caution about dual-eligible two-year-olds recorded under the working-parent entitlement rather than FRAS.
  • DfE period evidence: Early years data collection for the 2024–25 financial year, with the official 12/38 summer + 14/38 autumn + 12/38 spring weighting.
  • DfE entitlement/rate evidence reused from the prior audit: 2024–25 operational guide and funding-formula technical note.
  • Source-audit medium: authoritative HTML, ODS, CSV, and XLSX sources; no PDFs were in scope and 0 pages were rendered.
  • Year: model period 2024; HMRC target period 2024–25; DfE extended-caseload observation January 2025.
  • Scope: incremental PR changes plus unresolved prior findings.
  • Reviewed head SHA: 9f447c687c0c37d7608c7317a92d1db41e153654
  • Mode: incremental from 2505d3344376d9aa5d7d260fc3a2962e9d65bbdb

Branch Status

The PR is 11 commits ahead of and 2 commits behind main. A rebase is recommended, but the review used the exact PR head and merge-base diff, so staleness did not affect these findings.

Prior Finding Status

  1. [C1 RESOLVED] The live release workflow builds at 512 epochs and gates upload on successful tests. The checked-in workflow runs build → test → upload without TESTING=1; upload has no failure-bypassing condition. It explicitly uses one OA clone, so this is a 512-epoch, one-clone release build. — .github/workflows/push.yaml:29-79; policyengine_uk_data/datasets/create_datasets.py:69-75
  2. [C2 RESOLVED] The unsupported extended-childcare spending target remains removed. No changed source or implementation reintroduced it.
  3. [C3 RESOLVED] The registry-shape tests now match the sparse target registry. They require four caseload targets, only the three supported spending targets, and explicitly require extended spending to be absent. Independent focused execution passed. — policyengine_uk_data/tests/test_childcare_targets.py:18-29,89-96
  4. [C4 RESOLVED] The tolerance tests no longer leak global state. Both committed TFC overrides are asserted at 0.25; the synthetic override uses monkeypatch, and two same-process restoration rounds reproduced the original mapping exactly. — policyengine_uk_data/tests/test_childcare_targets.py:47-64
  5. [C5 RESOLVED] The underidentified hours mean/SD have been removed from the optimiser. The optimiser now has exactly four take-up inputs and four bounds; 15.019/4.972 are fixed, explicitly labelled assumptions shared by the simulation and FRS build. Independent dynamic verification confirmed the four-parameter contract and rejected the former six-parameter interface. — policyengine_uk_data/datasets/childcare/takeup_rate.py:15-28,31-47,76-85,164-187; policyengine_uk_data/datasets/frs.py:34-37,1479-1490
  6. [A4 RESOLVED] £2.778bn is now correctly labelled a modelled weighted sum, not expenditure actually paid by DfE.policyengine_uk_data/datasets/childcare/targets.py:61-70
  7. [A5 RESOLVED in production wording] The module and changelog now call ±25% a QA judgement and admit that the local 1.02x result lacks a cited reproducible release log. A stale test comment remains as S5. — policyengine_uk_data/datasets/childcare/targets.py:153-163; changelog.d/childcare-calibration-targets.fixed.md:3
  8. [S1 RESOLVED] The sparse-target comment now correctly identifies only extended spending as absent.
  9. [S4 RESOLVED] Direct official HMRC publication and ODS links are now present and independently corroborate both TFC targets.

Critical (Must Fix)

  1. [C6] The release-calibrated ±25% TFC tolerance is applied unconditionally to the deliberately reduced-fidelity PR smoke build, leaving required CI red. The PR workflow sets TESTING=1, which selects 32 calibration epochs and one OA clone, while the release workflow uses 512 epochs. Nevertheless, tolerance() has no build-mode input and test_childcare_hits_its_calibration_target applies the same 0.25 threshold in both environments. Exact-head CI therefore fails TFC spending at 0.597726x (£0.377882bn / £0.6322bn), a 40.227% miss; the suite reports 1 failed, 614 passed, 3 skipped, 1 xfailed. Independent verification reproduced the arithmetic and traced the failure to this PR's new unconditional threshold. This is not a dependency/import failure, a target-source mismatch, or evidence that a 512-epoch release build misses. Keep ±25% for the release gate, but define an explicit smoke-build contract (or run enough PR-build calibration to satisfy the release contract) so PR CI can merge without weakening release validation. — policyengine_uk_data/datasets/childcare/targets.py:120-173; policyengine_uk_data/tests/test_childcare.py:45-81; .github/workflows/pull_request.yaml:52-76; failed Test job

Should Address

  1. [A1 STILL OPEN] The global ±40% fallback remains for every non-TFC target. A 39% miss still passes regardless of the source precision. Retain it only as a documented provisional fallback until release evidence supports programme-specific thresholds. — policyengine_uk_data/datasets/childcare/targets.py:149-168
  2. [A2 STILL OPEN] The extended target still compares annual model period 2024 with a January-2025 end-of-rollout stock. The code discloses the proxy, but incorrectly says a better comparator would require unpublished monthly registrations. DfE publishes termly observations and a 12/38 + 14/38 + 12/38 weighting; applying the published national two-year-old counts gives about 235,897 rather than 242,500 and lowers the combined comparator from 621,500 to about 614,897. Use the termly series for a documented annualisation/sensitivity check, or correct the claim and retain January explicitly as a proxy. The opening period summary should also distinguish HMRC financial-year targets from the DfE January snapshot. — policyengine_uk_data/datasets/childcare/targets.py:31-36,73-80,95-108
  3. [A3 STILL OPEN, narrowed] The live release gate is safe, but its structural regression test remains bypassable and overstates “full fidelity.” Mutation testing found that inline TESTING=1, echo make data/test/upload, make test || true, and a changed OA-clone count all pass undetected. Pin normalised executable build/test/upload commands, reject swallowed failures and inline smoke configuration, and assert the intended clone count. Describe the live contract as “512 epochs, one OA clone,” not full fidelity. — policyengine_uk_data/tests/test_childcare.py:121-152; .github/workflows/push.yaml:29-79
  4. [A6] Move the fixed hours assumptions out of the executable optimiser module. Foundational frs.py now imports two constants from childcare/takeup_rate.py, which also imports SciPy, Microsimulation, release metadata, and Hugging Face configuration. The exact dependency set imports successfully and performs no network I/O, so this is nonblocking, but it reverses dependency direction and relies on SciPy transitively. Put the constants in a lightweight neutral assumptions module shared by both call sites. — policyengine_uk_data/datasets/frs.py:34-38,1479-1485; policyengine_uk_data/datasets/childcare/takeup_rate.py:1-28
  5. [A7] Carry DfE's dual-eligibility recording caveat beside the extended target. DfE warns that a substantial proportion of two-year-olds eligible for both FRAS and the working-parent entitlement were recorded under working parent contrary to guidance. Treating all 242,500 as cleanly matching the model's extended scheme can misallocate calibration mass between extended and targeted childcare. Disclose the caveat and consider a joint/combined calibration check where administrative classification is unreliable. — policyengine_uk_data/datasets/childcare/targets.py:38-48,95-108
  6. [A8] Add a committed regression test for the C5 fix. Independent verification proves the optimiser has four bounded take-up inputs and uses fixed hours assumptions in both draw sites, but the committed suite does not pin that contract. A focused test should reject the former six-value interface and assert that both simulation and FRS construction consume the shared assumptions. — policyengine_uk_data/datasets/childcare/takeup_rate.py:31-47,76-85,164-187; policyengine_uk_data/datasets/frs.py:1479-1490

Suggestions

  1. [S2 STILL OPEN] Do not infer 512-epoch provenance from the absence of TESTING=1. A local run may inspect a previously built or downloaded artifact; label it non-smoke / provenance unknown unless build metadata proves its epoch count. — policyengine_uk_data/tests/test_childcare.py:44-53
  2. [S3 STILL OPEN] Correct stale workflow history and merged-PR prose. The same configuration did not run continuously “since July 2025”: release testing briefly used TESTING=1 in December 2025, and upload-after-tests predates July. The same module also still calls already-merged #473 “ready to merge.” State only the configuration verified for release 1.56.16/current workflow and remove the obsolete paragraph. — policyengine_uk_data/datasets/childcare/targets.py:127-132,185-187; changelog.d/childcare-calibration-targets.fixed.md:3
  3. [S5] Align remaining provenance wording with the corrected production note. The test still says a “published artefact measures 1.02x,” despite the module saying the result came from an uncaptured local build. It also calls both HMRC values “exact,” although the published £632.2m top-up is rounded to £0.1m. Say “official published outturns” and describe ±25% as a provisional QA threshold pending a captured 512-epoch result. — policyengine_uk_data/tests/test_childcare_targets.py:54-59; policyengine_uk_data/datasets/childcare/targets.py:149-163
  4. [S6] Preserve reproducible provenance for long-lived model aggregates. If £2.778bn remains in documentation, record the exact dataset revision, PolicyEngine-UK SHA/version, command, and captured aggregate output; otherwise label it an illustrative uncaptured model measurement. — policyengine_uk_data/datasets/childcare/targets.py:61-70

Source Audit Summary

Category Count
Official TFC target values corroborated 2
New direct HMRC links validated 2
Source/value mismatches 0
Material period/proxy qualifications 2
PDFs / pages rendered 0 / 0

HMRC's £632.2m top-up and 1,085,020 used-child figures are correct and conceptually align with the model outputs. Calendar-2024 HMRC monthly top-ups sum to £638.2m, so replacing the fiscal-year denominator would move the smoke ratio from 0.598x to roughly 0.592x; source-period mapping cannot explain the approximately 40% smoke miss. No numerical source mismatch entered the report.

Validation Summary

Check Result
Regulatory Accuracy No enacted-rule mismatch; C5 resolved; A2/A7 proxy qualifications remain
Reference Quality Both HMRC targets and direct links corroborated; residual provenance wording only
Code Patterns C6 smoke/release contract failure; A6 reverse dependency
Formatting Ruff check/format and git diff --check pass
Test Coverage 10 focused repository tests pass; 4 meta-tests pass; A3/A8 gaps remain
Source Value Audit 0 mismatches; 2 official target values confirmed
CI Status Failing: 3 checks pass; required Test check fails (1 failed / 614 passed / 3 skipped / 1 xfailed)

Review Severity: REQUEST_CHANGES

Next Steps

Separate the 32-epoch smoke acceptance contract from the 512-epoch release threshold and restore green required CI without weakening the release gate. Then address the termly/dual-eligibility target caveats and harden the gate/fixed-hours regression tests.

To auto-fix issues: run the fix-pr workflow for this PR.

C6 of María's review: the ±25% Tax-Free Childcare tolerance is a release
threshold, but tolerance() had no notion of build mode, so it also gated the
32-epoch pull-request build — which came in at 0.598x and left CI red.

A reduced-epoch build cannot be held to a release threshold. The same code
measured 1.12x and then 0.60x on consecutive smoke runs, a swing no
release-representative bound absorbs. SMOKE_TOLERANCE is ±60%: it catches a
target that lost or doubled its population and nothing finer, which is what
32 epochs can honestly support. The release thresholds are unchanged, and
tolerance(smoke=True) takes the wider of the two, so nothing held tighter
than ±60% is loosened by it.

A6: the fixed hours assumptions move to childcare/assumptions.py, which
imports nothing. frs.py was reaching into the calibration module — and so
transitively SciPy, Microsimulation and the HuggingFace configuration — to
read two floats.

A3: the gate test pins the build, test and upload commands exactly and the
OA clone count, so `make test || true`, `echo make data` and an inline
TESTING=1 no longer pass it. The contract is described as 512 epochs and one
clone rather than full fidelity.

A8: a test pins the four-parameter optimiser contract and that both draw
sites consume the shared assumptions.

S2, S3, S5: a non-smoke build is labelled provenance-unknown; the workflow
history no longer claims continuity since July 2025 and no longer calls the
merged #473 ready to merge; the tolerance rationale says published outturn
and provisional QA threshold rather than exact and measured.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

C6 is right and it was my error: I tightened the tolerance without giving the tolerance a notion of which build it was grading. Fixed in 1a7ebb1.

C6 — the smoke build now has its own contract. tolerance() takes a smoke flag; SMOKE_TOLERANCE is ±60% and tolerance(smoke=True) returns the wider of the two, so no release threshold is loosened by it. ±60% is deliberately weak, and the module says so: a 32-epoch build can show that the pipeline runs and no target is out by an order of magnitude, and it cannot show the numbers are right. The evidence for not setting it any tighter is your own — the same code measured 1.12x and then 0.598x on consecutive smoke runs, so anything much under this flakes at random. The release thresholds are untouched and remain what validates the artefact.

Worth recording that the swing itself is the finding here. Yesterday's green smoke run at 1.12x and today's red one at 0.598x differ by more than the entire ±25% band, on the same targets. That is a stronger argument for your A1 position than anything in the module: a pull-request build cannot support programme-specific thresholds either, and they will have to come from the first push.yaml log.

A6 — my mistake, and thank you for catching it. frs.py was reaching into the calibration module, and so transitively SciPy, Microsimulation and the HuggingFace configuration, to read two floats. The assumptions now live in childcare/assumptions.py, which imports nothing.

A3 — the gate test pins the three commands exactly and asserts PE_UK_DATA_OA_CLONES == "1". Your four mutations all fail it now: make test || true, echo make data, an inline TESTING=1, and a changed clone count. The contract is described as 512 epochs and one clone rather than full fidelity.

A8test_the_optimiser_fits_take_up_rates_only pins the four-parameter interface and that both draw sites consume the shared assumptions.

S2, S3, S5 — a non-smoke build is now labelled provenance-unknown; the July 2025 continuity claim is replaced with the configuration verified for 1.56.16, noting the December 2025 TESTING=1 period; #473 is no longer described as ready to merge; the test comment says published outturn and provisional QA threshold.

A1, A2, A7 I am leaving. A1 needs the release log, as above. On A2 you are right that I overstated it — DfE does publish the termly series and the 12/38 + 14/38 + 12/38 weighting, and ~235,897 rather than 242,500 is a real difference. That is a target change on evidence I have not worked through, and the same is true of the dual-eligibility caveat in A7. Both belong in their own PR rather than folded into this one, and I would rather land the CI fix first than widen the diff again.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

  • HMRC TFC evidence: Tax-Free Childcare statistics, June 2025, the official ODS tables, and HMRC methodology. These continue to support £632.2m government top-up and 1,085,020 children with used accounts for 2024–25.
  • DfE caseload evidence: Funded early education and childcare, reporting year 2025, including its dual-eligibility recording warning.
  • DfE period evidence: Early years data collection for 2024–25, with the official 12/38 summer + 14/38 autumn + 12/38 spring weighting.
  • Source-audit medium: authoritative HTML, ODS, CSV, and XLSX evidence reused from checksum-verified prior audits; no PDFs and 0 pages rendered.
  • Year: model period 2024; HMRC target period 2024–25; DfE extended-caseload observation January 2025.
  • Scope: incremental PR changes plus unresolved prior findings.
  • Reviewed head SHA: 1a7ebb11487628383980ab274da4df37cdbabc87
  • Mode: incremental from 9f447c687c0c37d7608c7317a92d1db41e153654

Branch Status

The PR is 12 commits ahead of and 2 commits behind main. A rebase is recommended, but the review used the exact PR head and merge-base diff, so staleness did not affect the findings.

Prior Finding Status

  • C1–C5 remain RESOLVED. The checked-in release workflow is safe; extended spending remains absent; registry/tolerance-state tests remain corrected; and the optimiser still has four bounded take-up inputs with fixed shared hours assumptions.
  • [C6 RESOLVED] PR build and test steps now select the 32-epoch smoke contract, while release build and test steps remain non-smoke and retain ±25% TFC thresholds. The exact prior 0.597726x case fails release and passes smoke; exact-head CI is green.
  • [A1 STILL OPEN] The global ±40% release fallback remains for every non-TFC target.
  • [A2 STILL OPEN] January 2025 remains a disclosed but unannualised comparator for annual model period 2024.
  • [A3 STILL OPEN, narrowed] Exact workflow commands, ordering, and one clone are now pinned, but the structural guard still misses test-step environment/skip mutations.
  • A4, A5, and A6 RESOLVED. £2.778bn is labelled modelled; ±25% is labelled a QA judgement; and fixed hours now live in dependency-free childcare/assumptions.py.
  • [A7 STILL OPEN] The DfE dual-eligibility classification warning is still absent beside the extended target.
  • [A8 STILL OPEN, partially addressed] A committed C5 regression test exists, but independent mutations prove its source-text assertions do not protect the claimed behavior.
  • S1, S2, and S4 RESOLVED. Sparse-target wording, non-smoke provenance labeling, and direct HMRC links are corrected.
  • [S3 STILL OPEN, narrowed] Production workflow history is corrected, but the changelog retains the superseded “full-fidelity/since July 2025/exact” account.
  • [S5 STILL OPEN, narrowed] The unit-test wording is corrected, but targets.py still calls rounded HMRC top-up an “exact” outturn.
  • [S6 STILL OPEN] The retained £2.778bn model aggregate still lacks a captured dataset/code/command provenance trail.

Critical (Must Fix)

None. The checked-in release workflow is safe, all four required checks pass, and no official source/value or enacted-rule mismatch was found.

Should Address

  1. [A1 STILL OPEN] Replace the global ±40% release fallback with evidence-based per-programme thresholds when a 512-epoch result is available. A 39% release miss still passes every non-TFC target regardless of source precision. — policyengine_uk_data/datasets/childcare/targets.py:152-171
  2. [A2 STILL OPEN] Use DfE's termly series for a documented sensitivity/annualisation, or correct the claim and keep January explicitly as a proxy. DfE's published 12/38 + 14/38 + 12/38 weighting gives about 235,897 two-year-olds rather than 242,500, lowering the combined comparator from 621,500 to about 614,897. The opening summary also incorrectly groups the January DfE stock with HMRC financial-year outturns. — policyengine_uk_data/datasets/childcare/targets.py:31-36,73-80,103-108; DfE termly method
  3. [A3 STILL OPEN, narrowed] Complete the release-gate regression guard. The live release workflow is safe, but independent mutations show the guard stays green if Run tests sets TESTING=1 (weakening release checks to smoke), if Build/Run tests use a false if, or if inspected TESTING values are YAML integer 1. Require TESTING to be absent from workflow, job, build, and test scopes; reject skip conditions on build/test; and directly pin the 32/512 selector. — policyengine_uk_data/tests/test_childcare.py:124-176; .github/workflows/push.yaml:29-79
  4. [A7 STILL OPEN] Carry DfE's dual-eligibility recording caveat beside the extended target. A substantial share of two-year-olds eligible for both FRAS and working-parent support were recorded under working parent contrary to guidance. Separately calibrating extended and targeted childcare can therefore convert administrative classification noise into take-up differences. — policyengine_uk_data/datasets/childcare/targets.py:38-48,95-108; DfE reporting-year-2025 release
  5. [A8 STILL OPEN] Replace the C5 source-text test with behavioral protection. The implementation is correct, but the committed test still passes after inserting params = params[:4] (accepting the old six-value interface) and after changing both normal draws to literal (30, 10) while leaving unused constant names in the modules. Dynamically assert six inputs reject, four accept, four bounds exist, and both draw sites consume the shared assumptions. — policyengine_uk_data/tests/test_childcare_targets.py:131-161; policyengine_uk_data/datasets/childcare/takeup_rate.py:37-74,156-168; policyengine_uk_data/datasets/frs.py:1479-1487
  6. [A9] Scope the ±60% smoke override to TFC spending. Exact-head CI reports ratios 0.60, 0.81, 0.93, 0.81, 1.12, 0.79, 0.96; only TFC spending needs more than its existing threshold. The global max(release, 0.6) widens six unrelated checks. A 0.41x mutation passes all six today but fails all six with a keyed {("spending", "tfc"): 0.6} override. Preserve each other target's release threshold until a smoke run demonstrates a specific need. — policyengine_uk_data/datasets/childcare/targets.py:174-203; policyengine_uk_data/tests/test_childcare_targets.py:102-128; exact Test job
  7. [A10] Make KNOWN_MISSES build-aware and bounded before it is used. The map is empty now, so no current result is hidden. With a synthetic entry, however, zero, doubled, and 10x outputs all XFAIL; the companion passes those catastrophic outcomes and can falsely demand removal when a 32-epoch smoke result happens to enter the release band. Apply release exemptions only to release checks, keep the ordinary smoke catastrophe check, skip release-miss retirement in smoke, and validate an expected miss interval. — policyengine_uk_data/tests/test_childcare.py:67-99; policyengine_uk_data/datasets/childcare/targets.py:206-218
  8. [A11] Add a programme-independent upper bound for every tolerance. The current registry is safe, and exact TFC overrides are pinned. But an unpinned override of 1.1 or 99 passes all relevant committed tolerance tests and admits zero and doubled outputs in release and smoke. Assert 0 < release <= 1 and 0 < smoke <= 1; if the documented smoke cap is intentional, assert smoke <= SMOKE_TOLERANCE rather than the current reversed >= check. — policyengine_uk_data/tests/test_childcare_targets.py:50-67,102-128; policyengine_uk_data/datasets/childcare/targets.py:193-203
  9. [A12] Correct the empirical smoke-run provenance used to justify ±60%. Public CI shows 1.11x at 2505d334, then 0.60x at the different commit 39ec2161, and 0.60x at this head. No same-SHA rerun supports “same code,” displayed 1.12x, or random flaking; 39ec2161 changed FRS, optimiser, targets, and tests. Say that different 32-epoch branch builds ranged from 1.11x to 0.60x and cite both runs, or produce same-SHA reruns before attributing the swing to randomness. The evidence still supports separating smoke from release, but not the stronger claim. — policyengine_uk_data/datasets/childcare/targets.py:174-190; policyengine_uk_data/tests/test_childcare_targets.py:102-116; changelog.d/childcare-calibration-targets.fixed.md:4; 1.11x run; 0.60x run

Suggestions

  1. [S3 STILL OPEN, narrowed] Rewrite the changelog as one coherent final account. It still says “full fidelity,” continuous 512 epochs “since July 2025,” and “exact” HMRC figures despite the production module correctly recording one OA clone, the December-2025 smoke interval, and rounded top-up. Its new “order-of-magnitude breakage and nothing finer” claim also contradicts the actual (0.4, 1.6) interval, which rejects a doubling. — changelog.d/childcare-calibration-targets.fixed.md:3-4
  2. [S5 STILL OPEN, narrowed] Replace the remaining “exact outturns” phrase with “official published outturns.” HMRC publishes £632.2m rounded to £0.1m. — policyengine_uk_data/datasets/childcare/targets.py:157-164
  3. [S6 STILL OPEN] Preserve reproducible provenance for £2.778bn. Record the exact dataset revision, PolicyEngine-UK revision, command, and captured aggregate output, or call it an illustrative uncaptured model measurement. — policyengine_uk_data/datasets/childcare/targets.py:61-70
  4. [S7] Align the smoke-contract docstring and test name with max(...). The code intentionally loosens tighter release thresholds in smoke, but the docstring says the tighter bound remains in both builds and the test is named “never loosens.” State the real invariant: non-smoke calls retain release thresholds; smoke calls use only documented smoke overrides. — policyengine_uk_data/datasets/childcare/targets.py:193-203; policyengine_uk_data/tests/test_childcare_targets.py:119-128

Source Audit Summary

Category Count
Official TFC target values corroborated 2
Source/value mismatches 0
Material DfE period/classification qualifications 2
Model-run provenance claims needing correction 1
PDFs / pages rendered 0 / 0

HMRC still supports the TFC denominators and the model metrics remain aligned. The new ±60% number is an engineering smoke contract, not an official tolerance. A2 and A7 remain comparability qualifications rather than official-value errors.

Validation Summary

Check Result
Regulatory Accuracy No enacted-rule or official-value mismatch; A2/A7 remain
Reference Quality Official targets corroborated; smoke-run provenance inaccurate as written
Code Patterns Live runtime/release gate safe; global smoke scope and latent guard mechanisms need hardening
Formatting Ruff check/format and git diff --check pass
Test Coverage 13 focused tests pass in smoke and non-smoke; verified mutation gaps in A3/A8/A10/A11
Source Value Audit 2 official HMRC values confirmed; 0 mismatches
CI Status Passing: 4/4 checks, Test job 618 passed / 3 skipped / 1 xfailed

Review Severity: COMMENT

Next Steps

Keep the green smoke/release split, but key the smoke override to TFC spending and harden the release guard, known-miss mechanism, tolerance caps, and C5 behavioral regression coverage. Correct the unsupported smoke-run provenance and retain A2/A7 as explicit calibration limitations.

To auto-fix issues: run the fix-pr workflow for this PR.

A9: max(release, SMOKE_TOLERANCE) widened all seven checks when only
Tax-Free Childcare spending needed it. The other six ran at 0.79 to 1.12 on
the same build, so they keep their release thresholds and a regression in
any of them still fails the pull request. SMOKE_TOLERANCES is keyed, and a
test asserts nothing outside it is loosened.

A12: I claimed the same code measured 1.12x and then 0.60x. It did not.
Those were different commits — 1.11x at 2505d33 and 0.60x at 39ec216 —
and no same-SHA rerun exists, so the swing cannot be attributed to
randomness. Both runs are now cited, and the text says what the evidence
supports: a range across branch builds, which is still enough to show a
32-epoch build cannot carry a 25% threshold.

S7: the docstring said a tighter release threshold survived into the smoke
build while max() did the opposite, and the test was named for the
invariant the code did not have. Both now describe what the keyed lookup
actually does.

A11: a test rejects any tolerance above the smoke contract, so an unpinned
1.1 or 99 can no longer admit a zeroed or doubled target.

S3, S5: the changelog drops "full fidelity", the July 2025 continuity claim
and "exact" for the same reasons the module already did, and no longer
describes a (0.4, 1.6) interval as order-of-magnitude.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

A12 is a straight error on my part, and thank you for checking it rather than taking the claim. I wrote that the same code measured 1.12x and then 0.60x. I checked your citation: 1.11x was at 2505d334 and 0.60x at 39ec2161, different commits, and no same-SHA rerun exists. Neither commit changes how the extended hours are drawn or what TFC pays, so I still think under-convergence is the likely explanation — but that is an argument, not a measurement, and I had presented it as one. The module, the changelog and the test now say a range across branch builds, cite both runs, and note that no same-SHA rerun supports attributing it to randomness. The conclusion survives: 32 epochs cannot carry a 25% threshold either way.

A9 — right, and the mutation you ran makes the case. max(release, SMOKE_TOLERANCE) widened all seven checks to serve one. SMOKE_TOLERANCES is now keyed to ("spending", "tfc"), and test_the_smoke_contract_is_scoped_to_the_target_that_needs_it asserts every other target keeps its release threshold in both builds, so your 0.41x case fails six of them again.

S7 — the docstring and the test name were both describing an invariant the code did not have. max() loosens a tighter release threshold; the docstring claimed the tighter bound survived into the smoke build. That is fixed by the keying above, and both now say what the lookup does.

A11test_no_tolerance_admits_a_lost_or_doubled_population asserts 0 < allowed <= SMOKE_TOLERANCE for every target in both builds, so an unpinned 1.1 or 99 can no longer pass everything.

S3, S5 — the changelog now matches the module: one OA clone rather than full fidelity, the December 2025 TESTING=1 interval rather than continuity since July, published rather than exact, and it no longer calls a (0.4, 1.6) interval order-of-magnitude.

On the rest. A3, A8 and A10 are all fair — source-text assertions are weak protection, and I would rather replace them with the behavioural versions you describe than half-do it here. A2 and A7 are target changes on DfE evidence I have not worked through; the termly weighting giving ~235,897 rather than 242,500 is a real correction and deserves its own review, not a late commit on a green PR. A1 needs the release log. I will open issues for A2, A7 and the A3/A8/A10 test hardening rather than leave them in review comments — say if you would rather they landed here.

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.

2 participants