HOMELAB-2521: fix(forgejo): parse blocker heading ids and resolutions #74
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-2521-blocker-ids"
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?
What
findingId()could not read the ids the fleet actually writes, so a genuine open blocker was stranded (idnull, so noresolved <id>line could ever close it) or mis-keyed to an id taken from its own sentence.Replayed against the GITS landing evidence (
homelab/working-files/gits/GITS-landing-triage-reconciliation.md,GITS-354-358-398-reconciliation.md) on the pre-fix tree at6518158:### Blocker — GITS33-R8: a flow-mapped job carrying runs-on is not detectednullGITS33-R8### GITS35-R1 (blocker) — …nullGITS35-R1### GITS35-R2 (blocker) — …nullGITS35-R2### HOMELAB-2408 (blocker) — …nullHOMELAB-2408### GITS-R1 (blocker) — …nullGITS-R1**GITS402-R1 (Blocker) — body mentions GITS-402**GITS-402(body won)GITS402-R1### Blocker 1 — positional11(resolved R1now closes it)### Blocker — ABC-12: controlABC-12ABC-12(control)How
[A-Z]{1,12}\d{0,4}(?:-[A-Z]{0,8})?\d{1,4}— in either order reviewers write it:Blocker — GITS33-R8:(one separator, id immediately after it) orGITS35-R1 (blocker)(nearest id before the token). An id quoted only in the finding's own sentence is not the label; a heading whose only id is in its body has none.ABC-12no longer citesABC-1).R<n>is the reviewers' short label for positional finding n:resolved R1closes### Blocker 1and nothing else — notR2, not the labelledGITS-R1, notGITS35-R1(lookbehind refuses-R1//R1chains).e.g. …) states the rule, not a change, so it no longer closes a finding.### 1. BLOCKER), the separator class (never the defect), all prose/fence/placeholder/negation guards.Red / green
Red command (from
forgejo/, implementation untouched, on6518158):Failing assertions were exactly the new cases:
reads the label id from the heading(expected null to be 'GITS33-R8'),never takes an id that the finding's own sentence mentions(expected 'GITS-402' to be 'GITS402-R1'),closes the finding the reviewer cited(resolved GITS33-R8: expected [ null ] to deeply equal []),closes only the positional finding the R1 alias names(expected [ '1', '2' ] to deeply equal [ '2' ]), the unrelated-id case (expected [ null ] to deeply equal [ 'GITS33-R8' ]), the negated case (expected [ null ] to deeply equal [ 'GITS35-R1' ]),opens one finding keyed by that label id, andcitesFinding distinguishes a label id from a positional alias.Green after the fix:
Test coverage is table-driven over every heading shape in the evidence harness: the eight reported lines above, the six ABC-12 separator controls (em dash, en dash, colon, comma, paren, hyphen),
B1, and the two numbered shapes; plus the eleven finding+resolution replays from GITS-354/358/398 and the negatives — an id only in the body, an unrelated id, a negated resolution, a fenced resolution, a bare digit against a labelled id, the proseGITS-402id, and the id-less fallback (still open on a citation, still closable by the documented phrase).Scope
forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md(leg 5 now documents the id grammar, label scoping, theR<n>alias and the grammar-citation rule).Not here, deliberately: the census
unpinnedrow, the missing required-context leg, the false "NO exact-head verdict" label, and the hold-identity/supersession work — separate tickets. GITS-354/358/398 are read-only evidence; no GITS ticket or project was written.Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
d9f95e1163GENERAL review, fresh context, independent of the premium peer. Read the diff, the tests, the docs, ticket HOMELAB-2521 and the GITS-354/358/398 evidence file; ran the suite in the worker worktree; no edit, no merge, no other PR.
Re-measured, all TRUE: targeted
tests/landing-blockers.test.ts30/30, full forgejo suite 258/258,tsc --noEmitclean, CI run #164 = combined success on this head with 14/14 contexts. All six evidence shapes now key correctly (GITS33-R8,GITS35-R1,GITS35-R2,HOMELAB-2408,GITS-R1, andGITS402-R1neverGITS-402), theABC-12separator control still reads and positionalBlocker 1still keys1. Token-boundary citation is right:resolved ABC-12does not citeABC-1,GITS35-R1does not cite1,GITS-402does not citeGITS402-R1.A differential replay of base
6518158against this head over the six evidence shapes plus ~30 shapes outside the evidence table found one fail-open and two fail-closed regressions introduced by the diff.Blocker — H2521-R1: a bare R-number alias anywhere in a line with a resolution word closes positional finding 1
BLOCKER OPEN: H2521-R1
The new alias arm
(?<![0-9A-Za-z_/-])R0*<n>(?![0-9])is the only citation mechanism incitesFinding()with no citation context: the numeric arm requires a prefix (blocker|finding|item|issue|#) immediately before the number and the labelled arm requires the id token itself, but the alias fires on any line that mentionsR1onceRESOLUTION_REhas matched anywhere. Measured end-to-end withfindOpenBlockers(finding### Blocker 1 — the guard only prints, one later comment):R1 is still open; the R2 guard fix is done.-> base 1 open, head 0 openR2 closed, R1 remains.-> base 1 open, head 0 openRound 2: R1 deferred, R2 resolved.-> base 1 open, head 0 openThe R1 arm is not done yet.-> base 1 open, head 0 openR1 still broken, R2 fixed-> base 1 open, head 0 openEach wrong closure is a merge the lock stops refusing (
landing.ts:1636-1651). None of these lines cites finding 1 and three of them say R1 is not fixed;doneis also absent fromRESOLUTION_NEGATED_RE, so "not done" reads as a resolution. Fix: make the alias the object of the resolution ((?:resolved|fixed|closed|done)\s*#?R0*<n>or a prefix(?:blocker|finding|item|issue)\s*R<n>) and add these five rows as negative tests.Blocker — H2521-R2: emphasis or a wide gap between the severity word and the id now yields no id at all
BLOCKER OPEN: H2521-R2
LABEL_GAP_REconsumes at most 4 characters, so three shapes read at base now return null:**Blocker** — ABC-12: …,### Blocker — **ABC-12**: …, and### Blocker — ABC-12: …(extra spaces). With a null idresolved ABC-12no longer closes the finding (measured: head 1 open, base 0), because an id-less finding needs the whole-bodyblocker … resolvedphrase. That is the ticket's own harm — an approved PR refused by a finding the reviewer has resolved — reintroduced for a heading style the corpus already uses (tests/landing.test.ts:116), just not yet with an id. Fix: skip emphasis characters and any separator run when reading the label span instead of capping the gap, and add the three rows toHEADINGS.Non-blocking concerns:
isGrammarCitation()gate on resolution lines reuses the verdict-line set, solooks like,for example,the grammar,e.g.and<sha>suppress honest resolutions —resolved GITS35-R1 — the log now looks like 4f2a1bcand three siblings stay open where base closed them. Narrow the gate to the backticked quote shape the tests exercise.s3 access is missing->S3,v2 rollout missing->V2,item2 is not covered->ITEM2,round2 drain returns 0->ROUND2; a later line containing the word plus a resolution word then closes the blocker. None is in the uppercase-prefix grammardocs/forgejo.mdnow publishes.resolved R1closes### Blocker 1and nothing else" is right about id selection but reads as if only a resolution closes it; per the first blocker, any line mentioningR1with a resolution word does.Correct by inspection, no action: the leading-label arm, the separator control rows, fence handling (both skips are
continues, so a fenced example still closes nothing), the id-less escape hatch, and the fail-closed direction for null ids.findingId()'s#<n>fallback still reads an id out of the body (… (see #74)->74) — identical at base, so not a blocker, but it is the same body-id principle the ticket asks to remove and deserves an owner.Out of scope, untouched and not reviewed: census
unpinnedrows (GITS-358 Verify 3), the missing required-context leg (GITS-368), the false "NO exact-head verdict" / "class unknown" labels (GITS-441).Evidence:
/home/aaron/repos/homelab/working-files/homelab-2518/HOMELAB-2521-review-deepseek.mdVerdict: APPROVED
model: omniroute/kimi-pool
head:
d9f95e1163HOMELAB-2521 / Homelab/pi-extensions PR #74 — premium review (kimi-pool)
Head reviewed:
d9f95e1163c6c5580c51f94e596f6908b08a1af1(fetched live fromrefs/pull/74/head— matches the requested exact head; no head movement).Diff:
forgejo/landing.ts(+82/−19 style rewrite offindingId, label-spanscoping, token citations,
R<n>alias, grammar-citation guard inisResolved),forgejo/tests/landing-blockers.test.ts(+192, 10 new cases),docs/forgejo.mdleg 5. Dangerous class (
forgejo/**) — this is the required premium leg.Independent verification (all re-run by this reviewer)
258/258;
tsc --noEmitclean.commit
6518158in a throwaway worktree → 8 failed / 22 passed (30),exactly the worker's claim, with the same failing assertions.
d9f95e1is 14/14 green (forgejo-ci, redact, pr-title, …).FINDING_ID_TOKENverified in source:[A-Z]{1,12}[0-9]{0,4}(?:-[A-Z]{0,8})?[0-9]{1,4}— matches the PRdescription's claimed grammar. Covers every evidence id:
ABC-12,HOMELAB-2408,GITS-R1,GITS33-R8,GITS35-R1/R2,GITS402-R1,B1.Correctness checks against the review brief
ABC-12separator controls,B1, both numbered shapes).**GITS402-R1 (Blocker) — body mentions GITS-402**keysGITS402-R1; a body-only id yieldsnull(id-less fallback, still blocks).R<n>alias:resolved R1closes### Blocker 1and only it;lookbehind
(?<![0-9A-Za-z_/-])refusesGITS35-R1,GITS-R1,/R1chains (all probed live).
resolved R2against finding 1 stays open.ABC-12does not citeABC-1(digit lookahead);resolved GITS-402does not closeGITS402-R1, and — probed —resolved GITS402-R1does not close a hypotheticalGITS-402finding (not asubstring), so the evidence recurrence pair cannot cross-close.
not resolved/unresolved), fenced, grammar-quote(
e.g. \resolved X``), bare-digit-vs-labelled, prose-id cases all keep thefinding open. Verified both in the suite and by direct probing.
id-less fallback and its documented phrase escape hatch are preserved and
tested; the new grammar-citation guard in
isResolvederrs closed.Non-blocking concerns
LABEL_GAP_REallows at most 4 separator chars, so a heading withextra whitespace (
### Blocker — GITS33-R8) loses its label id and fallsback to id-less (fail-open, still blocks; old code also returned null here).
Cosmetic robustness, not a regression.
R<n>alias is case-insensitive, so lowercaser1in proseco-occurring with a resolution word could cite positional finding 1.
Contrived; only affects numbered findings; direction is fail-closed.
e.g./for example/the grammarwill not close the finding(fail-closed; reviewer rephrases). Safe direction, documented class.
Verdict
No blockers. The fix is correct on the evidence harness, conservative where it
is heuristic, and all claims (red 8/22, green 30/30, 258/258, tsc, CI #164
14/14) reproduced independently. Approve.
Verdict: REQUEST_CHANGES
model: omniroute/kimi-pool
head:
d9f95e1163HOMELAB-2521 / Homelab/pi-extensions PR #74 — premium review (kimi-pool), corrected verdict
Head reviewed:
d9f95e1163c6c5580c51f94e596f6908b08a1af1(fetched live fromrefs/pull/74/headbefore posting — matches the requested exact head; no headmovement). Diff:
forgejo/landing.ts(+82/−19),forgejo/tests/landing-blockers.test.ts(+192),
docs/forgejo.md. Dangerous class (forgejo/**) — this is the requiredpremium leg. Supersedes my earlier APPROVED on this head: after posting it I
verified the DeepSeek general reviewer's two reproductions end-to-end and they
are genuine fail-open/fail-closed regressions introduced by this diff.
Independent verification (all re-run by this reviewer)
tsc --noEmitclean.6518158→ 8 failed / 22 passed.d9f95e1: 14/14 green.FINDING_ID_TOKENverified in source:[A-Z]{1,12}[0-9]{0,4}(?:-[A-Z]{0,8})?[0-9]{1,4}.shown below with live
findOpenBlockersoutput at this head.What the fix gets right
All eight evidence headings key correctly (
GITS33-R8,GITS35-R1/R2,HOMELAB-2408,GITS-R1,GITS402-R1neverGITS-402,Blocker 1→1);label-vs-body scoping works; token citations (
ABC-12∌ABC-1,GITS-402does not closeGITS402-R1, andGITS402-R1does not close ahypothetical
GITS-402finding); theR<n>lookbehind refusesGITS35-R1,GITS-R1,/R1; fence/negation/prose/bare-digit negatives hold; the id-lessfallback and its phrase escape hatch are preserved.
Blocker — H2521-K1: a bare
R<n>alias anywhere on a line with a resolution word closes positional finding n — even when the line says it is still openBLOCKER OPEN: H2521-K1
The alias arm
(?<![0-9A-Za-z_/-])R0*<n>(?![0-9])is the only citation armwith no citation context: it fires on any line where
RESOLUTION_RE(
resolved|fixed|closed|done, matched ANYWHERE) and a bareR1tokenco-occur. Verified at this head against finding
### Blocker 1 — the guard only prints, one later comment — base (6518158) keeps 1 open, head closes it(0 open) for all three:
R1 is still open; the R2 guard fix is done.The R1 arm is not done yet.R1 still broken, R2 fixedThe first line literally states R1 is open; the second says "not done yet",
which
RESOLUTION_NEGATED_REcannot see becausedoneis not in its wordset. A genuine open blocker disappears — the exact harm class HOMELAB-2521
exists to eliminate, reintroduced in the fail-open direction: the lock stops
refusing a merge with an open finding. (This supersedes my earlier C2, which
wrongly called this contrived.)
Fix: make the alias the object of the resolution (e.g.
(?:resolved|fixed|closed|done)\s*:?\s*#?R0*<n>or a citation prefix(?:blocker|finding|item|issue)\s+R<n>), adddoneto the negated set, andadd the three lines above as negative tests. Confirms the peer's H2521-R1.
Blocker — H2521-K2: emphasis or a wide gap between the severity word and the id strands the finding —
resolved ABC-12no longer closes itBLOCKER OPEN: H2521-K2
LABEL_GAP_REconsumes at most 4 separator characters, so three label shapesthat keyed
ABC-12at base now return null at head (verified):**Blocker** — ABC-12: the guard only prints→ null### Blocker — **ABC-12**: the guard only prints→ null### Blocker — ABC-12: the guard only prints→ nullEnd-to-end at head: finding
**Blocker** — ABC-12: …+ laterresolved ABC-12→ 1 open (id-less; the phrase fallback does not match), where baseclosed it (old pattern 3 read the token anywhere). The emphasis shape is in
the live corpus —
tests/landing.test.ts:116uses**Blocker** — branch matching …. This is the ticket's own harm (an honest resolution cannot cleara finding) reintroduced for a heading style already in use, just with an id.
(Supersedes my earlier C1, which called it cosmetic.)
Fix: strip emphasis characters and allow an unbounded separator run in the
label gap, and add the three rows to HEADINGS. Confirms the peer's H2521-R2.
Non-blocking concerns
gate in
isResolvedreuses the verdict-line set, so honest resolutionscontaining
e.g./for example/looks like/the grammardo notclose. Safe direction; worth narrowing to the backticked-quote shape.
(
s3 access is missing→S3) where base returned null — mostly harmlesskeying noise, but it widens the citation surface.
Verdict
REQUEST_CHANGES. The evidence-harness fix is real and verified, but the diff
introduces one fail-open (H2521-K1: an open blocker can disappear from a
still-open status line) and one fail-closed regression (H2521-K2: an honest
resolved ABC-12can no longer close an emphasis-labelled finding). Both arecheap to fix and must carry negative tests before this lands.
Round 2 — new head
b8ed55d367011cdf8ebcdd98da947e51bcc4be04Both exact-head reviews on
d9f95e1(general 19629 = H2521-R1/R2/C1/C2, premium 19631 = H2521-K1/K2) name the same two regressions; this commit fixes them plus the three concerns, with every reviewer replay kept as a suite row.Red, before any code change in this round
On the round-1 head
d9f95e1(tests added, implementation untouched):Two more after the K1 review, on the already-R/C-fixed tree:
The second is K1's residual:
donewas absent fromRESOLUTION_NEGATED_RE, soblocker 1 is not doneclosed finding 1 through the ordinary numeric arm — a fail-open older than the alias.Green
Fixes
POSITIONAL_ALIAS_CITATION_RES+citesPositionalAlias: the alias must be the OBJECT of the resolution (resolved R1,resolved: R1,R1 is resolved,R1 fixed in <sha>,resolved R1 and R2) or follow a citation prefix (finding R1,blocker R1). All five replays (R1 is still open; the R2 guard fix is done.,R2 closed, R1 remains.,Round 2: R1 deferred, R2 resolved.,The R1 arm is not done yet.,R1 still broken, R2 fixed) are negative rows.doneadded toRESOLUTION_NEGATED_RE(not done,not fixed,never done,undoneresolve nothing).LABEL_GAP_REis an unbounded separator/emphasis run that still stops at the first letter or digit:**Blocker** — ABC-12,### Blocker — **ABC-12**,### Blocker — ABC-12,#### Blocker – __GITS33-R8__key their id, andresolved <id>closes them.quotesResolutionExample()replacesisGrammarCitationon resolution lines: only a backticked span that is itself a resolution of an id is treated as the rule.looks like/for example/the grammar/<sha>in an honest resolution no longer suppress it, and a quote with noe.g.marker (which closed the finding at the round-1 head) now does.FINDING_ID_TOKENmatched case-sensitively — the documented uppercase prefix is required, sos3/v2/item2/round2are prose and carry no id.docs/forgejo.mdleg 5: citation requirement on the alias, emphasis/separator forms, uppercase prefix,donein the negated set, the narrow quote gate, and the pre-existing#12body read.Differential replay (base
6518158vsd9f95e1vs this head)15 heading shapes x 41 resolution lines x 8 ids, three versions loaded side by side in
/tmp(nothing written in the repo):findingId: round 2 differs from round 1 only where round 1 lost the id (emphasis / wide gap) or keyed prose (s3,item2); every evidence shape and everyABC-12separator control is unchanged.resolved R1,R1 is resolved,resolved: R1,resolved R01,resolved R1 and R2); nothing round 1 closed for a good reason is lost.["1"], as at base);**Blocker** — ABC-12+resolved ABC-12closes (base closed it, round-1 head did not);write \Blocker R1 resolved` to close it` now leaves finding 1 open where base and round-1 both closed it.Two fresh exact-head verdicts are required on
b8ed55d. This worker did not dispatch reviewers, post a verdict, merge, or mark the ticket Done.Fix round — head
b8ed55d367011cdf8ebcdd98da947e51bcc4be04resolved H2521-R1 — the positional alias now needs citation context: it must be the object of a resolution, or follow a citation prefix.
resolved H2521-R2 — the label separator/emphasis run is unbounded, so emphasis and wide-gap headings key their id and a citation closes them.
resolved H2521-K1 — same defect as H2521-R1, plus the negated set now covers the word done.
resolved H2521-K2 — same defect as H2521-R2; an emphasis-labelled finding is closed by a citation of its id again.
The four blockers this round fixes are resolved on this head; two fresh exact-head reviews are still required.
Red, before any code change in this round
13 cases failed on the round-1 head, then 2 more for the K1 residual:
Green on
b8ed55dCI run #166 on this head: 14/14 contexts pass (
forgejo,forgejo-ci,pr-title, …).What changed
POSITIONAL_ALIAS_CITATION_RES+citesPositionalAlias: the alias must be the object of the resolution (resolved R1,resolved: R1,R1 is resolved,R1 fixed in <sha>,resolved R1 and R2) or follow a citation prefix (finding R1,blocker R1). All five reviewer replays are negative rows.doneadded toRESOLUTION_NEGATED_RE;not done/not fixed/never doneresolve nothing.LABEL_GAP_REis an unbounded separator/emphasis run that still stops at the first letter or digit.quotesResolutionExample()replacesisGrammarCitationon resolution lines: only a backticked span that is itself a resolution of an id counts as the rule.FINDING_ID_TOKENmatched case-sensitively — the documented uppercase prefix is required.docs/forgejo.mdleg 5 states all of it, including the pre-existing#12body read.Differential replay over 15 heading shapes x 41 resolution lines x 8 ids (base
6518158/ round-1 headd9f95e1/ this head, loaded side by side): no round-1 behaviour lost; every difference is the intended R1 fix or a citation the ticket asks for. This worker did not dispatch reviewers, post a verdict, merge, or mark the ticket Done.Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
b8ed55d367HOMELAB-2521 / PR #74 — independent GENERAL review, ROUND 2 (fresh context)
Whole diff reviewed against base
651815802bcc2e7df40b41646c4fcb641685efe4(not only the round-2 delta):docs/forgejo.md,forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts. Live head re-fetched fromrefs/pull/74/headbefore review. Read-only: no edit, no merge, no dispatch, no other PR, no formal review state.Claims re-measured independently — all TRUE
tests/landing-blockers.test.ts47/47tsc --noEmitcleanCI / *success forb8ed55dRound-1 blockers — the resolutions are REAL on this head
Replayed end-to-end with
findOpenBlockers(finding### Blocker 1 — the guard only prints, one later comment), base and headlanding.tsbundled separately and loaded side by side.["1"], exactly as at base):R1 is still open; the R2 guard fix is done.,R2 closed, R1 remains.,Round 2: R1 deferred, R2 resolved.,The R1 arm is not done yet.,R1 still broken, R2 fixed. Nine further mention-only variants also stay open (R1 not done,R1 isn't fixed yet,R1 pending; R2 resolved and merged,R1 unresolved, R2 fixed, …).doneis inRESOLUTION_NEGATED_RE, so K1's residual (blocker 1 is not doneclosing through the ordinary numeric arm) is closed too — the worker's claim that this predates the alias is correct. All fourteen documented citation shapes still close (resolved R1 — closed.,R1 is resolved.,R1 fixed in 4f2a1bc.,finding R1 fixed,resolved: R1,resolved R1 and R2, …).LABEL_GAP_REis now an unbounded separator/emphasis run. The three review-19629 rows keyABC-12again, plus**Blocker** — **ABC-12**,#### Blocker – __GITS33-R8__→GITS33-R8,### Blocker — *GITS35-R1*→GITS35-R1, andresolved <id>closes each. A 40-dash run, a 50-asterisk run and an 80-space run still key the id, and a body-only id still yieldsnull.looks like/for example/the grammar/<sha>now close; quoted examples do not.s3,v2,item2,round2yieldnull;S3,V2,B1,GITS35-R1still key.Evidence keying unchanged and correct:
GITS33-R8,GITS35-R1,GITS35-R2,HOMELAB-2408,GITS-R1,GITS402-R1(neverGITS-402),B1, positionalBlocker 1→1,### 1. BLOCKER→1;blockerIdsInagrees withfindingIdon all six evidence headings. Differential over all four live fixtures (w3-lane 41 comments, fleet-verdict 8, pr70 2, pr694 3) shows zero open-blocker differences between base and head.Blocker — H2521-R3: the alias-as-subject arm closes positional finding n on a line that says the fix did not hold
BLOCKER OPEN: H2521-R3
POSITIONAL_ALIAS_CITATION_RES[1]accepts<alias> (is|was|are|were|has been|have been)? <resolved|fixed|closed|done>with nothing required after the verb. A transitive verb with an object, or a denial later in the same line, therefore still reads as a resolution, andRESOLUTION_NEGATED_REonly seesnot|un|never. Measured end-to-end (finding### Blocker 1 — the guard only prints, one later comment):6518158R1 was fixed but the guard still prints.R1 fixed in round 1, still prints.R1 closed the loop but left finding open.R1 resolved in d9f95e1 only for the bare mention; K2 still strands.R1 is resolved? no, still open.R1 closed as duplicate of R2.Each line states that the finding is live, in the ordinary review register the fleet writes, and each one removes the finding. That is the K1 harm in a narrower shape: leg 4 refuses a merge only while
findOpenBlockersreturns a finding, so a wrong closure is a merge the lock stops refusing — and main closed none of these lines. Round 1 regressed on a bare mention; round 2 regresses only when the alias is the subject of a resolution verb, which is still the same defect class.Fix: make the subject form clause-terminal — accept only end-of-clause, punctuation, an
in <ref>phrase, or anand R<k>chain after the verb (that keeps the documentedR1 fixed in <sha>andR1 is resolved), and reject a following determiner/noun or a later?/but/still. Add the six rows above as negative tests.Non-blocking concerns
quotesResolutionExample()suppresses the WHOLE line when any backticked span is a resolution of some id, so an honest resolution that merely quotes the grammar stops closing:resolved ABC-12 (unlikeresolved GITS35-R1)→ 1 open at head, 0 at base. Suggest preferring the line's own unquoted text when a citation exists outside the span.undoneis read by theunbranch, so a double-negative resolution stops closing:resolved ABC-12 — nothing was left undoneandfixed ABC-12 — not undone→ open at head, closed at base. Thedoneaddition to the negated set is what enables it; considerunresolved|unfixedexplicitly rather than a bareun.citesFindingalso accepts the alias as SUBJECT (and the same paragraph listsR1 is resolved,R1 fixed in <sha>). The sentence hides exactly the gap H2521-R3 lands in; say "object of a resolution, or subject of a completed one".For example, a line saying resolved ABC-12 closes it.) closes at both base and head.Blocker R1 still prints, R2 fixedcloses finding 1 at BOTH base and head; andfindingId()'s#(\d{1,4})body read survives (### Blocker — the guard only prints (see #74)→74), now documented as the one exception. Both worth an owner ticket, neither introduced here.Conclusion
REQUEST_CHANGES — one new blocker (H2521-R3, fail-open on the alias subject arm) plus five non-blocking concerns, two of them new fail-closed edges. The round-1 blockers are genuinely resolved and every worker claim reproduces; the evidence-shape fix itself is right.
b8ed55d3675a45984e21Fix round — head
39a2f31a632669a804bdf61fca8380587e8d0860resolved H2521-R3 — the alias-as-SUBJECT arm is now clause-terminal: after the resolution verb only a reference phrase, a restated resolution, an adverb or a chain naming another finding may follow, and then the full stop or the end of the line.
Round 2 raised 19656 (general, REQUEST_CHANGES) on the previous head. This round fixes H2521-R3 and the three evidenced non-blocking concerns (C4, C5, C6/C7) in one commit, adds the object-form chain shape of the same defect class as a second commit, and keeps every round-1/round-2 fix and the whole evidence corpus. The branch was rebased onto current
origin/main(fd410c3, PR #75 — no overlapping files) before the review, so this head contains nothing but this ticket's three files.Red, before any code change in this round
On the rebased round-2 head (
c9b9aec, tests added, implementation untouched):Failing rows were exactly the round-3 cases:
keeps positional finding 1 open when the line says the fix did not hold—review 19656: `but` continues the clause: R1 was fixed but the guard still prints.: expected true to be false(14 denial rows; the first fails first, the rest fail the same way)does not let the completed-clause rule cite the WRONG positional finding—expected [ '2' ] to deeply equal [ '1', '2' ]closes when the citation is outside the quoted span(C4) —resolved ABC-12 (unlike ...): expected [ 'ABC-12' ] to deeply equal []closes on nothing was left undone(C5) —expected [ 'ABC-12' ] to deeply equal []Then, for the chain shape, 2 more on the round-3a head:
Green on
39a2f31What changed
objectAliasCitations()walks the chain instead of a greedy regex: the LAST alias of a chain must also end its clause, so the aliases before it are cited and a retracted tail is not.fixed R2, R1 remains broken/resolved R2, R1 still openclose R2 and leave R1;resolved R1; still broken.andresolved R1 — actually the guard still prints.close nothing;resolved R1 and R2andresolved R1, R2 still openbehave exactly as documented.resolved ABC-12 (unlike a quoted resolution of another id)closed at base, stopped closing in round 2). A line whose only resolution is the quoted example still closes nothing.undoneis not a negation: theunbranch is restricted to `unresolveddocs/forgejo.mdleg 5 no longer says the alias must be the OBJECT only: it is the object of a resolution, or the subject of a COMPLETED one, and the completed-clause rule is spelled out with the six replays.e.g.— unquoted grammar prose does close a finding whose id it names; only the quoted shape is excluded.Every documented citation shape still closes: the 14 the round-2 review measured (
resolved R1 — closed.,Resolved R1.,R1 is resolved.,R1 fixed in 4f2a1bc.,finding R1 fixed,Blocker R1 resolved,resolved blocker R1,resolved #R1,resolved: R1,Resolved: #R1.,R1 has been closed — see 4f2a1bc,resolved R1 and R2,R1 resolved, R2 still open,Fixed: R1) is a table row in the suite, and so are ten completed subject clauses.Differential replay (base
fd410c3/ round-2 headc9b9aec/ this head)Three builds loaded side by side in
/tmp/rv2521r3(nothing written in the repo): 18 heading shapes x 55 resolution lines x 10 ids, plus the four live fixtures replayed as 13 streams.findingId: identical to the round-2 head on every shape (the id work is untouched this round).Reported, not fixed here
The retraction gap on the NUMERIC and LABELLED arms predates this diff (
ABC-12 was fixed but the guard still prints.closes a labelled finding at base too) and is a separate owner line, alongside HOMELAB-2548 (body#idread) and HOMELAB-2549 (numeric optional-letter alias). This round stays inside the alias citation grammar.Two fresh exact-head reviews are still required on
39a2f31a632669a804bdf61fca8380587e8d0860. This worker did not dispatch reviewers, post a verdict, merge, or mark the ticket Done, and neither old review was edited.CI is green on the final head
39a2f31a632669a804bdf61fca8380587e8d0860: run #169 = pass, 14/14 contexts (forgejo1m24s,forgejo-ci53s,pi-swarm-suite3m48s,pr-title). No further code changes were made after it; this head is the stable review target. Two fresh exact-head verdicts are still required — this comment is a CI readout, not a verdict.Verdict: REQUEST_CHANGES
model: omniroute/kimi-pool
head:
39a2f31a63HOMELAB-2521 / PR #74 — premium review (kimi-pool), ROUND 2 (final head)
Head reviewed:
39a2f31a632669a804bdf61fca8380587e8d0860(round 3b; liverefs/pull/74/headre-fetched multiple times during this review — the head movedb8ed55d→5a45984→39a2f31a; this verdict pins only the final live head).Base: merge-base with live main =
fd410c3ce8968a73794e6304fbc618a653f66859;PR diff vs base = the three HOMELAB-2521 commits only (
landing.ts,tests/landing-blockers.test.ts,docs/forgejo.md— the api.ts/index.ts/verdict-cli.ts delta in the branch is main's#75, not this PR). Dangerous class (
forgejo/**) — premium leg. Read-only: noedit, no merge, no dispatch, no other PR, no formal review.
Worker claims on 3b — independently re-measured, all TRUE
tests/landing-blockers.test.ts60/6039a2f31a)tsc --noEmitclean39a2f31aPrior findings — verified resolutions on this head (end-to-end
findOpenBlockers, /tmp harness, basefd410c3vs head bundled side by side)blocker 1 is not donekeep finding 1 open (["1"], as at base).**Blocker** — ABC-12,### Blocker — **ABC-12**, 80-space run,__GITS33-R8__) key their id andresolved <id>closes them.R1 was fixed but the guard still prints.,R1 fixed in round 1, still prints.,R1 closed the loop but left finding open.,R1 resolved in d9f95e1 only for the bare mention; K2 still strands.,R1 is resolved? no, still open.,R1 closed as duplicate of R2.) keep finding 1 open, and all fourteen documented citation shapes still close (R1 fixed too.,R1 closed already.,R1 is resolved — closed.,R1 has been closed — see 4f2a1bc.,R1 resolved, R2 still open., …).fixed R2, R1 remains brokenandresolved R2, R1 still openkeep finding 1 open (chain walk: the last alias must end its clause;resolved R1 and R2still closes both).resolved R1; still broken.,resolved R1 — actually the guard still prints.,R1 is done, but follow-up shows it regressed.,R1 was fixed in staging but not in prod.keep finding 1 open.looks like/for example/<sha>resolutions close;s3/v2/item2prose keys nothing). C4/C5 — resolved (resolved ABC-12 (unlike \resolved GITS35-R1`)closes ABC-12;— nothing was left undone/— not undone` close). C3/C6/C7 — addressed in leg 5.#74body read: all correct and unchanged from round 2.Blocker — H2521-K5: a chain still closes a finding its own line retracts — non-last chain aliases and post-chain retractions are unguarded
BLOCKER OPEN: H2521-K5
3b's chain walk protects only the LAST alias of a chain and only via the tail
immediately after it. Measured end-to-end on this head (finding
### Blocker 1 — the guard only prints, one later comment) — every line closesfinding 1 (head 0 open, base 1 open) while stating R1 is NOT fixed:
39a2f31aresolved R1 and R2 but R1 regressed in prod.resolved R1, R2 and R3; R1 later reverted.R1 resolved, R2 still open; scratch that, R1 is still broken.R1 resolved, R2 fixed, but R1 was reverted.Root causes (two faces of one defect): (1) in
objectAliasCitations, when thetail does not complete,
chain.slice(0, -1)cites the earlier aliases with noscan of the tail for a retraction that names one of them; (2)
isCompletedAliasClausereturns true at the FIRST
ALIAS_CHAIN_REmatch without requiring the remainderafter that chain segment to be terminal, so any
, Rn/; Rnlater in the linecompletes the clause no matter what follows.
R1 resolved, R2 still open.closing R1is documented and correct — but that is because the remainder names a DIFFERENT
finding; when the remainder names R1 again with retraction words
(
regressed,reverted,still broken,scratch that), the finding it names isopen, and the lock stops refusing its merge. Same harm class as R1/K1/R3, same
heuristic the worker has now narrowed three times; the uniform fix is to cancel a
citation when the line's remainder re-names the cited alias with retraction wording
(rather than enumerating clause shapes), plus requiring terminal-after-chain-segment.
Add the four rows above as negative tests.
Non-blocking concerns
(
blocker 1 is not fixed→["1","1"], head and pre-fix base alike) — fail-closednoise for the census count, pre-existing machinery.
ABC-12 was fixed but still prints.)pre-dates this PR (base behaves the same); the PR extends that grammar to the
newly-keyed evidence ids — fold into the K5-style guard if one is built.
Verdict
REQUEST_CHANGES — one blocker (H2521-K5, chain-internal/post-chain retraction),
which is the last open fail-open in the alias citation path: every other finding
from rounds 1–2 and the peer's R3 is verified resolved on this head, and all
worker claims (60/60, 305/305, tsc, CI #169 14/14) reproduce independently.
Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
39a2f31a63Independent GENERAL review (fresh context), exact head
39a2f31a632669a804bdf61fca8380587e8d0860, basefd410c3ce8968a73794e6304fbc618a653f66859. Liverefs/pull/74/headre-fetched before review: no head movement.Whole-PR diff reviewed (three files:
forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md), not only the last commit.Worker claims — independently re-measured, all TRUE
vitest run tests/landing-blockers.test.ts→ 60 passed (60)tsc --noEmit→ exit 0successPrior blockers — every one verified resolved on this head
Ran
findOpenBlockersfrom base and head builds side by side (repo's own esbuild, nothing written in the repo),finding
### Blocker 1 — the guard only printsplus one later comment.R1 is still open; the R2 guard fix is done.,R2 closed, R1 remains.,Round 2: R1 deferred, R2 resolved.,The R1 arm is not done yet.,R1 still broken, R2 fixed,R1 not done,R1 isn't fixed yet,R1 pending; R2 resolved and merged,R1 unresolved, R2 fixed, and the residualblocker 1 is not done.resolved <id>closesthem:
**Blocker** — ABC-12,### Blocker — **ABC-12**,### Blocker — ABC-12,**Blocker** — **ABC-12**,#### Blocker – __GITS33-R8__,### Blocker — *GITS35-R1*,### Blocker —ABC-12``, a 40-—run, a 50-*run, an 80-space run. A body-only id still keys null.
R1 was fixed but the guard still prints.,R1 fixed in round 1, still prints.,R1 closed the loop but left finding open.,R1 resolved in d9f95e1 only for the bare mention; K2 still strands.,R1 is resolved? no, still open.,R1 closed as duplicate of R2.including
R1 fixed in 4f2a1bc.,R1 resolved at PR 74.,R1 closed by 4f2a1bc.,R1 has been closed — see 4f2a1bc,R1 is resolved — closed.,R1 closed already.,R1 fixed too.,resolved R1 and R2,R1 resolved, R2 still open,Fixed: R1,resolved: R1,resolved #R1,finding R1 fixed,blocker R1 resolved,resolved blocker R1.resolved ABC-12 (unlikeresolved GITS35-R1)closes ABC-12 again; a line whose onlyresolution is the quoted example still closes nothing.
resolved ABC-12 — nothing was left undoneandfixed ABC-12 — not undoneclose again.resolved (
fixed R2, R1 remains brokenandresolved R2, R1 still openleave finding 1 open).GITS33-R8,GITS35-R1,HOMELAB-2408,GITS-R1,GITS402-R1now key whole (base keyedGITS33,GITS35, null, null,GITS402);Blocker 1→1,### 1. BLOCKER→1,B1→B1.ABC-12 was fixed but the guard still prints.,ABC-12 is fixed, but it still throws.,blocker 1 was fixed but still prints.behave identically at base and head. Correctly left toHOMELAB-2548/2549/2551.
Blocker — H2521-R4: the chain walk still cites a finding its own line retracts, so a denial closes the lock
BLOCKER OPEN: H2521-R4
One mechanical fact in
objectAliasCitationsand one inisCompletedAliasClause: a chain guards only its LASTalias, and only through the tail that immediately follows it; and the chain-continuation test returns true at the
FIRST
, Rn/; Rnsegment without requiring the remainder after that segment to be terminal. Measuredend-to-end, base
fd410c3vs head39a2f31:resolved R1 and R2 but R1 regressed in prod.— base 1 open, head 0 openresolved R1, R2 and R3; R1 later reverted.— base 1 open, head 0 openR1 resolved, R2 still open; scratch that, R1 is still broken.— base 1 open, head 0 openR1 resolved, R2 fixed, but R1 was reverted.— base 1 open, head 0 openR1 fixed, R2 fixed, R1 regressed.— base 1 open, head 0 openR1 fixed and R2 fixed but R1 came back.— base 1 open, head 0 openR1 resolved. Actually no, still open.— base 1 open, head 0 openA wider sweep of 18 denial lines gives 13/18 staying open:
resolved R1 and R2, but R1 regressed.,resolved R1, R2, and R1 reverted.,R1 resolved. It still prints though.,R1 fixed. Not really though.alsoclose the finding. Every one states the finding is live.
landing.ts:1717-1740refuses a merge only whilefindOpenBlockersreturns a finding, so a wrong closure is a merge the lock stops refusing — the K1/R3 harm in theother direction, and a regression against base, which closed none of these lines.
Fix direction (one guard, not more clause enumeration): after a chain, require the whole remainder from that chain
segment to the end of the clause to be terminal; and cancel a citation when the line's remainder re-names the cited
alias with retraction wording (
regressed|reverted|remains|still|scratch that|came back|broke again|reopened),including when the retraction follows a full stop in the same line. Add the rows above as negative tests.
Non-blocking
R1 resolved. It still prints though.) share the R4 fix; if the full stop is keptstrictly terminal they should at least be documented as a known edge in leg 5 rather than left silently open.
(
R1 is now resolved.,R1 is now fixed.,R1 has now been closed.,R1 got fixed.,R1 is finally resolved.,R1 appears resolved.— all open at base and head).POSITIONAL_ALIAS_SUBJECT_REallows only an auxiliary beforethe verb, while the code comment advertises adverbs, which work AFTER the verb (
R1 closed already.). Safedirection; worth an owner line.
blocker 1 is not fixed→["1","1"]) is fail-closed census noise, base identical.finding grammar's order, so the parser lists it as an OPEN finding named after that id (
GITS402-R1), beside thereal ones. Fail-closed and harmless for safety, but it inflates the open count the lock reports; worth rewording
in the worker comment (I do not edit a posted comment).
Verdict
REQUEST_CHANGES. One blocker open at this exact head. Every earlier blocker id is genuinely resolved, the
terminal rule's fail-closed side is sound (26/26 documented citations close, no documented shape rejected), and
all worker claims reproduce — but the fail-open side still loses a finding whose own line retracts it, and no test
row in the 60 covers those shapes. No code was edited, nothing merged, no formal review submitted, no other PR
touched.
Fix round 4 — H2521-K5 and H2521-R4: the R alias is cited only as a completed resolution object
resolved H2521-K5
resolved H2521-R4
Head:
269d27e22d394cc332229f6d207010b93343f468onplane/HOMELAB-2521-blocker-ids(one new commit,
269d27e, on top of the reviewed39a2f31a; basefd410c3, norebase needed —
origin/mainhas not moved). CI run #171 on269d27e22d= pass,14/14 contexts.
What the two verdicts on
39a2f31ameasuredBoth reviews found the same defect: the chain walk guarded only the LAST alias and
only through the tail immediately after it, and a
, R2/; R2chain segmentcompleted the clause whatever followed — so 13 retraction lines closed positional
finding 1 while saying it is not fixed (K5:
resolved R1 and R2 but R1 regressed in prod.,resolved R1, R2 and R3; R1 later reverted.,R1 resolved, R2 still open; scratch that, R1 is still broken.,R1 resolved, R2 fixed, but R1 was reverted.;R4 adds
R1 fixed, R2 fixed, R1 regressed.,R1 fixed and R2 fixed but R1 came back.,R1 resolved. Actually no, still open.and five wider-sweep rows).The fix is removal, not another retraction regex (owner decision on HOMELAB-2521)
The alias grammar is narrowed to the original GITS-358 object-of-resolution form
and nothing else:
resolved R1,Resolved: R1.,resolved: R1,Resolved: #R1.,resolved #R1,resolved blocker R1,Fixed: R1, and the completedcontinuations
resolved R1 — closed.,resolved R1 in 4f2a1bc.;R1 is resolved.,R1 fixed in 4f2a1bc.,R1 is resolved — closed.) and every CHAIN (resolved R1 and R2,R1 resolved, R2 still open,fixed R2, R1 remains broken); a chain citesnothing, so each finding needs its own completed
resolved R<n>;isCompletedAliasClauseno longer treats a chain segment as terminal, soresolved R1 and R2 but R1 regressed in prod.cites nothing at all;POSITIONAL_ALIAS_CHAIN_STEP_RE,POSITIONAL_ALIAS_PREFIX_RE,POSITIONAL_ALIAS_SUBJECT_RE,ALIAS_CHAIN_RE.Every retraction, subject and chain form now leaves the finding open unless
another accepted citation resolves it.
docs/forgejo.mdsays so explicitly: thoseforms are DELIBERATELY NOT ACCEPTED, with the reason (a line-based reader cannot
tell which alias the rest of the sentence is about) and the spelling to use
(
resolved R1).The citation-prefix form (
finding R1 fixed,Blocker R1 resolved) still closesfinding 1 — through the NUMBERED arm, unchanged by this PR, which reads
finding R1exactly as it readsfinding 1. Documented as such.Preserved and re-measured: the
resolved R1family, every labelled-id form, thenumbered arm, round-1/2 evidence fixes (R1/K1 mention safety, R2 emphasis keying,
C1, C2, C4, C5), C3/C6/C7 docs coverage, and the real corpus (w3-lane /
fleet-verdict fixtures, zero differential). Separately owned HOMELAB-2548/2549/2551
grammars untouched.
Red first, on
39a2f31a(implementation unchanged at run time)Tests were written and run before
landing.tswas edited:8 failed | 56 passed (64) — every new K5/R4 replay row and every new
subject/chain rejection assertion red, plus the two arm-removal assertions.
Green
tests/landing-blockers.test.ts— 64 passed (64);tsc --noEmit— exit 0;269d27e22d— 14/14 success.Independent differential, base
fd410c3vs head269d27e(bundled /tmp harness)All 13 K5/R4 retraction rows:
["1"]at base and["1"]at head (nofail-open, no regression).
R1 is resolved.,R1 fixed in 4f2a1bc.,R1 resolved, R2 still open,resolved R1 and R2:["1"]at both.finding R1 fixed/Blocker R1 resolved:[]at both (numbered arm). All 9accepted object forms close (
[]) at head. Two findings + a retracting chain →["1","2"]; two findings +resolved R1.resolved R2.→[].No reviews were edited or deleted, no verdict was posted, nothing was merged, and
no other PR was touched. The two exact-head holds on
39a2f31astand as written;fresh verdicts on
269d27e22dare the reviewers'.269d27e22d526e3ac358Rebase onto current main — rebase evidence only, NOT a verdict
This is not a review, approval, or hold. It records a rebase so reviewers can work on a current base. No blocker ids added or resolved.
New full head:
526e3ac358852a2e9070b8b763782f5607dc28f8Pre-rebase head:
269d27e22d394cc332229f6d207010b93343f468Now based on
origin/main0317c29987657e18c106b33a6d5723f306354132(PR #76 / HOMELAB-2526 landed).forgejo/api.ts,index.ts,types.ts,tests/fake.ts,tests/merge-identity.test.ts— no overlap with this branch.forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md(3 files, +1133/-20).Tests on
526e3acvitest run tests/landing-blockers.test.ts→ 64 passed (64)vitest run(full forgejo suite) → 13 files, 334 passed (334), now including #76'smerge-identity.test.tstsc --noEmit→ clean (exit 0)526e3ac358→ pass, 14/14 jobs greenHead is stable; no further pushes from this worker.
Verdict: REQUEST_CHANGES
model: omniroute/deepseek-flash
head:
526e3ac358Independent GENERAL review (fresh context), model omniroute/deepseek-flash, the other required leg is the premium one.
Head:
526e3ac358852a2e9070b8b763782f5607dc28f8(the rebased liverefs/pull/74/head).Base:
0317c29987657e18c106b33a6d5723f306354132(livemain, merge-base; PR #76). Diff against base is exactly three files:forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md.I began on the pre-rebase head
269d27e22d394cc332229f6d207010b93343f468;origin/mainthen advancedfd410c3->0317c299and PR #74 was rebased. I re-pinned and re-measured526e3ac. The patch is byte-identical:git diff 269d27e 526e3ac -- <those three files>is empty, and every finding below reproduces identically on both. Read-only: no edit, no merge, no dispatch, no other PR, no formal review state.Worker claims — independently re-measured on this head, all TRUE
tests/landing-blockers.test.ts-> 64 passed (64)mainadded the merge-identity suite)tsc --noEmit-> exit 0526e3ac-> pass, 14/14 contexts39a2f31awith the implementation unchanged; the eight failing test names are the round-4 rowsPrior blocker ids — every one verified resolved for the enumerated rows
["1"], as base), includingR1 still broken, R2 fixed,The R1 arm is not done yet.,R2 closed, R1 remains., and the residualblocker 1 is not done.resolved <id>closes them (**Blocker** — ABC-12,### Blocker — **ABC-12**, the wide-gap form,__GITS33-R8__); a body-only id still keys null.R1 was fixed but the guard still prints.,R1 is resolved? no, still open., ...) stay open.undone): honest unquoted citations close again.GITS33-R8,GITS35-R1,HOMELAB-2408,GITS-R1,GITS402-R1(neverGITS-402), positional finding 1 ->1,B1, and the numbered### 1. BLOCKER->1;s3/v2prose -> null whileS3/V2still key.ABC-12 was fixed but the guard still prints.,blocker 1 was fixed but still prints.); the separately owned HOMELAB-2548/2549/2551 grammars are untouched.Blocker — H2521-R5: the object form's terminal full stop still closes a finding the same line retracts
BLOCKER OPEN: H2521-R5
isCompletedAliasClause()treats the first.after the accepted object form as terminal and never inspects the rest of the line. So a retraction in the next sentence of the same line closes the finding it retracts — the K5/R4 harm, now through the only accepted form. Measured end-to-end (positional finding 1 plus one later comment), base0317c299versus this head:resolved R1. Actually no, still open.resolved R1. R1 is still broken.resolved R1 — closed. But R1 regressed.resolved R1. It still prints though.resolved R1 — closed. Actually the guard still prints.Every row is a fail-open regression against base (base closed none of them) in the register the fleet writes, and each wrong closure is a merge the lock stops refusing.
It also contradicts the new contract:
docs/forgejo.mdleg 5 now states that "any RETRACTION of the alias ... or the sentence after it" is deliberately not accepted. That is true for the removed subject form (R1 resolved. Actually no, still open.now stays open) but false for the accepted object form. A reviewer reading leg 5 is told a retraction cannot close when it still can.Minimal fix, either way is small: when a completed object citation is followed by a full stop, either require the remainder of the line to be terminal too, or scan that remainder for a retraction that re-names the alias (
still|regressed|reverted|remains|reminder|actually|though|broken), or narrow leg 5's claim to the subject form and document the object-form full-stop edge explicitly. Add the five rows above as negative tests.Non-blocking observation — H2521-C10: a documented accepted form is rejected
docs/forgejo.mdleg 5 listsresolved R1, already.as an accepted completed citation, butALIAS_TAIL_ADVERB_REdoes not consume the leading comma, so that line stays open whileresolved R1 already.closes. Fail-closed (the safe direction) but a doc/code mismatch; accept the comma or drop the comma form from the docs.Scope
Touched nothing: no edit, no delete, no merge, no dispatch, no other PR, no formal review row. GITS-354/358/398 read-only and no GITS ticket or project written. The ticket stays In Progress; the second (premium) exact-head verdict is still required, and the PR is not landable while H2521-R5 is open.
resolved H2521-R5 — the accepted
resolved R<n>citation now covers the whole line, so no later sentence can retract what it just cited.resolved H2521-C10 — the documented
resolved R1, already.closes; docs and code agree.New full head:
73d113f0e45f8092ac7326df999068497157e53fOne commit on top of the reviewed round-4 head; base still
origin/main0317c29987657e18c106b33a6d5723f306354132. Diff vs base is still exactly three files:forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md.CI run #175 on
73d113f0e4= pass, 14/14 contexts.Red, before any implementation change on
526e3acTests were written and run with
landing.ts/docs/forgejo.mduntouched:keeps positional finding 1 open on every next-sentence retraction—R5 — a retraction in the sentence after the citation: resolved R1. Actually no, still open.: expected true to be false(first of 16 negative rows; every adversarial neighbour failed the same way)
closes on the documented comma-adverb form — docs and code agree (H2521-C10)—resolved R1, already.: expected false to be truedoes not let a next-sentence retraction cite the WRONG positional finding—expected [ '2' ] to deeply equal [ '1', '2' ]forresolved R1. R2 open; R1 regressed.What changed (minimal — no new arm, no grammar widening)
isCompletedAliasClauseno longer stops at the object form's full stop.retractsAliasLaterreads the remainder of the LINE, per sentence: a later sentence that re-names the cited alias vetoes the citation, and a later sentence naming no alias vetoes on a still-open / denial / regression word (still,not,no,but,however,yet,again,actually,apologies,correction,open,reopen*,remains,regress*,revert*,rolled back,came back,broken,break*,fail*,throw*,print*,unfixed,unresolved,pending,deferred).resolved R1. R2 still open.andresolved R1 — closed. R2 fixed too.still cite R1 — the round-4 rows that depend on that keep passing. The rule is fail-closed: a wrong closure un-blocks a merge, a wrong refusal is one more review comment.ALIAS_TAIL_ADVERB_REconsumes the leading comma (and a dash run), so the form leg 5 advertises —resolved R1, already.— is a fully covered completed form, retraction rule included (resolved R1, already. No, still open.stays open). Docs now say the comma is part of the tail and state the whole-line rule.Grammar unchanged: no alias-as-subject arm, no chains, no citation-prefix arm; the
numbered and labelled arms (HOMELAB-2548/2549/2551) are untouched.
Green on
73d113ftests/landing-blockers.test.ts→ 68 passed (68) [was 64]tsc --noEmit→ exit 073d113f0e4→ 14/14 contexts successDifferential, base
0317c299vs this head (bundled /tmp harness, nothing written in the repo)["1"]at base and["1"]at head,citesFindingfalse.Rows: the five review rows, plus
resolved R1. The guard still prints.,Resolved: R1. Not really though.,resolved R1. It came back in prod.,resolved R1. The wiring was reverted.,resolved R1 in 4f2a1bc. But the drain returns 0.,resolved R1 — closed. however the guard prints again,resolved R1. Reopened after the revert.,resolved R1 — closed. Scratch that: still broken.,resolved R1, already. R1 broke again.,resolved R1, already. No, still open.,resolved R1. R1 shipped in 4f2a1bc too?resolved R1,Resolved R1.,resolved: R1,Resolved: #R1.,resolved #R1,resolved blocker R1,Fixed: R1,resolved R1 — closed.,resolved R1 in 4f2a1bc.,resolved R1, already.,resolved R1 already.,resolved R1, too.,resolved R01.,resolved R1 — see 4f2a1bc, and the three "prose about another finding" rows).unchanged at
["1"].resolved R1. R2 open; R1 regressed.→["1","2"],resolved R2. R1 regressed.→["1"],resolved R1. R2 is still broken.→["2"],resolved R1.\nresolved R2.→[].Out-of-scope observation (reported, not touched)
resolved #R1. …andresolved blocker R1. …reach finding 1 through the numbered arm(
#/blocker+R1reads exactly as#1/blocker 1), so a next-sentence retraction therestill closes — the same pre-existing gap as
finding R1 fixed, owned byHOMELAB-2548/2549/2551. This PR does not touch that arm; recorded here so it is not mistaken
for an R5 residual.
No self-review, no verdict, no merge, no Done. Two fresh exact-head verdicts on
73d113fare required; this head is stable and will not move.
Round 6 evidence — head
269d28b07340f9cd3d4d13b84debddadf33e2578Recovery-worker post after the round-6 worker was terminated (gateway 503) after its push. Read-only: no code edit, no new commit, no head change, no blocker id added or cleared, not a verdict. Live
refs/pull/74/headandrefs/heads/plane/HOMELAB-2521-blocker-idsboth read269d28b07340f9cd3d4d13b84debddadf33e2578; base unchanged at0317c29987657e18c106b33a6d5723f306354132; diff vs base is still exactly three files (forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md).Red first (implementation untouched), round-5 head
Green on this head
Structural check at this head
retractsAliasLater,ALIAS_RETRACTION_WORD_REandALIAS_MENTION_REare absent fromforgejo/landing.ts;ALIAS_CITATION_LINE_REis present. The round-5 retraction-tail scanner was replaced by a whole-line (^…$) allowlist over the acceptedresolved R<n>object forms, so any extra sentence, clause or prose fails the allowlist and the finding stays open.Logs:
working-files/homelab-2518/HOMELAB-2521-red-round6.log,HOMELAB-2521-green-round6-targeted.log,HOMELAB-2521-green-round6-full.log,HOMELAB-2521-typecheck-round6.log; notes section "Round 6" inHOMELAB-2521-worker.md. Two fresh exact-head reviews on269d28b073are still required.Verdict: APPROVED
model: omniroute/deepseek-flash
head:
269d28b073GENERAL review, fresh independent context, round 6. Head pinned above is the full live head, re-fetched from refs/pull/74/head and from the branch ref immediately before this write; base is live main
0317c29987. The diff against base is exactly three files — forgejo/landing.ts, forgejo/tests/landing-blockers.test.ts, docs/forgejo.md — and I read the whole diff, not only the round-6 delta, together with ticket HOMELAB-2521 (including the binding course-correction comment and the full-line allowlist contract), all of the ticket and PR comments, and the worker file's round 6 red and green sections. Read-only: no edit, no new commit, no merge, no dispatch, no other PR, no formal review state.Worker claims reproduce on this head. Targeted suite 73 passed (73); full forgejo suite 13 files, 343 passed (343); tsc --noEmit exit 0 with no output; CI run 177 on this exact head, 14 of 14 contexts success. The red-first claim also holds up under an independent reconstruction: loading the round-5 implementation and this head side by side, the round-6 rejected table wrongly cites five of its twenty-two rows on the round-5 tree and none of them here, while the eighteen documented allowlisted forms close on both — so the four failing blocks the worker recorded are the real round-6 assertions, not a fabricated count.
The course correction is implemented as written, and I verified this structurally rather than trusting the assertions. There is no retraction scanner left in the module: the round-5 helper, its alias-mention regex, its retraction word list, the digit normaliser, the completed-clause and object-chain helpers and the tail-phrase regexes are all absent from source, no per-sentence split exists anywhere in the module, and the only word-class regex remaining near resolutions is the pre-existing narrative/stance pattern this diff does not touch. Acceptance is one whole-line anchored allowlist applied to the trimmed line, with nothing after it to scan, so no second sentence can be rescued. Concretely, a citation followed by an extra sentence about another finding does not cite anything, and neither does a completion followed by a sibling sentence — each finding needs its own allowlisted line, exactly as leg 5 of the docs now states.
Every prior blocker is genuinely closed at this head, measured end to end with the finding plus a later comment and with base and head loaded side by side. The mention-versus-citation class stays open on twenty-seven replay rows, including the residual not-done case that the negated set now covers. Emphasis and wide-gap headings key their id again — emphasis, wide space runs, long dash and asterisk runs — and a citation of that id closes them, while a body-only id still keys nothing. The alias-as-subject and chain arms are removed, not narrowed, so all thirteen chain and retraction rows stay open. The next-sentence retraction class is closed too: all sixteen rows stay open here, and I additionally fuzzed every accepted stem against twenty-eight appended retraction suffixes, one hundred and ninety-six combinations, with zero behavioural difference from base and no appended sentence or clause closing anything. The quoted-sibling and undone cases close again, and the documented comma-adverb form closes, so code and docs agree on the one form the docs advertise.
The evidence headings key correctly and strictly better than base: six shapes that base read as null now key, and the line whose id appears only in its own sentence keys its label rather than the sentence id, which base got wrong. Positional and separator controls are unchanged. On the labelled and numeric arms I ran a base-versus-head differential over fifteen heading shapes and fifty-six resolution lines: the only differences are the intended alias closures, the labelled-id fixes this ticket asks for, and three deliberate documented rules — token matching, the narrow quote gate, and the id-less fallback applying only to genuinely id-less findings. Nothing agreed was lost: all eighteen documented allowed forms close, all twenty-two documented-not-accepted forms stay open, and every form written in leg 5 closes. The real corpus is covered by the suite, including the live emphasis-labelled heading shape.
Non-blocking observations, none of which raise a blocker id. The hash-prefixed and noun-prefixed alias spellings still reach a positional finding through the pre-existing numbered arm, so a retraction written that way still closes it; that arm is byte-identical to base, behaves identically on one hundred and ninety-six measured rows, is documented in leg 5, and belongs to the separately owned grammar tickets, so I did not expand into it. The allowlisted reference slot is broad enough that a denial quoted inside a backticked reference still closes — the same on the round-5 tree, so not a round-6 regression, and a denial can only ride inside that single reference slot, never as a second sentence; narrowing it is a later refinement. A heading that is now keyed by a short label requires a citation of that label, which fails closed and is the ticket's own fix. The PR body still quotes round-1 numbers; the authoritative evidence is the round-6 ticket comment, the worker file and CI run 177.
No blocker is open at this head. I approve the GENERAL leg. The ticket stays In Progress and the premium exact-head leg is still required before landing.
Verdict: REQUEST_CHANGES
model: omniroute/k3.1-pro
head:
269d28b073tier: premium
Independent PREMIUM review, fresh context, of the complete diff against base
0317c29987. Live refs/pull/74/head and the branch were re-fetched and remain269d28b073. No code edits, merge, agents, other-PR actions, or formal review.I independently reran the targeted suite (73/73), the full forgejo suite (13 files, 343/343), and tsc --noEmit (exit 0). CI run 177 on this exact head is green in all 14 contexts. The round-6 red log records 4 failed / 69 passed before the implementation change.
The original evidence headings all key correctly, including GITS402-R1 rather than GITS-402; emphasis and 40-dash/50-asterisk/80-space gaps work. All sampled documented object forms close, including C10 comma adverbs. The enumerated R1/R2/K1/K2/R3/K5/R4/R5 bare-alias replays stay open, C4/C5 honest resolutions close, and the sampled labelled/ordinary-numeric controls match base apart from the intended fixes. The retraction helpers, denial-word scanner, alias subject arm, and chain implementation are gone.
Blocker — H2521-K6: documented alias variants bypass the full-line allowlist through the numeric arm
BLOCKER OPEN: H2521-K6
citesFinding returns the older numeric-arm match before it calls citesPositionalAlias. The new ^...$ allowlist therefore constrains bare R only; the documented #R1 and blocker/finding/item/issue R1 object forms still close finding 1 when extra prose, a chain, or an explicit retraction follows.
My independent exact-head findOpenBlockers probes returned no open finding (wrong closure) for resolved #R1. R2 still open.; resolved #R1. Actually no, still open.; resolved blocker R1. R2 still open.; resolved blocker R1. Actually no, still open.; resolved finding R1. R2 still open.; resolved item R1. Actually no, still open.; resolved issue R1. Actually no, still open.; resolved #R1 and R2; and resolved blocker R1 and R2.
The base also closes these rows, so this is not an allegation of a numeric regression. It is a direct miss of the binding round-6 contract that ANY extra sentence, clause, or prose after an accepted R object form leaves the finding open. The fix can give the documented alias forms precedence over the numeric fallback without expanding into the separately owned general numeric grammar. Add these variants to the rejection matrix; the current 73 tests exercise the bypass only with bare R1, which correctly stays open.
Full findings: working-files/homelab-2518/HOMELAB-2521-review-kimi-round6.md.
269d28b0730f0b980c16resolved H2521-K6
Head:
0f0b980c16240f61a05de2c32d8f40efcae90be2(plane/HOMELAB-2521-blocker-ids, ONE new head, replaces269d28b)Base:
f910731d9b22a3274c84c4d6c4cc410a41ec6f7c(rebase onto main after #77; the round-6 commitdfdf68cis the rebased copy of269d28b, so patch content is unchanged by the rebase)Diff vs base is still exactly three files:
forgejo/landing.ts,forgejo/tests/landing-blockers.test.ts,docs/forgejo.md.Root cause
Round 6 anchored
citesPositionalAlias()to the whole trimmed line, butcitesFinding()returned the NUMBERED arm's match first. That arm's prefix (#orblockers?|blocking|findings?|items?|issues?) accepts one trailing letter, so#R1andblocker R1— the alias SPELLING — were cited by the numbered arm and the allowlist never ran. All nine premium replay variants closed finding 1 again on top of an extra sentence, chain or retraction.Fix — precedence, not grammar
An
R<n>spelling that a resolution word governs is decided by the full-line allowlist and only by it: the numbered arm scans all of its matches and skips one shaped…R<digits>whenRESOLUTION_REmatches the text before it, then falls through tocitesPositionalAlias(). The citation-prefix form (finding R1 fixed,Blocker R1 resolved,blocker #R1 fixed) has no resolution word before the alias and keeps closing through the numbered arm (docs leg 5). No new arm, no numeric-grammar change; HOMELAB-2548/2549/2551 untouched.docs/forgejo.mdleg 5 now states the rule and names the K6 replays.Evidence — red first, on the unrevised salvaged code
Tests added before any
landing.tschange (headdfdf68c, worktree clean apart from the test file):leaves finding 1 OPEN on every K6 replay variant—resolved #R1. R2 still open.: expected true to be falsekeeps the extra-prose rule on both sides of a two-finding review—expected [ '2' ] to deeply equal [ '1', '2' ](finding 1 wrongly closed)gates an alias that a resolution word governs mid-line too—Update: resolved #R1. R2 still open.: expected true to be falseEvidence — green on
0f0b980c16All nine K6 replays now stay OPEN, on a single finding and on both sides of a two-finding review, and cite no sibling:
resolved #R1. R2 still open.,resolved #R1. Actually no, still open.,resolved blocker R1. R2 still open.,resolved blocker R1. Actually no, still open.,resolved finding R1. R2 still open.,resolved item R1. Actually no, still open.,resolved issue R1. Actually no, still open.,resolved #R1 and R2,resolved blocker R1 and R2.Accepted whole-line forms still close:
resolved #R1,resolved #R1.,Resolved: #R1.,resolved #R1 — closed.,resolved #R1 in 4f2a1bc.,resolved #R1, already.,Fixed: #R1.,resolved blocker|finding|item|issue R1and their— closed/in <sha>/, alreadytails, and zero-paddedresolved R01. Prefix form and plain numerics unchanged:finding R1 fixed,Blocker R1 resolved,blocker #R1 fixed,resolved blocker 1,resolved #1,finding 1 is closed,item 2 closed,#12 is closed.Differential (previous head
dfdf68cvs0f0b980, /tmp probe, nothing written in the repo)26 rows identical at both heads except the nine K6 rows, which flip
true -> false. Every documented form, prefix form and plain-numeric row is unchanged — including the pre-existing plain-numeric retraction gap (finding 1 fixed. It still prints.istrueat both heads, owned by HOMELAB-2548/2549/2551).CI run #180 on
0f0b980c16→ pass, 14/14 contexts.No self-review, no verdict, no merge, no Done; ticket stays In Progress.
Verdict: APPROVED
model: omniroute/deepseek-flash
head:
0f0b980c16GENERAL review, fresh independent context, round 7. The head pinned above is the full live head, re-fetched through the API from the pull request and confirmed identical to the branch ref and to the worktree I tested BEFORE this write; base is live main
f910731d9b, and it is an ancestor of the head. The diff against base is exactly three files — forgejo/landing.ts, forgejo/tests/landing-blockers.test.ts, docs/forgejo.md — and I read the whole diff, not only the round-7 delta, together with ticket HOMELAB-2521, every fix comment on this PR, the worker file's round-7 section and the premium round-6 review file. Read-only throughout: no edit, no new commit, no merge, no agents, no other pull request, and no formal review state.The worker's claims reproduce on this head. The targeted suite is 79 passed of 79, the full forgejo suite is 13 files and 349 passed of 349, tsc --noEmit exits 0 with no output, and CI run 180 on this exact head is green in all fourteen contexts. The red-first claim holds under my own reconstruction rather than on the worker's word: loading the round-6 implementation and this head side by side, all nine replay rows return true on the round-6 tree where the round-7 tests assert false, which is exactly the three failing blocks the red log records.
The round-7 change is precedence and nothing more, and I verified that structurally. The numeric arm now scans every one of its matches and skips one whose matched text ends in R followed by digits when a resolution word matches the text before it, then falls through to the whole-line allowlist. There is no new citation arm, no change to the general numeric grammar, no retraction scanner and no alias subject or chain arm anywhere in the source. The citation-prefix spelling keeps closing because no resolution word precedes it there, exactly as leg five now states.
H2521-K6 is resolved on measurement, not on argument. All nine premium replay rows stay open on a single positional finding and on both sides of a two-finding review, and none of them cites a sibling; I added twenty-three neighbours of my own — capitalised and colon forms, zero-padded aliases, hash and plural-noun spellings, bold-resolved, fixed, closed and done, semicolon and dash joins, prose prefixes and mid-line object forms — and every one stays open. In the other direction thirty documented whole-line object forms still close, including the adverb, reference and restated-resolution tails and the zero-padded alias, and the citation-prefix form and every plain numeric row behave exactly as at base.
I then fuzzed the two claims against each other: seven resolution prefixes by eight alias spellings by eighteen tails, one thousand and eight lines, base, round 6 and head loaded side by side. There is not one false-to-true flip between round 6 and this head, so nothing newly closes; every difference is true-to-false, the fail-closed direction; and no line the head accepts carries a retraction or extra-clause tail. A separate twenty-row wrong-closure hunt — retraction in the next sentence, denial, a sibling sentence about a second finding, a quoted example, a mid-line object form, a fenced span, partial and hedge forms — keeps the finding open on every row.
Every prior blocker class replays open at this head. The bare-alias mention class including the residual not-done case is open, the negated set now carries done, emphasis and wide-gap headings key their ids across forty-dash, fifty-asterisk and eighty-space runs while a body-only id still keys nothing, the alias subject and chain arms are absent so all their replay rows stay open, the quoted-sibling and undone cases close again, and the documented comma-adverb form closes. All six evidence headings key whole, including GITS402-R1 and never GITS-402, and token matching holds in both directions.
Three observations, none of which raises a blocker id. First, because the numeric arm no longer rescues the alias spelling, four undocumented shapes that round 6 happened to close now refuse — no space after the colon in a hash alias, a trailing semicolon, a spaced trailing colon, and a bare trailing comma. All four are fail-closed, a wrong refusal is one more review comment while a wrong closure unblocks a merge, and none is a form leg five advertises; double space and tab forms still close. Second, a chain naming the alias no longer closes its sibling, which is the standing chain rule applied consistently and again fail-closed. Third, the citation-prefix and alias-as-subject spellings followed by a retraction still reach a positional finding through the pre-existing numeric arm; that arm is identical at base, round 6 and this head on every row I measured, it is outside the scope the round-6 reviewer set for this fix, and it belongs to the separately owned grammar tickets, so I did not expand into it.
No blocker is open at this head. The round-7 fix is minimal, precedence-shaped, fail-closed, and it closes the one contract gap the premium round-6 review named without widening the grammar. I approve the GENERAL leg on this exact head. The ticket stays In Progress and the premium exact-head leg on a different declared model is still required before landing.
Verdict: APPROVED
model: omniroute/k3.1-pro
head:
0f0b980c16tier: premium
Independent PREMIUM review, fresh context, of the complete diff against base
f910731d9b. Live refs/pull/74/head was re-fetched and remains0f0b980c16. Declared model omniroute/k3.1-pro. No code edits, merge, agents, other-PR actions, or formal review.I independently reran the targeted suite (79/79), the full forgejo suite (13 files, 349/349), and tsc --noEmit (exit 0). CI run 180 on this exact head is green in all 14 contexts. The round-7 red log records 3 failed / 76 passed before the implementation change.
H2521-K6 is resolved. The numbered arm no longer rescues hash or noun-prefixed R1 spellings outside the full-line allowlist. All nine K6 replays stay open in single-finding and two-finding reviews without citing a sibling. Another 1,227 adversarial alias-object probes across verbs, noun and hash spellings, mid-line prefixes, chains, extra prose, and retractions found no wrong closure. Citation-prefix finding R1 fixed still closes, 32 accepted object/prefix forms close, and 45 plain-numeric base-versus-head assertions are unchanged.
All enumerated R1/K1, R2/K2, R3, K5/R4, and R5 rows behave correctly. Original evidence headings, emphasis and wide gaps, honest C1/C4/C5/C10 resolutions, and the removed scanner/subject/chain structure were also verified. All findings are closed. I approve.
Full findings: working-files/homelab-2518/HOMELAB-2521-review-kimi-round7.md.