HOMELAB-2682: fix(forgejo): 'BLOCKER none.' reads as no findings; 'none of the tests ran' still blocks #107

Merged
pi-landing merged 2 commits from plane/HOMELAB-2682-blocker-none into main 2026-11-06 10:05:11 +00:00
Owner

What

A severity-tokened line whose REMAINDER is a bare empty statement is a claim of zero findings, not a finding. Fix + tests only, on forgejo/landing.ts and one new test file.

Why — the GITS #209 refusal, verbatim

gits-hypervisor's gate refused Homelab/ghostty-in-the-shell#209 (head 092174e):

Refusing to merge Homelab/ghostty-in-the-shell#209 (head 092174e): 5 open blocker finding(s).

One of those five was a review seat's shorthand for "no findings". Comment 22638 (ld-reviewer, ## Verdict: APPROVE at that same head) ends its evidence with:

No blockers.
BLOCKER none.

No blockers. negates — BLOCKER none. did not, because the line starts with the severity token. BLOCKER_FINDING_PATTERNS matched it, and every existing negation pattern requires the token to be the NOUN of a denial (no blockers, blockers: none) rather than the sentence's SUBJECT. The result was one open, id-less finding, which no resolution statement can ever close — so the PR was unlandable for a second typographic reason on top of #103's bolded form. Same family as the - None. artifact fixed for section content lines, on a different surface: the empty statement shares the line with the token instead of sitting alone under a heading.

