fix(destinations): withhold at the API, not just in the chart
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m14s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 3m49s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m14s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 3m49s
Code review found the disclosure the whole design was meant to prevent.
R1 was written as a rendering rule and implemented as one: canRenderBar
stopped the bar being drawn, but GET /api/schools/{urn} still carried the
cohort and every published category. cohort - sum(published) returned
Whitley Bay's withheld further-education figure exactly — 18 pupils — to
any caller, and the RSC payload put it in the browser too.
app.py already stated the principle for admission_distance: this endpoint
is public and unauthenticated, so a field left in the payload is a
published field. The same reasoning applies here and did not get applied.
_mask_for_disclosure now closes both identities before serialisation —
categories sum to the cohort, and disadvantaged + other = all — by adding
secondary suppression until every row and column hides none or at least
two. My first attempt picked the smallest published cell as the companion
and a new test caught it choosing a zero, which protects nothing: the
residual still resolved to 18. The companion must carry pupils.
DfE's own aggregates are no longer served. Nothing rendered them, and one
spanning a single suppressed component names it.
Cost, measured over 262 mainstream secondaries: the all-pupils bar
survives on 94% rather than 100%. Zero lone-suppressed groups remain.
The e2e helper now tells a missing feature apart from missing data: it
fails if the API serves no destinations key at all, and skips if the key
is served but the annual DAG has not populated the marts. Failing on the
second would redden the staging gate for unrelated commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
This commit is contained in:
1 parent
68a192e430
commit
102397fe69
11 files changed
+318
-122
No files matched your search
@@ -22,7 +22,7 @@ const ALL_PUBLISHED = [
|
||||
|
||||
const fullPhase: DestinationPhase = {
|
||||
cohort_year: '2022/23',
|
||||
groups: { all: { cohort: 180, categories: ALL_PUBLISHED, aggregates: {} } },
|
||||
groups: { all: { cohort: 180, categories: ALL_PUBLISHED } },
|
||||
};
|
||||
|
||||
const suppressedPhase: DestinationPhase = {
|
||||
@@ -36,7 +36,6 @@ const suppressedPhase: DestinationPhase = {
|
||||
cell('apprenticeship', 8), cell('employment', 6),
|
||||
cell('not_sustained', 5), cell('not_captured', 4),
|
||||
],
|
||||
aggregates: {},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -14,7 +14,6 @@ const phase: DestinationPhase = {
|
||||
{ category: 'employment', pupils: 13, percentage: 13.5, status: 'published' },
|
||||
{ category: 'not_sustained', pupils: 6, percentage: 6.3, status: 'published' },
|
||||
],
|
||||
aggregates: {},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import {
|
||||
canAggregate, aggregateCells, canRenderPublishedAggregate,
|
||||
canAggregate, aggregateCells,
|
||||
canRenderBar, toBarSegments, CARD_GROUPS,
|
||||
type DestinationCell, type DestinationGroup, type DestinationCategory,
|
||||
} from '@/lib/destinations';
|
||||
@@ -19,7 +19,6 @@ const fullGroup = (): DestinationGroup => ({
|
||||
pub('apprenticeship', 8, 180), pub('employment', 6, 180),
|
||||
pub('not_sustained', 5, 180), pub('not_captured', 4, 180),
|
||||
],
|
||||
aggregates: {},
|
||||
});
|
||||
|
||||
describe('canAggregate — R2, computing from components', () => {
|
||||
@@ -47,26 +46,6 @@ describe('aggregateCells', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('canRenderPublishedAggregate — R2, a total DfE published itself', () => {
|
||||
it('allows it when no component is suppressed', () => {
|
||||
expect(canRenderPublishedAggregate([
|
||||
pub('school_sixth_form', 75, 180), pub('sixth_form_college', 21, 180),
|
||||
])).toBe(true);
|
||||
});
|
||||
|
||||
it('REFUSES it when exactly one component is suppressed — the aggregate identifies it', () => {
|
||||
expect(canRenderPublishedAggregate([
|
||||
pub('school_sixth_form', 75, 180), sup('sixth_form_college'),
|
||||
])).toBe(false);
|
||||
});
|
||||
|
||||
it('allows it when two or more components are suppressed', () => {
|
||||
expect(canRenderPublishedAggregate([
|
||||
sup('school_sixth_form'), sup('sixth_form_college'),
|
||||
])).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('canRenderBar — R1', () => {
|
||||
it('allows a bar when the whole group is published', () => {
|
||||
expect(canRenderBar(fullGroup())).toBe(true);
|
||||
|
||||
@@ -17,7 +17,6 @@ const phase = (categories = 1) => ({
|
||||
category: 'school_sixth_form' as const,
|
||||
pupils: 75, percentage: 41.7, status: 'published' as const,
|
||||
})),
|
||||
aggregates: {},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
@@ -39,7 +39,6 @@ function toGroup(payload: DestinationGroupPayload): DestinationGroup {
|
||||
return {
|
||||
cohort: payload.cohort ?? 0,
|
||||
cells: payload.categories,
|
||||
aggregates: payload.aggregates,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -5,9 +5,13 @@
|
||||
* DfE suppresses individual cells with `c`, and the destination categories sum
|
||||
* to the cohort. So subtracting the published cells from the cohort total
|
||||
* recovers a lone suppressed cell exactly — which is the case on 22% of
|
||||
* mainstream secondaries. The guards below are what stop this module's
|
||||
* consumers doing that by accident, and they are why a percentage is never
|
||||
* reconstructed from a partial sum.
|
||||
* mainstream secondaries.
|
||||
*
|
||||
* The guards here are the SECOND line of defence, not the first. Not drawing a
|
||||
* number does nothing to stop it being computed, so the real fix lives in
|
||||
* backend/data_loader.py::_mask_for_disclosure, which withholds a companion
|
||||
* cell before the figures ever leave the server. These functions keep the UI
|
||||
* honest about what it draws from an already-safe payload.
|
||||
*
|
||||
* See docs/superpowers/specs/2026-08-28-destination-measures-design.md.
|
||||
*/
|
||||
@@ -40,8 +44,6 @@ export interface DestinationCell {
|
||||
export interface DestinationGroup {
|
||||
cohort: number;
|
||||
cells: DestinationCell[];
|
||||
/** Aggregates DfE published itself, keyed by slug. */
|
||||
aggregates: Partial<Record<'sustained_education' | 'sustained_all', DestinationCell>>;
|
||||
}
|
||||
|
||||
/** Display order, which is also bar order: education, then work, then absence. */
|
||||
@@ -80,15 +82,6 @@ export function aggregateCells(
|
||||
return { pupils, percentage: (pupils / cohort) * 100 };
|
||||
}
|
||||
|
||||
/**
|
||||
* R2, the other direction: DfE published this total itself. Showing it beside
|
||||
* the components is safe only when it spans no suppressed component, or two or
|
||||
* more. Exactly one, and the total names the withheld figure.
|
||||
*/
|
||||
export function canRenderPublishedAggregate(components: DestinationCell[]): boolean {
|
||||
return suppressedCount(components) !== 1;
|
||||
}
|
||||
|
||||
/** R1: a bar is drawable only when nothing in the group is withheld. */
|
||||
export function canRenderBar(group: DestinationGroup): boolean {
|
||||
return group.cohort > 0 && group.cells.every(c => c.status === 'published');
|
||||
|
||||
@@ -616,8 +616,6 @@ import type { DestinationCell, PupilGroup } from './destinations';
|
||||
export interface DestinationGroupPayload {
|
||||
cohort: number | null;
|
||||
categories: DestinationCell[];
|
||||
/** Totals DfE published itself. Never computed here — see lib/destinations.ts. */
|
||||
aggregates: Partial<Record<'sustained_education' | 'sustained_all', DestinationCell>>;
|
||||
}
|
||||
|
||||
export interface DestinationPhase {
|
||||
|
||||
Reference in new issue
Block a user