Hand a revoked review's reports back to the community gate

The first revoke rebuilt a reviewed row's community reports as one
unbucketed event on the row's last_seen, which included the review date.
A later report on the original day then counted as a day apart, bucketed
reporters lost the three-reporter rule, a row the community had published
on its own was unpublished, and the review's placeholder count of 1
carried into the ledger.

Now the ledger keeps the real events: approving a pending number no longer
pops its state, the quorum recheck keeps the state of a reviewed row whose
only other support is community reports, and reports that arrive while
the review holds a row up are recorded too. A reviewed row is never
demoted by a bucket recheck. On revoke, a row with a ledger state goes
back to the gate (count and dates from its events when the review had
published it), and one without is unpublished. Production data: a no-op
merge stays at v52, revoking #27's number gives v53, a rerun stays put.

Tests: assert_revoked_review_hands_reports_back_to_the_gate covers each
case above. The approval test asserted the ledger forgot the report once
the review published it; that loss is the bug, so it now asserts the
report is kept unpublished. The revoke test's settled row now stays
published, a legacy reviewed row covers the no-ledger path, and an
FCC-only revoke step makes the version bump depend on the revocation
alone (later version numbers shift by one).

Author: SysAdminDoc

CHANGELOG.md

@@ -7,7 +7,9 @@ All notable changes to CallShield will be documented in this file.
 - A number the maintainer approved can now be taken back. Its entry in
   `spam_numbers_approved.json` gets `"revoked": true` and a date, and the next
   merge strips the review from the row. A row the review alone was holding up
-  is unpublished. A number listed twice stops the merge with an error naming
+  is unpublished. One with community reports goes back to the usual gate with
+  the reports it actually got, so the review's date and count can't help it
+  through. A number listed twice stops the merge with an error naming
   it, since two dates made its evidence flip and every phone re-download the
   database on each merge. So does deleting an approval outright, which used to
   leave the review in the row for two years. Not-spam votes no longer make a

data/README.md

@@ -239,11 +239,13 @@ it goes in `spam_numbers_approved.json` with a link to the report. The merge
 then publishes it, keeping the pending community reports in its count, and the
 review is the row's own evidence, so later reports can't demote it.
 
-A revoked approval comes off at the next merge. The row keeps whatever its
-other sources say and loses the review's evidence and description. A row the
-review alone was holding up is unpublished, including one that only community
-reports back. Those reports go back to the pending ledger, counted as one day's
-reports on the row's last day, so the usual gate decides whether it returns.
+The pending ledger keeps a reviewed number's community reports, including
+ones that arrive after the review, with their real days and reporters. A
+revoked approval comes off at the next merge. The row keeps whatever its other
+sources say and loses the review's evidence and description. A row with
+community reports goes back to the usual gate on those reports alone, so it
+stays if they carry it and is held if they don't. A row the review alone was
+holding up is unpublished.
 
 Every evidence record carries `expires_at_epoch_ms`, and phones stop matching
 a downloaded number or range once its last record expires, so a row

scripts/merge_community_reports.py

@@ -382,22 +382,31 @@ def require_listed_reviews(existing: dict[str, dict], listed: set[str]) -> None:
             )
 
 
+def keeps_community_ledger(entry: dict) -> bool:
+    """A reviewed row whose only other support is community reports.
+
+    Its reports stay in the pending ledger with their real days and reporters,
+    so revoking the review hands them back to the community gate intact.
+    """
+    support = canonical_source_ids(entry)
+    return MAINTAINER_SOURCE in support and support - {MAINTAINER_SOURCE} <= {"community_reports"}
+
+
 def revoke_maintainer_approvals(
     existing: dict[str, dict],
     pending: dict[str, dict],
     revoked: set[str],
-    today: str,
-    pending_cutoff: str,
 ) -> tuple[int, int]:
     """Take the review back off every row whose approval is revoked.
 
     Its evidence, source and description come off. A row another source still
-    backs stays published without them. A row the review carried is unpublished,
-    and its community reports go back to the pending ledger as one report on the
-    row's last day, which can delay a later promotion but never hurry one. Running
-    this twice changes nothing.
+    backs stays published without them. A row with community reports in the
+    ledger goes back to the community gate, which runs right after this and
+    keeps it or unpublishes it on those reports alone. A row the review alone
+    carried, or whose reports have aged out of the ledger, is unpublished.
+    Running this twice changes nothing.
     """
-    stripped = 0
+    released = 0
     removed = 0
     for number in sorted(revoked):
         entry = existing.get(number)
@@ -416,18 +425,25 @@ def revoke_maintainer_approvals(
             part for part in entry.get("description", "").split("; ") if part != MAINTAINER_DESCRIPTION
         )
         support = canonical_source_ids(entry)
+        state = pending.get(number)
         if support - {"community_reports"}:
