diff --git a/scripts/review-queue.py b/scripts/review-queue.py index a4708577..a3d3f5b5 100644 --- a/scripts/review-queue.py +++ b/scripts/review-queue.py @@ -519,12 +519,38 @@ class Rant: agent: str = "" -#: The handle R5 writes into a rant's `progress`: the word `issue` and the number. Anchored -#: on that word rather than on a bare `#N`, because the same line normally names the PR too -#: (`issue #2110 (closed) → PR #2111 merged`) and a PR number is not what this row reports. -#: The tail continues the list only in the forms a writer actually uses for it — `#1/#2`, -#: `#1, #2`, `#1 and #2` — so a sentence that mentions an unrelated number later stays out. -_PROGRESS_ISSUE = re.compile(r"(?i)\bissues?\s*#(\d+)((?:\s*(?:,|/|&|\band\b)\s*#\d+)*)") +#: The handle R5 writes into a rant's `progress`: the word `issue` and the number **as the +#: statement the line begins with**. Anchored on that word rather than on a bare `#N`, +#: because the same line normally names the PR too (`issue #2110 (closed) → PR #2111 +#: merged`) and a PR number is not what this row reports. The tail continues the list only +#: in the forms a writer actually uses for it — `#1/#2`, `#1, #2`, `#1 and #2`. +#: +#: Line-anchored, and not merely word-anchored, because `progress` is prose as well as a +#: record. A cycle writes a sentence there whenever it decides *not* to take a rant up yet +#: — "the gate is closed because issue #2633 is open" — and a substring rule reads that +#: mention as this entry's handle. Measured on issue #2658: `_progress_issues` answered +#: `[2633]` for that sentence and `[2637]` for `issue #2637`. A false pair is not cosmetic: +#: this row is the reading R5's fail-closed rule rests on, so it tells the next cycle the +#: rant was already filed and the rant stops moving without anyone deciding to stop it — +#: and because the issue-side fallback below runs only for an entry that names **no** +#: number, the wrong pairing also suppresses the reading that would have answered. +#: +#: The cost is entries that spell the handle inside a sentence rather than at the start of +#: a line (`Merged. Issue #2565 closed by PR #2566 …`, `Done: issue #2352 -> PR #2362`). +#: They lose their pairing here, and the fallback below does not recover them: it reads an +#: `Origin:` line, which only the older convention wrote. Whether that costs a rendered row +#: depends on the number, not on the spelling. A number belonging to another project — `the +#: research direction of journal issue #124` — was never this entry's handle, so refusing it +#: is this predicate working, not a price it pays. A number of this repository's own, +#: spelled mid-sentence, is a pairing lost; a `completed` entry renders no row, so the loss +#: appears only if such an entry returns to an open state, where nothing else answers it. +#: Measured on the 13-entry ledger of the host that wrote this rule: 10 pairings change, +#: every one of them `completed`, and no rendered row moves. A ledger carrying another +#: agent's open rants is where an open row moves, and there the number decides which of the +#: two groups that row falls in. +_PROGRESS_ISSUE = re.compile( + r"(?im)^[ \t]*[*_]{0,2}issues?[ \t]*#(\d+)((?:\s*(?:,|/|&|\band\b)\s*#\d+)*)" +) _PROGRESS_MORE = re.compile(r"#(\d+)") @@ -541,6 +567,14 @@ def _progress_issues(progress: object) -> list[int]: written as its continuation. A `progress` that names no issue is not an error — it is a rant that has not been taken on (or one filed under the older convention, which the caller resolves from the issue side). + + **It has to begin its line.** The handle and the prose share one field, so the shape + that tells them apart is where the token sits: a pairing R5 wrote is a statement + (`issue #2637 → PR #2640 …`), while a number inside a sentence is a mention + (`… because issue #2633 is open …`), and only the first one says this rant became an + issue. `^` is applied with `re.M`, so a `progress` that appends a correction after a + newline still carries the handle written earlier, and a mention anywhere on a line + that already holds other words pairs with nothing. """ text = progress if isinstance(progress, str) else "" first = _PROGRESS_ISSUE.search(text) diff --git a/tests/test_review_queue.py b/tests/test_review_queue.py index 2c4d9f0e..26751a3c 100644 --- a/tests/test_review_queue.py +++ b/tests/test_review_queue.py @@ -2566,6 +2566,118 @@ def test_an_entry_that_names_its_own_issue_is_not_re_read_from_the_issue_side(mo assert links.states_asked == [], "and an open issue is answered by the listing already" +def test_a_sentence_that_mentions_another_issue_is_not_read_as_this_rants_handle(monkeypatch): + """The handle and the prose share one field, so the shape has to be where it sits. + + A cycle writes a sentence into `progress` whenever it decides not to take a rant up + yet — the gate is closed, another issue is open, so this rant has no issue. A + substring search read that mention as this entry's pairing: `_progress_issues` answered + the mentioned number, the row rendered as filed, and the next cycle neither opened the + issue nor took the rant, so it stopped moving without anyone deciding to stop it + (issue #2658). + + A pairing is a statement, and a statement begins its line. + """ + tool = _fresh_tool() + + assert tool._progress_issues( + "R5 is closed this cycle because issue #2633 is open, and a rant becomes work " + "only by opening its issue first." + ) == [], "a mention inside a sentence is not a handle" + assert tool._progress_issues("issue #2637") == [2637], "the statement R5 defines pairs" + assert tool._progress_issues("") == [], "no number is not an error" + + +def test_a_handle_spelled_inside_a_sentence_is_the_price_the_anchor_pays(monkeypatch): + """The cost of the statement rule, pinned as intended rather than as an oversight. + + `progress` records finished work in prose as often as it records a handle, and a writer + spells the handle there as part of that sentence. The line anchor does not read it, so + the pairing is lost — and the issue-side fallback cannot recover it, because it looks + for an `Origin:` line that only the older convention wrote. + + Pinned so that a change which recovers these has to say here which mention it refuses + instead: widening the anchor is a trade between the two groups, never a free gain. The + second assertion is the remedy that needs no change to the rule — the same entry pairs + as soon as its handle is what the line begins with. + """ + tool = _fresh_tool() + + assert tool._progress_issues( + "Merged. Issue #2565 closed by PR #2566 (squash ffdf65f1)" + ) == [], "a handle spelled mid-sentence is the pairing the anchor gives up" + assert tool._progress_issues( + "issue #2565 — merged by PR #2566 (squash ffdf65f1)" + ) == [2565], "the same pairing survives once the handle begins the line" + assert tool._progress_issues( + "registered as SILICON SCIENCE issue #142" + ) == [], "another project's number is not this entry's handle" + + +def test_the_handle_is_read_where_a_statement_may_begin(monkeypatch): + """Every spelling that *is* the handle pairs; every spelling that mentions one does not. + + The accepting side is what keeps the anchor from becoming a false absence: entries are + written by hand into a free-text field, so the token arrives with markdown emphasis, on + its own line after a newline, or with the numbers continued in the three forms a writer + uses. The rejecting side is the defect itself, and each case is a sentence that names a + number while saying something else about it. + """ + tool = _fresh_tool() + + paired = { + "issue #2637": [2637], + " issue #2637 → PR #2640 merged": [2637], + "**issue #2637** → PR #2640 merged": [2637], + "issue #1985/#1986": [1985, 1986], + "issue #1985, #1986": [1985, 1986], + "issue #1985 and #1986": [1985, 1986], + "done in two parts:\nissue #1985 → PR #1986 merged": [1985], + } + for progress, expected in paired.items(): + assert tool._progress_issues(progress) == expected, progress + + unpaired = { + "the gate is closed because issue #2633 is open": [], + "Merged. Issue #2565 closed by PR #2566": [], + "issue #1985 → PR #1986 merged": [1985], # control: the same text, stated + "Half 1 (#1086 age alarm): PR #1088 MERGED, issue #1086 CLOSED": [], + "no number here at all": [], + } + for progress, expected in unpaired.items(): + assert tool._progress_issues(progress) == expected, progress + + +def test_a_narrated_number_leaves_the_rant_looking_unfiled(monkeypatch): + """Which is the honest row: the rant has no issue, so the next cycle should file one. + + The end-to-end shape of the defect, and the direction the fix has to move — the row + must offer the work rather than report it done. The issue-side reading is spent, and + finds nothing, because the issue the sentence mentioned is not this rant's origin. + """ + tool = _fresh_tool() + stamp = "2026-10-29T09:23:40+08:00" + links = _links( + monkeypatch, + tool, + [_row(stamp, "pending", + progress="R5 is closed this cycle because issue #2633 is open, and a rant " + "becomes work only by opening its issue first.")], + issues=[(2633, "another rant's issue, with no origin line of this one")], + ) + + (rant,) = tool.open_rant_rows(None, tool.REPO) + + assert (rant.issues, rant.closed_issues, rant.unread_issues) == ([], [], []) + assert "no issue yet" in tool.render_rant(rant), ( + "a rant with no issue must render as work to do, not as work already filed" + ) + assert links.origins_asked != [], ( + "the fallback runs precisely because the entry stated no handle — the wrong pair " + "used to suppress it" + ) + + def test_an_entry_with_no_progress_still_reads_the_issue_side(monkeypatch): """The fallback is what keeps older entries from rendering as never filed.