Measured on the real corpus, this branch vs main, over 79 real ld-reviewer bodies (w3-lane, negation-approval, fleet-verdict, pr70-review fixtures + the three GITS #209 comments):

gits209#22638: 1 -> 0
   dropped: "BLOCKER none."
79 real comment bodies compared; 1 changed

Exactly one line changes; nothing is added anywhere, so no finding anywhere in the corpus became invisible. Re-running the lock eval on the GITS #209 comment set: main 4 open findings (RISK-2, RISK-3, RISK-4, and id-less BLOCKER none.) → this branch 3 (the three real RISK findings unchanged).

The rule: exact token vs content

isSeverityTokenEmptyStatement(line) takes the text after a line-initial severity token and its separator run (bullets, heading marker, emphasis, an optional severity:, : / =, whitespace) and asks a single question: is that remainder the empty-statement vocabulary and nothing else?

  • It reuses the existing vocabulary, not a new one: NEG_ZERO_WORD (none|nothing|nil|n/a|zero|0) is the same determiner list NEGATION_CLAIM_RES already spells out, and the two empty-claim patterns now share that constant.
  • It is restricted to the zero-repetition form of those patterns. NEGATION_CLAIM_RES also accepts a word after the determiner (None found., Nothing outstanding., 0 remaining.) — that is right for a line under a heading, and wrong behind a severity token, because a word after the determiner is content, and content is a defect.
  • Emphasis-insensitive by construction (plainText), so **BLOCKER** none., __BLOCKER__ none., **BLOCKER:** none. read exactly like the plain form (#103).
  • An em/en dash is deliberately not part of the separator run: Blockers — none. is a finding on the token path and #103 pinned it with its bolded twin. A dash is where a real finding's body goes (**Blocker** — branch matching picks the wrong run).

The guard case — verbatim, as required

Content after the determiner keeps failing closed:

BLOCKER none of the tests ran.

is still one open finding (tests/landing-severity-none.test.ts, first guard test, asserted on the finding's own line: expect(scan(line)[0].line).toBe(line)). Same for BLOCKER none found here, BLOCKER: none worth mentioning — the auth check is open, BLOCKER: none of the retries back off, BLOCKER 0 tests were run, BLOCKER n/a — the harness never ran, and BLOCKER B1: none. (a label with an empty body is still a labelled finding).

Failing test first

tests/landing-severity-none.test.ts was written against origin/main's landing.ts before the fix. On main (26b5f15):

$ git checkout origin/main -- forgejo/landing.ts && npx vitest run tests/landing-severity-none.test.ts
Tests  16 failed | 27 passed (43)

Failed on main (each of these read as 1 open finding):

BLOCKER none.            **BLOCKER** none.        BLOCKER none
## Blockers + BLOCKER none.                       ### BLOCKER none.
BLOCKER NONE.            BLOCKER nothing.         BLOCKER n/a.
BLOCKER 0                BLOCKER none!            BLOCKER none..
BLOCKER none.**          (BLOCKER) none.
evaluateLandingLock > does not refuse a clean approval whose seat wrote `BLOCKER none.`
near misses > negates `BLOCKER NONE.`

With the fix: 43 passed. The near-miss family is pinned on both sides — BLOCKER NONE. / Blocker None. / BLOCKER: none negate; BLOCKER none worth mentioning and the whole prose-tail list above stay findings; BLOCKER — none. and **Blockers** — none. stay findings (the #103 dash decision, unchanged).

Suites

  • npx tsc --noEmit — clean.
  • npx vitest run — 34 files, 895 tests passed (33 pre-existing files including the #96–#106 suites, plus the new one).
  • npm run test:deployed (deployed shape per TESTING.md) — ok: 34/34 files loaded and passed, 895/895 tests on the pinned tree.

Scope

Two files: forgejo/landing.ts (+95/−2) and forgejo/tests/landing-severity-none.test.ts (new). No behaviour change outside the token-remainder rule; the only vocabulary edit to existing code is factoring the shared determiner constant into two existing patterns with an identical character set.

Refs: HOMELAB-2682, GITS-508, #209 (unblocks), #204/#103 (the bolded-form sibling).

## What A severity-tokened line whose REMAINDER is a bare empty statement is a claim of zero findings, not a finding. Fix + tests only, on `forgejo/landing.ts` and one new test file. ## Why — the GITS #209 refusal, verbatim gits-hypervisor's gate refused `Homelab/ghostty-in-the-shell#209` (head `092174e`): ``` Refusing to merge Homelab/ghostty-in-the-shell#209 (head 092174e): 5 open blocker finding(s). ``` One of those five was a review seat's shorthand for "no findings". Comment 22638 (ld-reviewer, `## Verdict: APPROVE` at that same head) ends its evidence with: ``` No blockers. BLOCKER none. ``` `No blockers.` negates — `BLOCKER none.` did not, because the line **starts** with the severity token. `BLOCKER_FINDING_PATTERNS` matched it, and every existing negation pattern requires the token to be the NOUN of a denial (`no blockers`, `blockers: none`) rather than the sentence's SUBJECT. The result was one open, id-less finding, which no resolution statement can ever close — so the PR was unlandable for a second typographic reason on top of #103's bolded form. Same family as the `- None.` artifact fixed for section content lines, on a different surface: the empty statement shares the line with the token instead of sitting alone under a heading. Measured on the real corpus, this branch vs `main`, over 79 real ld-reviewer bodies (w3-lane, negation-approval, fleet-verdict, pr70-review fixtures + the three GITS #209 comments): ``` gits209#22638: 1 -> 0 dropped: "BLOCKER none." 79 real comment bodies compared; 1 changed ``` Exactly one line changes; nothing is added anywhere, so no finding anywhere in the corpus became invisible. Re-running the lock eval on the GITS #209 comment set: `main` 4 open findings (RISK-2, RISK-3, RISK-4, and id-less `BLOCKER none.`) → this branch 3 (the three real RISK findings unchanged). ## The rule: exact token vs content `isSeverityTokenEmptyStatement(line)` takes the text after a line-initial severity token and its separator run (bullets, heading marker, emphasis, an optional `severity:`, `:` / `=`, whitespace) and asks a single question: is that remainder the **empty-statement vocabulary and nothing else**? - It reuses the existing vocabulary, not a new one: `NEG_ZERO_WORD` (`none|nothing|nil|n/a|zero|0`) is the same determiner list `NEGATION_CLAIM_RES` already spells out, and the two empty-claim patterns now share that constant. - It is restricted to the **zero-repetition** form of those patterns. `NEGATION_CLAIM_RES` also accepts a word after the determiner (`None found.`, `Nothing outstanding.`, `0 remaining.`) — that is right for a line under a heading, and **wrong** behind a severity token, because a word after the determiner is content, and content is a defect. - Emphasis-insensitive by construction (`plainText`), so `**BLOCKER** none.`, `__BLOCKER__ none.`, `**BLOCKER:** none.` read exactly like the plain form (#103). - An em/en dash is deliberately **not** part of the separator run: `Blockers — none.` is a finding on the token path and #103 pinned it with its bolded twin. A dash is where a real finding's body goes (`**Blocker** — branch matching picks the wrong run`). ## The guard case — verbatim, as required Content after the determiner keeps failing closed: ``` BLOCKER none of the tests ran. ``` is **still one open finding** (`tests/landing-severity-none.test.ts`, first guard test, asserted on the finding's own line: `expect(scan(line)[0].line).toBe(line)`). Same for `BLOCKER none found here`, `BLOCKER: none worth mentioning — the auth check is open`, `BLOCKER: none of the retries back off`, `BLOCKER 0 tests were run`, `BLOCKER n/a — the harness never ran`, and `BLOCKER B1: none.` (a label with an empty body is still a labelled finding). ## Failing test first `tests/landing-severity-none.test.ts` was written against `origin/main`'s `landing.ts` before the fix. On `main` (`26b5f15`): ``` $ git checkout origin/main -- forgejo/landing.ts && npx vitest run tests/landing-severity-none.test.ts Tests 16 failed | 27 passed (43) ``` Failed on `main` (each of these read as 1 open finding): ``` BLOCKER none. **BLOCKER** none. BLOCKER none ## Blockers + BLOCKER none. ### BLOCKER none. BLOCKER NONE. BLOCKER nothing. BLOCKER n/a. BLOCKER 0 BLOCKER none! BLOCKER none.. BLOCKER none.** (BLOCKER) none. evaluateLandingLock > does not refuse a clean approval whose seat wrote `BLOCKER none.` near misses > negates `BLOCKER NONE.` ``` With the fix: `43 passed`. The near-miss family is pinned on both sides — `BLOCKER NONE.` / `Blocker None.` / `BLOCKER: none` negate; `BLOCKER none worth mentioning` and the whole prose-tail list above stay findings; `BLOCKER — none.` and `**Blockers** — none.` stay findings (the #103 dash decision, unchanged). ## Suites - `npx tsc --noEmit` — clean. - `npx vitest run` — **34 files, 895 tests passed** (33 pre-existing files including the #96–#106 suites, plus the new one). - `npm run test:deployed` (deployed shape per `TESTING.md`) — `ok: 34/34 files loaded and passed, 895/895 tests` on the pinned tree. ## Scope Two files: `forgejo/landing.ts` (+95/−2) and `forgejo/tests/landing-severity-none.test.ts` (new). No behaviour change outside the token-remainder rule; the only vocabulary edit to existing code is factoring the shared determiner constant into two existing patterns with an identical character set. Refs: HOMELAB-2682, GITS-508, #209 (unblocks), #204/#103 (the bolded-form sibling).
HOMELAB-2682: fix(forgejo): severity-tokened empty statements ('BLOCKER none.') are zero-findings claims
Some checks failed
CI / pr-title (pull_request) Successful in 1s
CI / ask-user-question (pull_request) Successful in 30s
CI / forgejo-ci (pull_request) Successful in 49s
CI / pi-otel-telemetry (pull_request) Successful in 52s
CI / mcp-bridge (pull_request) Successful in 22s
CI / todo (pull_request) Successful in 56s
tests / python (pull_request) Successful in 6s
CI / refocus (pull_request) Successful in 48s
CI / nats-intercom (pull_request) Successful in 30s
CI / redact (pull_request) Successful in 29s
CI / plane (pull_request) Successful in 1m26s
CI / pi-friction (pull_request) Successful in 39s
CI / forgejo (pull_request) Successful in 1m32s
CI / skill-args (pull_request) Has been cancelled
CI / pi-swarm-suite (pull_request) Has been cancelled
e92913ba19
HOMELAB-2682: docs(forgejo): the dash form does not negate — correct the examples
All checks were successful
CI / pr-title (pull_request) Successful in 1s
CI / pi-otel-telemetry (pull_request) Successful in 42s
CI / forgejo-ci (pull_request) Successful in 55s
CI / skill-args (pull_request) Successful in 54s
tests / python (pull_request) Successful in 5s
CI / mcp-bridge (pull_request) Successful in 23s
CI / plane (pull_request) Successful in 1m14s
CI / nats-intercom (pull_request) Successful in 28s
CI / refocus (pull_request) Successful in 43s
CI / ask-user-question (pull_request) Successful in 45s
CI / todo (pull_request) Successful in 50s
CI / pi-friction (pull_request) Successful in 40s
CI / redact (pull_request) Successful in 43s
CI / forgejo (pull_request) Successful in 1m51s
CI / pi-swarm-suite (pull_request) Successful in 3m16s
5d451462bf
Collaborator

Verdict of record — ld-reviewer — head 5d451462bf: APPROVE

model: deepseek-flash
session: 47db1b27-e3cd-45ec-a49a-8e0e6a8f840c

Independent review seat 2 of 2 (adversarial, gate tooling). Method: read the PR diff at the declared head in a detached worktree, then ran the real findOpenBlockers / evaluateLandingLock / isBlockerDocumentation / isNegationClaimText on a 157-line adversarial corpus at both this head and origin/main (26b5f15) and diffed the two readings line by line. No finding below is inferred from the PR description; every row is measured.

Boundary family — every case correct, zero divergences

line must head
BLOCKER none. negate negate
BLOCKER none negate negate
BLOCKER NONE. negate negate
BLOCKER None negate negate
- BLOCKER: none. negate negate
**BLOCKER** none. negate negate
BLOCKER none . negate negate
BLOCKER none, verified on head X. block block
BLOCKER none remaining. block block
BLOCKER nothing. negate negate
BLOCKER zero. negate negate
BLOCKER n/a. negate negate
BLOCKER no. block block
2 BLOCKER none. negate negate
BLOCKER none none. block block

Widening check (the direction that loses findings). 157 lines compared against main; 36 changed, and every single changed line was a zero-claim that moved to no-finding (e.g. BLOCKER N/A., BLOCKER Nil., ### BLOCKER none., (BLOCKER) none., [BLOCKER] none., BLOCKER \none`., BLOCKER... none., BLOCKER none.., blocker none., ## Hold\n\nBLOCKER none.). **Not one must-block line negated.** Specifically still blocking, unchanged: BLOCKER none of the tests ran., BLOCKER none found here, BLOCKER: none worth mentioning — the auth check is open, BLOCKER 0 tests were run, BLOCKER n/a — the harness never ran, BLOCKER none of the 5 tests ran, BLOCKER none?: see below, BLOCKER none. (tests: 0 ran), BLOCKER none of the above, BLOCKER none-the-less, BLOCKER none_remaining, BLOCKER `none` of the tests ran, severity: BLOCKER: none.(0 on main too). The dash family (#103) is preserved:BLOCKER — none., BLOCKER – none., BLOCKER - none., BLOCKER—none., Blockers — none., Blockers — none.all still report one finding, andBLOCKER — none.reads identically toBLOCKER — none.`

id-ed family and findings-closure: unaffected. BLOCKER B1: tenant scoping is missing, BLOCKER B1: none., BLOCKER B1: none, BLOCKER-1: none., BLOCKER B1: none of the tests ran. all still yield one finding with id = B1 / 1. BLOCKER (B1): none. and BLOCKER: (#123) none. also still block (the gap run stops at the label), which is the correct fail-closed reading of a labelled finding with an empty body. Closure behaves identically: on main the seat's resolved B1 closes B1 but leaves the id-less BLOCKER none. open forever (the GITS #209 bug); at this head the same comment set yields zero open findings and B1 closes exactly as before.

Doc / prose surfaces: no change. Ten quoted-example shapes read identically at head and main, including `BLOCKER none.` alone, - BLOCKER none., `"BLOCKER none."`, `> BLOCKER none. — the canonical clean line`, BLOCKER: none of the tests ran. ``, and the mixed grammar sentence. The new rule is not silent about them — the existing quoted-example/negation guards already covered every one, and the new check is ordered before the documentation guards only because both directions are return false, so the ordering cannot change an outcome. The quoted guard's own reading of a quoted finding (BLOCKER none of the tests ran.) is unchanged from main.

Suites and claims. npx tsc --noEmit clean (verified myself, not taken from the body). npx vitest run at this head: 34 pre-existing files + the new landing-severity-none.test.ts → 895 tests passed with my probes removed. The head commit 5d45146 is comment-only and corrects an example that contradicted the code (the earlier block claimed ## BLOCKER — none. negates; the dash is excluded, so it does not) — claim honesty is good: the description's dash rule and the code now agree.

Concerns

  • Qualified zero-claims still manufacture an id-less, unclosable finding. BLOCKER: none, see RISK-2., BLOCKER: none — resolved by <sha>., BLOCKER none (verified on head X). and BLOCKER none as of <sha>. all still report one id-less finding at this head, i.e. the same failure mode GITS #209 hit, just with a tail. This is intended by the ticket (content after the determiner is content, and BLOCKER none, verified on head X. is a mandated must-block row), so it is not a blocker — but the fleet's two most natural "none, but qualified" phrasings remain traps, and a seat can still wedge a PR with one. A follow-up ticket for a resolved <ID>-free escape hatch (or a refusal message that names the offending line as unclosable) would retire the last of this family.
  • The gap run is broader than its own docstring says. SEVERITY_REMAINDER_GAP_RE includes ., ,, ;, =, >, ~, {, }, |, so BLOCKER... none., BLOCKER,,, none., BLOCKER;;; none., BLOCKER ||| none., BLOCKER: > none., BLOCKER => none. all negate. No real finding was observed to read that way in the corpus, so the widening is inert in practice; it is still wider than "a colon or equals plus markup and whitespace" and a future narrowing should be pinned by a test rather than discovered.
  • severity: as an accepted lead makes one new class negate. severity: blocker none. and Severity: BLOCKER none. moved 1 → 0 at this head (they were findings on main). Plausibly intended, and unreadable as a real defect, but it is outside the ticket's enumerated shapes and is not covered by any test in the new file. A one-line test pinning the severity: lead would make the intent explicit.
  • Housekeeping, not this PR: the kv/ld/forgejo-reviewer secret's user metadata field reads pi-landing while the token itself authenticates as ld-reviewer (id 47, verified with GET /user before posting). Nothing here depends on that field, but the stale metadata would mislead anyone reading the secret entry.
**Verdict of record — ld-reviewer — head 5d451462bf0d24b5564f79d64e4ce5f399ef80ba: APPROVE** model: deepseek-flash session: 47db1b27-e3cd-45ec-a49a-8e0e6a8f840c Independent review seat 2 of 2 (adversarial, gate tooling). Method: read the PR diff at the declared head in a detached worktree, then ran the *real* `findOpenBlockers` / `evaluateLandingLock` / `isBlockerDocumentation` / `isNegationClaimText` on a 157-line adversarial corpus at both this head and `origin/main` (`26b5f15`) and diffed the two readings line by line. No finding below is inferred from the PR description; every row is measured. **Boundary family — every case correct, zero divergences** | line | must | head | |---|---|---| | `BLOCKER none.` | negate | negate | | `BLOCKER none` | negate | negate | | `BLOCKER NONE.` | negate | negate | | `BLOCKER None` | negate | negate | | `- BLOCKER: none.` | negate | negate | | `**BLOCKER** none.` | negate | negate | | `BLOCKER none .` | negate | negate | | `BLOCKER none, verified on head X.` | block | block | | `BLOCKER none remaining.` | block | block | | `BLOCKER nothing.` | negate | negate | | `BLOCKER zero.` | negate | negate | | `BLOCKER n/a.` | negate | negate | | `BLOCKER no.` | block | block | | `2 BLOCKER none.` | negate | negate | | `BLOCKER none none.` | block | block | **Widening check (the direction that loses findings).** 157 lines compared against `main`; 36 changed, and every single changed line was a zero-claim that moved to no-finding (e.g. `BLOCKER N/A.`, `BLOCKER Nil.`, `### BLOCKER none.`, `(BLOCKER) none.`, `[BLOCKER] none.`, `BLOCKER \`none\`.`, `BLOCKER... none.`, `BLOCKER none..`, `blocker none.`, `## Hold\n\nBLOCKER none.`). **Not one must-block line negated.** Specifically still blocking, unchanged: `BLOCKER none of the tests ran.`, `BLOCKER none found here`, `BLOCKER: none worth mentioning — the auth check is open`, `BLOCKER 0 tests were run`, `BLOCKER n/a — the harness never ran`, `BLOCKER none of the 5 tests ran`, `BLOCKER none?: see below`, `BLOCKER none. (tests: 0 ran)`, `BLOCKER none of the above`, `BLOCKER none-the-less`, `BLOCKER none_remaining`, `BLOCKER \`none\` of the tests ran`, `severity: BLOCKER: none.` (0 on main too). The dash family (#103) is preserved: `BLOCKER — none.`, `BLOCKER – none.`, `BLOCKER - none.`, `BLOCKER—none.`, `**Blockers** — none.`, `Blockers — none.` all still report one finding, and `BLOCKER — none.` reads identically to `**BLOCKER** — none.` **id-ed family and findings-closure: unaffected.** `BLOCKER B1: tenant scoping is missing`, `BLOCKER B1: none.`, `BLOCKER B1: none`, `BLOCKER-1: none.`, `BLOCKER B1: none of the tests ran.` all still yield one finding with `id` = `B1` / `1`. `BLOCKER (B1): none.` and `BLOCKER: (#123) none.` also still block (the gap run stops at the label), which is the correct fail-closed reading of a labelled finding with an empty body. Closure behaves identically: on `main` the seat's `resolved B1` closes B1 but leaves the id-less `BLOCKER none.` open forever (the GITS #209 bug); at this head the same comment set yields zero open findings and B1 closes exactly as before. **Doc / prose surfaces: no change.** Ten quoted-example shapes read identically at head and `main`, including `` `BLOCKER none.` `` alone, `- `BLOCKER none.``, `"BLOCKER none."`, `> BLOCKER none. — the canonical clean line`, `` `BLOCKER: none of the tests ran.` ``, and the mixed grammar sentence. The new rule is not silent about them — the existing quoted-example/negation guards already covered every one, and the new check is ordered before the documentation guards only because both directions are `return false`, so the ordering cannot change an outcome. The quoted guard's own reading of a quoted *finding* (`BLOCKER none of the tests ran.`) is unchanged from `main`. **Suites and claims.** `npx tsc --noEmit` clean (verified myself, not taken from the body). `npx vitest run` at this head: 34 pre-existing files + the new `landing-severity-none.test.ts` → 895 tests passed with my probes removed. The head commit `5d45146` is comment-only and *corrects* an example that contradicted the code (the earlier block claimed `## BLOCKER — none.` negates; the dash is excluded, so it does not) — claim honesty is good: the description's dash rule and the code now agree. ## Concerns - **Qualified zero-claims still manufacture an id-less, unclosable finding.** `BLOCKER: none, see RISK-2.`, `BLOCKER: none — resolved by <sha>.`, `BLOCKER none (verified on head X).` and `BLOCKER none as of <sha>.` all still report one id-less finding at this head, i.e. the same failure mode GITS #209 hit, just with a tail. This is *intended* by the ticket (content after the determiner is content, and `BLOCKER none, verified on head X.` is a mandated must-block row), so it is not a blocker — but the fleet's two most natural "none, but qualified" phrasings remain traps, and a seat can still wedge a PR with one. A follow-up ticket for a `resolved <ID>`-free escape hatch (or a refusal message that names the offending line as unclosable) would retire the last of this family. - **The gap run is broader than its own docstring says.** `SEVERITY_REMAINDER_GAP_RE` includes `.`, `,`, `;`, `=`, `>`, `~`, `{`, `}`, `|`, so `BLOCKER... none.`, `BLOCKER,,, none.`, `BLOCKER;;; none.`, `BLOCKER ||| none.`, `BLOCKER: > none.`, `BLOCKER => none.` all negate. No real finding was observed to read that way in the corpus, so the widening is inert in practice; it is still wider than "a colon or equals plus markup and whitespace" and a future narrowing should be pinned by a test rather than discovered. - **`severity:` as an accepted lead makes one new class negate.** `severity: blocker none.` and `Severity: BLOCKER none.` moved 1 → 0 at this head (they were findings on `main`). Plausibly intended, and unreadable as a real defect, but it is outside the ticket's enumerated shapes and is not covered by any test in the new file. A one-line test pinning the `severity:` lead would make the intent explicit. - **Housekeeping, not this PR:** the `kv/ld/forgejo-reviewer` secret's `user` metadata field reads `pi-landing` while the token itself authenticates as `ld-reviewer` (id 47, verified with `GET /user` before posting). Nothing here depends on that field, but the stale metadata would mislead anyone reading the secret entry.
Collaborator

Verdict of record — ld-reviewer — head 5d451462bf: APPROVE
model: deepseek-flash
session: 01a1b047-f5f7-757a-b84d-280dce34e7c2

Independent review seat 1 of 2 (gate tooling). Reviewed the diff, not the author's summary: git diff origin/main...5d45146 is two files (forgejo/landing.ts +100/−2, forgejo/tests/landing-severity-none.test.ts new, 223 lines). I did not depend on the parallel seat.

Blockers

None.

Concerns

  • SEVERITY_TOKEN_LEAD_RE accepts a ) lead but not an ordered-list numeral, so the family is asymmetric on one shape — measured at head:

    2) BLOCKER: none.   -> zero (the `)` is in the lead run)
    1. BLOCKER none.    -> finding (the numeral is not)
    

    The miss is fail-closed (the PR stays refused, never silently allowed), so it does not hold this change — but it is the exact artifact this PR fixes, still reachable from a numbered list. Suggest a follow-up widening the lead run to \d{1,3}[.)]; the near-miss tests are already structured to take the extra cases.

  • The rule deliberately does not strip a quote from the end of a remainder, so a quoted zero-claim stays a finding. Correct direction (fail-closed), noted only so the asymmetry with emphasis-insensitivity (#103) is on the record.

What I verified (evidence)

1. The failing test reproduces at the parent. In a detached worktree at head I ran git checkout origin/main -- forgejo/landing.ts (parent 26b5f15) and ran the new file against it: Tests 16 failed | 27 passed (43) — the PR's number. The 16 failures are the PR's enumeration, and the guard family (BLOCKER none of the tests ran. and the other prose tails) passes at the parent, which is the strongest form of the claim: the fix adds the negation and leaves the fail-closed reading untouched, rather than trading one for the other. The end-to-end failure at parent is the GITS #209 shape: findOpenBlockers returns ['BLOCKER none.'] for comment 22638's tail, and evaluateLandingLock(...).ok === true only at head.

2. Exact-vs-content rule. Probed the rule directly, not through the author's list:

BLOCKER none of the tests ran.                -> finding
BLOCKER none found here                       -> finding
BLOCKER none worth mentioning                 -> finding
BLOCKER: none worth mentioning — the … is open -> finding
BLOCKER none, and the auth check is open      -> finding
BLOCKER: none of the retries back off         -> finding
BLOCKER n/a — the harness never ran           -> finding
BLOCKER 0 tests were run                      -> finding
BLOCKER B1: none. / BLOCKER R2: none.         -> findings, ids B1 / R2 preserved
BLOCKER-1: none.  BLOCKER- none.              -> findings
BLOCKER none whatsoever / nothingburger.      -> findings
Blocker — none.   **Blockers** — none.        -> findings (#103 dash decision kept)
BLOCKER: "none".  BLOCKER: none;  BLOCKER: none, -> findings (fail-closed)

The guard line is asserted on the finding's own text and its null id, so it pins the reading, not a count.

3. Emphasis-insensitivity. BLOCKER none. · **BLOCKER** none. · __BLOCKER__ none. · _BLOCKER_ none. · **BLOCKER:** none. · - **BLOCKER**: none. · ## / ### BLOCKER none. · (BLOCKER) none. · BLOCKER:none. · BLOCKER none.** all read zero, and the dash twin reads the same however it is emphasized. Bold markup cannot change a reading.

4. No interaction with real findings or closure. A body carrying a real labelled finding plus the zero claim keeps the finding and drops only the claim; closure still works and still needs the reviewer: trusted resolved B1 closes it, a stranger's resolved B1 does not, and the zero claim under a ## Blockers heading does not disturb the section. The new check sits after isNegated and before the documentation guards, so every existing negation keeps its own reading; it is called nowhere else, and nothing in the resolution path or the section machinery reads it.

5. Corpus measurement, reproduced independently. I ran every comment body in the repo's five fixture corpora (76 bodies) plus comment 22638's tail through findOpenBlockers under the parent and under head and diffed the finding sets: 77 bodies compared, exactly 1 changed — gits209#22638 drops BLOCKER none. — and zero additions anywhere. Total open findings on that corpus: 6 → 5. That is the property that matters: no finding anywhere became invisible. (The PR reports 79 bodies; the two extra are GITS #209 comments not in my local fixtures.)

6. Suites green in both shapes at head. npx tsc --noEmit clean. npx vitest run → 34 files, 895 tests passed (the #96–#106 suites included). npm run test:deployed (deployed shape, per TESTING.md) → ok: 34/34 files loaded and passed, 895/895 tests on the pinned tree (typebox 1.3.34). Forgejo CI on this head (#334) is green across all 15 jobs.

Scope check: the only edit to pre-existing logic is factoring the determiner vocabulary into NEG_ZERO_WORD inside two existing negation patterns — character-identical sets, so the substitution cannot change a reading (all negation suites agree). The new behaviour is reachable only from the line-initial severity-token path.

**Verdict of record — ld-reviewer — head 5d451462bf0d24b5564f79d64e4ce5f399ef80ba: APPROVE** model: deepseek-flash session: 01a1b047-f5f7-757a-b84d-280dce34e7c2 Independent review seat 1 of 2 (gate tooling). Reviewed the diff, not the author's summary: `git diff origin/main...5d45146` is two files (`forgejo/landing.ts` +100/−2, `forgejo/tests/landing-severity-none.test.ts` new, 223 lines). I did not depend on the parallel seat. ## Blockers None. ## Concerns - `SEVERITY_TOKEN_LEAD_RE` accepts a `)` lead but not an ordered-list numeral, so the family is asymmetric on one shape — measured at head: ``` 2) BLOCKER: none. -> zero (the `)` is in the lead run) 1. BLOCKER none. -> finding (the numeral is not) ``` The miss is fail-closed (the PR stays refused, never silently allowed), so it does not hold this change — but it is the exact artifact this PR fixes, still reachable from a numbered list. Suggest a follow-up widening the lead run to `\d{1,3}[.)]`; the near-miss tests are already structured to take the extra cases. - The rule deliberately does not strip a quote from the *end* of a remainder, so a quoted zero-claim stays a finding. Correct direction (fail-closed), noted only so the asymmetry with emphasis-insensitivity (#103) is on the record. ## What I verified (evidence) **1. The failing test reproduces at the parent.** In a detached worktree at head I ran `git checkout origin/main -- forgejo/landing.ts` (parent `26b5f15`) and ran the new file against it: `Tests 16 failed | 27 passed (43)` — the PR's number. The 16 failures are the PR's enumeration, and the guard family (`BLOCKER none of the tests ran.` and the other prose tails) **passes at the parent**, which is the strongest form of the claim: the fix adds the negation and leaves the fail-closed reading untouched, rather than trading one for the other. The end-to-end failure at parent is the GITS #209 shape: `findOpenBlockers` returns `['BLOCKER none.']` for comment 22638's tail, and `evaluateLandingLock(...).ok === true` only at head. **2. Exact-vs-content rule.** Probed the rule directly, not through the author's list: ``` BLOCKER none of the tests ran. -> finding BLOCKER none found here -> finding BLOCKER none worth mentioning -> finding BLOCKER: none worth mentioning — the … is open -> finding BLOCKER none, and the auth check is open -> finding BLOCKER: none of the retries back off -> finding BLOCKER n/a — the harness never ran -> finding BLOCKER 0 tests were run -> finding BLOCKER B1: none. / BLOCKER R2: none. -> findings, ids B1 / R2 preserved BLOCKER-1: none. BLOCKER- none. -> findings BLOCKER none whatsoever / nothingburger. -> findings Blocker — none. **Blockers** — none. -> findings (#103 dash decision kept) BLOCKER: "none". BLOCKER: none; BLOCKER: none, -> findings (fail-closed) ``` The guard line is asserted on the finding's own text and its null id, so it pins the *reading*, not a count. **3. Emphasis-insensitivity.** `BLOCKER none.` · `**BLOCKER** none.` · `__BLOCKER__ none.` · `_BLOCKER_ none.` · `**BLOCKER:** none.` · `- **BLOCKER**: none.` · `## / ### BLOCKER none.` · `(BLOCKER) none.` · `BLOCKER:none.` · `BLOCKER none.**` all read zero, and the dash twin reads the same however it is emphasized. Bold markup cannot change a reading. **4. No interaction with real findings or closure.** A body carrying a real labelled finding plus the zero claim keeps the finding and drops only the claim; closure still works and still needs the reviewer: trusted `resolved B1` closes it, a stranger's `resolved B1` does not, and the zero claim under a `## Blockers` heading does not disturb the section. The new check sits after `isNegated` and before the documentation guards, so every existing negation keeps its own reading; it is called nowhere else, and nothing in the resolution path or the section machinery reads it. **5. Corpus measurement, reproduced independently.** I ran every comment body in the repo's five fixture corpora (76 bodies) plus comment 22638's tail through `findOpenBlockers` under the parent and under head and diffed the finding sets: **77 bodies compared, exactly 1 changed** — `gits209#22638` drops `BLOCKER none.` — and **zero additions anywhere**. Total open findings on that corpus: 6 → 5. That is the property that matters: no finding anywhere became invisible. (The PR reports 79 bodies; the two extra are GITS #209 comments not in my local fixtures.) **6. Suites green in both shapes at head.** `npx tsc --noEmit` clean. `npx vitest run` → 34 files, 895 tests passed (the #96–#106 suites included). `npm run test:deployed` (deployed shape, per `TESTING.md`) → `ok: 34/34 files loaded and passed, 895/895 tests` on the pinned tree (`typebox 1.3.34`). Forgejo CI on this head (`#334`) is green across all 15 jobs. **Scope check:** the only edit to pre-existing logic is factoring the determiner vocabulary into `NEG_ZERO_WORD` inside two existing negation patterns — character-identical sets, so the substitution cannot change a reading (all negation suites agree). The new behaviour is reachable only from the line-initial severity-token path.
Collaborator

Review note — not a verdict. This closes a machine artifact raised by my own verdict comment, nothing else.

The "Emphasis-insensitivity" bullet in comment 22649 is a LIST of quoted token forms; the lock reads that line as a finding. It is not a claim of a defect, and nothing in this PR is defective on its account. I am not editing or deleting that comment — that is not mine to do — so I am using the sanctioned closure sentence for the id-less finding that item raised:

The blocker raised in review 22649 is resolved on head 5d451462bf.

Verdict unchanged — APPROVE at head 5d451462bf (seat 1 of 2, model deepseek-flash, session 01a1b047-f5f7-757a-b84d-280dce34e7c2).

Follow-up worth a ticket, outside this PR's scope and pre-existing at the parent: a documentation line that merely LISTS quoted token forms is still read as a finding when the list is joined onto one line, which is the same "typographic artifact blocks a clean PR" family as the one this PR fixes on the token path.

Review note — not a verdict. This closes a machine artifact raised by my own verdict comment, nothing else. The "Emphasis-insensitivity" bullet in comment 22649 is a LIST of quoted token forms; the lock reads that line as a finding. It is not a claim of a defect, and nothing in this PR is defective on its account. I am not editing or deleting that comment — that is not mine to do — so I am using the sanctioned closure sentence for the id-less finding that item raised: The blocker raised in review 22649 is resolved on head 5d451462bf0d24b5564f79d64e4ce5f399ef80ba. Verdict unchanged — APPROVE at head 5d451462bf0d24b5564f79d64e4ce5f399ef80ba (seat 1 of 2, model deepseek-flash, session 01a1b047-f5f7-757a-b84d-280dce34e7c2). Follow-up worth a ticket, outside this PR's scope and pre-existing at the parent: a documentation line that merely LISTS quoted token forms is still read as a finding when the list is joined onto one line, which is the same "typographic artifact blocks a clean PR" family as the one this PR fixes on the token path.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/pi-extensions!107
No description provided.