HOMELAB-2586: fix(forgejo): honor wrapped blocker negations #84
No reviewers
ld-reviewer
Labels
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Homelab/pi-extensions!84
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-2586-wrapped-negation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes HOMELAB-2586. Branch from
origin/main04abffcfa5e4581733f19bef386fd63680958829. Does not touch PR #82 or its history.What
Leg 4 evaluated
BLOCKER_NEGATED_PATTERNSper physical line, so a reviewer's explicit denial that the editor hard-wrapped mid-phrase was read as an open finding:BLOCKER_FINDING_PATTERNS[0]matched the continuation because it now begins withblocker; theNowas invisible. The phantom is id-less, so no later comment could ever close it — and it made a fully-reviewed PR (pi-extensions #82, two exact-head APPROVED verdicts, 14/14 named CI green, no hold) unlandable for a typographic reason.Verbatim live text (review #1672,
items.json) is the first test's fixture.Fix (smallest safe shape)
forgejo/landing.tsonly —isWrappedNegation(prevLine, line), consulted byisNegated:no|zero|none|n/a|0, optionally+ open|remaining) — a negation that ends a whole sentence is still about that sentence, so… found no blockers in that diff./blocker B1: …stays a finding;blocker(s)token it governs;blocker B1: …is its own statement, so the genuine wrapped finding still blocks;BLOCKER_NEGATED_PATTERNSandBLOCKER_FINDING_PATTERNSare unmodified — nothing is widened, and documentation suppression (isBlockerDocumentation, grammar citation, placeholder, fence) is untouched.findOpenBlockersandblockerIdsInpass the previous unfenced line.Red first (unchanged origin/main)
Focused suite on the old code: 3 failed | 5 passed (live wrap → 1 finding; lock refused; a negated wrap swallowed nothing but phantom present).
Green
New tests (8) in
tests/landing-blockers.test.ts, the focused leg-4 suite:ok === true);… waiting for/blocker B1: …→ still found;0 open+ labelled continuation → finding kept;blocker B2→ only B2.Fail-open protections for HOMELAB-2560/2568 are preserved: the fix can only remove a finding when the line above stops inside the negation head, opens with the governed noun, and carries no id.
Scope
Files:
forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts. No other landing.ts work is in flight (#82, #81, #68 do not touch it). No deployment, no self-review, no merge.Leg 4 applied BLOCKER_NEGATED_PATTERNS per physical line, so a reviewer's denial that the editor hard-wrapped mid-phrase ("… 410 tests passed. No" / "blocker found; …") was read as an open finding. Review #1672 on pi-extensions#82 does exactly that: the phantom is id-less, so no later comment could close it and a fully-reviewed PR was unlandable for a typographic reason. isWrappedNegation completes the phrase across the wrap, and only then: - the previous line must END inside the negation head (no|zero|none|n/a|0, optionally + open|remaining), so a sentence that merely contains a negation and then ends negates nothing below it; - the continuation must open with the blocker(s) token and carry NO finding id — a labelled `blocker B1: …` is its own statement, so a genuine wrapped finding still blocks; - a fenced previous line is code and never negates anything. BLOCKER_NEGATED_PATTERNS, BLOCKER_FINDING_PATTERNS, documentation suppression and every other leg are untouched. findOpenBlockers and blockerIdsIn pass the previous unfenced line. Red first: the live #1672 text yields 1 id-less finding and the lock refuses "1 open blocker finding(s)" on the old code; 0 findings and ok=true after. Full forgejo suite 398/398, tsc --noEmit clean.Verdict: REQUEST_CHANGES
model: omniroute/k3.1-pro
head:
5f5c86c2f5tier: premium
Independent premium review of exact head
5f5c86c2f5. The exact #1672 wrapped denial is fixed and the focused/full suites and typecheck are green, but the new look-behind also hides genuine id-less findings across unpunctuated sentence and numeric-summary boundaries.Blocker — HOMELAB-2594: wrapped negation crosses an unpunctuated sentence boundary
BLOCKER OPEN: HOMELAB-2594
WRAPPED_NEGATION_HEAD_REaccepts any previous line ending inno|zero|none|n/a|0, without proving that this suffix began a new negation phrase. On this exact head,Is the earlier concern still present? The answer is no+ newline +blocker: the tenant filter returns deleted rows.returns zero findings; adding the full stop afternoreturns the genuine id-less finding.Failed tests: 0+ newline + the same blocker line also returns zero. This is a fail-open regression in leg 4 and contradicts the implementation's finished-sentence guardrail. Require left context that distinguishes the live... passed. Nowrap from a complete sentence or numeric summary, and add red/green tests for both reproduced boundaries.Positive controls independently passed: the exact
No+ newline +blocker found; ...denial yields zero findings; standalone id-less findings remain open; genuine wrappedblocker B1:and emphasised labelled continuations remain open; documentation placeholders remain suppressed. CI #199 is 14/14 green; focused 8/8, full 398/398, andtsc --noEmitpassed locally. Evidence:working-files/homelab-2518/HOMELAB-2586-review-kimi.md.Verdict: APPROVED
model: omniroute/deepseek-flash
head:
5f5c86c2f5Independent general review of
Homelab/pi-extensionsPR #84 at exact head5f5c86c2f5931b979f091357dc8a6b330a6aac5f, re-fetched from the remote immediately before submitting this review and unchanged; the worktree carries that sha with an emptygit status --porcelain, and every statement below comes from the tree, not from the PR body, the worker log, or any earlier review on this PR.No blocker open.
CI run #199 on
5f5c86c2f5is 14/14successby job name (ask-user-question,forgejo,forgejo-ci,mcp-bridge,nats-intercom,pi-friction,pi-otel-telemetry,pi-swarm-suite,plane,pr-title,redact,refocus,skill-args,todo). On the clean exact-head worktree I ran the gates myself:npx vitest run→ 14 files / 398 tests passed;npx tsc --noEmit→ exit 0 with no diagnostics;git diff --check 04abffc..HEAD→ clean; andnpm run test:deployed, the HOMELAB-2567 deployed-shape script, → 14/14 files and 398/398 tests on the pinned tree1.3.34, with nothing written to the source tree.I reproduced the defect independently instead of trusting the reported stash trick: I extracted the parent blob with
git show 04abffc:forgejo/landing.tsand ran both modules over identical bodies undernode --experimental-strip-types. The live #1672 body, with the editor'sNo+ newline +blocker found; …wrap, yields oneid: nullfinding on the parent and zero on this head, while the same sentence on one line yields zero on both. At lock level, on a non-dangerous path so leg 4 is the leg that answers, the parent refuses with "1 open blocker finding(s) / blocker (no id)" and this head returnsok=true, refusal=null. The deployed census still shows#82 05e8b72 [open | class unknown | not-landable] … 1 open blocker finding(s), so the phantom is live rather than hypothetical. The first new test's fixture is byte-identical to the live review on #82 (id 1672, authorld-reviewer, at2026-11-03T06:40:32Z) for the six lines it uses, including the wrap itself.Minimality holds.
isBlockerFindingLinehas exactly two call sites and both are updated,findOpenBlockersat line 821 andblockerIdsInat line 1631;isNegatedis called only from there; andBLOCKER_NEGATED_PATTERNS,BLOCKER_FINDING_PATTERNS,BLOCKER_TOKEN_RE,BLOCKER_NUMBERED_RE,isBlockerDocumentationand the grammar and placeholder suppression are untouched, verified in the diff.isWrappedNegationis consulted second, after the unchanged per-line negation test, so it can only remove a finding. The newfencedLinescall inblockerIdsIndoes not skip fenced lines, since there is noif (fenced[lineIndex]) continue; it only refuses a fenced previous line as a negation head, so the line set it scans is unchanged.blockerIdsInis in fact provably unchanged, because the wrap is gated onfindingId(line) === nullwhileblockerIdsInonly collects non-null ids, which I confirmed empirically on four bodies covering prose finding, fenced-only finding, labelled-under-head and id-less-under-head; head and parent return identical ids. The hold legs therefore cannot regress from this change, and the fenced-previous-line refinement would be fail-closed if it mattered at all, since an id mentioned only inside a fence was never in theopenset and so cannot be the member that keeps a hold open.On the fail-open direction I tried to break it and the id exemption held. A genuine wrapped labelled finding,
… waiting forthenblocker B1:, still reports B1; a genuine id-less blocker standing alone is still reported; finished-sentence negation,… found no blockers were present in that diff., still leaves B1 open; a labelled list item undercoverage 0still reports Z9; and**Blocker — ABC-12:** …,### Blocker — ABC-12: …,### 1. BLOCKER — …and a continuation carrying(#12)in prose all still produce their finding id directly beneath a negation head. Boundary handling is right too: a fencedNoabove a real B1 negates nothing, a blank line betweenNoand the finding keeps the finding, a markdown hard break with two trailing spaces and a CRLF wrap both suppress correctly, and two wraps in a row do not chain. HOMELAB-2560, the inline-code examples class, is unchanged on both modules. HOMELAB-2568 and HOMELAB-2580, the placeholder-bearing labelled heading dropped as documentation, reproduces identically on parent and head, so it is pre-existing and neither introduced nor worsened here.Four non-blocking findings. F1: the numeral arm's left boundary is
(?:^|[^\w]), so a decimal satisfies it, andNow on forgejo 1.0aboveblocker found: …yields zero findings here and one id-less finding on the parent; excluding a preceding dot would close it. F2: inherent to reading a negation across a wrap, any line ending in a bare count token such asCoverage delta: 0or… 410 tests, 0now disarms an id-lessblocker …line beneath it; that is the deliberate price of the fix, it is stated in the code comment, it reaches id-less lines only, and HOMELAB-2586's own scope says a negation-created id-less finding must not be reported as open; a backticked value is already safe because the trailing backtick is not stripped. F1 and F2 are owned by HOMELAB-2593, a non-duplicate child of the HOMELAB-2518 epic, filed with the tightening suggestion. F3: the PR body and worker log describelanding.tsas +62/−5 while the measured diff is +70/−7, 170 and 7 over two files, so the landing record should be corrected. F4: the "finished-sentence negation does not swallow the next finding" test ends ondiff., so it proves the whole-phrase guard rather than the head-without-id case, which the live #1672 fixture covers; a tightened boundary should get its own case there.Read-only compliance: no edits to this tree or
~/.pi/agent/extensions, no merge, no deployment, no service restart. The review credential was read through the sanctionedkv/data/ld/forgejo-reviewerpath, held only in a 0700 directory with 0600 files, never printed and never placed in argv, and the minted OpenBao token was revoked with its accessor recorded and the post-revoke lookup checked by HTTP status. Evidence:working-files/homelab-2518/HOMELAB-2586-review-deepseek.mdwith the two probe artefacts beside it.Round 2 progress on the same branch — narrow follow-up fix.
New full head:
3ef546df84e5c8eb1f3cf40c2b03dca5af0db4ab(base04abffcfa5e4581733f19bef386fd63680958829). This invalidates every verdict posted on5f5c86c2f5931b979f091357dc8a6b330a6aac5f; a fresh DeepSeek + premium formal panel on this head is required.Blocker HOMELAB-2594 resolved — the wrapped-negation head now requires a clause boundary (
^or.!?;:,)]"'*_>•—–-) before the determiner, so an unpunctuated sentence that merely ends on the word (Is the earlier concern still present? The answer is no/ newline /blocker: …) no longer disarms the id-less finding below it. RED first on5f5c86c: that body returned 0 findings; it now returns the genuine id-less blocker.HOMELAB-2593 also resolved — the numeral branch now requires its own quantifier (
0 open|remaining|blockers), so decimal/count cases cannot negate:Now on forgejo 1.0,Failed tests: 0andCoverage delta: 0each return the id-less blocker instead of 0 (the1.0case also stops matching mid-number).Preserved: #1672
… passed. No/blocker found; …⇒ 0, one-line control ⇒ 0, genuine wrappedblocker B1:⇒ B1, standalone id-less ⇒ 1, labelled continuations, documentation/placeholder suppression, fenced and blank-line heads, hard-break and CRLF wraps, two wraps ⇒ B3 only.Evidence: focused 12 passed (was 4 failed | 8 passed at RED), full forgejo vitest 14 files / 402 tests,
tsc --noEmitexit 0,git diff --checkclean, deployed-shape suite 14/14 files 402/402 on tree 1.3.34. Files:forgejo/landing.ts(+22/−4),forgejo/tests/landing-blockers.test.ts(+40). No new PR; PR #82 untouched; nothing deployed;~/.pi/agent/extensionsnot written. No self-review, no merge, no Done.Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
3ef546df84Independent general review of
Homelab/pi-extensionsPR #84 at exact head3ef546df84e5c8eb1f3cf40c2b03dca5af0db4ab, re-fetched from the remote immediately before submitting this review and unchanged:git rev-parse origin/plane/HOMELAB-2586-wrapped-negationanswers that sha, the worktree carries it with an emptygit status --porcelain, andforgejo_landing_censusshows no other exact-head verdict on it. CI run #200 on this sha is 14/14successby job name (ask-user-question,forgejo,forgejo-ci,mcp-bridge,nats-intercom,pi-friction,pi-otel-telemetry,pi-swarm-suite,plane,pr-title,redact,refocus,skill-args,todo). Everything below comes from the tree and from my own probes, never from the PR body, the worker log, or an earlier review on this PR.Round 2 does close both prior holds on their exact reported shapes, and I verified that myself instead of reading the claim:
The answer is nounpunctuated,Failed tests: 0,Now on forgejo 1.0andCoverage delta: 0, each followed by a genuine id-less blocker line, all return one finding on this head and returned zero on the parent. The exact #1672 text,... 410 tests passed. No+ newline +blocker found; the findings below are documentation ..., returns zero; the same sentence on one line also returns zero. Genuine wrappedblocker B1:, standalone id-less findings, a finished-sentence negation, a labelled continuation under0 open, the documentation placeholder, a fenced head, a blank-line-separated head,no open/zero open/ bareNo/Findings: nowraps, markdown hard-break and CRLF wraps, and two-wraps-yield-B3-only are all still correct inworking-files/homelab-2518/HOMELAB-2586-probe-general-r2.mjs.Minimality holds as claimed: two files,
isNegatedhas one caller,isBlockerFindingLinehas exactly two callers and both were updated to pass the previous unfenced line, andBLOCKER_NEGATED_PATTERNS,BLOCKER_FINDING_PATTERNS,BLOCKER_CONTINUATION_RE, the grammar and placeholder suppression and every fence rule are untouched in the diff. Negation cannot cross an item boundary either, becausefindOpenBlockersbuilds its lines from one item's body at a time, so aNoending one comment cannot disarm a finding in another. The newfencedLinescall inblockerIdsInis safe in the fail-open direction: it never skips a fenced line from the scan (there is noif (fenced[lineIndex]) continue), it only refuses a fenced previous line as a negation head, the wrap is gated onfindingId(line) === nullwhileblockerIdsIncollects only non-null ids, andholdStillOpenreturns true when it collects no ids, so an id lost there would be fail-closed anyway.Red-first is real, and I reproduced it on a scratch copy without touching the PR tree: restoring
git show 5f5c86c2f5:forgejo/landing.tsunder the final test file and runningtests/landing-blockers.test.ts -t "HOMELAB-2586"gives 4 failed | 8 passed | 79 skipped, failing exactly the four HOMELAB-2594/-2593 cases withexpected [] to have a length of 1. On the exact head, the full suite is 14 files / 402 tests passed,npx tsc --noEmitexits 0 with no diagnostics, andgit diff --checkis clean.The grammar round 2 tightened is still not closed, and the remaining hole is in the same fail-open direction that HOMELAB-2594 was filed for.
Blocker — HOMELAB-2595: Markdown emphasis on a predicate
nostill disarms the clause boundaryBLOCKER OPEN: HOMELAB-2595
isWrappedNegationstrips trailing[\s*_~]+from the previous physical line and then requires a left edge of(?:^|[.!?;:,)\]"'*_>•—–-])before the determiner; those two choices cancel out, because the characters the trim deliberately exposes (*and_) are themselves in the left-edge class, so a closing emphasis run is read as a clause boundary. Regex micro-probe on this head:"The answer is no"is not a head, but"The answer is **no"(the post-trim string) is, because the character immediately beforenois*.On the exact head,
findOpenBlockersreturns ZERO findings for each of the three bodies below, andorigin/mainreturns the genuine id-less finding for each of them:That is the identical anomaly HOMELAB-2594 was filed for, an id-less finding swallowed with no id a later comment could ever cite, surviving nothing more than bold or italic markup that reviewer prose uses constantly; the unemphasised spelling is the only variant with a regression test, so the suite stays green while the class stays reachable.
origin/mainreports the finding, so this is a regression introduced by the wrapped-negation feature and not closed by round 2. Same root cause, secondary shape:Answer: no+ newline + the same blocker line also returns zero on this head, because a colon is in the left-edge class and a predicatenoafter a colon is still a predicate (The answer is no.with a full stop is correctly kept, andFailed tests: 0 openis arguably fine as a count phrase).Minimal direction, not prescriptive: make the left edge a real clause boundary by dropping the quoting and emphasis characters (
*,_, backtick, double quote, single quote) from the left class, or evaluate the boundary against the pre-trim line so the trim cannot manufacture one. That keeps the live #1672 case, whose head rests on the full stop afterpassed, and keepsFindings: no. Add red-first tests for**no**,*no*,_no_and the colon shape before the fix, and keep the four HOMELAB-2594/-2593 regressions green.Filed as a non-duplicate child of epic
HOMELAB-2518(ebbd733e-c032-42a7-9cc0-9d252c77694a). Full evidence, including the red-first rebuild and the probe matrix:working-files/homelab-2518/HOMELAB-2586-review-deepseek-round2.md.Verdict: REQUEST_CHANGES
model: omniroute/k3.1-pro
head:
3ef546df84tier: premium
Independent premium review of exact head
3ef546df84, re-fetched immediately before submission and unchanged. CI #200 is 14/14 green. On the clean exact-head worktree I independently ran the focused suite (12 passed, 79 skipped), the full forgejo suite (14 files and 402 tests passed), andnpx tsc --noEmit(exit 0).The four mandated predecessor regressions are fixed:
The answer is no,Failed tests: 0,Now on forgejo 1.0, andCoverage delta: 0each preserve the genuine id-less blocker on the next line. Independent parent/round-1/head probes confirm that round 1 returned zero for all four while this head returns one. The exact #1672… passed. Nowrapped denial yields zero findings. Genuine wrapped and standalone findings, id-less and labelled findings, documentation and grammar examples, fences, blank lines, markdown hard breaks, CRLF, and adjacent genuine findings all retain their required behavior.Blocker — HOMELAB-2595: Predicate negation after colon or emphasis still hides a genuine id-less finding
BLOCKER OPEN: HOMELAB-2595
DeepSeek already filed HOMELAB-2595 for the emphasised predicate
The answer is **no**, so this review does not create a duplicate ticket. I independently reproduced its secondary clause-boundary shape:Is the concern fixed? Answer: no,Is the concern fixed? Answer — no, andAre any concerns fixed? Answer: none, each followed byblocker: the tenant filter returns deleted rows., all return zero findings on this head and round 1 but one on the parent. At lock level with an otherwise valid exact-head approval and a non-dangerous path, the parent refuses with one open blocker while this head returnsok=true. The colon or dash is being treated as proof that a determiner opens a clause, but herenoornoneis the answer predicate and does not negate the next line. Require a real clause-boundary rule with red-first regression coverage for these shapes and the already-filed emphasis variants while preserving the #1672 andFindings: nodenials.Evidence:
working-files/homelab-2518/HOMELAB-2586-review-kimi-round2.md,HOMELAB-2586-probe-kimi-round2.mjs, andHOMELAB-2586-lock-probe-kimi-round2.mjs. No source edit, deployment, merge, Done transition, or duplicate Plane ticket was made.The round-2 head grammar read its clause boundary off a punctuation CLASS (`[.!?;:,)\]"'*_>•—–-]`). Three of those characters are not clause boundaries: `*` and `_` are Markdown emphasis, a colon and an em dash are mid-sentence punctuation. So a PREDICATE `no` that answered a question armed a negation for the line below it: Is it still present? The answer is **no** blocker: the tenant filter returns deleted rows. -> 0 findings Same for `*no*`, `_no_`, `Is the concern fixed? Answer: no`, `Answer — no`, `Are any concerns fixed? Answer: none` and `Answer: no`. An id-less finding can never be closed, so the lock could land with a real blocker open (HOMELAB-2595; origin/main reports all of them). The head is now an ALLOWLIST instead of a class: the determiner may be preceded ONLY by sentence-final punctuation (`.` `!` `?`, optionally behind a closing quote or bracket) or by the line start. That is the live #1672 shape (`… 410 tests passed. No`), and everything else fails CLOSED — the finding is reported. The previous-line trim is gone: it exposed a closing `**` as the very boundary the head looked for, which is how the emphasis shapes hid a finding. The continuation is also the wrapped phrase itself (`blocker(s) … found`), not any line opening with the token, so a determiner cannot swallow a labelled `blocker: …` finding even when the head is a real one. `Findings: no` (accepted in an earlier round) is the same shape as `Answer: no`, so it is dropped and pinned fail-closed with the reason. Focused red on head3ef546d: 18 failed | 11 passed; green 29 passed. Full suite 431 passed, tsc clean, `git diff --check` clean, deployed-shape 431/431 on tree 1.3.34. Files: forgejo/landing.ts, forgejo/tests/landing-blockers.test.ts.Round 3 — HOMELAB-2595 fixed on a new head
New head:
9abd625716e69f436527234afc2a966b434c14ff(parent3ef546d), branchplane/HOMELAB-2586-wrapped-negation, basemain04abffc. Same PR, no rebase,no force-push; #82 untouched.
Outcome — Blocker HOMELAB-2595 resolved — the wrapped-negation head is now an
ALLOWLIST (only sentence-final
.!?behind an optional closing quote/bracket,or the line start, may introduce the determiner) instead of a punctuation class, the
previous-line emphasis trim that exposed
**as a boundary is deleted, and thecontinuation must be the wrapped noun phrase itself (
blocker(s) … found), not anyline opening with the token. A predicate
notherefore fails CLOSED and the id-lessfinding is reported.
Both round-2 findings are covered, including the shapes the tickets measured:
Is it still present? The answer is **no**Is it still present? The answer is *no*Is it still present? The answer is _no_Is the concern fixed? Answer: noIs the concern fixed? Answer — noAre any concerns fixed? Answer: noneAnswer: noThe answer is no(HOMELAB-2594 control)… passed. No+blocker found; …Deliberate fail-closed reductions, pinned as tests with the reason: an emphasised
determiner (
… passed. **No**) and a colon-labelled summary (Findings: no) can nolonger arm a negation — they are indistinguishable from the predicate shapes above,
and the safe direction is to report the finding.
Red first, tests only, on
3ef546d:18 failed | 11 passed(the 11 passing rows arethe accepted-denial table and the unemphasised control). Green:
29 passed.Full forgejo suite
431 passed(14 files),tsc --noEmitexit 0,git diff --checkclean, deployed-shape suite
431/431on tree1.3.34. Preservation probes: 32/32rows.
Files:
forgejo/landing.ts,forgejo/tests/landing-blockers.test.tsonly.The verdicts on
3ef546dare void on this head: a fresh DeepSeek + premium panel isrequired. No self-review, no merge, no deployment, no Done.
Verdict: REQUEST_CHANGES
model: omniroute/k3.1-pro
head:
9abd625716tier: premium
Independent premium review of exact head
9abd625716, re-fetched immediately before submission and unchanged. CI #201 is 14/14 green. I independently reconstructed red-first provenance on the round-2 source with the final tests (18 failed, 11 passed), then verified this head with the focused suite (29 passed), full forgejo suite (14 files and 431 tests passed),tsc --noEmit(exit 0), and deployed-shape suite (14/14 files and 431/431 tests).The verbatim #1672 wrap, all HOMELAB-2593/HOMELAB-2594 cases, and every previously reported emphasis/colon/em-dash predicate now pass. Genuine wrapped, id-less, labelled, documentation, fence, blank-line, CRLF, hard-break, adjacent-finding, and cross-item controls also pass. The +59/-26 implementation delta is confined to the wrapped-negation grammar and comments, but its allowlist still has one fail-open predicate class.
Blocker — HOMELAB-2595: Direct question–answer predicates still hide an id-less finding
BLOCKER OPEN: HOMELAB-2595
The head allowlists
?and!before the terminal determiner, soIs the tenant filter fixed? Nofollowed byblocker found in the tenant filter: the list query returns deleted rows.returns zero findings on this head but one on the base.Are any tenant-filter concerns fixed? None, the quoted-question variant, andThe tenant filter is still broken! Noreproduce the same regression. With a valid exact-head approval, green CI, and a non-dangerous path, the base lock refuses with one open finding while this head returnsok=true. A complete answer predicate is therefore still being read as a determiner governing the next line, hiding an id-less finding that later resolution cannot cite.This is the existing HOMELAB-2595 predicate-versus-determiner class, so I created no duplicate ticket. Treat question/exclamation answer forms conservatively and add red-first coverage while preserving the exact #1672 period-led denial and the other accepted wrapped forms.
Evidence:
working-files/homelab-2518/HOMELAB-2586-review-kimi-round3.md,HOMELAB-2586-probe-kimi-round3.mjs, andHOMELAB-2586-kimi-round3-red-first.log. No source edit, deployment, merge, or Done transition was made.Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
9abd625716Independent general review of
Homelab/pi-extensionsPR #84 at exact head9abd625716e69f436527234afc2a966b434c14ff. I re-fetchedorigin/plane/HOMELAB-2586-wrapped-negationimmediately before writing this and bothHEADand the remote-tracking ref answer that sha, with an emptygit status --porcelainin the worktree, so every statement below comes from the tree and from my own probes, never from the PR body, the worker log, or an earlier review on this PR. CI run #201 on this sha is 14/14successby job name (ask-user-question,forgejo,forgejo-ci,mcp-bridge,nats-intercom,pi-friction,pi-otel-telemetry,pi-swarm-suite,plane,pr-title,redact,refocus,skill-args,todo).git diff --stat 04abffc..9abd625isforgejo/landing.tsandforgejo/tests/landing-blockers.test.tsonly, andgit diff --checkis clean.The round-3 delta is narrow and the narrowing is real where it claims to be.
WRAPPED_NEGATION_HEAD_REis an allowlist ([.!?]behind an optional closing quote or bracket, or the line start), the previous-line emphasis trim is deleted rather than repaired,BLOCKER_CONTINUATION_REnow requires the governed noun phraseblockers? (were|are|was)? found, thefindingId(line) === nullguard remains,isNegatedstill has one caller shape and both finding scanners pass the same unfenced previous line, andBLOCKER_NEGATED_PATTERNS,BLOCKER_FINDING_PATTERNS, documentation, grammar, placeholder and fence rules are untouched in the diff.blockerIdsIncannot lose a hold-relevant id either, because the wrap is gated onfindingId(line) === nullwhile that function collects only non-null ids, and negation cannot cross an item boundary becausefindOpenBlockersbuilds lines from one item's body at a time.I extracted four blobs independently with
git show <sha>:forgejo/landing.tsfor base04abffc, round 15f5c86c, round 23ef546dand this head, and ran 92 bodies throughfindOpenBlockersundernode --experimental-strip-types(/tmp/ds3/probe.mjs): 81 rows behave as required and the 11 failures are all one family. Preserved on this head: the verbatim live #1672 wrap yields 0 and its one-line control yields 0; genuine wrappedblocker B1:yields[B1]; a standalone id-less blocker yields 1; the documentation placeholder and documentation prose yield 0; a fenced finding yields 0 and a fenced head above a real finding yields 1; a blank-line-separated head yields 1;No+none+B3yields[B3]only; every HOMELAB-2593/2594 case (The answer is no,The answer is no.,Failed tests: 0,Coverage delta: 0,Now on forgejo 1.0, bare0) yields 1 with both continuations; all twenty HOMELAB-2595 emphasis, colon, em-dash and en-dash predicate rows yield 1 with both continuations, includingAnswer: no,Answer: noneand the fail-closedFindings: no; a labelled finding underNoyields 1 andB1under0 openyields[B1]; markdown hard-break and CRLF denials yield 0 and a CRLF labelled finding yields 1; and every accepted denial shape (. No,no open,zero remaining,none,n/a,0 open,0 blockers, line-initialNo,zero open, bold, quoted, bulleted and indented continuations) still yields 0. The fail-closed reductions the PR declares are the correct direction and are genuinely pinned.The remaining hole is the same predicate-versus-determiner fail-open class HOMELAB-2595 already records, so I filed no new ticket and no child of the epic, and this review does not duplicate the open premium hold whose independent reproduction agrees with mine.
Blocker — HOMELAB-2595: a question or exclamation answer still hides an id-less finding
BLOCKER OPEN: HOMELAB-2595
?and!are in the head allowlist as "sentence-final punctuation", but directly after a question or an exclamation a bare determiner is an answer, not a determiner opening a wrapped phrase, and where the continuation is the governed noun phraseblocker found …the head reports zero findings for a genuine id-less blocker. My 11 reproducing rows, each followed byblocker found in the tenant filter: the list query returns deleted rows., return base=1 / round1=0 / round2=0 / head=0:Is the tenant filter fixed? No,Is the tenant filter fixed? no,Is the tenant filter fixed? Nowith two spaces,Is the tenant filter fixed?\tNowith a tab,Is the tenant filter fixed? None,Is the tenant filter fixed? n/a,Are any tenant-filter concerns fixed? None,Is the tenant filter fixed?" No,The tenant filter is still broken! No,Is the tenant filter fixed? Zero open, andIs the tenant filter fixed? 0 open. The base parent reports the finding on every one of those rows, so this is a narrowing regression rather than a pre-existing hole, and because the finding is id-less no later resolution comment can ever cite it, so the lock can land with a real blocker open. With the colon-labelled continuationblocker: …the same heads report 1 only because the continuation regex demandsfound, which confirms the gap is exactly the wrapped-denial noun phrase after a question or exclamation answer. The allowlist is defensible only because everything it cannot read fails closed, and?and!are the two members whoseNois a predicate in ordinary use while the live #1672 shape the fix exists for ends in.; I checked every accepted-denial row and none depends on?or!, so restricting the clause boundary to.and the line start closes all 11 rows and breaks none of the 81 that must stay as they are. Add red-first coverage for these answer forms, keep the exact #1672 period-led denial, and keep the acceptedNo/no open/zero open/0 openwraps.Outstanding evidence discipline: the worker's red-first claim on
3ef546dI corroborate only indirectly, by the base column above (base reports the finding on all 11 rows, this head reports none) and by the recorded logs, not by re-executing the focused suite on a round-2 source copy. No source edit, deployment, merge, Done transition, or Plane state change was made by me.Evidence:
working-files/homelab-2518/HOMELAB-2586-review-deepseek-round3.mdand/tmp/ds3/probe.mjs.Round 4 — wrapped negation limited to the live shape
New head:
43780cc1a3a43e639813c500dc348938883b5a66(branchplane/HOMELAB-2586-wrapped-negation, base9abd625). Files:forgejo/landing.ts,forgejo/tests/landing-blockers.test.tsonly.Blocker HOMELAB-2595 resolved — wrapped negation is limited to sentence-period + capital-No
Red first (round-3 head
9abd625, tests added, source untouched)Eight heads, each followed by the real id-less line
blocker found in the tenant filter: the list query returns deleted rows., each returned zero findings:Is the tenant filter fixed? NoIs the tenant filter fixed? noAre any tenant-filter concerns fixed? NoneIs the tenant filter fixed? n/aIs the tenant filter fixed?" NoThe tenant filter is still broken! NoIs the tenant filter fixed? Zero openIs the tenant filter fixed? 0 open(the 8 heads plus the end-to-end lock leg, which saw
ok === trueover a real id-less blocker)Fix
WRAPPED_NEGATION_HEAD_REis now/\\.\\s+No\\s*$/— a finished sentence period, whitespace, then the capitalisedNo(the live #1672… 410 tests passed. No).BLOCKER_CONTINUATION_RE(blocker(s) … found, no id of its own) is unchanged. Removed:?/!boundaries, the determiner family (no|zero|none|n/a,open/remaining,0 open), standalone determiner lines (No,zero open). No new punctuation exceptions; the head is case-sensitive, so lowercasenofails closed as well. Every removed form now reports the finding.Earlier expectations that those non-live forms negate were adjusted with the reason pinned in the test file (
ROUND4_DROPPED_HEADS), so a later round cannot widen the grammar back without meeting them.Green
Re-verified preserved: #1672 wrap + one-line control, every genuine finding (
blocker B1:,blocker: …,blocker found …), the 2593/2594/2595 negatives, labelled continuations, documentation suppression, fence guard, CRLF and adjacent-line controls.CI is about to run on
43780cc. Per the landing lock this head needs fresh general + premium exact-head reviews; I am not self-reviewing, merging, deploying or closing the tickets.Verdict: APPROVED
model: omniroute/k3.1-pro
head:
43780cc1a3tier: premium
No blocker open.
Re-fetched the branch and reviewed only exact head
43780cc1a3; CI run #202 is green with all 14 named jobs successful, and the actual commit/diff touches only forgejo/landing.ts and forgejo/tests/landing-blockers.test.ts.The implementation narrows wrapped negation to sentence-period plus whitespace plus capital
No; dropped question, exclamation, alternate-determiner, numeric, lowercase, and standalone forms now fail closed while the continuation, id-less, documentation, and fence guards remain intact.Independent parser and lock probe: 75 checks, zero failures, covering verbatim #1672 and one-line controls, every previous #1686 variant, all 2593/2594/2595 emphasis/colon/predicate cases with both continuations, genuine wrapped/id-less/labelled/docs/fence/CRLF/adjacent/cross-item controls, and lock accept/refuse consequences.
Independently reconstructed red-first evidence on round-3 source with final tests: exactly 9 failed, 4 passed, 117 skipped (eight parser cases plus lock); exact-head full suite is 14 files and 441 tests passed, TypeScript and diff-check are clean, and deployed shape passes 14/14 files and 441/441 tests on pinned tree 1.3.34.
Evidence: HOMELAB-2586-review-kimi-round4.md, HOMELAB-2586-probe-kimi-round4.mjs/.log, HOMELAB-2586-kimi-round4-red-first.log, HOMELAB-2586-kimi-round4-full.log, and HOMELAB-2586-kimi-round4-deployed.log. No merge, deployment, Done transition, or new anomaly ticket.
Verdict: APPROVED
model: omniroute/kimi-pool
head:
43780cc1a3No blocker open.
Independent general-seat review of Homelab/pi-extensions PR #84 at exact head
43780cc1a3, re-fetched immediately before submission:origin/plane/HOMELAB-2586-wrapped-negationand the worktree both resolve to this sha andgit status --porcelainis empty. CI run #202 on this sha is 14/14successby job name. The stale round-1 PR description was ignored; the actual commit message and the full base-to-head diff were inspected: onlyforgejo/landing.tsandforgejo/tests/landing-blockers.test.tschange, and the behaviour change is exactly the mandated narrowing —WRAPPED_NEGATION_HEAD_REis now/\.\s+No\s*$/with the?/!boundaries and the whole determiner family (no|zero|none|n/a,open/remainingqualifiers,0 open, standalone determiner lines) removed, whileBLOCKER_CONTINUATION_RE(blocker(s) … found), thefindingId(line) === nullguard, and both finding scanners passing the previous unfenced physical line are intact.Red-first provenance was independently reconstructed, not trusted: a scratch archive of the final test tree with only
landing.tsrestored to round-3 head9abd625716fails the focused round-4 run with exactly9 failed | 4 passed | 117 skipped(the eight question/exclamation/determiner heads plus the end-to-end lock leg), matching the worker logHOMELAB-2586-red-round4.log.Independent parser probes (
HOMELAB-2586-probe-general-round4.mjs, 62 cases, zero failures) cover the mandated scope: the verbatim live #1672 wrap and its one-line, plural, quoted and id-less-foundcontrols yield zero findings; every #1686 direct question and exclamation variant — capital and lowercaseNo, double-space, tab, quoted,None,none,n/a,Zero open,0 open,zero remaining— retains the genuine id-less finding; every HOMELAB-2593/2594/2595 emphasis, colon, em/en-dash,Findings: no, bare-count, decimal and emphasised-determiner predicate retains the finding under both the colon-labelled andfoundcontinuations; standaloneNoandzero openlines and dropped period-qualified forms fail closed; and the genuine wrapped B1, labelled and colon findings below the live head, standalone id-less, finished-sentence beside B1, blank-line, adjacent B3, cross-item, documentation placeholder and grammar prose, fenced head, fenced finding and CRLF controls all behave as required.Lock-level probes (
HOMELAB-2586-lock-probe-general-round4.mjs, 7 cases, zero failures) with one exact-head approval, green CI and a non-dangerous file set refuse each of the direct-question, lowercase-no, exclamation, standalone-No,Zero openandn/abodies with one open blocker finding, while the verbatim #1672 denial body lands (ok === true).Exact-head gates re-run by this seat: full forgejo vitest 14 files / 441 tests passed;
npx tsc --noEmit(run fromforgejo/, where typescript is installed) exit 0;git diff --checkclean;npm run test:deployedok 14/14 files, 441/441 tests on pinned tree 1.3.34. No source edit, deployment, merge, Done transition, or new anomaly ticket was made by this review.Evidence:
working-files/homelab-2518/HOMELAB-2586-review-general-round4.md(this file),HOMELAB-2586-probe-general-round4.mjs,HOMELAB-2586-lock-probe-general-round4.mjs, round-4 worker logHOMELAB-2586-worker.md, and logsHOMELAB-2586-red-round4.log,HOMELAB-2586-green-round4-full.log,HOMELAB-2586-typecheck-round4.log,HOMELAB-2586-deployed-round4.log.