From 97a9e8261bfc22a9ccc917ae7606365f531adc3b Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 10:53:52 -0500 Subject: [PATCH] fix(asvs): survey progress counted a read-and-parked cell as never read `ASVS-CURRENT.md` reported "123 of 345 requirements have been read ... 222 have not" directly above its own table saying **221** unverified. Two numbers for one quantity, on the page that IS the record under ADR 0156. The renderer computed survey progress from `DECIDED_VERDICTS`, which deliberately excludes `needs-review` -- correctly, for verdict COUNTS, since that cell was examined and then left open on purpose. But the survey line does not report what was decided, it reports what was **read**, and a `needs-review` cell has been read. The constant's own docstring says so: *"the second was examined and left open on purpose."* The code used the right set for the wrong question. Adds `EXAMINED_VERDICTS = DECIDED_VERDICTS | {"needs-review"}` and uses it for progress only. Verdict counts, and the `inherited` warning about a decided verdict lacking a `last_verified` date, keep `DECIDED_VERDICTS` -- a parked cell is not an inherited verdict. **Why this survived every gate since the renderer was written.** With zero `needs-review` cells the two sets are identical, so no test, no CI run and no reading of the output could tell them apart. It became observable only when the scorecard acquired its first one (11.4.4, in the V7/V11 baseline sweep) -- a latent defect that needed new *data*, not new code, to show itself. That is the shape this repo keeps finding: a control that passes because the case it gets wrong has never occurred. The regression test asserts the two printed figures agree, because a reader comparing them is the only thing that ever noticed. Proved red against `HEAD` before being trusted green -- the renderer was reverted with `git show HEAD:` and the file restored afterwards, since `git stash` is shared with the primary checkout and the worktree gate refuses it. ruff check + format, mypy strict, and all 35 scorecard tests pass. --- scripts/asvs/scorecard.py | 17 +++++++++++++++-- tests/test_asvs_scorecard.py | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/scripts/asvs/scorecard.py b/scripts/asvs/scorecard.py index 663ad95d..55b3276a 100644 --- a/scripts/asvs/scorecard.py +++ b/scripts/asvs/scorecard.py @@ -49,6 +49,17 @@ #: the first was never examined, the second was examined and left open on purpose. DECIDED_VERDICTS: Final[frozenset[str]] = frozenset({"pass", "partial", "fail", "na"}) +#: A verdict whose cell has been READ against the requirement text. Not the same set as +#: :data:`DECIDED_VERDICTS`, and the difference is `needs-review`: that cell was examined and then +#: left open on purpose, so it belongs in survey PROGRESS while staying out of the verdict counts. +#: +#: Survey progress used `DECIDED_VERDICTS` until the scorecard acquired its first `needs-review` cell +#: (11.4.4, the V7/V11 baseline sweep). The page then reported "123 of 345 read … 222 have not" +#: directly above its own table saying 221 unverified — two numbers for one quantity, on the page +#: that IS the record. The bug had been latent since the renderer was written: with zero +#: `needs-review` cells the two sets are identical, so no test and no CI run could tell them apart. +EXAMINED_VERDICTS: Final[frozenset[str]] = DECIDED_VERDICTS | {"needs-review"} + #: How far from the recorded line the expected token may drift before the anchor is considered broken. #: Anchors name a TOKEN rather than a bare line number precisely so that ordinary edits above a cell's #: evidence do not thrash every anchor in the file; the window keeps the line number meaningful without @@ -458,8 +469,10 @@ def render_current(cells: list[Cell], *, anchor_sha: str) -> str: """ n = count(cells) total = sum(n.values()) - decided = [c for c in cells if c.verdict in DECIDED_VERDICTS] - examined = [c for c in decided if c.last_verified] + # EXAMINED, not DECIDED: a `needs-review` cell was read and then parked, so it is survey progress + # even though it carries no verdict. Counting it as unread made this line contradict the table + # below it (see EXAMINED_VERDICTS). + examined = [c for c in cells if c.verdict in EXAMINED_VERDICTS and c.last_verified] unexamined = total - len(examined) pct = (100.0 * len(examined) / total) if total else 0.0 inherited = sum(1 for c in cells if c.verdict in DECIDED_VERDICTS and not c.last_verified) diff --git a/tests/test_asvs_scorecard.py b/tests/test_asvs_scorecard.py index ebd6502a..ffdc13c6 100644 --- a/tests/test_asvs_scorecard.py +++ b/tests/test_asvs_scorecard.py @@ -366,6 +366,33 @@ def test_render_leads_with_survey_progress_not_a_headline_score() -> None: assert "deadbeef" in out +def test_render_counts_a_needs_review_cell_as_read_not_as_unexamined() -> None: + """Survey progress counts what was READ; `needs-review` was read and then parked on purpose. + + Latent from the day the renderer was written and invisible until the scorecard acquired its first + `needs-review` cell: with none present, "decided" and "examined" are the same set, so nothing + could distinguish them. The symptom was two numbers for one quantity on the page that IS the + record — a survey line saying N have *not* been read, directly above a table whose `unverified` + row said N-1. + + The assertion that matters is the LAST one: the two figures the page prints must agree, because a + reader comparing them is the only thing that ever noticed. + """ + cells = [ + Cell(id="1.1.1", level=1, verdict="pass", last_verified="2026-08-02"), + Cell(id="1.1.2", level=2, verdict="unverified"), + Cell(id="2.1.1", level=3, verdict="needs-review", last_verified="2026-08-02"), + ] + out = render_current(cells, anchor_sha="x") + + assert ( + "**2 of 3 requirements have been read against the ASVS text (66.7%).** 1 have not." in out + ) + # ...and it stays OUT of the verdict counts, which is why the two sets differ at all. + assert "| Needs review | 1 |" in out + assert "| **Unverified** | **1** |" in out + + def test_render_flags_a_decided_verdict_carrying_no_verified_date_as_inherited() -> None: cells = [Cell(id="1.1.1", level=1, verdict="pass")] # decided, but never dated out = render_current(cells, anchor_sha="x")