From 27f2adcc7aed6972880d1303166194695c17ae7f Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 17:12:40 +0200 Subject: [PATCH 1/5] =?UTF-8?q?fix(tests):=20#245=20=E2=80=94=20delete=20t?= =?UTF-8?q?he=20arrival=20check;=20it=20subscribed=20us=20to=20another=20r?= =?UTF-8?q?epo's=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C-86 is that this repository's release path can be blocked by another repository's routine work. PR #239 was written to reduce that and shipped a check that reproduced it. MEASURED, ON THE LIVE REGISTRY. The drift check had two halves. `changed` is filtered to the names each partner declares. `arrived` was not — it ran over every row of every table this package depends on. Each partner reads 13 of those 25 rows. Eight belong to no repository here: three platform key slots, three caller keys for other repos, and two contract rows. So views-appwrite issuing one key for someone else turned both partner parametrizations red and blocked a merge to main, which is the release to FAO. The same test's docstring, seven lines above, said "Silent through: prose edits, [meta] bumps, and rows belonging to anyone else." The prose described the check that was designed; the code implemented a wider one. ITS SELF-DEFENCE WAS FALSE. The failure message claimed `[contract.*]` "arrived exactly this way, and nothing else here would have seen it". `[contract]` is a top-level TABLE, and `test_every_table_in_the_registry_is_classified_here` — thirty lines above, in the same file — catches exactly that. The case it cited as its reason to exist was already covered by its neighbour. MUTATION-PROVEN IN BOTH DIRECTIONS against the real registry: upstream issues an API key for another repo -> silent (was: red build) a third partner's contract row lands -> silent (was: red build) a value this partner reads rotates -> fires a name this partner reads is removed -> fires The second one is not hypothetical: views-productionapi is a forthcoming consumer, and its contract row landing would have reddened both existing partners. AND THE STOPPING RULE IS NOW IN THE FILE, above the check, because this check has been widened once already: it fires on exactly one condition — a row this partner declares differs between the pinned edition and the sibling's main — and any proposal to add a condition must name the delivery failure it prevents and show that failure is not already prevented by assert_env_declared at run time. Deliberately NOT done: adopting `[edition].obliges_consumers` (views-appwrite#76, shipped as registry v1.6.0). Deleting `arrived` fixes the shipping problem; adopting the flag is an addition, and it would move a safety property onto another repository's labelling discipline. `changed` compares bytes and depends on nobody's self-description. Deferred with a trigger in C-86. Register: C-86's "⚠ CORRECTED" note becomes CORRECTED-and-RESOLVED, with the measurement; its rate claim is now accurate, which it was not before. C-90 gets a partial mitigation — the tautological proof and the untested `registry_current` are #246's and it stays open. Suite 410 passed / 1 skipped / 40 xfailed, ruff clean. The one failure is #248's. Closes #245. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- reports/technical_risk_register.md | 38 ++++++++++++++++++++---------- tests/test_env_declaration.py | 27 +++++++++++---------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index f2c2f83..317467d 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -355,6 +355,12 @@ There is a real question underneath, and it should be decided rather than inheri **`registry_current` has no test.** The module `tests/seam_registry.py` was extracted for one reason: two copies of the reader disagreed about whether to read the sibling's `main` or its working tree, and reading the working tree is issue #196 verbatim. The function that settles it is called by five tests and is the subject of none. Replacing its body with `rev-parse HEAD` — the defect it exists to prevent — leaves the suite at its exact baseline. Three of its error branches are executed by nothing. +**Partial mitigation 2026-08-12 (#245) — the false-alarm half is gone; the two proof defects are not.** + +`arrived` is **deleted**. Measured before deleting: each partner reads 13 of the 25 rows in the tables this package depends on, and 8 of those rows belong to no repository here — so the check subscribed this repository to another repo's changelog. Mutation-proven after: an unrelated API key and a third partner's contract row are silent; a rotation and a removal still fire. The docstring and the code now agree, and the stopping rule sits above the check. + +**Still open, and routed to #246**: the partition's mutation proof is a tautology (`base` is built from `_TABLE_ROLE`, so `_unclassified_tables(base)` is empty for every possible input), and `registry_current` — the function `tests/seam_registry.py` was extracted to create — has no test, so replacing it with a working-tree read leaves the suite at baseline. This entry stays open until both land. + Cross-refs: **C-86** (whose partial-mitigation paragraph this falsifies), **C-89** (the sibling defect in the no-copy scan), **C-93**, **C-91**, ADR-014 §1/§2, issue #196. --- @@ -509,18 +515,26 @@ description, where deferrals go to be forgotten: Cross-refs for this amendment: **C-57** (the drift detector this rides on), ADR-016 §7a/§7b, views-appwrite#76 (**delivered**, registry v1.6.0). -**⚠ CORRECTED 2026-08-12 — the partial mitigation above overstates what shipped.** The -paragraph beginning *"Partial mitigation 2026-08-11"* says the replacement checks "match on -the facts this repository actually declares". **One of the three does not.** The arrival -half of the drift check is computed over every row of every table this package depends on, -with no filter for the names it reads — 8 of 25 rows on the live registry belong to other -repositories. So the false-alarm class this entry records is *narrowed*, not removed: -observation-only edits no longer fire it, but any new coordinate for any repo still does. - -The measurement quoted above (v1.4.4 → v1.5.2, three editions, green) remains true and was -not the wrong measurement — none of those editions added a row. It was simply not a -measurement of the case that now fires. Registered as **C-90**, and this entry's rate claim -should be re-stated once that is fixed rather than before. +**⚠ CORRECTED 2026-08-12, and RESOLVED the same day (#245).** The paragraph beginning +*"Partial mitigation 2026-08-11"* claimed the replacement checks "match on the facts this +repository actually declares". **One of the three did not.** The drift check's arrival half +ran over every row of every table this package depends on, with no filter for the names it +reads — measured on the live registry, **each partner reads 13 of 25 rows, and 8 belong to +no repository here**. So an API key issued upstream for someone else reddened this +repository and blocked a release, which is this entry's own failure class arriving through +the guard meant to reduce it. + +**The arrival half is now deleted** (#245). The drift check fires on exactly one condition — +a row this partner declares differs between the pinned edition and the sibling's `main` — +and that stopping rule is written above the check itself. Its self-defence turned out to be +false: it claimed `[contract.*]` *"arrived exactly this way and nothing else here would have +seen it"*, but `[contract]` is a top-level **table**, caught by the partition check directly +above it. + +Mutation-proven both ways against the live registry: an unrelated API key and a third +partner's contract row are now **silent**; a rotation and a removal of a row this partner +reads still **fire**. The rate claim in the paragraph above is therefore accurate as +written, which it was not before. --- diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index f6f3bd3..55f0267 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -736,6 +736,20 @@ def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): **Silent through:** prose edits, ``[meta]`` bumps, and rows belonging to anyone else. Measured across the real v1.4.4 -> v1.5.2 window (three editions, one week): green. + + **The stopping rule, because this check has been widened once already.** It fires on + exactly one condition: *a row named in this partner's* ``expected_class`` *differs + between the pinned edition and the sibling's* ``main``. Nothing else. + + Any proposal to add a condition must name the delivery failure it prevents and show + that failure is not already prevented by ``assert_env_declared`` at run time. If it + is, the condition is refused. An arrival half was added and deleted for failing that + test: it fired on every row of every table this package depends on — measured, 12 of + 25 rows are ones this partner never reads and 8 belong to no repository here — so an + API key issued upstream for someone else blocked a release. It defended itself by + claiming ``[contract.*]`` "arrived exactly this way and nothing else here would have + seen it", which is false: ``[contract]`` is a top-level *table*, and + ``test_every_table_in_the_registry_is_classified_here`` above catches those. """ repo = require_sibling("views-appwrite") module, _, expected_class = _PARTNER_ENV[partner] @@ -745,19 +759,6 @@ def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): names = set(expected_class) then, now = _projection(pinned, names), _projection(current, names) - arrived = sorted( - set(_rows(current, _TABLES_WE_DEPEND_ON)) - set(_rows(pinned, _TABLES_WE_DEPEND_ON)) - ) - assert not arrived, ( - f"[{partner}] the registry gained coordinate(s) {arrived} in a table this package " - f"reads, since the edition it was verified against " - f"(v{module.SEAM_CONTRACT_VERSION}). Decide whether this package must adopt them " - "— that is a human read, which is why this reports rather than guesses — then " - "move SEAM_CONTRACT_VERSION and SEAM_CONTRACT_COMMIT together. This is the half " - "of the deleted edition check that was worth keeping: `[contract.*]` arrived " - "exactly this way, and nothing else here would have seen it." - ) - changed = _describe_changes(then, now, names) assert not changed, ( f"[{partner}] the registry changed something this package reads, since the " From ff1de31e8422784181ddbe1dcaab4dbd25b1da4c Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 17:24:38 +0200 Subject: [PATCH 2/5] =?UTF-8?q?fix(tests):=20#245=20review=20=E2=80=94=20a?= =?UTF-8?q?=20counterfactual=20written=20as=20history,=20and=20a=20rule=20?= =?UTF-8?q?that=20read=20backwards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent reviews of the deletion. The deletion itself is sound; three of my own claims about it were not, and one of them is the defect this whole epic is about. I WROTE A MUTATION RESULT IN THE PAST TENSE. The docstring, the commit message and C-86 all said an upstream key "reddened this repository and blocked a release". Measured: ZERO rows have arrived in the depended-on tables since the pin — the arrival half was green on every real edition it ever saw, from its introduction on 2026-08-11 to its deletion a day later. And `protect_main` carries `deletion, non_fast_forward, pull_request` and no required status check, so a red build blocks nothing today anyway. What is true: a mutation shows it WOULD fire, and it WOULD block once C-81 lands. Writing the counterfactual as history, in a change whose subject is prose outrunning code, is the thing itself. THE STOPPING RULE READ BACKWARDS. It said a proposed widening must show its failure is "not already prevented by assert_env_declared at run time". Applied literally that ADMITS the arrival half — an unadopted upstream coordinate is not prevented by it — and REJECTS the removal detection this check performs, because a removed coordinate IS caught at run time when the launcher cannot supply the name. The rule now asks the question that actually discriminates: state the delivery failure prevented, AND the false-alarm surface measured against the live registry; if that surface includes rows this package does not read, it is refused. "8 BELONG TO NO REPOSITORY HERE" IS SIX. The other two are this package's own delivery labels — both rows declare `producer = "views-postprocessing"` — and they are covered by tests/test_product.py, not by this check. C-90's pre-existing text had this right; I upgraded a careful claim into a false one and propagated it to four places. AND THE DELETION'S REAL COST WAS RECORDED NOWHERE. A coordinate views-appwrite issues FOR this package, or a second `[contract.*]` row for a future partner, is now silent until a human reads the registry — no test, no runtime assert. C-90 had posed exactly this question ("they cannot both stand") and my mitigation answered only the half that flattered the change. Now stated in the docstring and decided in C-90. THE SILENCE DIRECTION NOW HAS A CHECK. The proofs either side of this one are positive — rotation fires, a rename fires — so a third widening would have met no objection and the rule forbidding it would have been prose with a guard on the other half only. Mutation- proven by re-adding an arrival half and watching it fail. C-86: two of three, not one. "Every top-level table must be classified" still fires regardless of obligation, and HAS — `[edition]` at v1.6.0, an edition the registry itself calls "additive and opt-in; obliges nobody", reddened this repo. Recorded as the entry's remaining rate risk with the `obliges_consumers` deferral as its fix. ADR-014 §5: ADR-016 §1 and §7b both asserted the deleted behaviour as current, and §7b is the section C-86 cross-references. Both corrected with an erratum naming the cost. C-90's Trigger, Location, title and present-tense body all repointed at the two defects that remain. Suite 412 passed / 1 skipped / 40 xfailed, ruff clean. The one failure is #248's. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- .../016_ci_read_access_to_private_siblings.md | 19 ++++-- reports/technical_risk_register.md | 56 ++++++++++++----- tests/test_env_declaration.py | 63 +++++++++++++++---- 3 files changed, 106 insertions(+), 32 deletions(-) diff --git a/docs/ADRs/016_ci_read_access_to_private_siblings.md b/docs/ADRs/016_ci_read_access_to_private_siblings.md index ae5cf56..ee77a84 100644 --- a/docs/ADRs/016_ci_read_access_to_private_siblings.md +++ b/docs/ADRs/016_ci_read_access_to_private_siblings.md @@ -44,9 +44,11 @@ this repository makes about a different repository** is still true. The clearest example. Each partner's `appwrite_env.py` records which edition of the shared configuration registry it was verified against, by naming a commit in another repository. Tests open that repository at that commit and ask whether anything this -package reads has moved since — a rename, a reclassification, a rotated value, or a new -coordinate that has arrived. *(Until 2026-08-11 they asked a cruder question — whether the -edition **label** still matched — which fired on every upstream edit. See §7b's erratum.)* +package reads has moved since — a rename, a reclassification, a rotated value. *(Until +2026-08-11 they asked a cruder question — whether the edition **label** still matched — +which fired on every upstream edit. A second attempt also asked whether any coordinate had +**arrived**, which fired on other repositories' rows and was deleted on 2026-08-12. See +§7b's erratum.)* This is not hypothetical housekeeping. On 2026-08-05 that test failed — the registry had moved to a new edition while nobody here was looking. Two days earlier, the same family of @@ -248,9 +250,14 @@ So the two categories above become three: - **Reachability checks** read a pinned commit; movement is noise. - **Differential tripwires** read **both** — a pinned edition as the baseline, the sibling's `main` as the comparison — and fire when something *we declare* differs between - them, or when a coordinate arrives in a table we read. This is the shape the registry - check now has. It catches a rotation, which is invisible to the other two, and it is - silent through prose edits and version bumps. + them. This is the shape the registry check now has. It catches a rotation, which is + invisible to the other two, and it is silent through prose edits and version bumps. + + *(Erratum, 2026-08-12. This paragraph read "…or when a coordinate **arrives** in a table + we read" and called that the check's current shape. That half was deleted the same day: it + compared every row of every table this package depends on, so another repository's key + reddened this one. The cost of removing it — a coordinate issued **for** this package now + goes unnoticed until a human reads the registry — is carried in register C-90.)* *(Corrected twice on 2026-08-11, and the second time is the instructive one. The first implementation compared against the sibling's **working tree** — so a developer whose clone diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 317467d..9638bd7 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -334,22 +334,24 @@ Cross-refs: **C-57** (the scan's own entry and its history), **C-93** (why the a --- -### C-90: The replacement drift checks assert less than their docstrings say, and one of their mutation proofs is a tautology +### C-90: A mutation proof that cannot fail, and the untested function a module was extracted to create | Field | Value | |-------|-------| | ID | C-90 | | Tier | 2 — this is the entry PR #239 was written to close, reopened by the code that closed it. It reinstates release-blocking false alarms on the path that is this project's production release, and it does so under a docstring saying the opposite. | | Source | `/code-review max` on PR #239 post-merge, 2026-08-11; verified by direct measurement against views-appwrite `origin/main` | -| Trigger | views-appwrite declares any new coordinate in `connection`, `target`, `secret` or `contract` — including one belonging to another repository entirely. On the current registry that is 8 of 25 rows' worth of surface. | +| Trigger | **Both remaining halves are proof defects, not runtime ones.** (a) Someone mutates `_unclassified_tables` or `_TABLE_ROLE` and believes the tautological proof covers it. (b) Someone changes `registry_current` — the reason `tests/seam_registry.py` exists — and the suite stays green. *(The original trigger, an unrelated coordinate arriving upstream, died with `arrived` in #245.)* | | Owner | This repository. | -| Location | `tests/test_env_declaration.py:732` (`arrived`), `:890` (the tautology), `tests/seam_registry.py:113` (`registry_current`). | +| Location | `tests/test_env_declaration.py` — `test_the_table_partition_would_catch_a_new_table_and_a_vanished_one` (the tautology); `tests/seam_registry.py::registry_current` (untested). Function names, not line numbers: this entry has cited stale ones before. | -**`arrived` is not filtered by the names this package reads.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use. +**~~`arrived` is not filtered by the names this package reads.~~ RESOLVED 2026-08-12 (#245) — deleted; see the mitigation below. Left visible because the reasoning it prompted is the entry's most useful part.** `changed` is; `arrived` is computed over every row of every table this package depends on. Measured on the live registry: **25 rows, 8 of which this package never reads** — six of them keys and callers belonging to other repositories. So views-appwrite issuing one more key for an unrelated repo turns both partner parametrizations red here, with a message demanding a `SEAM_CONTRACT` re-pin for a coordinate this package cannot use. That is the exact failure class C-86 records and that PR #239 was written to remove, and the same test's docstring seven lines above says **"Silent through: prose edits, `[meta]` bumps, and rows belonging to anyone else."** The prose describes the check that was designed; the code implements a wider one. -There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt — `[contract.*]` arrived exactly that way and nothing else here would have seen it. That argues for table-granularity on arrival and row-granularity on change. If that is the intent it is defensible, and then the docstring is wrong; if the docstring is right, the code is. **They cannot both stand.** +There is a real question underneath, and it should be decided rather than inherited: a *new* coordinate in a table we read may be one we must adopt. That argues for table-granularity on arrival and row-granularity on change. **They cannot both stand.** + +**DECIDED 2026-08-12 (#245): the docstring won, and the cost is real.** The arrival half was deleted. A coordinate views-appwrite issues *for this package* — or a second `[contract.*]` row for a future partner such as views-productionapi — is now **silent** until a human reads the registry: no test, no run-time assert, nothing. `assert_env_declared` cannot see it, because it iterates the names this package already declares. That is the accepted price of not being reddened by every unrelated row, and it is recorded here rather than left to be discovered. *(The half's own defence — that `[contract.*]` "arrived exactly this way and nothing else here would have seen it" — was false at table granularity, where the partition check catches it, and true at row granularity, which is exactly the cost now accepted.)* **The partition's mutation proof cannot fail.** `assert not _unclassified_tables(base)` where `base = {name: {} for name in _TABLE_ROLE}` reduces to `set(_TABLE_ROLE) - set(_TABLE_ROLE)`, empty for every possible input. Its message — *"the real registry's tables must all classify"* — asserts a fact about a file this test never opens. It is decoration inside the test whose own docstring is about removing decoration. @@ -515,14 +517,29 @@ description, where deferrals go to be forgotten: Cross-refs for this amendment: **C-57** (the drift detector this rides on), ADR-016 §7a/§7b, views-appwrite#76 (**delivered**, registry v1.6.0). -**⚠ CORRECTED 2026-08-12, and RESOLVED the same day (#245).** The paragraph beginning -*"Partial mitigation 2026-08-11"* claimed the replacement checks "match on the facts this -repository actually declares". **One of the three did not.** The drift check's arrival half -ran over every row of every table this package depends on, with no filter for the names it -reads — measured on the live registry, **each partner reads 13 of 25 rows, and 8 belong to -no repository here**. So an API key issued upstream for someone else reddened this -repository and blocked a release, which is this entry's own failure class arriving through -the guard meant to reduce it. +**⚠ CORRECTED 2026-08-12, and PARTLY RESOLVED the same day (#245).** The paragraph +beginning *"Partial mitigation 2026-08-11"* claimed the replacement checks "match on the +facts this repository actually declares". **Two of the three did not**, and an earlier +draft of this correction said one — the review that caught it is the reason this paragraph +is longer than it wants to be. + +**The first, now fixed.** The drift check's arrival half ran over every row of every table +this package depends on, with no filter for the names it reads. Measured on the live +registry: each partner reads **13 of 25** rows, and **6 belong to other repositories** +(three platform key slots, three caller keys). The other two rows this package does not +read are its own delivery labels — both declare `producer = "views-postprocessing"` — and +they are covered by `tests/test_product.py`, not by this check. *An earlier draft of this +paragraph said "8 belong to no repository here", which upgraded a careful claim in C-90 +into a false one and propagated it to four places.* + +**And it never actually fired in anger.** Measured: **zero rows have arrived in the +depended-on tables since the pin** — the arrival half was green on every real edition it +ever saw, from its introduction on 2026-08-11 to its deletion on 2026-08-12. An earlier +draft said an upstream key "reddened this repository and blocked a release". That is a +mutation result written in the past tense. What is true: a mutation shows it *would* fire, +and it *would* block once C-81's required check lands — `protect_main` today carries +`deletion, non_fast_forward, pull_request` and no required status check, which is this +entry's own "latent rather than live" paragraph. **The arrival half is now deleted** (#245). The drift check fires on exactly one condition — a row this partner declares differs between the pinned edition and the sibling's `main` — @@ -533,8 +550,17 @@ above it. Mutation-proven both ways against the live registry: an unrelated API key and a third partner's contract row are now **silent**; a rotation and a removal of a row this partner -reads still **fire**. The rate claim in the paragraph above is therefore accurate as -written, which it was not before. +reads still **fire**. A new test asserts the silence direction, so a third widening meets +an objection rather than a paragraph. + +**The second inaccuracy stands, and is this entry's remaining rate risk.** *"Every +top-level table upstream must be classified here"* fires on any new table regardless of +whether this repository declares anything about it — and **it has already fired for exactly +that reason**: `[edition]` arrived at v1.6.0, an edition the registry's own `[meta]` calls +*"additive and opt-in; obliges nobody"*, and it reddened this repository. That event is +recorded further down this entry as the partition "earning its keep", which is true of the +detection and not of the cost. Adopting `[edition].obliges_consumers` is the deferral that +would fix it, and its trigger is below. --- diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 55f0267..01b96c3 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -739,17 +739,27 @@ def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): **The stopping rule, because this check has been widened once already.** It fires on exactly one condition: *a row named in this partner's* ``expected_class`` *differs - between the pinned edition and the sibling's* ``main``. Nothing else. - - Any proposal to add a condition must name the delivery failure it prevents and show - that failure is not already prevented by ``assert_env_declared`` at run time. If it - is, the condition is refused. An arrival half was added and deleted for failing that - test: it fired on every row of every table this package depends on — measured, 12 of - 25 rows are ones this partner never reads and 8 belong to no repository here — so an - API key issued upstream for someone else blocked a release. It defended itself by - claiming ``[contract.*]`` "arrived exactly this way and nothing else here would have - seen it", which is false: ``[contract]`` is a top-level *table*, and - ``test_every_table_in_the_registry_is_classified_here`` above catches those. + between the pinned edition and the sibling's* ``main``. + + Any proposal to widen it must state two things: which delivery failure it prevents, + **and its false-alarm surface measured against the live registry**. If that surface + includes rows this package does not read, it is refused. (An earlier version of this + rule asked instead whether ``assert_env_declared`` already prevented the failure at + run time. That rule is wrong in both directions: an unadopted upstream coordinate is + not prevented by it, so arrival detection would pass — and a removed coordinate *is* + caught by it, so the removal detection this check does perform would fail.) + + An arrival half failed the surface test and was deleted (#245): it compared every row + of every depended-on table, and measured, each partner reads 13 of 25 while 6 belong + to other repositories entirely. It defended itself by claiming ``[contract.*]`` + "arrived exactly this way and nothing else here would have seen it" — false at table + granularity, where ``test_every_table_in_the_registry_is_classified_here`` catches it. + + **What that deletion costs, stated rather than implied.** A coordinate views-appwrite + issues *for this package* is now silent until a human reads the registry — no test, no + run-time assert, nothing. So is a second ``[contract.*]`` row for a future partner. + That is the accepted price of not being reddened by every unrelated row, and it is + carried in register C-90 rather than here. """ repo = require_sibling("views-appwrite") module, _, expected_class = _PARTNER_ENV[partner] @@ -965,6 +975,37 @@ def edition(value: str) -> dict: ) +@pytest.mark.parametrize("partner", _PARTNERS) +def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner): + """The stopping rule above, as a check rather than a paragraph. + + The proofs either side of this one are positive — rotation fires, a rename fires. + Nothing asserted the *silence* direction, so a third widening of this check would + have met no objection, and the rule forbidding it would have been prose with a guard + on the other half only. That is this file's own recurring defect (C-80, C-82). + """ + _, _, expected_class = _PARTNER_ENV[partner] + names = set(expected_class) + mine = next(iter(names)) + + pinned = {"target": {mine: {"class": "target", "value": "unchanged"}}} + current = { + "target": {mine: {"class": "target", "value": "unchanged"}}, + # Somebody else's coordinate, arriving in a table this package depends on. + "secret": {"SOMEBODY_ELSES_API_KEY": {"class": "secret", "slot": "theirs"}}, + } + + changed = _describe_changes( + _projection(pinned, names), _projection(current, names), names + ) + assert not changed, ( + f"[{partner}] the drift check objected to a row this package does not read: " + f"{changed}. It must be silent on another repository's coordinates — an arrival " + "half that was not was deleted in #245 for exactly this, and re-adding one is " + "refused by the stopping rule in the check's own docstring." + ) + + @pytest.mark.parametrize("partner", _PARTNERS) def test_the_drift_check_would_catch_a_rename(partner): """A gated test that cannot fail is decoration — so prove this one bites in CI. From 79abda7b3e3feb4bfa8a3171bb9cebda7e9de1e7 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 17:41:18 +0200 Subject: [PATCH 3/5] =?UTF-8?q?fix(tests):=20#245=20review-diff=20?= =?UTF-8?q?=E2=80=94=20the=20silence=20guard=20was=20not=20pointed=20at=20?= =?UTF-8?q?the=20check=20it=20named?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /review-diff on the remediation, and it found the remediation's own defect. Same class, one level up, in the change that was fixing it. THE SILENCE GUARD DID NOT GUARD THE CHECK. Its docstring said "the stopping rule above, as a check rather than a paragraph" and claimed a third widening "would have met no objection" — implying it now would. It would not. The test called `_describe_changes` directly, the helper UNDERNEATH the check, and the deleted arrival half never lived in that helper. Proven: re-adding `arrived` to the real check, exactly as it was before #245, left the whole suite green at 41 passed. AND THE MUTATION PROOF I OFFERED FOR IT WAS INVALID. I mutated the TEST — adding arrival logic inside the test body — and watched it fail. That proves the assertion works on input I hand it. It proves nothing about the code the test is supposed to be watching. A proof has to mutate the subject, not the instrument. It now drives the real check through monkeypatched registry readers, the pattern #242 arrived at for the same reason. Re-proven properly: re-widening the real check fails it, naming the offending row. Also fixed while in there: `mine = next(iter(names))` picked an arbitrary set element, so the fixture varied per process. `sorted(...)[0]` instead — declared, not incidental. And one near-miss worth recording: my first attempt to drop an unused local used a `sed` pattern that matched three OTHER tests and silently deleted their `module = ...` binding. Ruff caught it as seven F821s, restored from a `cp` backup. The lesson is the standing one — anchor edits on unique text, not on a pattern that happens to be unique in the region you are looking at. Register: C-89's sibling lesson recorded on C-86's mitigation — a guard must be pointed at the thing it claims to guard, which is the sentence this arc keeps re-learning. Suite 412 passed / 1 skipped / 40 xfailed, ruff clean. The one failure is #248's. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- reports/technical_risk_register.md | 9 +++++++ tests/test_env_declaration.py | 42 ++++++++++++++++-------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 9638bd7..ed976c9 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -553,6 +553,15 @@ partner's contract row are now **silent**; a rotation and a removal of a row thi reads still **fire**. A new test asserts the silence direction, so a third widening meets an objection rather than a paragraph. +*(That silence test needed two attempts, and the first is worth recording because it is +this entry's own disease. It called `_describe_changes` directly — the helper underneath +the check — and was worthless: re-adding the deleted arrival half to the real check left +the whole suite green, because that half never lived in the helper the test was asking. +The "mutation proof" offered for it was invalid too: it mutated the test rather than the +code, which proves nothing. It now drives the real check through monkeypatched readers, +and re-adding the arrival half fails it. **A guard must be pointed at the thing it claims +to guard**, which is the sentence this whole entry keeps re-learning.)* + **The second inaccuracy stands, and is this entry's remaining rate risk.** *"Every top-level table upstream must be classified here"* fires on any new table regardless of whether this repository declares anything about it — and **it has already fired for exactly diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index 01b96c3..aa9bb00 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -976,34 +976,36 @@ def edition(value: str) -> dict: @pytest.mark.parametrize("partner", _PARTNERS) -def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner): +def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner, monkeypatch): """The stopping rule above, as a check rather than a paragraph. The proofs either side of this one are positive — rotation fires, a rename fires. - Nothing asserted the *silence* direction, so a third widening of this check would - have met no objection, and the rule forbidding it would have been prose with a guard - on the other half only. That is this file's own recurring defect (C-80, C-82). + Nothing asserted the *silence* direction, so a third widening of this check would have + met no objection and the rule forbidding it would have been prose with a guard on the + other half only. That is this file's own recurring defect (C-80, C-82). + + **It drives the real check**, through the real registry readers, rather than calling + the projection helpers underneath it. A first version called ``_describe_changes`` + directly and was worthless: re-adding the deleted arrival half to the check left the + whole suite green, because that half never lived in the helper this was asking. A + guard has to be pointed at the thing it claims to guard. """ - _, _, expected_class = _PARTNER_ENV[partner] - names = set(expected_class) - mine = next(iter(names)) + mine = sorted(_PARTNER_ENV[partner][2])[0] - pinned = {"target": {mine: {"class": "target", "value": "unchanged"}}} - current = { + pinned = { + "meta": {"version": "0.0.0-fixture"}, + "connection": {"APPWRITE_ENDPOINT": {"class": "connection", "value": "e"}}, "target": {mine: {"class": "target", "value": "unchanged"}}, - # Somebody else's coordinate, arriving in a table this package depends on. - "secret": {"SOMEBODY_ELSES_API_KEY": {"class": "secret", "slot": "theirs"}}, } + # The same registry, plus one coordinate belonging to another repository. + current = {**pinned, "secret": {"SOMEBODY_ELSES_API_KEY": {"class": "secret"}}} - changed = _describe_changes( - _projection(pinned, names), _projection(current, names), names - ) - assert not changed, ( - f"[{partner}] the drift check objected to a row this package does not read: " - f"{changed}. It must be silent on another repository's coordinates — an arrival " - "half that was not was deleted in #245 for exactly this, and re-adding one is " - "refused by the stopping rule in the check's own docstring." - ) + here = sys.modules[__name__] + monkeypatch.setattr(here, "require_sibling", lambda name: Path("/nonexistent")) + monkeypatch.setattr(here, "_registry_at", lambda repo, ref: pinned) + monkeypatch.setattr(here, "_registry_current", lambda repo: current) + + test_nothing_this_repo_reads_has_changed_since_the_pin(partner) @pytest.mark.parametrize("partner", _PARTNERS) From 046ca06ca244ce5d24b1d41ac2455888ba1a9a97 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 17:54:46 +0200 Subject: [PATCH 4/5] =?UTF-8?q?fix(tests):=20#245=20iteration=202=20?= =?UTF-8?q?=E2=80=94=20nothing=20proved=20the=20check=20FIRES?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iteration 2's review, and it found the sharpest thing on this branch. THE DRIFT CHECK'S ENTIRE ASSERTION COULD BE DELETED WITH A GREEN SUITE. Measured: neuter `assert not changed` and the suite stays at 412 passed. My silence guard was the only test driving the real check, and it is purely negative — it asserts the check does NOT raise. Nothing asserted it does. AND MY DOCSTRING VOUCHED FOR THE NEIGHBOURS. It said "the proofs either side of this one are positive — rotation fires, a rename fires". Neither is about this check. The rotation proof calls `_describe_changes`, the helper underneath it — which is precisely the defect I diagnosed and fixed for the silence guard one commit earlier. The rename proof calls `_declared_classes` and re-types the CLASS check's comparison inline; it belongs to a different check entirely, under this one's name. So I applied "a guard must be pointed at the thing it claims to guard" to one of the three tests in the cluster, then used that fix as a docstring warrant for the other two. C-90's rewritten Trigger says "Both remaining halves are proof defects" and enumerates two; this was a third of the same class, in the same file, which made the word "Both" falsifiable by a one-line mutation. Fixed by using the harness I had already built for only the direction that cannot fail: `test_the_drift_check_fires_when_a_row_this_partner_reads_rotates` drives the same real check with a rotated row and `pytest.raises`. Both directions now mutation-proven THROUGH THE CHECK: neuter the assertion -> the twin fails; re-add the arrival half -> the silence guard fails. The vouching for the neighbours is deleted rather than repaired; they are still worth having, they are simply not about this. TWO REGISTER CORRECTIONS, one of which is the same mistake inside its own correction. "6 belong to OTHER REPOSITORIES ENTIRELY" — no. Three are caller keys for other consumers; three are platform slots still marked status="planned" that belong to no repository at all, and one of those names "un_fao delivery" among the identities it would be issued for. C-90's original "belong to no repository here" was the careful wording. I had upgraded it, corrected the upgrade, and upgraded it again two sentences from the correction. And a quotation that was never written: the text pointed "further down this entry" for the partition "earning its keep". That phrase appears nowhere in the repository. The real record is 81 lines ABOVE, and says something narrower. Now quoted verbatim, direction fixed. Also: the silence fixture filed a `secret`-class name under `[target]` — harmless, since both sides carry the identical row, but its two neighbours each assert fixture honesty and this one did not. It now picks a genuinely `target`-class name and asserts it. Suite 414 passed / 1 skipped / 40 xfailed, ruff clean. The one failure is #248's. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- reports/technical_risk_register.md | 13 +++++--- tests/test_env_declaration.py | 52 ++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index ed976c9..504a11c 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -525,8 +525,12 @@ is longer than it wants to be. **The first, now fixed.** The drift check's arrival half ran over every row of every table this package depends on, with no filter for the names it reads. Measured on the live -registry: each partner reads **13 of 25** rows, and **6 belong to other repositories** -(three platform key slots, three caller keys). The other two rows this package does not +registry: each partner reads **13 of 25** rows, and **6 belong to no repository here** — +three caller keys for other consumers, and three platform key slots still marked +`status = "planned"`, one of which names *"un_fao delivery"* among the identities it would +be issued for. C-90's original wording was "belong to no repository here"; an earlier draft +of this paragraph upgraded it to "other repositories entirely", which is the same +overstatement this paragraph exists to correct, two sentences from correcting it. The other two rows this package does not read are its own delivery labels — both declare `producer = "views-postprocessing"` — and they are covered by `tests/test_product.py`, not by this check. *An earlier draft of this paragraph said "8 belong to no repository here", which upgraded a careful claim in C-90 @@ -567,8 +571,9 @@ top-level table upstream must be classified here"* fires on any new table regard whether this repository declares anything about it — and **it has already fired for exactly that reason**: `[edition]` arrived at v1.6.0, an edition the registry's own `[meta]` calls *"additive and opt-in; obliges nobody"*, and it reddened this repository. That event is -recorded further down this entry as the partition "earning its keep", which is true of the -detection and not of the cost. Adopting `[edition].obliges_consumers` is the deferral that +recorded **earlier** in this entry — the partition "went red on it unprompted", "which is +the whole reason that check is directional" — and that record is true of the *detection* +and silent about the *cost*. Adopting `[edition].obliges_consumers` is the deferral that would fix it, and its trigger is below. diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index aa9bb00..edca65d 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -751,7 +751,7 @@ def test_nothing_this_repo_reads_has_changed_since_the_pin(partner): An arrival half failed the surface test and was deleted (#245): it compared every row of every depended-on table, and measured, each partner reads 13 of 25 while 6 belong - to other repositories entirely. It defended itself by claiming ``[contract.*]`` + to no repository here. It defended itself by claiming ``[contract.*]`` "arrived exactly this way and nothing else here would have seen it" — false at table granularity, where ``test_every_table_in_the_registry_is_classified_here`` catches it. @@ -979,10 +979,15 @@ def edition(value: str) -> dict: def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner, monkeypatch): """The stopping rule above, as a check rather than a paragraph. - The proofs either side of this one are positive — rotation fires, a rename fires. - Nothing asserted the *silence* direction, so a third widening of this check would have - met no objection and the rule forbidding it would have been prose with a guard on the - other half only. That is this file's own recurring defect (C-80, C-82). + Paired with ``test_the_drift_check_fires_when_a_row_this_partner_reads_rotates``, + which is the positive direction through the same harness. Neither is optional: alone, + this one is a check that can only pass, and alone the twin says nothing about what the + check ignores. + + An earlier version of this docstring vouched for the two proofs either side — "rotation + fires, a rename fires" — and neither is about this check. One calls the helper + underneath it; the other belongs to the class check. That is this file's own recurring + defect (C-80, C-82), so the vouching is gone and the twin is real. **It drives the real check**, through the real registry readers, rather than calling the projection helpers underneath it. A first version called ``_describe_changes`` @@ -990,7 +995,12 @@ def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner, whole suite green, because that half never lived in the helper this was asking. A guard has to be pointed at the thing it claims to guard. """ - mine = sorted(_PARTNER_ENV[partner][2])[0] + expected_class = _PARTNER_ENV[partner][2] + mine = sorted(n for n, cls in expected_class.items() if cls == "target")[0] + assert expected_class[mine] == "target", ( + f"[{partner}] this fixture files {mine} under [target]; if this package no longer " + "classifies it that way the fixture is lying about the registry's shape." + ) pinned = { "meta": {"version": "0.0.0-fixture"}, @@ -1008,6 +1018,36 @@ def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner, test_nothing_this_repo_reads_has_changed_since_the_pin(partner) +@pytest.mark.parametrize("partner", _PARTNERS) +def test_the_drift_check_fires_when_a_row_this_partner_reads_rotates(partner, monkeypatch): + """The positive twin of the silence guard, through the same real check. + + Without this, the check's entire ``assert not changed`` could be deleted and the suite + would stay green — measured. Its two apparent proofs are not about it: the rotation + proof below calls ``_describe_changes``, the helper underneath, and the rename proof + calls ``_declared_classes``, which belongs to a different check altogether. So the + silence guard was the only test driving this one, and a guard that can only pass is + the same defect as a guard pointed at the wrong subject. + """ + expected_class = _PARTNER_ENV[partner][2] + mine = sorted(n for n, cls in expected_class.items() if cls == "target")[0] + + pinned = { + "meta": {"version": "0.0.0-fixture"}, + "connection": {"APPWRITE_ENDPOINT": {"class": "connection", "value": "e"}}, + "target": {mine: {"class": "target", "value": "at-the-pin"}}, + } + rotated = {**pinned, "target": {mine: {"class": "target", "value": "after-rotation"}}} + + here = sys.modules[__name__] + monkeypatch.setattr(here, "require_sibling", lambda name: Path("/nonexistent")) + monkeypatch.setattr(here, "_registry_at", lambda repo, ref: pinned) + monkeypatch.setattr(here, "_registry_current", lambda repo: rotated) + + with pytest.raises(AssertionError, match=mine): + test_nothing_this_repo_reads_has_changed_since_the_pin(partner) + + @pytest.mark.parametrize("partner", _PARTNERS) def test_the_drift_check_would_catch_a_rename(partner): """A gated test that cannot fail is decoration — so prove this one bites in CI. From 33a38f7bc391be1cd2a77098fcdb7561eb1e814b Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 12 Aug 2026 18:09:50 +0200 Subject: [PATCH 5/5] =?UTF-8?q?fix(tests):=20#245=20review-diff=202=20?= =?UTF-8?q?=E2=80=94=20a=20tautology,=20in=20the=20commit=20that=20fixed?= =?UTF-8?q?=20a=20tautology?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings, both mine, both classes I had just finished correcting elsewhere in the same commit. THE FIXTURE-HONESTY ASSERTION COULD NOT FAIL. I wrote: mine = sorted(n for n, cls in expected_class.items() if cls == "target")[0] assert expected_class[mine] == "target", ... `mine` is SELECTED by the condition it is then asserted to satisfy. True by construction, for every possible input. That is C-90's own first defect — `base = {name: {} for name in _TABLE_ROLE}` then `assert not _unclassified_tables(base)` — reproduced by me in the commit whose whole subject was "nothing proves this check fires". Replaced with the thing that can actually go wrong: assert the selection found anything. If a partner stops declaring a `target` coordinate the fixture has nothing to file and would prove nothing, and now it says so. Proven by pointing the selection at a class that does not exist and watching it fail. AND "THE ROTATION PROOF BELOW" IS ABOVE. It is at :929; the twin is at :1022 — 93 lines below it, not above. Same directional error as the "recorded further down this entry" pointer I fixed in the same commit, which pointed 81 lines the wrong way. Two in one change is a pattern rather than a slip: I describe a file's layout from memory instead of reading it. Suite 414 passed / 1 skipped / 40 xfailed, ruff clean. The one failure is #248's. Epic #241. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_env_declaration.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index edca65d..2672ed6 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -996,11 +996,15 @@ def test_the_drift_check_is_silent_on_a_row_this_partner_does_not_read(partner, guard has to be pointed at the thing it claims to guard. """ expected_class = _PARTNER_ENV[partner][2] - mine = sorted(n for n, cls in expected_class.items() if cls == "target")[0] - assert expected_class[mine] == "target", ( - f"[{partner}] this fixture files {mine} under [target]; if this package no longer " - "classifies it that way the fixture is lying about the registry's shape." + targets = sorted(n for n, cls in expected_class.items() if cls == "target") + assert targets, ( + f"[{partner}] declares no coordinate of class 'target', so this fixture has " + "nothing to file under [target] and would prove nothing. (An earlier version " + "asserted that the chosen name's class was 'target' — true by construction, " + "since the name was selected by that condition. A proof that cannot fail is the " + "defect this entry's own C-90 records.)" ) + mine = targets[0] pinned = { "meta": {"version": "0.0.0-fixture"}, @@ -1024,13 +1028,14 @@ def test_the_drift_check_fires_when_a_row_this_partner_reads_rotates(partner, mo Without this, the check's entire ``assert not changed`` could be deleted and the suite would stay green — measured. Its two apparent proofs are not about it: the rotation - proof below calls ``_describe_changes``, the helper underneath, and the rename proof + proof above calls ``_describe_changes``, the helper underneath, and the rename proof calls ``_declared_classes``, which belongs to a different check altogether. So the silence guard was the only test driving this one, and a guard that can only pass is the same defect as a guard pointed at the wrong subject. """ - expected_class = _PARTNER_ENV[partner][2] - mine = sorted(n for n, cls in expected_class.items() if cls == "target")[0] + targets = sorted(n for n, cls in _PARTNER_ENV[partner][2].items() if cls == "target") + assert targets, f"[{partner}] declares no 'target' coordinate to rotate" + mine = targets[0] pinned = { "meta": {"version": "0.0.0-fixture"},