fix(destinations): the masking pass can no longer exit unsafely
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m5s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m16s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 7m23s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m5s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m16s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 7m23s
Review found _mask_for_disclosure could return with its invariant broken and say nothing. add_companion only ever withheld a *published* cell, so a group with one suppressed category and every other one not_applicable — routine in special schools and AP, where few categories apply — left the loop with the lone suppressed cell still solvable. Reproduced on a nine-pupil cohort: one hidden cell, cohort served, residual intact. A disclosure-control pass that fails silently is worse than none, because everything downstream trusts it. The loop now runs until the invariant holds and escalates when no companion exists: the pupil group is dropped from the payload, and an empty block serialises as None so the section is absent rather than an empty shell. disclosure_invariant_holds() is exported so tests assert it directly instead of re-deriving it, and an exhaustive test sweeps all 81 suppression patterns of a four-category group. Also fixes a test that set up six measures and checked one: the loop was `for measure in ["school_sixth_form"]`. It now checks every measure, and against the real invariant — none hidden, or at least two, rather than "at least two", which the five published measures would have failed. No regression on real data: 262 mainstream secondaries, all-pupils bar still drawable on 94%, zero invariant violations, one disadvantaged group dropped by the new escalation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
This commit is contained in:
1 parent
102397fe69
commit
2e9b5c83c5
3 files changed
+181
-42
No files matched your search
+93
-36
@@ -839,10 +839,44 @@ def _format_cohort_year(year) -> str | None:
|
||||
return text
|
||||
|
||||
|
||||
def _mask_for_disclosure(groups: dict) -> None:
|
||||
"""Add secondary suppression until no withheld figure can be recovered.
|
||||
_PUPIL_GROUPS = ("disadvantaged", "other", "all")
|
||||
|
||||
Not rendering a number is not the same as not publishing it. This endpoint
|
||||
|
||||
def _lone_hidden_groups(groups: dict) -> list:
|
||||
"""Pupil groups hiding exactly one category — solvable by subtraction."""
|
||||
return [
|
||||
key for key, group in groups.items()
|
||||
if sum(1 for c in group["categories"] if c["status"] == "suppressed") == 1
|
||||
]
|
||||
|
||||
|
||||
def _lone_hidden_categories(groups: dict) -> list:
|
||||
"""Categories hidden in exactly one of several pupil groups."""
|
||||
lone = []
|
||||
categories = {c["category"] for g in groups.values() for c in g["categories"]}
|
||||
for category in categories:
|
||||
found = [
|
||||
c for g in groups.values() for c in g["categories"]
|
||||
if c["category"] == category
|
||||
]
|
||||
hidden = [c for c in found if c["status"] == "suppressed"]
|
||||
if len(hidden) == 1 and len(found) > 1:
|
||||
lone.append(category)
|
||||
return lone
|
||||
|
||||
|
||||
def disclosure_invariant_holds(groups: dict) -> bool:
|
||||
"""Every row and every column hides none, or at least two.
|
||||
|
||||
Public so the tests can assert it directly rather than re-deriving it.
|
||||
"""
|
||||
return not _lone_hidden_groups(groups) and not _lone_hidden_categories(groups)
|
||||
|
||||
|
||||
def _mask_for_disclosure(groups: dict) -> None:
|
||||
"""Withhold further cells until nothing suppressed can be solved for.
|
||||
|
||||
Not rendering a figure is not the same as not publishing it. This endpoint
|
||||
is public and unauthenticated, so anything left in the payload is
|
||||
published, whatever the UI chooses to draw — the same reasoning the
|
||||
admission_distance field carries in app.py.
|
||||
@@ -855,17 +889,19 @@ def _mask_for_disclosure(groups: dict) -> None:
|
||||
category suppressed in exactly ONE of the three gives itself away.
|
||||
|
||||
DfE's own answer is secondary suppression: withhold a second cell so the
|
||||
residual spans two unknowns and identifies neither. This does the same,
|
||||
iterating because each new suppression can break the other identity, and
|
||||
terminating because cells are only ever added to the suppressed set.
|
||||
residual spans two unknowns and identifies neither.
|
||||
|
||||
Mutates `groups` in place.
|
||||
Where no companion can do that — a sparse cohort whose every other category
|
||||
is `not_applicable`, which is common in special schools and alternative
|
||||
provision — there is nothing left to withhold, so the pupil group is
|
||||
DROPPED entirely. An earlier version simply gave up here and returned with
|
||||
the violation intact and no signal, which is the one outcome this function
|
||||
must never produce: a disclosure-control pass that fails silently is worse
|
||||
than none, because everything downstream trusts it.
|
||||
|
||||
Mutates `groups` in place. Guaranteed to return with
|
||||
disclosure_invariant_holds(groups) true.
|
||||
"""
|
||||
PAIRS = ("disadvantaged", "other", "all")
|
||||
|
||||
def cells(group_key):
|
||||
group = groups.get(group_key)
|
||||
return group["categories"] if group else []
|
||||
|
||||
def suppress(cell):
|
||||
if cell["status"] == "published":
|
||||
@@ -875,14 +911,13 @@ def _mask_for_disclosure(groups: dict) -> None:
|
||||
return True
|
||||
return False
|
||||
|
||||
def add_companion(candidates):
|
||||
def add_companion(candidates) -> bool:
|
||||
"""Withhold a second cell so the residual spans two unknowns.
|
||||
|
||||
The companion must carry pupils. Suppressing a zero looks like
|
||||
secondary suppression and protects nothing: the residual still equals
|
||||
the original withheld figure exactly. Where every remaining cell is
|
||||
zero there is no companion that helps, so the whole set goes — losing
|
||||
real data, but that beats publishing what DfE withheld.
|
||||
the original withheld figure exactly. Returns False when no cell can
|
||||
do the job, which escalates to dropping the group.
|
||||
"""
|
||||
published = [c for c in candidates if c["status"] == "published"]
|
||||
useful = sorted(
|
||||
@@ -891,31 +926,47 @@ def _mask_for_disclosure(groups: dict) -> None:
|
||||
)
|
||||
if useful:
|
||||
return suppress(useful[0])
|
||||
return any([suppress(c) for c in published])
|
||||
# Every remaining cell is zero or not applicable: withholding any of
|
||||
# them leaves the residual equal to the original figure.
|
||||
return False
|
||||
|
||||
changed = True
|
||||
while changed:
|
||||
# Fixpoint: each new suppression can break the other identity. Terminates
|
||||
# because every pass either adds a suppression, drops a group, or stops.
|
||||
while not disclosure_invariant_holds(groups):
|
||||
changed = False
|
||||
|
||||
# Column rule: a category must be suppressed in none of the three
|
||||
# pupil groups, or in at least two of them.
|
||||
categories = {c["category"] for key in PAIRS for c in cells(key)}
|
||||
for category in categories:
|
||||
found = [
|
||||
c for key in PAIRS for c in cells(key) if c["category"] == category
|
||||
for category in _lone_hidden_categories(groups):
|
||||
siblings = [
|
||||
c for g in groups.values() for c in g["categories"]
|
||||
if c["category"] == category
|
||||
]
|
||||
hidden = [c for c in found if c["status"] == "suppressed"]
|
||||
if len(hidden) == 1 and len(found) > 1:
|
||||
if add_companion(found):
|
||||
changed = True
|
||||
if add_companion(siblings):
|
||||
changed = True
|
||||
|
||||
# Row rule: within a group, none suppressed or at least two.
|
||||
for key in PAIRS:
|
||||
group_cells = cells(key)
|
||||
hidden = [c for c in group_cells if c["status"] == "suppressed"]
|
||||
if len(hidden) == 1:
|
||||
if add_companion(group_cells):
|
||||
changed = True
|
||||
for key in _lone_hidden_groups(groups):
|
||||
if add_companion(groups[key]["categories"]):
|
||||
changed = True
|
||||
|
||||
if changed:
|
||||
continue
|
||||
|
||||
# Nothing left to withhold. Drop the groups that are still solvable,
|
||||
# and any category still solvable across the groups that remain.
|
||||
for key in _lone_hidden_groups(groups):
|
||||
del groups[key]
|
||||
changed = True
|
||||
|
||||
for category in _lone_hidden_categories(groups):
|
||||
for group in groups.values():
|
||||
for cell in group["categories"]:
|
||||
if cell["category"] == category and suppress(cell):
|
||||
changed = True
|
||||
|
||||
if not changed:
|
||||
# Unreachable given the two escalations above, but a masking pass
|
||||
# must never spin or exit unsafely. Withhold everything.
|
||||
groups.clear()
|
||||
return
|
||||
|
||||
|
||||
def _destinations_block(rows: list) -> dict | None:
|
||||
@@ -969,6 +1020,12 @@ def _destinations_block(rows: list) -> dict | None:
|
||||
|
||||
_mask_for_disclosure(groups)
|
||||
|
||||
# Masking can empty the block entirely — a sparse cohort where no group
|
||||
# could be made safe. Return None so the section is absent rather than
|
||||
# rendering an empty shell.
|
||||
if not groups:
|
||||
return None
|
||||
|
||||
return {"cohort_year": _format_cohort_year(latest_year), "groups": groups}
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user