Repository navigation
Apply the platform's own rules, and close the gaps four reviewers found - #5
Merged
Merged
Conversation
…unts Both metrics read a positive from the presence of something, and only the negative case had ever been seen — the dangerous way round, since a wrong assumption would leave the check permanently false and let a locked account through, in a design that is fail-closed everywhere else. Checked against the live API. A locked account carries `locked: true`; an unlocked one has no such key at all, so presence is a sound signal rather than a lucky guess. A global block returns a row from `bgtargets`. Both readings were already correct. The accounts came from the API itself — `list=globalblocks`, and the globalauth log — so nobody had to be singled out, and the fixtures here are synthetic reproductions of the confirmed shapes. No real account is named. Closes #1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A blocked account passed frwiki-sondage, because that policy codifies no eligibility and nothing enforced anything outside a policy's own list. But "no formal requirement" never meant "a blocked account may vote" — it meant the community had nothing to add to what the software already imposes. Global locks and global blocks now apply to every policy, and a site-wide block on the policy's own wiki applies whenever the policy is scoped to one. They are marked `source: platform` against the community's `source: policy`, so nobody mistakes a rule the software imposes for something a wiki wrote. A policy that states one itself keeps its own version and the baseline stands down, leaving the community's wording the authority. `baseline: false` opts out entirely. Boolean criteria are also named after what must hold rather than what is being looked for: "not globally locked", not "globally locked ... passed", which made a reader invert it in their head. The fan-out per check rose from 4 to 6, so the derived rate limit came down from 40/min to 26/min on its own. That is the design working — the budget is the constant and the limit follows it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing the offset documentation earlier cut mid-sentence and left the notes as a fragment: "...the proposal being posted. between them if a particular vote needs that precision." A policy file whose whole purpose is to faithfully carry what a community wrote is the worst place to leave that. Also restores a clause dropped from the original wording — "(ingangstijd/datum van stemvoorstel dient ter referentie)", which names the proposal's timestamp as the reference point, and is precisely the sentence the notes discuss. The second source page was the more serious omission. It records a 2018 proposal to add an activity requirement for person-elections that was NOT adopted. Citing it without saying so is misleading: a reader following the link finds activity requirements and would reasonably assume they apply. The notes now say it did not pass, and record that Dutch voting rights once earned are held for life, so nothing here expires. Scanned the other four entries for the same damage; none found. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven places asserted that stewards place global locks and blocks, plus a test file named after the claim. That attribution was never checked against anything — it is plausible, and plausible is not the standard for a tool whose whole value is showing its working. The state is what the API reports and what the policies care about: an account is globally locked, or a global block applies to it. Who acted, and under what authority, is a separate fact this tool does not read and should not assert. Nothing about the checks changes; only what we claim to know about them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ages The repository URL was written out three times in Python, one of them a constant named REPOSITORY that nothing imported. It now lives in mediawiki.py, the lowest module, and app.py imports it. It matters more than tidiness: the User-Agent carries that URL as the contact address WMF uses to reach an operator, and a rename would have left it stale in exactly that field. Separately, two pieces of prose claimed something the code does not do. "/policies — what it decomposes to" reads as though the tool decomposes a community's wording into rules, and "when we last read the policy page" reads as though the tool read it. A person does both. The boundary is now stated outright in app.py and the README: this tool never reads policy pages and never interprets prose. A person writes the rules; the tool executes them; the original wording travels with the verdict so a reader can audit that person's work. Inferring rules from text would leave nobody able to tell whether a verdict reflected the community's rule or the tool's reading of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An adversarial pass over the suite found the deduplication in baseline_rules untested: removing the filter, so a policy's own rule and the platform's implied one both apply, left all 41 tests green while enwiki-arbcom silently grew a duplicate is_blocked. Double-evaluating rarely flips a verdict — it corrupts the audit trail a voter is meant to follow by hand. Five tests now cover it, and removing the filter fails two of them. Writing those found a wrong belief of my own: the first version asserted no metric may repeat, which is false. German Wikipedia legitimately has two edit_count rules, 200 all-time and 50 within twelve months. Identity is the whole rule, not the metric name. A fidelity pass over the comments found prose claiming things the code does not do: - The README said edit_count cannot use at_most or fewer_than. It can, since the cap is widened per operator; what is refused is a threshold above the API's 500-row ceiling. - The rate-limit derivation said four upstream calls per check. The constant it claims to derive says six, and six is right — an auditor doing the arithmetic by hand would have got 40/min instead of 26. - Two places still said "our reading" of a community's policy, contradicting the interpretation boundary stated a few lines away in the same files. - The README's worked example carried a removed field, omitted a criterion, and never mentioned `source`. It is now generated from a real response. - mediawiki.py claimed contribution counts are cached. They are not, and should not be: two edit-count rules are two different questions. A readability pass found scar tissue: fourteen lines defending a three-line limiter, a comment that was a commit message, and dangling references to documents that exist nowhere in this repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…icy names Six plausible wrong-code changes were run against the suite. Four were caught. Two got through, and both mattered. Collapsing `indeterminate` into `not_eligible` passed every test. Those are different answers: one says we checked and you fall short, the other says we could not check, which is a reason to look by hand rather than to turn someone away. `test_traps.py` proved `rules.apply` keeps the distinction; nothing proved it survived the HTTP layer, which is where a consumer reads it. Making the account cache ignore which wiki also passed. No policy spans two wikis today, so nothing noticed — a future one would have read the first wiki's block status and groups for the second, silently. Separately, `load_policies`' docstring claimed it refuses "a rule we cannot answer" at startup, and it did not: a misspelt metric or operator loaded fine and became a 500 at request time. That is exactly the mistake someone editing policies.yaml would make, and that file is meant to be edited by people who do not read Python. It now fails at startup, naming the typo and listing the valid spellings. Making the code match the claim was the better fix than narrowing the claim. Also corrects a test docstring that counted four account lookups where there are three, and a comment whose second sentence described a rule appended by a different function. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
**An account with no CentralAuth record was refused on every policy.** The platform rules ask about global status on every check, so `global_account()` raising `UserNotFound` denied a real person everywhere — reporting "No such account" for an account whose local existence the response's own `queries` array proved one call earlier. Direction was exclusion. The refusal is right; the reasoning was not. Every account has been unified for years, so one without a CentralAuth record is anomalous. That is now its own named condition, `has_global_account`, applied with the other platform rules — so the person sees which requirement they failed instead of being told they do not exist. A lock is a CentralAuth attribute, so no record now correctly reads as "not locked" rather than raising. **A local block is not a platform rule.** A site-wide block on one wiki is that community's sanction under its own blocking policy, and whether it also removes a vote is that community's decision to write down — WMF Board elections disqualify only an account blocked on more than one project. Imposing it by default applied one community's sanction to policies that never asked for it, and broke meta-global, whose whole subject is movement-wide standing. Only conditions that hold movement-wide remain. **The deduplication ignored which wiki.** It keyed on metric name alone, so a policy stating a rule about one wiki suppressed the platform rule for another. Now keyed on metric and wiki together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measured: applying the same rule three times makes one upstream request, since lookups are memoised. Rules are ANDed, so a repeat cannot change a verdict. Deduplication therefore saved no traffic and prevented no error — it only made the criteria list shorter. For that it required deciding when two rules are "the same", and that decision was wrong. Keying on the metric alone meant a policy stating a rule about one wiki silently suppressed the platform rule for another. The fix keyed on wiki as well, which was correct but was still paying for a shorter list with a question that has no good general answer. Showing both is also the better reading. A community that states a condition the software already imposes has said something, and the response now records that it did, one criterion marked `policy` and one `platform`. Five tests went with it; one remains, asserting the repeat is allowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/policies served the file verbatim while /check also applied the platform's rules. A policy stating none of its own was therefore published as requiring nothing while three conditions were being checked — the endpoint built for auditing the tool misdescribing what the tool does, which is the one place that error costs the most. Both now read from a single resolve(), which returns every rule that will be evaluated paired with where it came from. The French note no longer calls that policy an empty case, because it is not one. When policies gain a parent (#4), only resolve() walks the chain; both callers follow without changing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A policy anchors on its own instant — "150 mainspace edits by 1 November", "two weeks before the vote opened". The tool could only measure from now, so asked on 15 November it silently answered "by 15 November" and counted a fortnight of edits the policy excludes. Precision was never the problem: we already measured to the second, from the wrong second. Callers may now pass `as_of`, and a future one is allowed, since asking before a vote opens is the ordinary case rather than an error. `checked_at` and `as_of` are reported separately. A verdict measured against November must not claim it was taken then. Writing that turned up a bug of its own: a `+` in a query string decodes to a space, so any caller passing a numeric timezone offset had their valid timestamp refused. Only the `Z` form worked. The sign is now restored on a trailing offset, without breaking the space-separated ISO form. Separately, the loader checked a rule's metric and operator by name but never its parameters, so `is_globally_locked` given a wiki, or `is_blocked` given none, both loaded fine and raised TypeError at request time as a 500 — the exact failure the validation exists to prevent. Parameters are now bound against the metric's signature at startup. Two test fixtures turned out to have been invalid all along and were only passing because nothing checked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s not got Both the enwiki and nlwiki entries said their date approximation errs towards admitting people. It does not. Measuring from now drifts in whichever direction the asking moment falls: after the policy's anchor the window runs too late and admits people it excludes, before the anchor it runs too early and excludes people it admits. Claiming a single safe direction is worse than claiming none, because a reader relies on it. Both notes now say what to do instead — pass `as_of` set to the moment the policy anchors on, and for the Dutch entry an `offset` on the edit-count rule for the gap between the proposal and the vote opening, which is set per vote and cannot live in a shared file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A criterion the tool could not evaluate reported `label` as the bare metric identifier, so one line of a verdict read "time_since_first_edit" while the next read "edits on Dutch Wikipedia" — the same key carrying an identifier on one branch and prose on the other. An earlier audit named this and it was only half-fixed: the successful path was corrected and the failing one was not. A metric knows what it is called before it discovers it cannot answer, so NotMeasurable now carries the label along with the reason. Found by running a real account with no edits on the wiki in question, which is the ordinary way to reach that branch and was not covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s findable An account with no edits on the wiki was reported as a rule that "could not be checked", so a policy whose only requirement was a first edit returned `indeterminate` for somebody who has plainly never edited there. Those are different absences: no edits is a fact we have, and it settles the comparison — there is no first edit, so it cannot have been long enough ago. A missing registration date, which MediaWiki genuinely does not hold for pre-2006 accounts, is the case that deserves "could not be checked". The first is now a definite failure carrying the absence as its observation, and only the second leaves the verdict open. Also splits the policy endpoints. /policies was the whole file — 8.7KB to answer "which policies exist", which does not scale past a handful. It now lists what there is to choose from, filterable by wiki, each entry linking to its own detail at /policies/<id>. The detail still shows every rule that would actually run, tagged by origin, so no view can describe a smaller check than /check performs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A rule that raised took the whole request with it — an HTML 500 that told a reader nothing, not even which rule went wrong, and discarded the rules that had answered perfectly well. A defect in one condition is our bug, not grounds to withhold the four conditions we did evaluate. Each rule is now applied in isolation. One that breaks is reported in place, named, with its error, and counted as neither passed nor failed — so the verdict cannot come out eligible while one of its conditions is unknown. The traceback goes to the server log, with the rule and its wiki but not the account, since that pairing is the one thing a log here creates that exists nowhere else. Upstream failure is deliberately still fatal to the request. If Wikimedia cannot be reached then every remaining rule is equally unanswerable, and a verdict listing one answer and three broken rules would look more complete than it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removed earlier on the grounds that it was a compatibility shim for a client that did not exist yet. The client exists and is deployed: wiki-polis sends `?user=&event=` from v2/app.py:1547. What did not exist was the URL pointing at this service, so the call had never been made and nothing failed to reveal it. "No consumer" was a claim about the world that a grep of the other repository would have settled, and I did not run it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Everything that landed after #2 merged, plus the fixes from a review panel run against it.
The bug this fixes
A globally locked account is currently reported
eligibleonfrwiki-sondage. That policy states no rules of its own, so nothing was checked at all.Conditions the software imposes movement-wide — a unified account, no global lock, no global block — now apply to every policy. Each criterion carries
source: platformorsource: policy, so a reader can see which requirements came from the community and which from the software.A local block is deliberately not among them. A site-wide block on one wiki is that community's sanction under its own blocking policy, and whether it also removes a vote is that community's decision to write down — WMF Board elections disqualify only an account blocked on more than one project. Imposing it by default would apply one community's sanction to policies that never asked for it.
What the panel found
Three blockers, all fixed:
queriesarray proved. Refusing is right — every account has been unified for years — but it is now its own named condition,has_global_account, so the person sees which requirement they failed.The deduplication is now gone entirely. Measured: applying the same rule three times makes one upstream request, since lookups are memoised, and rules are ANDed so a repeat cannot change a verdict. It saved no traffic and prevented no error — it only shortened a list, and for that it required deciding when two rules are "the same", which is where it went wrong.
as_ofA policy anchors on its own instant — "150 mainspace edits by 1 November", "two weeks before the vote opened". The tool could only measure from now, so asked on 15 November it silently answered "by 15 November". Precision was never the problem: we already measured to the second, from the wrong second.
Callers may now pass
as_of. A future one is allowed, since asking before a vote opens is the ordinary case.checked_atandas_ofare reported separately, because a verdict measured against November must not claim it was taken then.Writing that found a bug of its own: a
+in a query string decodes to a space, so any caller passing a numeric timezone offset had a valid timestamp refused.Both endpoints now agree
/policiesserved the file verbatim while/checkalso applied the platform rules — the endpoint built for auditing the tool misdescribing what the tool does. Both now read from oneresolve(). When policies gain a parent (#4), only that walks the chain.Validation
The loader checked a rule's metric and operator by name but not its parameters, so
is_globally_lockedgiven a wiki, oris_blockedgiven none, loaded fine and raisedTypeErrorat request time as a 500 — the exact failure the validation exists to prevent. Parameters are now bound against each metric's signature at startup. Two test fixtures turned out to have been invalid all along, passing only because nothing checked.Verified rather than assumed
The two steward-action checks are confirmed against a genuinely locked account and a genuinely globally-blocked one, found by asking the API rather than by choosing anyone. That closes #1, whose premise was wrong.
Corrected along the way: the Dutch policy entry, whose notes a careless regex had cut mid-sentence and whose citation of a rejected 2018 proposal was unexplained; and two notes claiming their date approximation errs towards admitting people, when it drifts in whichever direction the asking moment falls.
Testing
64 tests, up from 37.
ruffclean. Six adversarial mutations were run against the suite; two got through and both are now covered.🤖 Generated with Claude Code