Repository navigation
fix(planner): make a plan that misreports its own length unconstructible - #129
Merged
Merged
Conversation
Closes the two structural traps the plan-length report found, on today's code with today's behaviour unchanged, so the fixed-16-week change lands where the failure is impossible. Invariant 1 - a plan must tile its own `week_count`, split across two constructors because the claim has two halves. `MesocycleBlueprint`: the microcycle `week_no`s, in order, are exactly `range(start_week, end_week+1)`. `PlanBlueprint`: those numbers concatenated over all mesocycles are exactly `range(1, week_count+1)`. Tuple equality rather than a pairwise contiguity loop, which would pass vacuously on an empty `mesocycles` tuple. Invariant 2 - one read. `generate()` takes `mesocycle_spans(block_count_for( gap))` and derives `week_count` from `spans[-1].end_week`. This fixes nothing today: `week_count_for(gap)` is defined as `block_count_for(gap) * WEEKS_PER_BLOCK`, so both former call sites already routed through one formula and could not disagree. It fires the moment plan length gets a second source of truth, which is the next PR. `week_count_for` is left in place so `tests/test_plans_api.py:136` stays a cross-check rather than a tautology. Digest byte-identical over the 24-profile sweep, `b079fce09ee80126` both sides, so `GENERATOR_VERSION` stays at 8.0.0 per PR #127's precedent. Also deletes two comments that told the reader the schema cannot check something it can - `check (weekday = extract(isodow from scheduled_on) - 1)` is accepted by Postgres and bites, verified against the test database. No constraint added: the Python check fails earlier and names the day. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the two structural traps the plan-length measurement report found, on today's code with today's behaviour unchanged, so the fixed-16-week change (ruling 49/51/52) lands where the failure is impossible. Split out of the length PR by ruling 53, which amends ruling 51's last bullet.
Invariant 1 — a plan must tile its own
week_countSplit across two constructors because the claim has two independent halves:
MesocycleBlueprint: the microcycleweek_nos, in order, are exactlyrange(start_week, end_week + 1).PlanBlueprint: those numbers concatenated over all mesocycles are exactlyrange(1, week_count + 1).The plan-level tuple equality alone forbids a gap, an overlap, a wrong total, a start past week 1 and a week past the end — but it needs the per-mesocycle half too, or every span could claim weeks 1-3 while the microcycles still ran 1..N.
mesocyclestuple; equality againstrange(1, week_count + 1)cannot, becauseMIN_WEEK_COUNTis 1 so the expected tuple is never empty.microcycles=()is an explicit arm. Contiguity of the spans themselves was already guarded bytest_spans_tile_the_plan_exactly_once_starting_at_week_one— re-asserting it here would have been an inert check. The load-bearing new claim is the linkage: spans ↔ microcycles ↔week_count.Invariant 2 — the block count is read once
generate()now takesspans = mesocycle_spans(block_count_for(gap))and derivesweek_count = spans[-1].end_week.week_count_foris no longer imported bygenerate.py.week_count_for(gap)is defined asblock_count_for(gap) * WEEKS_PER_BLOCK, so both former call sites already routed through one formula and could not disagree — "nothing ties those two reads together" was true of the call sites but not of the values. The trap is real and it is the next PR's: it fires the moment plan length gets a second source of truth.week_count_foris deliberately left in place as independent arithmetic, sotests/test_plans_api.py:136(week_count == week_count_for(grade_gap)) stays a genuine cross-check of generate's span-derived answer instead of a tautology. Residual for the length PR: editingweek_count_foralone now has zero effect on a generated plan, and that test is the arm that catches it.No behaviour change
Plan digest over the 24-profile sweep (
test_phase_guide's own_SWEEP), blake2b overweek_no | phase | order_index | exercise_key | block_seconds:87b7b5e)b079fce09ee80126b079fce09ee80126Byte-identical, so⚠️ The baseline was re-measured at
GENERATOR_VERSIONstays at8.0.0per PR #127's precedent.87b7b5e— the four-block report'sf96268fb5edba7eddoes not reproduce, because it predates PR #128's re-dose.Sabotage A — a
week_countthat does not tile its mesocyclesThe control is live on both sides:
0and53raiseck_plan_week_count_in_rangebefore and after, and a dedicated arm pins that message so the new check cannot swallow the old one.Sabotage B — the block count read twice
Patch honours its
gap(block_count_for(gap) + 1). At gap 2 the plan is 4 blocks / 16 weeks, which is the length PR's exact target shape.Guards shown red with only the source fix stashed: 3 failed / 9 passed, headline
AssertionError: the extra block did not reach the length it reports/assert 16 == 20.block_countargument hides the trap. The real mechanism:server/domain/planner/__init__.py:46re-exports thegeneratefunction, which shadows the submodule of the same name — soimport server.domain.planner.generate as genbinds a function andsetattron it is a silent no-op. The first probe did exactly that and reported the trap as absent. You have to reach the module throughsys.modules. Pinned bytest_the_package_reexport_shadows_the_generate_submoduleso the next probe cannot repeat it.Two prose overclaims deleted
Both told the reader the schema cannot check something it demonstrably can, and both were the stated reason a check lives in Python — the shape that misdirects anyone later asking whether it belongs in the database.
MesocycleBlueprint's new error no longer ends "Nothing in the schema can check across rows". A CHECK cannot, but anEXCLUDE USING gistconstraint or a trigger can. The precise version survives in the module docstring: "the one invariant no CHECK can express because it spans rows."SessionBlueprintsaid "planned_sessionstores both and nothing in the schema keeps them in agreement", repeated attests/test_planner_periodisation.py:306. Verified against the test database — a plain CHECK expresses it, noEXCLUDEor trigger needed:isodow - 1, notdow: Postgresdowis 0=Sunday against this app's 0=Monday. It is CHECK-legal becausescheduled_onisDate, so it casts totimestamp(immutable) rather thantimestamptz(stable). The reason now has one home, narrowed to a claim about what is present rather than what is possible, at the site that executes it.No constraint added and no migration.
migrations/versions/0004_domain_schema.py:431carries onlyweekday BETWEEN 0 AND 6; the Python check fails earlier and names the actual weekday, which is the better failure to read.Gate
npm run checkgreen — 1282 web, 1327 server (1322 + five new arms). No allowlist row added,BASELINE_RATCHETuntouched.🤖 Generated with Claude Code