-            stripped += 1
+            released += 1
+            continue
+        if support and state is not None and (state["published"] or state["events"]):
+            if not state["published"]:
+                # The review published it, so its count and dates were partly
+                # the review's. The reports alone decide them now.
+                days = [event["day"] for event in state["events"]]
+                entry["reports"] = sum(event["count"] for event in state["events"])
+                entry["first_seen"], entry["last_seen"] = min(days), max(days)
+            entry["description"] = entry["description"] or COMMUNITY_DESCRIPTION
+            state["entry"] = entry
+            released += 1
             continue
         del existing[number]
         pending.pop(number, None)
         removed += 1
-        reports = max(0, int(entry.get("reports", 0)))
-        last = entry.get("last_seen")
-        if "community_reports" in support and reports and _is_valid_day(last, today) and last >= pending_cutoff:
-            event = {"key": f"revoked:{number}", "day": last, "bucket": "", "count": reports}
-            pending[number] = {"published": False, "entry": entry, "events": [event]}
-    return stripped, removed
+    return released, removed
 
 
 def apply_maintainer_approvals(
@@ -444,7 +460,8 @@ def apply_maintainer_approvals(
     GitHub waiting forever when it's a region no imported source covers
     (issue #27, a Munich number). The review is its own evidence record, so the
     row is no longer community-only and the quorum rechecks leave it alone. The
-    pending reports still count toward it. Running this twice changes nothing.
+    pending reports still count toward it and stay in the ledger, along with
+    later ones, in case the review is revoked. Running this twice changes nothing.
     """
 
     added = 0
@@ -468,7 +485,7 @@ def apply_maintainer_approvals(
             updated += 1
             continue
 
-        state = pending.pop(number, None)
+        state = pending.get(number)
         events = state["events"] if state else []
         days = [event["day"] for event in events] + [reviewed]
         base = dict(state["entry"]) if state else {}
@@ -656,16 +673,15 @@ def main(argv: list[str] | None = None):
     # as a community report day.
     approvals, revoked_numbers = validated_approvals(load_approved_numbers(), today)
     require_listed_reviews(existing, {approval["number"] for approval in approvals} | revoked_numbers)
-    revoked_stripped, revoked_removed = revoke_maintainer_approvals(
-        existing, pending, revoked_numbers, today, pending_cutoff
-    )
-    if revoked_stripped or revoked_removed:
-        print(f"Revoked approvals: {revoked_removed} rows unpublished, {revoked_stripped} kept on other sources")
+    revoked_released, revoked_removed = revoke_maintainer_approvals(existing, pending, revoked_numbers)
+    if revoked_released or revoked_removed:
+        print(f"Revoked approvals: {revoked_removed} rows unpublished, {revoked_released} left to their other sources")
 
     demoted = 0
     for number, entry in list(existing.items()):
         if canonical_source_ids(entry) != {"community_reports"}:
-            pending.pop(number, None)
+            if not keeps_community_ledger(entry):
+                pending.pop(number, None)
             continue
         state = pending.setdefault(number, legacy_community_state(entry, today))
         if state["published"] and (state.get("bucket_quorum") or not any(event["bucket"] for event in state["events"])):
@@ -776,10 +792,32 @@ def main(argv: list[str] | None = None):
                         entry["sources"] = sorted(set(entry.get("sources", [])) | {COMMUNITY_SOURCE})
                         if reported_at > entry.get("last_seen", ""):
                             entry["last_seen"] = reported_at
+                        if state is None and keeps_community_ledger(entry):
+                            state = {
+                                "published": False,
+                                "entry": {
+                                    "number": number,
+                                    "type": entry.get("type", spam_type),
+                                    "reports": 0,
+                                    "first_seen": reported_at,
+                                    "last_seen": reported_at,
+                                    "description": COMMUNITY_DESCRIPTION,
+                                    "sources": [COMMUNITY_SOURCE],
+                                },
+                                "events": [],
+                            }
+                            pending[number] = state
                         if state:
                             state["events"].append(event)
-                            state["entry"] = entry
-                            if bucket and state["published"] and not state.get("bucket_quorum") and not community_has_quorum(state):
+                            if state["published"]:
+                                state["entry"] = entry
+                            if (
+                                bucket
+                                and state["published"]
+                                and not state.get("bucket_quorum")
+                                and not has_maintainer_review(entry)
+                                and not community_has_quorum(state)
+                            ):
                                 state["published"] = False
                                 del existing[number]
                                 demoted += 1
@@ -892,7 +930,7 @@ def main(argv: list[str] | None = None):
         or demoted > 0
         or approved_added > 0
         or approved_updated > 0
-        or revoked_stripped > 0
+        or revoked_released > 0
         or revoked_removed > 0
     )
     if changed: