Document the Passage compatibility boundary - #1341
Conversation
pzfreo
left a comment
There was a problem hiding this comment.
Reviewed against the round-4 baseline (the live #184/#1339 texts vs actual main). The framing is right and honestly stated: the PR correctly acknowledges that #1248/#1293 already did the pin move and truthful declaration, doesn't manufacture another bump, and separates the deployed state from the future 0.4 migration. I ran the new test plus the adoption suite on the head — passes. The five-bullet paper model faithfully compresses the #184 facade clauses. Three findings, one substantive:
The main one: the PR body claims to freeze the >=0.2.6,<0.4.0 interval "in an executable test", but only the point version is executable — the interval, and especially its load-bearing upper bound, lives in prose (inline comment). Plus one omission in the paper model (the slot→passage disposition flip and its #1245 gate) and one unpinned boundary (documentation).
| def test_existing_passage_contract_is_truthfully_declared_before_f4b() -> None: | ||
| """Pin the current-family declaration separately from the future 0.4 migration.""" | ||
|
|
||
| assert INSTALLED_PACKAGE_VERSION == "0.3.1" |
There was a problem hiding this comment.
The adoption interval is not actually executable — only the point version is. The PR body says it "freezes the reviewed released interval >=0.2.6,<0.4.0 as the adoption boundary" and "pins … in an executable test", but this asserts == "0.3.1". The exact-literal ratchet is fine as a both-sides-changed-together convention (like the 25-family literal above), but the upper bound — the part the doc says "prevents accidental adoption of the planned 0.4 compatibility change" — has no executable form: when someone bumps the pin and dutifully updates this literal to "0.4.0", nothing fails and nothing routes them to the F4b gate. Add an interval assertion alongside the literal, e.g. parse INSTALLED_PACKAGE_VERSION and assert (0, 2, 6) <= v < (0, 4, 0) with a comment naming b123d-recognisers#184 / #1337 as the gate that must be ACCEPTed before the upper bound moves.
| the installed manifest really contains `passages`; the upper boundary prevents accidental adoption | ||
| of the planned 0.4 compatibility change. | ||
|
|
||
| The reviewed future 0.4 model is a distinct migration, not latent behavior in this declaration: |
There was a problem hiding this comment.
The paper model omits the one consumer-visible consequence of the 0.4 migration, and the crossing condition doesn't mention #1245. Under the revised #184, a principal split-junction void whose walls contain a Slot group flips ownership at 0.4: today the legacy finder never sees it, so the Slot is the accepted, counted, rendered claim (slots is fully supported); post-0.4 the rich passage wins (SLOT_SUPERSEDED_BY_PASSAGE) and the claim moves to a family this page declares Draftwright does not draw. As specified, crossing 0.4 would silently replace a rendered slot callout with nothing. Since this section is the consumer-side boundary statement, it should say so: add a bullet that adoption changes which family claims Slot-superseding split-junction voids, and condition the pin crossing 0.4 not only on the capability-contract representation but on #1245 resolving what a passage draws (or an explicit decision to accept the callout loss).
| "drawing_consumer", | ||
| ) | ||
| } == {"unsupported"} | ||
| assert passage_consumer["completeness"]["state"] == "deferred" |
There was a problem hiding this comment.
Minor: the unsupported lowering emits a sixth boundary this test doesn't pin — documentation: {"state": "supported", "evidence": ["docs/reference/recogniser-capabilities.md"]} — and this PR is precisely that evidence. Worth one more assertion (passage_consumer["documentation"]["state"] == "supported" plus the evidence path) so the boundary set checked here is exhaustive rather than four-of-six, and so deleting the new doc section can't leave the declaration claiming evidence that no longer exists.
|
Addressed all three review findings at exact
Re-run: 83 focused tests pass; Ruff and diff-check clean. CI restarted on the new head. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1341 +/- ##
==========================================
- Coverage 93.59% 93.57% -0.03%
==========================================
Files 61 62 +1
Lines 20803 21262 +459
Branches 3972 4080 +108
==========================================
+ Hits 19470 19895 +425
- Misses 774 790 +16
- Partials 559 577 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Outcome
Closes #1339 by making the already-deployed Passage declaration and the future F4b compatibility boundary independently reviewable and executable.
Current
mainis ahead of the stale premise that triggered #1339:b123d-recognisers==0.2.6and added the exhaustivepassagesdeclaration.0.3.1package.passagesintroduced in 0.2.6, andpending_family_declarations()is already empty.This PR therefore does not manufacture another dependency bump. It:
Passagefamily as a truthful unsupported Draftwright disposition tracked by Decide what a recognised Passage means to draftwright (new family in b123d-recognisers 0.2.6) #1245;>=0.2.6,<0.4.0as the adoption boundary;The later 0.4 adoption remains a separate coordinated PR updating the adapter/declaration model, validator, tests, exact pin and lockfile together after b123d-recognisers#184 and Draftwright #1337 are accepted and implemented.
Validation
uv run pytest -q tests/test_recogniser_capabilities.py tests/test_recogniser_adoption.py tests/test_external_recognition_boundary.py— 83 passeduv run ruff check tests/test_recogniser_capabilities.pygit diff --check