Skip to content

Commit 54ce20f

Browse files
CamSoperclaude
andauthored
content-review: close the loop between the claims detector and the fixer (#20968)
* content-review: close the loop between the claims detector and the fixer The nightly claims re-verification finds real bugs and the per-article review worker never acts on them. Observed end-to-end in #20927: concepts/providers/_index.md was flagged 2026-08-15 for a contradicted version pin, boosted to the front of the queue on that marker, reviewed, reported "0 contradicted" across 74 re-extracted claims, and merged a one-line unrelated repair — with the flagged bug still on master (it took a hand-written PR, #20961). The marker then cleared, so the entity returned to the pool to be re-flagged and re-missed on a ~2-day cycle. Three defects, three fixes: 1. The evidence never reached the worker. select-articles.py put only `len(stale_claims)` — an integer — into the queue item, while the review skill was told to "treat the ledger markers' entity keys and evidence as priority findings". It was being asked to act on data it was never given, so it re-derived from scratch and missed. The queue item now carries `stale_claim_markers` in full, and the skill documents them as must-address findings. 2. Unresolved markers were silently cleared. build_record rebuilt the ledger entry from the queue and dropped `stale_claims` whether or not anything had been done. A marker now retires only when the verdict names its entity in the new optional `resolved_claims` field; anything else carries forward with `unresolved_reviews` incremented. Since an unboundedly-boosted page would starve the queue, a marker that survives MARKER_ESCALATION_CAP (2) reviews is flagged `escalated`: still recorded and still visible, but no longer boosting — the signal that a human, not another identical pass, is the next step. 3. Markers on generated pages could never be cleared at all. select-articles.py globs content/docs/**/*.md, so an adapter-generated page (the pre-built policy-pack tables) can never be selected — while already_marked() excluded its entities from re-verification forever. Three entities were lost this way over five nights (iam-user-unused-credentials-90, redshift-backup-enabled, cloud-logging-retention-period-365). apply_markers() now skips pages with no source file and reports the verdict instead, leaving the entity in the pool; those contradictions are real but the fix is upstream of this repo. make test-review-pipeline and make lint pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hp4LqiP4u81uT48MS8Hea4 * content-review: keep escalated markers on the ledger through a review Review feedback on #20968 (thanks @unblocked) found a real hole in the escalation path, and a smaller inconsistency next to it. Escalated markers were filtered out of the queue item, but record-review.py rebuilds the ledger entry *from the queue* and writes back only what survives — so the first time an escalated marker's page was reviewed for any other reason (ordinary staleness, or --paths), that marker was dropped from the ledger entirely. already_marked() in reverify-claims.py would then stop seeing the entity and let it back into the nightly pool, restarting the detect/boost/miss/clear cycle this PR exists to break, just two reviews later. The PR body claimed the opposite invariant ("the marker stays on the ledger entry"), and it did not hold. Relatedly, the queue's `stale_claims` count was computed over all markers while `stale_claim_markers` carried only the non-escalated ones, so a page whose only marker was escalated advertised `stale_claims: 1` alongside an empty list — contradicting what SKILL.md tells the worker those two fields mean. Both come from the same root cause: escalation was being enforced at the wrong layer. It is a *boost* policy, not a visibility policy. So the queue item now carries every marker (new all_markers()) and the count is taken over that same list, while active_markers() — non-escalated only — is used solely for the scoring boost. Escalated markers travel, persist, and stay resolvable; they simply stop jumping the page to the front of the queue. SKILL.md documents the `escalated` flag so the worker knows what it means rather than inferring it. Regression coverage for the exact failure: an escalated marker rides the queue item, reaches a --paths review, survives build_record with its counter still incrementing, and can still be retired via resolved_claims; plus the count/list agreement. All three new select-articles checks fail against the previous code and pass against this one. make test-review-pipeline and make lint pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hp4LqiP4u81uT48MS8Hea4 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 579534e commit 54ce20f

5 files changed

Lines changed: 299 additions & 10 deletions

File tree

.claude/commands/review-existing-content/SKILL.md

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ Read `.content-review-queue.json` from the repo root (written by
5252
"feedback": { "yes": 4, "no": 9, "neg_rate": 0.6923, "multiplier": 1.27 }
5353
},
5454
"last_reviewed": null,
55+
"stale_claims": 1,
56+
"stale_claim_markers": [
57+
{ "entity_key": "version/pulumi-package",
58+
"verdict": "contradicted",
59+
"evidence": "CHANGELOG lists \"Save package source to `packages` in Pulumi.yaml on `package add`\" under 3.163.0, not 3.157.0.",
60+
"source": "gh api repos/pulumi/pulumi/releases",
61+
"checked_at": "2026-08-15",
62+
"unresolved_reviews": 0 }
63+
],
5564
"score": 0.91 }
5665
]
5766
}
@@ -60,8 +69,31 @@ Read `.content-review-queue.json` from the repo root (written by
6069
- `lane``priority` (scored pick) or `manual` (workflow_dispatch override).
6170
- `stale_claims` (when present) — count of this page's volatile claims the
6271
nightly re-verification found contradicted (see §Claims index below). A
63-
non-zero count is why the page jumped the queue: treat the ledger markers'
64-
entity keys and evidence as priority findings to re-check first.
72+
non-zero count is why the page jumped the queue.
73+
- `stale_claim_markers` (when present) — **those findings in full**, each with
74+
the `entity_key`, the `verdict`, the `evidence` the nightly verifier
75+
recorded, the `source` it reached, and `unresolved_reviews` (how many prior
76+
reviews saw this marker and left it unresolved). **These are the highest-
77+
priority findings in your queue and you must address every one of them.**
78+
The nightly lane has already done the expensive part — it identified the
79+
entity, reached an authoritative source, and wrote down what that source
80+
says — so start here rather than waiting to see whether your own claim
81+
extraction happens to re-derive the same finding. It may not: a page boosted
82+
for a contradicted version pin was once reviewed, reported "0 contradicted"
83+
across 74 re-extracted claims, and merged an unrelated one-line repair while
84+
the flagged bug stayed on master.
85+
A marker carrying `"escalated": true` has already survived two reviews
86+
unresolved, so it no longer jumps the page to the front of the queue and a
87+
human may be looking at it — but it is still a live finding and still yours
88+
to resolve if you can. Treat it like any other marker.
89+
For each marker, either apply the fix, or establish that the flag was wrong
90+
(the nightly verdicts are single-sample and do produce false positives —
91+
a synthetic module path for a locally generated SDK was once flagged as a
92+
broken import). Then list its `entity_key` in the verdict sentinel's
93+
`resolved_claims`. A marker you do not resolve is carried onto the next
94+
review with `unresolved_reviews` incremented, and after two such rounds it
95+
is escalated for a human — so silently skipping one does not make it go
96+
away, it just delays it.
6597
- `no_retire` — when true, retirement must never be proposed for this page.
6698
This is the **hard veto** on retirement — honor it regardless of evidence.
6799
- `reader_signals` / `signals` — Search Console and feedback-widget figures
@@ -335,6 +367,7 @@ the canonical ledger record, and uploads it to S3 keyed by slug.
335367
"skipped_findings": 2,
336368
"retirement": false,
337369
"clarity_flag": true,
370+
"resolved_claims": ["version/pulumi-package"],
338371
"applied": [
339372
{ "category": "claim", "file": "content/docs/iac/concepts/stacks/_index.md",
340373
"lines": [42, 43], "source": "verified-claims:c3" },
@@ -362,6 +395,12 @@ the canonical ledger record, and uploads it to S3 keyed by slug.
362395
`len(applied)`. The workflow's scope gate cross-checks these against the
363396
artifacts and the branch diff; for link fixes (which have no artifact) the
364397
declared lines must actually carry the link in the pre-fix file.
398+
- `resolved_claims`: optional; the `entity_key` of every
399+
`stale_claim_markers` entry you resolved this run — fixed, or shown to be a
400+
false positive (say which, in the PR body). Omit or leave empty when the
401+
queue item carried no markers. Anything you leave out is carried forward to
402+
the next review rather than cleared, so this list is the only way a marker
403+
retires.
365404
- `clarity_flag`: optional; `true` when you flagged a readthrough `reconception`
366405
for this page. Carries onto the ledger record so the page's structural
367406
follow-up is durable even when the verdict is `clean` or `fixed` (the

scripts/content-review/record-review.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ def load_queue_article(queue_path: Path) -> dict:
130130
# verbatim from the queue entry; null on a signal-blind run.
131131
"signals": a.get("signals"),
132132
"signals_available": bool((data.get("reader_signals") or {}).get("available")),
133+
# Stale-claim markers in full (entity_key/verdict/evidence/source/...).
134+
# carry_markers() needs them to decide which survive this review.
135+
"stale_claim_markers": a.get("stale_claim_markers") or [],
133136
}
134137

135138

@@ -243,6 +246,43 @@ def canonical_branch_pushed(slug: str) -> bool:
243246
# ---- derivation -------------------------------------------------------------
244247

245248

249+
# A stale-claim marker survives this many reviews that saw it and did not
250+
# resolve it; after that it is kept but flagged `escalated`, which stops
251+
# select-articles.py boosting the page (see MARKER_ESCALATION_CAP there).
252+
MARKER_ESCALATION_CAP = 2
253+
254+
255+
def carry_markers(article: dict, verdict: dict | None) -> list[dict]:
256+
"""Stale-claim markers that outlive this review, with their retry state.
257+
258+
The nightly re-verification writes a marker when a volatile claim goes
259+
contradicted; the marker boosts the page into this worker's queue. Before
260+
this function existed the marker simply vanished when the review's ledger
261+
record landed, whether or not anything had been done about it — so a review
262+
that missed the finding silently cleared the flag and the entity went back
263+
into the pool to be re-flagged, re-boosted, and re-missed on a ~2-day cycle
264+
(observed end-to-end in pulumi/docs#20927).
265+
266+
A marker is retired only when the verdict names its entity in
267+
`resolved_claims` — the worker asserting it either fixed the claim or
268+
determined the flag was wrong. Everything else is carried forward with
269+
`unresolved_reviews` incremented, and flagged `escalated` once that count
270+
reaches MARKER_ESCALATION_CAP so a human is the next step rather than
271+
another identical pass.
272+
"""
273+
resolved = {str(k) for k in ((verdict or {}).get("resolved_claims") or [])}
274+
carried: list[dict] = []
275+
for m in article.get("stale_claim_markers") or []:
276+
if not isinstance(m, dict):
277+
continue
278+
if m.get("entity_key") in resolved:
279+
continue
280+
seen = int(m.get("unresolved_reviews") or 0) + 1
281+
carried.append({**m, "unresolved_reviews": seen,
282+
"escalated": seen >= MARKER_ESCALATION_CAP})
283+
return carried
284+
285+
246286
def build_record(article: dict, verdict: dict | None, pr: dict | None,
247287
slug: str, claude_succeeded: bool = False,
248288
branch_exists: bool = False) -> dict:
@@ -285,6 +325,9 @@ def build_record(article: dict, verdict: dict | None, pr: dict | None,
285325
"signals_available": bool(article.get("signals_available")),
286326
"reviewed_at": datetime.now(timezone.utc).date().isoformat(),
287327
}
328+
markers = carry_markers(article, verdict)
329+
if markers:
330+
rec["stale_claims"] = markers
288331

289332
if verdict is None:
290333
if claude_succeeded and not branch_exists:
@@ -456,6 +499,64 @@ def check(name, cond):
456499
check("signal-blind queue -> signals null, signals_available False",
457500
blind_article["signals"] is None
458501
and blind_article["signals_available"] is False)
502+
# --- stale-claim marker retention -------------------------------
503+
marker = {"entity_key": "version/pulumi-package", "verdict": "contradicted",
504+
"evidence": "CHANGELOG says 3.163.0", "source": "gh release view",
505+
"checked_at": "2026-08-15"}
506+
marked = {**article, "stale_claim_markers": [marker]}
507+
508+
rec_m = build_record(marked, {"verdict": "clean", "reason": "accurate"},
509+
None, marked["slug"])
510+
check("a review that resolves nothing carries the marker forward",
511+
[m["entity_key"] for m in rec_m["stale_claims"]] == ["version/pulumi-package"])
512+
check("carried marker counts the miss",
513+
rec_m["stale_claims"][0]["unresolved_reviews"] == 1
514+
and rec_m["stale_claims"][0]["escalated"] is False)
515+
check("carried marker keeps its evidence",
516+
rec_m["stale_claims"][0]["evidence"] == "CHANGELOG says 3.163.0")
517+
518+
rec_r = build_record(marked, {"verdict": "fixed", "fixes": 1,
519+
"resolved_claims": ["version/pulumi-package"]},
520+
None, marked["slug"])
521+
check("a resolved marker retires", "stale_claims" not in rec_r)
522+
523+
second = {**article, "stale_claim_markers": [rec_m["stale_claims"][0]]}
524+
rec_e = build_record(second, {"verdict": "clean", "reason": "x"},
525+
None, second["slug"])
526+
check("second unresolved review escalates the marker",
527+
rec_e["stale_claims"][0]["unresolved_reviews"] == 2
528+
and rec_e["stale_claims"][0]["escalated"] is True)
529+
530+
# The other half of the round-trip: select-articles.py hands escalated
531+
# markers through, so they must survive rather than be filtered out
532+
# here — otherwise the ledger loses them and the entity re-enters the
533+
# nightly pool.
534+
esc_marker = {**marker, "entity_key": "version/old-miss",
535+
"unresolved_reviews": 2, "escalated": True}
536+
with_esc = {**article, "stale_claim_markers": [marker, esc_marker]}
537+
rec_esc = build_record(with_esc, {"verdict": "clean", "reason": "x"},
538+
None, with_esc["slug"])
539+
carried = {m["entity_key"]: m for m in rec_esc["stale_claims"]}
540+
check("escalated marker persists through a review",
541+
"version/old-miss" in carried)
542+
check("escalated marker stays escalated and keeps counting",
543+
carried["version/old-miss"]["escalated"] is True
544+
and carried["version/old-miss"]["unresolved_reviews"] == 3)
545+
check("an escalated marker can still be resolved",
546+
"stale_claims" not in build_record(
547+
{**article, "stale_claim_markers": [esc_marker]},
548+
{"verdict": "fixed", "fixes": 1,
549+
"resolved_claims": ["version/old-miss"]},
550+
None, article["slug"]))
551+
552+
rec_none = build_record(article, {"verdict": "clean", "reason": "x"},
553+
None, article["slug"])
554+
check("no markers -> no stale_claims key", "stale_claims" not in rec_none)
555+
556+
rec_incomplete = build_record(marked, None, None, marked["slug"])
557+
check("marker survives an incomplete run too",
558+
rec_incomplete["stale_claims"][0]["unresolved_reviews"] == 1)
559+
459560
blind_rec = build_record(blind_article, None, None, blind_article["slug"])
460561
check("signal-blind record persists null/false",
461562
blind_rec["signals"] is None and blind_rec["signals_available"] is False)

scripts/content-review/reverify-claims.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ def load_ledger(ledger_dir: Path) -> dict[str, dict]:
233233
return entries
234234

235235

236-
def apply_markers(ledger: dict[str, dict], stale: list[dict], today: date) -> dict[str, dict]:
236+
def apply_markers(ledger: dict[str, dict], stale: list[dict], today: date,
237+
repo_root: Path | None = None) -> dict[str, dict]:
237238
"""Fold stale entity verdicts into the affected pages' ledger entries.
238239
239240
Returns {slug: updated entry (without bookkeeping keys)} for every entry
@@ -243,7 +244,19 @@ def apply_markers(ledger: dict[str, dict], stale: list[dict], today: date) -> di
243244
not synthesized: every changed entry is uploaded to the same S3 key
244245
record-review.py owns, so a two-field stub would silently destroy that
245246
page's status / reviewed_at / attempts / tier / score. The entity simply
246-
stays unmarked and comes back around on a later rotation."""
247+
stays unmarked and comes back around on a later rotation.
248+
249+
A page with no markdown source on disk is skipped for a different and more
250+
important reason. `select-articles.py` builds its candidate set by globbing
251+
`content/docs/**/*.md`, so a page rendered by a Hugo content adapter (the
252+
pre-built policy-pack tables, generated from `data/`) can never be selected
253+
for review — which means a marker written there can never be cleared, and
254+
`already_marked()` would then exclude that entity from re-verification
255+
permanently. Three entities were lost that way over five nights before this
256+
guard existed. Such a contradiction is real and worth acting on, but the
257+
action is upstream of this repo (fix the `data/` source or the product
258+
metadata behind it), so the verdict is reported and left in the pool rather
259+
than converted into a marker nothing can retire."""
247260
changed: dict[str, dict] = {}
248261
for s in stale:
249262
marker = {
@@ -254,6 +267,12 @@ def apply_markers(ledger: dict[str, dict], stale: list[dict], today: date) -> di
254267
"checked_at": today.isoformat(),
255268
}
256269
for page in s["pages"]:
270+
if repo_root is not None and not (repo_root / page["path"]).is_file():
271+
warn(
272+
f"{page['path']} has no source file (generated page); reporting "
273+
f"{s['entity_key']} without a marker no review could ever clear"
274+
)
275+
continue
257276
entry = ledger.get(page["path"])
258277
if entry is None:
259278
warn(
@@ -395,7 +414,7 @@ def check_entity(key: str) -> dict:
395414
report["meta"]["n_demoted"] = sum(1 for r in results if r["demoted_from"])
396415

397416
if stale:
398-
changed = apply_markers(ledger, stale, today)
417+
changed = apply_markers(ledger, stale, today, repo_root)
399418
uri = os.environ.get("CONTENT_REVIEW_LEDGER_URI", "").strip()
400419
for slug, entry in sorted(changed.items()):
401420
local = Path(args.ledger_dir) / f"{slug}.json"
@@ -413,6 +432,7 @@ def check_entity(key: str) -> dict:
413432

414433

415434
def self_test() -> int:
435+
import tempfile
416436
failures = []
417437

418438
def check(name, cond):
@@ -514,6 +534,31 @@ def snap(slug, reviewed_at, *claims):
514534
check("re-marking is idempotent",
515535
len(changed2["docs-a"]["stale_claims"]) == 1)
516536

537+
# Generated pages have no markdown source, so select-articles.py can never
538+
# queue them and a marker there could never be cleared. Report, don't mark.
539+
with tempfile.TemporaryDirectory() as td:
540+
root = Path(td)
541+
(root / "content/docs").mkdir(parents=True)
542+
(root / "content/docs/a.md").write_text("real page\n")
543+
gen = [{"entity_key": "numerical/pack-count", "verdict": "contradicted",
544+
"evidence": "", "source": "",
545+
"pages": [{"path": "content/docs/reference/generated.md",
546+
"slug": "docs-reference-generated"}]}]
547+
led = {"content/docs/reference/generated.md": {
548+
"path": "content/docs/reference/generated.md",
549+
"slug": "docs-reference-generated", "status": "clean"}}
550+
check("sourceless page gets no marker",
551+
apply_markers(led, gen, today, root) == {})
552+
check("sourceless page's ledger entry is left untouched",
553+
"stale_claims" not in led["content/docs/reference/generated.md"])
554+
# ...while a page that does exist on disk is still marked normally.
555+
led2 = {"content/docs/a.md": {"path": "content/docs/a.md",
556+
"slug": "docs-a", "status": "clean"}}
557+
on_disk = [{**gen[0], "pages": [{"path": "content/docs/a.md",
558+
"slug": "docs-a"}]}]
559+
check("page with a source file is still marked",
560+
set(apply_markers(led2, on_disk, today, root)) == {"docs-a"})
561+
517562
check("already_marked sees the marker",
518563
already_marked("version/pulumi-gcp", ents["version/pulumi-gcp"], ledger))
519564
check("already_marked ignores other entities",
@@ -522,7 +567,6 @@ def snap(slug, reviewed_at, *claims):
522567

523568
# Health-observation meta: the early-exit paths must say why they stopped
524569
# (signal-health.py's reverify signal reads these fields).
525-
import tempfile
526570

527571
def run_report(d: Path, extra_env_unset: list[str]) -> dict:
528572
saved = {k: os.environ.pop(k) for k in extra_env_unset if k in os.environ}

0 commit comments

Comments
 (0)