From 870f737807fe795e7f40f30206948f85ca7eaa48 Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 2 Sep 2026 17:20:54 -0700 Subject: [PATCH 01/34] fix(heartbeat): accept markdown-formatted already-reviewed exits (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit evaluatePrReviewCompletionEvidence recognized the reviewer's idempotent "already reviewed" exit only in the plain shape `already reviewed at for `. Two real Ally runs on 2026-09-02 exited cleanly per the one-review-per-(PR, head) contract and were still classified pr_review_output_missing, flipping Ally to `error`: - b7a984bf (onprem-k8s#3023): "**Already reviewed at `8b237675…`** — no action taken." (sha directly after `at`, bold, backticks) - 3ace1eef (penstock-vault-node#554): "**Already reviewed at 2026-09-02T20:41:53Z for `90193c30…`**" (the backtick between `for` and the sha defeats `\bfor\b\s+[0-9a-f]`) Both were transient_failure_retry wakes carrying a stale head; the reviewer found an Ally review attesting the live head and correctly did not post a duplicate verdict. Replace the inline regex with prReviewOutputHasAlreadyReviewedSkip: tolerates backticks/bold/italics between tokens, accepts the sha either after `for` or directly after `at` (optionally after a `head`/`commit` noun), still requires a 7–40 hex sha (a sha-less claim stays `missing`), and rejects a negated clause ("not already reviewed at …"). Tests: both real run texts verbatim, a `head ` variant, and two masking guards (no sha; negated). Mutation-tested: restoring the old regex fails the three new shape tests; dropping the negation guard fails the negation test; loosening the sha to {0,40} fails the no-sha test. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 56 +++++++++++++++++++ server/src/services/heartbeat.ts | 35 +++++++++++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 2fd5bff54b0b..6a9e0de3a094 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -726,6 +726,62 @@ describe("evaluatePrReviewCompletionEvidence", () => { ).toEqual({ status: "already_reviewed" }); }); + // BLO-31374: the same idempotent exit as the reviewer actually writes it — + // markdown-formatted, and with the sha either after `for` or directly after + // `at`. Both texts are verbatim openings of real Ally runs on 2026-09-02 that + // exited cleanly and were still classified `pr_review_output_missing`, + // flipping Ally to `error`. + it.each([ + { + label: "run b7a984bf — sha directly after `at`, bold + backticks", + summary: + "**Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`** — no action taken.\n\n" + + "The wake was a `transient_failure_retry` carrying a stale head (`0936fba6…`). Against the live PR state:\n\n" + + "- **Live head:** `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` (the wake-time SHA is superseded).\n" + + "- **Existing review:** `5096237327` — `allyblockcast[bot]` (Bot), state `COMMENTED`, submitted 2026-09-02T23:28:20Z, " + + "body starting `## Ally — Consolidated PR Review` with exactly one `Reviewed head:` line attesting `8b237675…`.\n\n" + + "Posting again would be a duplicate verdict on the same head, which the one-review-per-(PR, head) contract prohibits. Exiting cleanly.", + }, + { + label: "run 3ace1eef — timestamp + `for` + backticked sha", + summary: + "Exiting without posting — the idempotency check proves this head was already reviewed.\n\n" + + "## Wake disposition: already reviewed\n\n" + + "**`Blockcast/penstock-vault-node#554`** — the wake carried head `8d47ae36`, which has been superseded. " + + "Live head is **`90193c30abb9a75ac17e167b9aea8ca83cebc2cb`**, and the PR is **merged**.\n\n" + + "**Already reviewed at 2026-09-02T20:41:53Z for `90193c30abb9a75ac17e167b9aea8ca83cebc2cb`** (review `5094874877`).\n\n" + + "No review posted, no PR state touched.", + }, + { + label: "sha after a `head` noun", + summary: "Already reviewed at head 90193c30abb9a75ac17e167b9aea8ca83cebc2cb; skipping.", + }, + ])("BLO-31374: accepts a markdown-formatted already-reviewed exit ($label)", ({ summary }) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ + status: "already_reviewed", + }); + }); + + // Masking guard: an "already reviewed" claim that cites no sha is not an + // idempotency exit — nothing ties it to a head — and stays `missing`. + it("BLO-31374: rejects an already-reviewed claim that cites no sha", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "Already reviewed at 2026-09-02T20:41:53Z; nothing further to do.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + + // Masking guard: a negated clause describes the opposite situation. + it("BLO-31374: rejects a negated already-reviewed clause", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: + "This head was not already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the draft review was never posted.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + it("accepts archived Network-Management-Portal skips", () => { expect( evaluatePrReviewCompletionEvidence( diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 008a2f7362a3..e191072e7569 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9363,6 +9363,39 @@ function prReviewOutputHasSelfReviewSkip( return handlePattern.test(text); } +// BLO-31374: the idempotent "already reviewed" exit. The reviewer found an Ally +// review whose body attests the LIVE head and exited without posting, per the +// one-review-per-(PR, head) contract. Routine on `transient_failure_retry` wakes, +// which carry the head captured at the original wake and are often stale by the +// time the retry runs. Real shapes, all of which must classify `already_reviewed`: +// "already reviewed at 2026-05-26T04:38:27Z for 86fd374dc3b4…" (plain) +// "**Already reviewed at 2026-09-02T20:41:53Z for `90193c30abb9…`**" (run 3ace1eef) +// "**Already reviewed at `8b237675b19f…`** — no action taken." (run b7a984bf) +// The previous inline regex accepted only the plain shape: a backtick between +// `for` and the sha, or the sha following `at` directly, fell through to +// `pr_review_output_missing` and flipped Ally to `error`. The cited sha is the +// live head, which the wake context cannot know (the wake head is exactly what +// went stale), so the clause is anchored to a 7–40 hex sha rather than to the +// wake head — a sha-less "already reviewed" claim is NOT accepted — and a +// negated clause ("not already reviewed at …") is rejected. +function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { + // Markdown that may sit between tokens: whitespace, backticks, bold/italic. + const md = "[\\s`*_]*"; + const pattern = new RegExp( + "(\\b(?:not|never|wasn['\u2019]?t|isn['\u2019]?t|hasn['\u2019]?t)\\s+(?:been\\s+)?)?" + + `\\balready\\s+reviewed\\s+at${md}` + + // optional " for" (plain shape) and/or a "head"/"commit" noun + `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + + "([0-9a-f]{7,40})(?![0-9a-f])", + "gi", + ); + for (const m of text.matchAll(pattern)) { + if (m[1]) continue; + return true; + } + return false; +} + export function evaluatePrReviewCompletionEvidence( contextSnapshot: Record | null | undefined, output: { @@ -9384,7 +9417,7 @@ export function evaluatePrReviewCompletionEvidence( if (/\bgh\s+pr\s+review\b[\s\S]{0,400}\bexit["']?\s*:\s*0\b/i.test(text)) { return { status: "posted_review" as const }; } - if (/\balready\s+reviewed\s+at\b[\s\S]{0,160}\bfor\b\s+[0-9a-f]{7,40}\b/i.test(text)) { + if (prReviewOutputHasAlreadyReviewedSkip(text)) { return { status: "already_reviewed" as const }; } if ( From b479ef0998356cb323611954375473f9fd08845e Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 2 Sep 2026 17:33:43 -0700 Subject: [PATCH 02/34] fix(heartbeat): veto negated, hedged and did-not-post already-reviewed clauses (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Ally review of #1613 (two Important findings on the compensating guard, not on the widening itself): - The negation prefix used a bare `\s+`, so `**not** already reviewed at ` and `*not* already reviewed …` slipped past the guard the widened shape was tolerating markdown for. The prefix now uses the same `[\s`*_]+` class (`+`, so `notalready` is not a negation) and also absorbs `yet`/`been`. - The widened clause accepted hedged and prior-head narration the old `for`-anchored regex rejected ("could not confirm whether … already reviewed at ", "Unclear if already reviewed at ", "the prior head was already reviewed at … I did not post"). The helper now returns false when prReviewOutputHasPostedReviewNegation fires — it vetoes neither real run text — and skips a clause preceded in the same sentence by a hedge cue (unclear / unsure / uncertain / not sure / whether / if). - Suggestion taken: the negation capture is a named group. Tests: four negation forms, three hedge/prior-head forms, and a precedence case where a hedge in an earlier sentence must not veto a later unhedged clause. Mutation-tested: reverting the prefix to `\s+` fails the bold and italic cases; removing the posted-negation veto fails the did-not-post case; removing the hedge veto fails both hedge cases. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 38 +++++++++++++++++-- server/src/services/heartbeat.ts | 19 ++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 6a9e0de3a094..d0ecc4910135 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -772,14 +772,44 @@ describe("evaluatePrReviewCompletionEvidence", () => { ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); }); - // Masking guard: a negated clause describes the opposite situation. - it("BLO-31374: rejects a negated already-reviewed clause", () => { + // Masking guard: a negated clause describes the opposite situation — and the + // negation must survive the same markdown the clause tolerates (Ally review + // of #1613: a bare `\s+` prefix let `**not**` through). + it.each([ + { label: "plain not", summary: "This head was not already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the draft review was never posted." }, + { label: "bold not", summary: "This head was **not** already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the draft review was never posted." }, + { label: "italic not", summary: "It was *not* already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`." }, + { label: "not yet been", summary: "Has not yet been already reviewed at 8b237675b19fa5ae061821fd3b1d87cd8cd1836f." }, + ])("BLO-31374: rejects a negated already-reviewed clause ($label)", ({ summary }) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + + // Masking guard: hedged or prior-head narration from a run that did NOT post + // (Ally review of #1613). The old `for`-anchored regex rejected all three. + it.each([ + { label: "could not confirm whether", summary: "I could not confirm whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the API call failed." }, + { label: "unclear if", summary: "Unclear if already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`. Aborting before the post step." }, + { label: "prior head, did not post", summary: "The prior head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`, but the branch moved and I did not post a review for the new head." }, + ])("BLO-31374: rejects hedged or prior-head already-reviewed narration ($label)", ({ summary }) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + + // Precedence guard: a bare `notalready` is not a negation, and a sentence + // that merely contains "if" elsewhere still classifies once the clause itself + // is unhedged. + it("BLO-31374: a hedge in an earlier sentence does not veto a later unhedged clause", () => { expect( evaluatePrReviewCompletionEvidence(reviewerContext, { summary: - "This head was not already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the draft review was never posted.", + "I checked whether a review exists. Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", }), - ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + ).toEqual({ status: "already_reviewed" }); }); it("accepts archived Network-Management-Portal skips", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index e191072e7569..d528207c7522 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9376,21 +9376,32 @@ function prReviewOutputHasSelfReviewSkip( // `pr_review_output_missing` and flipped Ally to `error`. The cited sha is the // live head, which the wake context cannot know (the wake head is exactly what // went stale), so the clause is anchored to a 7–40 hex sha rather than to the -// wake head — a sha-less "already reviewed" claim is NOT accepted — and a -// negated clause ("not already reviewed at …") is rejected. +// wake head — a sha-less "already reviewed" claim is NOT accepted. Three vetoes +// keep the widened shape from masking a run that did NOT post (per Ally review +// of #1613): a negated clause ("was **not** already reviewed at …" — the +// negation prefix tolerates the same markdown the clause does, so bold/italic +// cannot slip past it), a hedge in the same sentence ("could not confirm +// whether … already reviewed at …", "Unclear if already reviewed at …"), and +// the shared posted-review negation cues ("… did not post …"). function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. const md = "[\\s`*_]*"; const pattern = new RegExp( - "(\\b(?:not|never|wasn['\u2019]?t|isn['\u2019]?t|hasn['\u2019]?t)\\s+(?:been\\s+)?)?" + + // `+`, not `*`: a bare `notalready` must not read as a negation. + "(?\\b(?:not|never|wasn['\u2019]?t|isn['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + `\\balready\\s+reviewed\\s+at${md}` + // optional " for" (plain shape) and/or a "head"/"commit" noun `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + "([0-9a-f]{7,40})(?![0-9a-f])", "gi", ); + if (prReviewOutputHasPostedReviewNegation(text)) return false; for (const m of text.matchAll(pattern)) { - if (m[1]) continue; + if (m.groups?.negated) continue; + // Hedge cue earlier in the same sentence: the clause is a question the run + // could not answer, not a finding. + const sentenceBefore = text.slice(Math.max(0, m.index - 80), m.index); + if (/\b(?:unclear|unsure|uncertain|not\s+sure|whether|if)\b[^.\n]{0,60}$/i.test(sentenceBefore)) continue; return true; } return false; From c9b6bc89703ca6719b1d61e4cc3b755e20ec7a82 Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 2 Sep 2026 18:06:36 -0700 Subject: [PATCH 03/34] fix(heartbeat): do not veto "did not post" on an already-reviewed exit; scope hedge to the governing clause (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second Ally pass on #1613, both findings in the compensating guards added by the previous commit: - prReviewOutputHasPostedReviewNegation was semantically inverted for this branch: not posting is the defining property of an already-reviewed exit, so "Already reviewed at — I did not post a second review" fell to pr_review_output_missing. Both real fixtures happened to phrase the skip without tripping it, which is why the earlier check gave a false all-clear. Replaced by a prior/stale-head cue scoped to the clause's own sentence (before: "the prior head was already reviewed at …"; after: "…, but the branch moved"), which is the signal that actually distinguishes the fixture the veto was added for. The real b7a984bf text mentions a "stale head" in a different sentence and still classifies. - The hedge veto used a period as the only sentence boundary and bare `whether|if` as the cue, so "Checked whether a prior review exists: already reviewed at " (or with , ; —) inherited the hedge. The cue now has to govern the clause ("unclear whether", "could not confirm whether") and the scope ends at . : ; , — – or a newline. - Suggestions: weren't/aren't added to the negation alternation; a glued `notalready` row documents that it is neither a negation nor the clause. Tests: Ally's five did-not-post phrasings and seven joiner variants as positive rows, plus the plural negation and glued-token rows (118 in file). Mutation-tested: removing the prior-head veto fails the prior-head fixture; loosening the cue to bare whether|if fails the joiner rows; reverting the boundary to period-only fails the colon/comma/dash rows. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 47 ++++++++++++++++--- server/src/services/heartbeat.ts | 36 +++++++++----- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index d0ecc4910135..88e7c49de064 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -800,16 +800,49 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); - // Precedence guard: a bare `notalready` is not a negation, and a sentence - // that merely contains "if" elsewhere still classifies once the clause itself - // is unhedged. - it("BLO-31374: a hedge in an earlier sentence does not veto a later unhedged clause", () => { + // Not vetoed: stating that no second review was posted is the defining + // property of this exit (second Ally pass on #1613 — a posted-negation veto + // sent all five of these to `missing`). + it.each([ + "**Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`** — I did not post a second review.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; I didn't post again.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`. Did not post a duplicate verdict.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — could not post a duplicate; contract forbids it.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; unable to post a second verdict on the same head.", + ])("BLO-31374: a correct skip that says it did not post again still classifies (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + + // Precedence guard: a hedge or bare `if`/`whether` in an earlier clause does + // not veto a later unhedged clause — whatever joins the two clauses. + it.each([ + "I checked whether a review exists. Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + "Checked whether a prior review exists: already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Checked whether a prior review exists, already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Checked whether a prior review exists — already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Checked whether a prior review exists; already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Skipping the post step if a review exists; already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Determining if this is a duplicate: already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: an earlier clause's hedge or bare if/whether does not veto the clause (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + + // `notalready` is not a negation and not the clause: no word boundary before + // `already`, so the shape does not match at all and the run stays `missing`. + it("BLO-31374: a glued `notalready` is neither a negation nor the clause", () => { expect( evaluatePrReviewCompletionEvidence(reviewerContext, { - summary: - "I checked whether a review exists. Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + summary: "This head was notalready reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", }), - ).toEqual({ status: "already_reviewed" }); + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + + it("BLO-31374: rejects a plural negation", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "These heads weren't already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); }); it("accepts archived Network-Management-Portal skips", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index d528207c7522..537132f08f21 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9377,31 +9377,45 @@ function prReviewOutputHasSelfReviewSkip( // live head, which the wake context cannot know (the wake head is exactly what // went stale), so the clause is anchored to a 7–40 hex sha rather than to the // wake head — a sha-less "already reviewed" claim is NOT accepted. Three vetoes -// keep the widened shape from masking a run that did NOT post (per Ally review +// keep the widened shape from masking a run that did NOT post (per Ally reviews // of #1613): a negated clause ("was **not** already reviewed at …" — the // negation prefix tolerates the same markdown the clause does, so bold/italic -// cannot slip past it), a hedge in the same sentence ("could not confirm -// whether … already reviewed at …", "Unclear if already reviewed at …"), and -// the shared posted-review negation cues ("… did not post …"). +// cannot slip past it), a hedge that governs the clause in the same sentence +// ("could not confirm whether … already reviewed at …", "Unclear if already +// reviewed at …"), and a prior/stale-head cue in the same sentence ("the prior +// head was already reviewed at …, but the branch moved"). NOT vetoed: "did not +// post" — not posting is the defining property of this exit, so the shared +// prReviewOutputHasPostedReviewNegation cues would invert the branch (second +// Ally pass: five plausible phrasings of a correct skip fell to `missing`). +// Sentence scope for both cues ends at . : ; , — – or a newline, so a clause +// joined by a colon or dash does not inherit a hedge from the previous clause. function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. const md = "[\\s`*_]*"; const pattern = new RegExp( - // `+`, not `*`: a bare `notalready` must not read as a negation. - "(?\\b(?:not|never|wasn['\u2019]?t|isn['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + + // `+`, not `*`: the negation must be its own word, separated from `already` + // by whitespace or markdown. + "(?\\b(?:not|never|wasn['\u2019]?t|weren['\u2019]?t|isn['\u2019]?t|aren['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + `\\balready\\s+reviewed\\s+at${md}` + // optional " for" (plain shape) and/or a "head"/"commit" noun `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + "([0-9a-f]{7,40})(?![0-9a-f])", "gi", ); - if (prReviewOutputHasPostedReviewNegation(text)) return false; + // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. + const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; + const clauseAfter = /^[^.\n]*/; + // A hedge that governs the clause ("unclear whether …", "could not confirm + // whether …") — not a bare `if`/`whether` anywhere nearby. + const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|did\s+not|didn['\u2019]?t)\s+\w+\s+(?:if|whether))\b/i; + // The clause is about a superseded head, not the live one. + const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; - // Hedge cue earlier in the same sentence: the clause is a question the run - // could not answer, not a finding. - const sentenceBefore = text.slice(Math.max(0, m.index - 80), m.index); - if (/\b(?:unclear|unsure|uncertain|not\s+sure|whether|if)\b[^.\n]{0,60}$/i.test(sentenceBefore)) continue; + const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; + if (hedge.test(before) || priorHead.test(before)) continue; + const after = clauseAfter.exec(text.slice(m.index + m[0].length, m.index + m[0].length + 160))?.[0] ?? ""; + if (priorHead.test(after)) continue; return true; } return false; From 7eadad1b54449eed1ede29ae2474ea96487aa200 Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 2 Sep 2026 18:08:35 -0700 Subject: [PATCH 04/34] test(heartbeat): pin the hedge cue and the clause boundary independently (BLO-31374) The joiner rows added in the previous commit passed with either guard change alone (a tight boundary hides a loose cue and vice versa), so the previous commit message overstated what its mutation testing showed for those two. These rows discriminate them: a bare `whether`/`if` in the same clause that does not govern the review clause (fails when the cue is loosened to bare whether|if), and a governing hedge about something else in the previous clause joined by ; : , (fails when the boundary reverts to period-only). Co-Authored-By: Claude Fable 5.1 --- .../__tests__/heartbeat-context-summary.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 88e7c49de064..b41f708ce5b5 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -827,6 +827,22 @@ describe("evaluatePrReviewCompletionEvidence", () => { expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); }); + // Pins the two halves of the hedge veto independently. (1) A bare + // `whether`/`if` in the SAME clause that does not govern the review clause + // must not veto — only a governing hedge ("unclear whether", "could not + // confirm whether") does. (2) A governing hedge about something else in the + // PREVIOUS clause, joined by ; : , or —, is out of scope for the review + // clause. + it.each([ + "Whether the wake was stale is moot because the head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Regardless of if the wake head moved, this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Unclear whether CI is green; already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + "I could not confirm whether CI passed: already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Not sure if the lockstep check ran, already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: only a hedge governing the review clause in its own clause vetoes (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + // `notalready` is not a negation and not the clause: no word boundary before // `already`, so the shape does not match at all and the run stays `missing`. it("BLO-31374: a glued `notalready` is neither a negation nor the clause", () => { From 2654e1fc81f27048c3659f312628ca46eac015b8 Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 2 Sep 2026 18:19:14 -0700 Subject: [PATCH 05/34] fix(heartbeat): veto prior-head only before the clause; tolerate markdown inside it (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third Ally pass on #1613, both findings in the prior/stale-head veto: - The veto was also applied to the text AFTER the clause, where it fires on the ordinary way a correct skip explains the wake ("Already reviewed at — the wake carried a stale head, superseded by this one"). The real b7a984bf text says exactly that and survived only because its clause happens to end at a period. The after-arm had no test coverage and is removed; only a prior-head SUBJECT before the clause vetoes. - The clause interior used bare \s+ between already/reviewed/at while every junction from `at` onward tolerated markdown, so "**Already reviewed** at " (markdown closing before the sha) missed. Same [\s`*_]+ class now (with `+`, so `alreadyreviewed` cannot match). - Suggestion: the hedge's second arm allows one to three words before if/whether ("could not fully confirm whether", "was not able to confirm whether"). Tests: Ally's five after-clause stale-head phrasings and three markdown-before-sha shapes as positive rows; glued `alreadyreviewed` and two multi-word hedges as negative rows (134 in file). Mutation-tested: restoring the after-arm fails 5 rows; reverting the interior to \s+ fails 3; reverting the hedge arm to one word fails 1. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 42 +++++++++++++++++++ server/src/services/heartbeat.ts | 31 +++++++------- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index b41f708ce5b5..f5a99722e0d3 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -843,6 +843,48 @@ describe("evaluatePrReviewCompletionEvidence", () => { expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); }); + // Stale/prior-head narration AFTER the clause is how a correct skip explains + // the wake (third Ally pass); only a prior-head subject BEFORE the clause + // vetoes. + it.each([ + "**Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`** — the wake carried a stale head, superseded by this one.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`, the wake head has moved since the original wake.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — the branch moved after the wake, so the wake SHA is not the live one.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the prior head in the payload is superseded.", + "Already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` (the earlier head `0936fba6` is stale) — no action taken.", + ])("BLO-31374: stale-head narration after the clause does not veto (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + + // Markdown closing before the sha: the clause interior tolerates the same + // markdown class as its tail. + it.each([ + "**Already reviewed** at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + "*Already reviewed* at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "`Already reviewed` at 8b237675b19fa5ae061821fd3b1d87cd8cd1836f.", + ])("BLO-31374: markdown that closes before the sha still matches (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + + it("BLO-31374: a glued `alreadyreviewed` is not the clause", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "alreadyreviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + + // Multi-word hedges still govern the clause. + it.each([ + "I could not fully confirm whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; aborting.", + "I was not able to confirm whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: a multi-word governing hedge still vetoes (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + // `notalready` is not a negation and not the clause: no word boundary before // `already`, so the shape does not match at all and the run stays `missing`. it("BLO-31374: a glued `notalready` is neither a negation nor the clause", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 537132f08f21..c32c73dc299f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9382,21 +9382,27 @@ function prReviewOutputHasSelfReviewSkip( // negation prefix tolerates the same markdown the clause does, so bold/italic // cannot slip past it), a hedge that governs the clause in the same sentence // ("could not confirm whether … already reviewed at …", "Unclear if already -// reviewed at …"), and a prior/stale-head cue in the same sentence ("the prior -// head was already reviewed at …, but the branch moved"). NOT vetoed: "did not -// post" — not posting is the defining property of this exit, so the shared +// reviewed at …"), and a prior/stale-head cue governing the clause ("the prior +// head was already reviewed at …"). NOT vetoed: "did not post" — not posting is +// the defining property of this exit, so the shared // prReviewOutputHasPostedReviewNegation cues would invert the branch (second // Ally pass: five plausible phrasings of a correct skip fell to `missing`). -// Sentence scope for both cues ends at . : ; , — – or a newline, so a clause -// joined by a colon or dash does not inherit a hedge from the previous clause. +// Also NOT vetoed: stale/prior-head narration AFTER the clause ("… — the wake +// carried a stale head, superseded by this one") — that is how a correct skip +// explains the wake (third Ally pass), so both cues look only at the text +// BEFORE the clause, and that scope ends at . : ; , — – or a newline, so a +// clause joined by a colon or dash does not inherit a hedge from the previous +// clause. function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. const md = "[\\s`*_]*"; const pattern = new RegExp( // `+`, not `*`: the negation must be its own word, separated from `already` - // by whitespace or markdown. + // by whitespace or markdown. The same `+` class joins already/reviewed/at, + // so "**Already reviewed** at ``" (markdown closing before the sha) + // matches while `alreadyreviewed` cannot. "(?\\b(?:not|never|wasn['\u2019]?t|weren['\u2019]?t|isn['\u2019]?t|aren['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + - `\\balready\\s+reviewed\\s+at${md}` + + `\\balready[\\s\`*_]+reviewed[\\s\`*_]+at${md}` + // optional " for" (plain shape) and/or a "head"/"commit" noun `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + "([0-9a-f]{7,40})(?![0-9a-f])", @@ -9404,18 +9410,15 @@ function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { ); // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; - const clauseAfter = /^[^.\n]*/; - // A hedge that governs the clause ("unclear whether …", "could not confirm - // whether …") — not a bare `if`/`whether` anywhere nearby. - const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|did\s+not|didn['\u2019]?t)\s+\w+\s+(?:if|whether))\b/i; - // The clause is about a superseded head, not the live one. + // A hedge that governs the clause ("unclear whether …", "could not fully + // confirm whether …") — not a bare `if`/`whether` anywhere nearby. + const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; + // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; if (hedge.test(before) || priorHead.test(before)) continue; - const after = clauseAfter.exec(text.slice(m.index + m[0].length, m.index + m[0].length + 160))?.[0] ?? ""; - if (priorHead.test(after)) continue; return true; } return false; From f4c3a57f41494c88df8b7dbaf2a14a51f84fc517 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 00:19:54 -0700 Subject: [PATCH 06/34] fix(heartbeat): veto a non-adjacent negation governing the already-reviewed clause (BLO-31374) Fourth Ally pass on #1613, masking direction: the negation guard only recognised a negation adjacent to `already`, so "There is no evidence this head was already reviewed at , so I posted a fresh verdict" / "I cannot see that this head was already reviewed at " reached `already_reviewed` (both false on master). A before-scope negation cue now sits alongside hedge/priorHead in the same clause scope; cannot/can't are listed explicitly because \bnot\b does not match inside cannot. Also: `at` is followed by [\s`*_]+ not *, so `at` with no separator is not the clause; the block comment now states the 120-character scope cap and why it is kept (an uncapped scan is quadratic on long outputs); prReviewOutputHasAlreadyReviewedSkip is exported and a direct 17-row table pins each veto boundary in one line. Tests: Ally's four non-adjacent phrasings as negative rows, a previous-clause negation as a positive row, the direct table (156 in file, 189 across sibling suites). Mutation-tested: removing the negation cue fails 6 rows; reverting the `at` separator to * fails 1. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 52 ++++++++++++++++++- server/src/services/heartbeat.ts | 22 ++++++-- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index f5a99722e0d3..d0dad31c03e8 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { buildPaperclipTaskMarkdown, derivePaperclipPrReview, - evaluatePrReviewCompletionEvidence, + evaluatePrReviewCompletionEvidence, prReviewOutputHasAlreadyReviewedSkip, mergeCoalescedContextSnapshot, summarizeHeartbeatRunContextSnapshot, summarizeHeartbeatRunListResultJson, @@ -885,6 +885,56 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // Masking guard (fourth Ally pass): a negation governing the clause from the + // subject position, not adjacent to `already`. All four were `false` on + // master and must stay `missing`. + it.each([ + "There is no evidence this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`, so I posted a fresh verdict.", + "I do not believe this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I cannot see that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Found no review; nothing indicates this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: rejects a non-adjacent negation governing the clause (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + + // A negation in the PREVIOUS clause does not reach the review clause. + it("BLO-31374: a negation in an earlier clause does not veto the clause", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "No changes were requested on the prior head; already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + }), + ).toEqual({ status: "already_reviewed" }); + }); + + // Direct table over the predicate: pins each veto boundary in one line. + describe("BLO-31374: prReviewOutputHasAlreadyReviewedSkip boundaries", () => { + const sha = "8b237675b19fa5ae061821fd3b1d87cd8cd1836f"; + it.each([ + [`Already reviewed at \`${sha}\` — no action taken.`, true], + [`already reviewed at 2026-05-26T04:38:27Z for ${sha}`, true], + [`**Already reviewed** at \`${sha}\`.`, true], + [`Already reviewed at head ${sha}; skipping.`, true], + [`Already reviewed at ${sha} — the wake carried a stale head, superseded by this one.`, true], + [`Checked whether a prior review exists: already reviewed at \`${sha}\`.`, true], + [`Unclear whether CI is green; already reviewed at \`${sha}\`.`, true], + [`Already reviewed at \`${sha}\`; I did not post again.`, true], + [`Already reviewed at 2026-09-02T20:41:53Z; nothing further to do.`, false], + [`This head was **not** already reviewed at \`${sha}\`.`, false], + [`There is no evidence this head was already reviewed at \`${sha}\`.`, false], + [`I cannot see that this head was already reviewed at \`${sha}\`.`, false], + [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], + [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], + [`The prior head was already reviewed at \`${sha}\`, but the branch moved.`, false], + [`alreadyreviewed at \`${sha}\`.`, false], + [`Already reviewed at${sha}.`, false], + ])("%s → %s", (text, want) => { + expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); + }); + }); + // `notalready` is not a negation and not the clause: no word boundary before // `already`, so the shape does not match at all and the run stays `missing`. it("BLO-31374: a glued `notalready` is neither a negation nor the clause", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index c32c73dc299f..278a73cb78b5 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9392,8 +9392,16 @@ function prReviewOutputHasSelfReviewSkip( // explains the wake (third Ally pass), so both cues look only at the text // BEFORE the clause, and that scope ends at . : ; , — – or a newline, so a // clause joined by a colon or dash does not inherit a hedge from the previous -// clause. -function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { +// clause. The same before-scope also carries a negation cue for the +// non-adjacent form ("no evidence this head was already reviewed at …", +// "I cannot see that this head was already reviewed at …" — fourth Ally pass: +// the adjacent form alone let these through as `already_reviewed`, the masking +// direction). The scope is capped at 120 characters before the clause; a +// comma-free run-on longer than that escapes the vetoes, which no real +// reviewer sentence has approached (commas are themselves boundaries), and an +// uncapped `[^…]*$` scan is quadratic on long outputs. +// Exported for direct table tests of the veto boundaries. +export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. const md = "[\\s`*_]*"; const pattern = new RegExp( @@ -9402,7 +9410,8 @@ function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // so "**Already reviewed** at ``" (markdown closing before the sha) // matches while `alreadyreviewed` cannot. "(?\\b(?:not|never|wasn['\u2019]?t|weren['\u2019]?t|isn['\u2019]?t|aren['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + - `\\balready[\\s\`*_]+reviewed[\\s\`*_]+at${md}` + + // `+` after `at` too: `at8b237675…` with no separator is not the clause. + "\\balready[\\s`*_]+reviewed[\\s`*_]+at[\\s`*_]+" + // optional " for" (plain shape) and/or a "head"/"commit" noun `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + "([0-9a-f]{7,40})(?![0-9a-f])", @@ -9415,10 +9424,15 @@ function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; + // A negation governing the clause from further back in the same clause + // ("no evidence … was already reviewed at", "cannot see that … was already + // reviewed at"). `cannot`/`can't` are listed because `\bnot\b` does not + // match inside `cannot`. + const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|without)\b/i; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; - if (hedge.test(before) || priorHead.test(before)) continue; + if (hedge.test(before) || priorHead.test(before) || negation.test(before)) continue; return true; } return false; From d955c48bac89237c2b8faedcb89434ca2ae9be3c Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 00:36:05 -0700 Subject: [PATCH 07/34] fix(heartbeat): anchor the negation cue to an epistemic head (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth Ally pass on #1613: the before-scope negation cue added last round was a bare word list, so the house phrasing of a CORRECT skip — "Exiting without posting since this head was already reviewed at " (verbatim opening of fixture 3ace1eef), "No action taken because …", "I did not post a duplicate because …" — fell to pr_review_output_missing (10 of 10 probed phrasings; 5 regress against master in the plain shape). It also partially re-opened the b479ef0 finding that "did not post" must never veto this exit. The cue is now anchored the way `hedge` is: a negation word followed within three words by an epistemic head (evidence / indication / believe / see / confirm / aware …). `without` is dropped (a preposition, not a clause negation). `failed to` joins the hedge's second arm so "failed to confirm whether …" vetoes like "unable to determine whether …". Tests: the ten skip phrasings as positive rows (two in the plain `at for ` shape), "failed to confirm whether" as a negative row, three more direct-table rows; test import block alphabetised (170 in file, 203 across sibling suites). Mutation-tested: reverting the cue to the bare word list fails 12 rows; removing the cue fails 6; removing `failed to` fails 2. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 32 ++++++++++++++++++- server/src/services/heartbeat.ts | 24 +++++++++----- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index d0dad31c03e8..3d105cbae0a2 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -2,8 +2,9 @@ import { describe, expect, it } from "vitest"; import { buildPaperclipTaskMarkdown, derivePaperclipPrReview, - evaluatePrReviewCompletionEvidence, prReviewOutputHasAlreadyReviewedSkip, + evaluatePrReviewCompletionEvidence, mergeCoalescedContextSnapshot, + prReviewOutputHasAlreadyReviewedSkip, summarizeHeartbeatRunContextSnapshot, summarizeHeartbeatRunListResultJson, } from "../services/heartbeat.js"; @@ -900,6 +901,32 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // A correct skip explains WHY it did not post, often with a negation word in + // the same clause and before the review clause (fifth Ally pass). Only an + // epistemic negation vetoes; these all classify. + it.each([ + "Exiting without posting since this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I did not post a duplicate because this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "There is no need to post again because this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "No action taken because this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Nothing to do here because the head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I don't need to post since the head was already reviewed at 2026-09-02T20:41:53Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f.", + "The contract does not permit a second verdict and this head was already reviewed at 2026-09-02T20:41:53Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f.", + "I cannot post a second verdict because this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "No second verdict is needed as this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Skipping — no duplicate verdict is permitted and this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: a non-epistemic negation explaining the skip does not veto (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ status: "already_reviewed" }); + }); + + it("BLO-31374: 'failed to confirm whether' is a governing hedge", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "I failed to confirm whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; aborting.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + // A negation in the PREVIOUS clause does not reach the review clause. it("BLO-31374: a negation in an earlier clause does not veto the clause", () => { expect( @@ -925,6 +952,9 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`This head was **not** already reviewed at \`${sha}\`.`, false], [`There is no evidence this head was already reviewed at \`${sha}\`.`, false], [`I cannot see that this head was already reviewed at \`${sha}\`.`, false], + [`Exiting without posting since this head was already reviewed at \`${sha}\`.`, true], + [`No action taken because this head was already reviewed at \`${sha}\`.`, true], + [`I failed to confirm whether this head was already reviewed at \`${sha}\`.`, false], [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], [`The prior head was already reviewed at \`${sha}\`, but the branch moved.`, false], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 278a73cb78b5..8d99ccb8e602 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9392,11 +9392,16 @@ function prReviewOutputHasSelfReviewSkip( // explains the wake (third Ally pass), so both cues look only at the text // BEFORE the clause, and that scope ends at . : ; , — – or a newline, so a // clause joined by a colon or dash does not inherit a hedge from the previous -// clause. The same before-scope also carries a negation cue for the +// clause. The same before-scope also carries an EPISTEMIC negation cue for the // non-adjacent form ("no evidence this head was already reviewed at …", // "I cannot see that this head was already reviewed at …" — fourth Ally pass: // the adjacent form alone let these through as `already_reviewed`, the masking -// direction). The scope is capped at 120 characters before the clause; a +// direction). It is anchored to an epistemic head (evidence / indication / +// believe / see / confirm …) exactly as `hedge` is anchored, because a bare +// negation word list vetoes the house phrasing of a CORRECT skip — "Exiting +// without posting since this head was already reviewed at …", "No action +// taken because …" (fifth Ally pass: 10 of 10 such phrasings fell to +// `missing`). The scope is capped at 120 characters before the clause; a // comma-free run-on longer than that escapes the vetoes, which no real // reviewer sentence has approached (commas are themselves boundaries), and an // uncapped `[^…]*$` scan is quadratic on long outputs. @@ -9421,14 +9426,17 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; // A hedge that governs the clause ("unclear whether …", "could not fully // confirm whether …") — not a bare `if`/`whether` anywhere nearby. - const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; + const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|failed\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; - // A negation governing the clause from further back in the same clause - // ("no evidence … was already reviewed at", "cannot see that … was already - // reviewed at"). `cannot`/`can't` are listed because `\bnot\b` does not - // match inside `cannot`. - const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|without)\b/i; + // An epistemic negation governing the clause from further back in the same + // clause ("no evidence … was already reviewed at", "cannot see that … was + // already reviewed at"): a negation word followed within three words by an + // epistemic head. NOT a bare negation — "Exiting without posting since …", + // "No action taken because …" are how a correct skip explains itself. + // `cannot`/`can't` are listed because `\bnot\b` does not match inside + // `cannot`. + const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t)\s+(?:\w+\s+){0,3}(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|see|seen|appears?|suggests?|confirms?|confirmed|aware)\b/i; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; From 5135b09c69be9503e2d6599b0f4cec7d6e216090 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 00:48:14 -0700 Subject: [PATCH 08/34] fix(heartbeat): veto a failed establishing verb with a that-complement (BLO-31374) Sixth Ally pass on #1613, masking direction: the two vetoes split the "could not establish it" class by complement, not meaning. `hedge` needs `if|whether`; `negation` needs an epistemic head. "I could not verify that this head was already reviewed at " satisfied neither (11 of 13 probed verbs leaked on the `that` form). The epistemic head list now includes the establishing verbs (verify / establish / determine / find / check / locate / ascertain / validate / prove / demonstrate / tell) with their inflections, so the veto's governing-form anchoring is intact and the ten correct-skip phrasings still classify. Tests: seven that-complement rows as negative cases plus two direct-table rows (177 in file, 210 across sibling suites). Mutation-tested: removing the establishing verbs from the list fails 9 rows. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 21 +++++++++++++++++++ server/src/services/heartbeat.ts | 8 ++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 3d105cbae0a2..0e4cbe0bb5e0 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -927,6 +927,25 @@ describe("evaluatePrReviewCompletionEvidence", () => { ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); }); + // Masking guard (sixth Ally pass): a `that`-complement of an establishing + // verb that the run could NOT complete. The hedge only sees `if|whether`. + it.each([ + "I could not verify that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not establish that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not determine that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not find that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I have not verified that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Unable to establish that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not check that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the API failed.", + "I couldn't verify that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I failed to establish that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: rejects a failed establishing verb with a that-complement (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + // A negation in the PREVIOUS clause does not reach the review clause. it("BLO-31374: a negation in an earlier clause does not veto the clause", () => { expect( @@ -955,6 +974,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Exiting without posting since this head was already reviewed at \`${sha}\`.`, true], [`No action taken because this head was already reviewed at \`${sha}\`.`, true], [`I failed to confirm whether this head was already reviewed at \`${sha}\`.`, false], + [`I could not verify that this head was already reviewed at \`${sha}\`.`, false], + [`Unable to establish that this head was already reviewed at \`${sha}\`.`, false], [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], [`The prior head was already reviewed at \`${sha}\`, but the branch moved.`, false], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8d99ccb8e602..71671588914e 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9431,12 +9431,14 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; // An epistemic negation governing the clause from further back in the same // clause ("no evidence … was already reviewed at", "cannot see that … was - // already reviewed at"): a negation word followed within three words by an - // epistemic head. NOT a bare negation — "Exiting without posting since …", + // already reviewed at", "could not verify that … was already reviewed at"): a + // negation word followed within three words by an epistemic head — including + // the establishing verbs (verify / establish / determine / find / check …), + // whose `that`-complement form the `hedge` cue cannot see (sixth Ally pass). NOT a bare negation — "Exiting without posting since …", // "No action taken because …" are how a correct skip explains itself. // `cannot`/`can't` are listed because `\bnot\b` does not match inside // `cannot`. - const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t)\s+(?:\w+\s+){0,3}(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|see|seen|appears?|suggests?|confirms?|confirmed|aware)\b/i; + const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\s+to|failed\s+to)\s+(?:\w+\s+){0,3}(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|see|seen|appears?|suggests?|confirms?|confirmed|aware|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tell)\b/i; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; From a5ae38358cf60426e7a7cc26fe2472bf1c970e97 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 00:53:17 -0700 Subject: [PATCH 09/34] chore(ci): backfill shard durations for 37 unmeasured server suites (BLO-31374) `check-shard-manifest-freshness` failed the #1613 policy job because 37 suites that landed on master had no entry in scripts/general-server-shard-durations.json. Backfilled with `node scripts/measure-general-server-shard-durations.mjs --update` (all 37 measured; the two "uncaughtException" lines in the run are the crash-run-marking suite's own deliberate throws). Freshness check now exits 0 locally. Co-Authored-By: Claude Fable 5.1 --- scripts/general-server-shard-durations.json | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/scripts/general-server-shard-durations.json b/scripts/general-server-shard-durations.json index 539cfd29b29c..46f762597091 100644 --- a/scripts/general-server-shard-durations.json +++ b/scripts/general-server-shard-durations.json @@ -11,18 +11,21 @@ "server/src/__tests__/acpx-local-execute.test.ts": 3547, "server/src/__tests__/acpx-local-skill-sync.test.ts": 52, "server/src/__tests__/activity-log-responsible-user.test.ts": 4715, + "server/src/__tests__/activity-log-transactional-publish.test.ts": 575, "server/src/__tests__/activity-service.test.ts": 10397, "server/src/__tests__/adapter-models.test.ts": 128, "server/src/__tests__/adapter-registry.test.ts": 30, "server/src/__tests__/adapter-session-codecs.test.ts": 29, "server/src/__tests__/agent-auth-jwt.test.ts": 134, "server/src/__tests__/agent-auth-middleware.test.ts": 96, + "server/src/__tests__/agent-budget-mirror-write.test.ts": 35, "server/src/__tests__/agent-hires-instructions-materialize.test.ts": 7694, "server/src/__tests__/agent-image-bump.test.ts": 57393, "server/src/__tests__/agent-inbox-lite-status-contract.test.ts": 11, "server/src/__tests__/agent-instructions-service.test.ts": 595, "server/src/__tests__/agent-invokability.test.ts": 14, "server/src/__tests__/agent-permissions-service.test.ts": 18, + "server/src/__tests__/agent-profile-change-gate-mixing.test.ts": 1717, "server/src/__tests__/agent-run-health.test.ts": 9, "server/src/__tests__/agent-scorecards.test.ts": 22, "server/src/__tests__/agent-secret-redaction.test.ts": 358, @@ -45,6 +48,7 @@ "server/src/__tests__/approval-insert.test.ts": 10, "server/src/__tests__/approval-linked-agent-migration.test.ts": 15122, "server/src/__tests__/approval-payload-title-guard.test.ts": 4684, + "server/src/__tests__/approval-revision-exit.test.ts": 1484, "server/src/__tests__/approval-withdraw-plugin-event.test.ts": 9363, "server/src/__tests__/approvals-service.test.ts": 4562, "server/src/__tests__/attachment-types.test.ts": 12, @@ -94,6 +98,8 @@ "server/src/__tests__/company-skills.test.ts": 106, "server/src/__tests__/config-pr-reviewer-pool.test.ts": 40, "server/src/__tests__/config-recovery-action-bounds.test.ts": 21, + "server/src/__tests__/crash-guard-install-wiring.test.ts": 1, + "server/src/__tests__/crash-run-marking.test.ts": 108, "server/src/__tests__/cursor-local-adapter-environment.test.ts": 530, "server/src/__tests__/cursor-local-adapter.test.ts": 29, "server/src/__tests__/cursor-local-execute.test.ts": 2579, @@ -140,6 +146,7 @@ "server/src/__tests__/execution-workspaces-derive-agent-cwd.test.ts": 4, "server/src/__tests__/execution-workspaces-service.test.ts": 49219, "server/src/__tests__/external-objects-service.test.ts": 11944, + "server/src/__tests__/external-wait-doc-contract.test.ts": 3, "server/src/__tests__/feedback-flush-controller.test.ts": 14, "server/src/__tests__/feedback-service.test.ts": 5336, "server/src/__tests__/feedback-share-client.test.ts": 23, @@ -154,6 +161,8 @@ "server/src/__tests__/git-checkout-identity.test.ts": 18930, "server/src/__tests__/git-worktree-ownership.test.ts": 4170, "server/src/__tests__/github-app-auth.test.ts": 219, + "server/src/__tests__/github-review-gate-authority.test.ts": 166, + "server/src/__tests__/github-review-posted-metric.test.ts": 10, "server/src/__tests__/github-status-delivery-outbox.test.ts": 11013, "server/src/__tests__/github-webhook.test.ts": 47153, "server/src/__tests__/google-sheets-gallery.test.ts": 12, @@ -200,11 +209,13 @@ "server/src/__tests__/heartbeat-project-env.test.ts": 109, "server/src/__tests__/heartbeat-provider-capacity-horizon.test.ts": 10247, "server/src/__tests__/heartbeat-queued-backlog-convergence.test.ts": 184313, + "server/src/__tests__/heartbeat-quota-recovery-wake-scope.test.ts": 2527, "server/src/__tests__/heartbeat-rate-limit-exhausted.test.ts": 8401, "server/src/__tests__/heartbeat-rate-limit-retry-schedule.test.ts": 13, "server/src/__tests__/heartbeat-recoverable-error-family.test.ts": 8, "server/src/__tests__/heartbeat-responsible-user-invariant.test.ts": 24618, "server/src/__tests__/heartbeat-retry-scheduling.test.ts": 47427, + "server/src/__tests__/heartbeat-retry-successor.test.ts": 2588, "server/src/__tests__/heartbeat-run-log.test.ts": 12, "server/src/__tests__/heartbeat-run-scoped-job-liveness-dispatch.test.ts": 9644, "server/src/__tests__/heartbeat-run-summary.test.ts": 18, @@ -220,8 +231,10 @@ "server/src/__tests__/heartbeat-timeout-outcome.test.ts": 6, "server/src/__tests__/heartbeat-timer-wake-coalescing.test.ts": 4980, "server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts": 18, + "server/src/__tests__/heartbeat-unmaterialized-desired-skills.test.ts": 2, "server/src/__tests__/heartbeat-wake-dispatch-retry.test.ts": 29126, "server/src/__tests__/heartbeat-wake-terminal-failed-gauge.test.ts": 17309, + "server/src/__tests__/heartbeat-worker-crash-marking.test.ts": 931, "server/src/__tests__/heartbeat-workspace-branch-containment.test.ts": 15513, "server/src/__tests__/heartbeat-workspace-finalize-branch.test.ts": 17883, "server/src/__tests__/heartbeat-workspace-session.test.ts": 1855, @@ -232,7 +245,11 @@ "server/src/__tests__/helpers/truncate-company-scoped-test-state.test.ts": 2250, "server/src/__tests__/hire-hook.test.ts": 21, "server/src/__tests__/http-log-policy.test.ts": 11, + "server/src/__tests__/human-gated-ageing-digest.test.ts": 2180, "server/src/__tests__/human-gated-ageing.test.ts": 85, + "server/src/__tests__/human-gated-gate-revalidation-backfill.test.ts": 18, + "server/src/__tests__/human-gated-gate-revalidation-wiring.test.ts": 259, + "server/src/__tests__/human-gated-gate-revalidation.test.ts": 8, "server/src/__tests__/in-review-gate.test.ts": 10, "server/src/__tests__/inbox-dismissals.test.ts": 8466, "server/src/__tests__/instance-settings-service.test.ts": 45, @@ -253,6 +270,7 @@ "server/src/__tests__/issue-execution-policy.test.ts": 50, "server/src/__tests__/issue-force-release.test.ts": 182, "server/src/__tests__/issue-goal-fallback.test.ts": 10, + "server/src/__tests__/issue-list-scheduled-retry-projection.test.ts": 360, "server/src/__tests__/issue-liveness.test.ts": 43, "server/src/__tests__/issue-monitor-convergence-guard.test.ts": 114, "server/src/__tests__/issue-monitor-convergence-message.test.ts": 9, @@ -269,18 +287,22 @@ "server/src/__tests__/issue-thread-interactions-telemetry.test.ts": 5130, "server/src/__tests__/issue-tree-control-service-unit.test.ts": 14, "server/src/__tests__/issue-tree-control-service.test.ts": 11269, + "server/src/__tests__/issues-alertmanager-aggregate-create.test.ts": 288, "server/src/__tests__/issues-getByIdentifier-legacy-fallback.test.ts": 5120, "server/src/__tests__/issues-getByLinearIssueId.test.ts": 6892, "server/src/__tests__/issues-identifier-provider.test.ts": 5175, "server/src/__tests__/issues-last-activity-at.test.ts": 20223, + "server/src/__tests__/issues-list-page-param-rejection.test.ts": 21, "server/src/__tests__/issues-list-query-parsing.test.ts": 62, "server/src/__tests__/issues-open-assignment-census.test.ts": 39821, "server/src/__tests__/issues-patch-evidence.test.ts": 7008, + "server/src/__tests__/issues-plugin-fencing-generation.test.ts": 2888, "server/src/__tests__/issues-user-context.test.ts": 11, "server/src/__tests__/join-request-dedupe.test.ts": 10, "server/src/__tests__/json-schema-secret-refs.test.ts": 6, "server/src/__tests__/k8s-job-liveness-run-scoped.test.ts": 630, "server/src/__tests__/k8s-job-liveness.test.ts": 37, + "server/src/__tests__/lapsed-monitor-grace-bound.test.ts": 8556, "server/src/__tests__/lifecycle-hook-command-audit.test.ts": 535, "server/src/__tests__/linear-webhook-fixture-replay.test.ts": 4757, "server/src/__tests__/linear-webhook-fixtures.test.ts": 10, @@ -289,20 +311,24 @@ "server/src/__tests__/log-redaction.test.ts": 14, "server/src/__tests__/logger-tz.test.ts": 58, "server/src/__tests__/mcp-http.test.ts": 18, + "server/src/__tests__/mcp-seed-scrub-coverage.test.ts": 6, "server/src/__tests__/metrics-ccrotate-capacity-deferred.test.ts": 17, "server/src/__tests__/metrics-service.test.ts": 269, "server/src/__tests__/monthly-spend-service.test.ts": 74, "server/src/__tests__/normalize-agent-mention-token.test.ts": 6, + "server/src/__tests__/numeric-env-bounds.test.ts": 95119, "server/src/__tests__/oidc-rbac.test.ts": 26, "server/src/__tests__/openclaw-gateway-adapter.test.ts": 148, "server/src/__tests__/opencode-k8s-seed-transport.test.ts": 17886, "server/src/__tests__/opencode-local-adapter.test.ts": 20, "server/src/__tests__/opencode-local-skill-sync.test.ts": 55, + "server/src/__tests__/orphaned-run-terminal-result.test.ts": 12, "server/src/__tests__/overdue-scheduled-retry-metrics.test.ts": 2730, "server/src/__tests__/paperclip-env.test.ts": 9, "server/src/__tests__/paperclip-node-role.test.ts": 126, "server/src/__tests__/paperclip-skill-utils.test.ts": 66, "server/src/__tests__/parse-status-filter.test.ts": 15, + "server/src/__tests__/pen2370-mirror-redaction.test.ts": 52, "server/src/__tests__/penstock-availability-gate.test.ts": 41, "server/src/__tests__/pi-local-adapter-environment.test.ts": 360, "server/src/__tests__/pi-local-execute.test.ts": 3016, @@ -319,6 +345,7 @@ "server/src/__tests__/plugin-dev-watcher.test.ts": 84, "server/src/__tests__/plugin-environment-driver-seam.test.ts": 46, "server/src/__tests__/plugin-event-outbox.test.ts": 7681, + "server/src/__tests__/plugin-events-ownership-check.test.ts": 25, "server/src/__tests__/plugin-execution-workspace-bridge.test.ts": 17, "server/src/__tests__/plugin-install-autobuild.test.ts": 13114, "server/src/__tests__/plugin-isolated-store-migration.test.ts": 235, @@ -339,6 +366,7 @@ "server/src/__tests__/plugin-tool-dispatcher-dbid.test.ts": 15, "server/src/__tests__/plugin-tool-dispatcher-pluginDbId.test.ts": 26, "server/src/__tests__/plugin-ui-static.test.ts": 2735, + "server/src/__tests__/plugin-webhook-not-ready-retryable.test.ts": 42, "server/src/__tests__/plugin-webhook-verification.test.ts": 2775, "server/src/__tests__/plugin-worker-manager.test.ts": 1094, "server/src/__tests__/pr-comment-review-gate-check.test.ts": 271, @@ -346,7 +374,9 @@ "server/src/__tests__/pr-reconciler-sweep.test.ts": 18, "server/src/__tests__/pr-review-gate-status-target.test.ts": 17, "server/src/__tests__/pr-review-issue-scope-locks.test.ts": 2026, + "server/src/__tests__/pr-review-request-ageing-digest.test.ts": 191, "server/src/__tests__/pr-review-request-ageing.test.ts": 34, + "server/src/__tests__/pr-review-state-reconciler.test.ts": 131, "server/src/__tests__/private-hostname-guard.test.ts": 81, "server/src/__tests__/process-crash-guard-exit.test.ts": 2859, "server/src/__tests__/process-crash-guard.test.ts": 409, @@ -369,6 +399,7 @@ "server/src/__tests__/redact-sensitive.test.ts": 11, "server/src/__tests__/remote-http-endpoint-guard.test.ts": 30, "server/src/__tests__/routine-run-telemetry.test.ts": 7148, + "server/src/__tests__/routine-scoped-retry.test.ts": 3, "server/src/__tests__/routines-service.test.ts": 15140, "server/src/__tests__/run-continuations.test.ts": 13, "server/src/__tests__/run-liveness.test.ts": 16, @@ -390,6 +421,7 @@ "server/src/__tests__/static-index-html.test.ts": 92, "server/src/__tests__/storage-local-provider.test.ts": 74, "server/src/__tests__/stranded-blocked-issue-reconciler.test.ts": 3747, + "server/src/__tests__/stranded-recovery-drain-metering.test.ts": 1, "server/src/__tests__/stranded-run-recovery.test.ts": 8, "server/src/__tests__/successful-run-handoff-liveness.test.ts": 817, "server/src/__tests__/summary-slots.test.ts": 7447, @@ -405,6 +437,7 @@ "server/src/__tests__/tool-gateway-service.test.ts": 12489, "server/src/__tests__/tool-gateway.test.ts": 32320, "server/src/__tests__/tool-oauth-legacy-backfill.test.ts": 8289, + "server/src/__tests__/transient-retry-floor-jitter.test.ts": 1992, "server/src/__tests__/trust-preset-resolver.test.ts": 17, "server/src/__tests__/trust-proxy.test.ts": 27, "server/src/__tests__/ui-branding.test.ts": 8, @@ -434,12 +467,15 @@ "server/src/services/gbrain-client-factory.test.ts": 51, "server/src/services/heartbeat-run-runtime-status.test.ts": 48, "server/src/services/heartbeat-stop-metadata.test.ts": 15, + "server/src/services/issue-comment-effects.test.ts": 755, "server/src/services/issue-thread-interactions.test.ts": 2107, "server/src/services/recovery/model-profile-hint.test.ts": 11, + "server/src/services/recovery/origins.test.ts": 2, "server/src/services/recovery/provider-failure-classification.test.ts": 17, "server/src/services/recovery/service.infra-class-continuation.test.ts": 7, "server/src/services/recovery/service.pause-durability.test.ts": 6, "server/src/services/recovery/strand-comment-provider-capacity.test.ts": 47, + "server/src/services/recovery/stranded-escalation-status.test.ts": 2, "server/src/services/recovery/successful-run-handoff.test.ts": 16, "server/src/services/responsible-user-denial-run-outcomes.test.ts": 16, "server/src/services/routines-formatter-cache.test.ts": 2287, @@ -447,6 +483,7 @@ "server/src/services/run-scratch.test.ts": 35, "server/src/services/session-workspace-cwd.test.ts": 14, "server/src/services/sweep-wake-preflight.test.ts": 24, + "server/src/services/wake-idempotency.test.ts": 107, "server/src/shutdown.test.ts": 67 } } From d3a7e514a25cc6ab24eae7379deff148fa8861b5 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 01:03:24 -0700 Subject: [PATCH 10/34] chore(ci): record shard duration for managed-checkout-partial-clone (BLO-31374) master added server/src/__tests__/managed-checkout-partial-clone.test.ts after the a5ae3835 backfill; the freshness check on the merge commit reported it as the one unmeasured suite (1 of 483). Measured with measure-general-server-shard-durations.mjs --update after merging master. Co-Authored-By: Claude Fable 5.1 --- scripts/general-server-shard-durations.json | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/general-server-shard-durations.json b/scripts/general-server-shard-durations.json index 46f762597091..6973118752a6 100644 --- a/scripts/general-server-shard-durations.json +++ b/scripts/general-server-shard-durations.json @@ -310,6 +310,7 @@ "server/src/__tests__/live-events-ws.test.ts": 45, "server/src/__tests__/log-redaction.test.ts": 14, "server/src/__tests__/logger-tz.test.ts": 58, + "server/src/__tests__/managed-checkout-partial-clone.test.ts": 1583, "server/src/__tests__/mcp-http.test.ts": 18, "server/src/__tests__/mcp-seed-scrub-coverage.test.ts": 6, "server/src/__tests__/metrics-ccrotate-capacity-deferred.test.ts": 17, From 6b082cd3202b7fca4ac13b011225fa8850ed32a8 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 02:35:48 -0700 Subject: [PATCH 11/34] fix(heartbeat): bind negated establishing verbs to the clause by complementizer (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seventh Ally pass. The establishing verbs added in the sixth pass are transitive over arbitrary objects, so they also vetoed correct skips whose negated object was something else — "I did not find a newer head so already reviewed at for " — turning six phrasings master accepted into false `missing`. The negation cue now has two kinds of head: evidence nouns and mental-state predicates, which are inherently about the claim and need no marker; and the establishing verbs (see / confirm / verify / establish / determine / find / check / locate …), which veto only with the complementizer (that / whether / if) that binds them to the clause. Known limit, documented: an elided complementizer ("I cannot say this head was already reviewed at …") is not vetoed. Masking direction, same pass: hedge stems now include unknown / unverified / unconfirmed / ambiguous / not clear / no idea and take `that`; a new `assumption` cue (possibly / probably / presumably / assuming / apparently / doubt / may or might have been) covers hedges with neither a negation nor a complementizer. Tests: seven over-veto regressions as positives (plain and markdown shape), the complementizer pair, eight hedge/assumption negatives, an earlier-clause assumption positive, three table rows. 234/234 across the suites; typecheck clean. Mutation: verbs without the marker → 8 rows fail; original hedge stems only → 5 fail; assumption cue not consulted → 4 fail. Structural follow-up (agent-emitted marker instead of free-text regexes) filed as BLO-31526. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 58 +++++++++++++++++++ server/src/services/heartbeat.ts | 50 ++++++++++++---- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 0e4cbe0bb5e0..004e4c10c10e 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -946,6 +946,61 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // Seventh Ally pass, over-veto direction: the establishing verbs are + // transitive over arbitrary objects, so a correct skip that negates finding + // or checking a DIFFERENT thing must still classify as a skip. Master + // accepted every one of these in the plain shape. + it.each([ + "I did not find a newer head so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "No newer review was found so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I could not find any reason to re-review so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I did not check the comments API but already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I could not see a newer head so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "Cannot tell you more; nothing else found so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I did not find a newer head so **already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`** — no action taken.", + ])("BLO-31374: a negated establishing verb with an unrelated object is still a skip (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ + status: "already_reviewed", + }); + }); + + // …while the same verb bound to the clause by a complementizer still vetoes. + it("BLO-31374: the complementizer is what binds a negated establishing verb to the clause", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "I did not find that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + }), + ).toMatchObject({ status: "missing", errorCode: "pr_review_output_missing" }); + }); + + // Seventh Ally pass, masking direction: hedges outside the original four + // stems, `that` after a hedge stem, and assumptions with no negation and no + // complementizer at all. + it.each([ + "Unknown whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "It remains unverified whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I have no idea whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "It is ambiguous whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "It is not clear that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I doubt this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "Possibly already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`, but I could not check.", + "Assuming this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` (not confirmed).", + ])("BLO-31374: rejects a hedge or assumption governing the clause (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + + // An assumption in the PREVIOUS clause does not reach the review clause. + it("BLO-31374: an assumption in an earlier clause does not veto the clause", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "Presumably the earlier run posted; already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f` — no action taken.", + }), + ).toEqual({ status: "already_reviewed" }); + }); + // A negation in the PREVIOUS clause does not reach the review clause. it("BLO-31374: a negation in an earlier clause does not veto the clause", () => { expect( @@ -975,6 +1030,9 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No action taken because this head was already reviewed at \`${sha}\`.`, true], [`I failed to confirm whether this head was already reviewed at \`${sha}\`.`, false], [`I could not verify that this head was already reviewed at \`${sha}\`.`, false], + [`I did not find a newer head so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I did not find that this head was already reviewed at \`${sha}\`.`, false], + [`Possibly already reviewed at \`${sha}\`, but I could not check.`, false], [`Unable to establish that this head was already reviewed at \`${sha}\`.`, false], [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 2a9cb7a8c250..da95938433ec 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9513,25 +9513,51 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { ); // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; - // A hedge that governs the clause ("unclear whether …", "could not fully - // confirm whether …") — not a bare `if`/`whether` anywhere nearby. - const hedge = /\b(?:(?:unclear|unsure|uncertain|not\s+sure)\s+(?:if|whether)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|failed\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; + // A hedge that governs the clause ("unclear whether …", "not clear that …", + // "could not fully confirm whether …") — not a bare `if`/`whether` anywhere + // nearby. Hedge stems take `that` as well as `if`/`whether`; the negated-verb + // arm keeps `if`/`whether` only, since its `that` form belongs to `negation`. + const hedge = + /\b(?:(?:unclear|unsure|uncertain|unknown|unverified|unconfirmed|ambiguous|not\s+sure|not\s+clear|no\s+idea)\s+(?:if|whether|that)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|failed\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; + // An assumption governing the clause with neither a negation nor a + // complementizer to hang on ("Possibly already reviewed at …", "Assuming this + // head was already reviewed at …", "I doubt this head was …"). Seventh Ally + // pass: nine hedges outside the four original stems reached `already_reviewed` + // in the backtick shapes this function added. + const assumption = /\b(?:possibly|probably|presumably|assuming|apparently|doubt(?:s|ed|ful)?|may\s+have\s+been|might\s+have\s+been)\b/i; // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; // An epistemic negation governing the clause from further back in the same // clause ("no evidence … was already reviewed at", "cannot see that … was - // already reviewed at", "could not verify that … was already reviewed at"): a - // negation word followed within three words by an epistemic head — including - // the establishing verbs (verify / establish / determine / find / check …), - // whose `that`-complement form the `hedge` cue cannot see (sixth Ally pass). NOT a bare negation — "Exiting without posting since …", - // "No action taken because …" are how a correct skip explains itself. - // `cannot`/`can't` are listed because `\bnot\b` does not match inside - // `cannot`. - const negation = /\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\s+to|failed\s+to)\s+(?:\w+\s+){0,3}(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|see|seen|appears?|suggests?|confirms?|confirmed|aware|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tell)\b/i; + // already reviewed at", "could not verify that … was already reviewed at"): + // a negation word followed within three words by an epistemic head. Two + // kinds of head, discriminated by what ties them to THIS clause: + // (a) evidence nouns and mental-state predicates are inherently about the + // claim, so no complement marker is required ("no evidence …"); + // (b) the establishing verbs (see / confirm / verify / establish / + // determine / find / check / locate …) are transitive over ARBITRARY + // objects, so they veto only with the complementizer (that / whether / + // if) that binds them to the already-reviewed clause. Without it, + // "I did not find a newer head so already reviewed at for " + // is a correct skip whose negated object is the newer head, not the + // review — the seventh Ally pass measured six such phrasings that the + // verb-only list (sixth pass) turned into false `missing`. + // Known limit: an elided complementizer ("I cannot say this head was + // already reviewed at …") is indistinguishable from an unrelated object and + // is not vetoed. + // NOT a bare negation — "Exiting without posting since …", "No action taken + // because …" are how a correct skip explains itself. `cannot`/`can't` are + // listed because `\bnot\b` does not match inside `cannot`. + const negationPrefix = + "\\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\\s+to|failed\\s+to)\\s+(?:\\w+\\s+){0,3}"; + const epistemicNoun = "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware)"; + const establishingVerb = + "(?:see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tell)\\s+(?:\\w+\\s+){0,3}(?:that|whether|if)"; + const negation = new RegExp(`${negationPrefix}(?:${epistemicNoun}|${establishingVerb})\\b`, "i"); for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; - if (hedge.test(before) || priorHead.test(before) || negation.test(before)) continue; + if (hedge.test(before) || assumption.test(before) || priorHead.test(before) || negation.test(before)) continue; return true; } return false; From abf6e7003e216c22af976cc10755c30b01308657 Mon Sep 17 00:00:00 2001 From: CTO Date: Thu, 3 Sep 2026 19:49:54 -0700 Subject: [PATCH 12/34] fix(heartbeat): anchor the assumption cue and close the elided complementizer (BLO-31374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eighth Ally pass, both directions of the same structural idea: a cue governs the clause only when it is adjacent to it or reaches it through a copula at the clause edge. Over-veto: the seventh-pass `assumption` cue was a bare word list over the whole before-scope, so it vetoed correct skips that hedge about the WAKE rather than the review — "The wake was probably a duplicate dispatch so already reviewed at for " — six phrasings master accepted. It is now end-anchored (adjacent, or bound by was/were/is/are/has been/had been). Masking: the residual the seventh pass documented as a known limit is closable on the same discriminator. "I cannot say this head WAS already reviewed at …" binds the negated verb to the clause by the copula immediately preceding it; a correct skip has so/but/and/because there instead. New `elidedComplement` cue covers say/state/assert/claim/tell/ confirm/verify/establish/determine/see/find/prove. Header comment corrected: the loop applies five cues, not three. Tests: six wake-assumption positives, four elided-complementizer negatives, two table rows. 246/246 across the suites; typecheck clean. Mutation: assumption back to a bare word list fails 7 rows; elidedComplement not consulted fails 5. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 32 ++++++++++++++ server/src/services/heartbeat.ts | 43 ++++++++++++++++--- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 004e4c10c10e..0eca7ea2fbea 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -992,6 +992,36 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // Eighth Ally pass, over-veto direction: hedging about the WAKE is the most + // natural thing this exit says, and the assumption stem must not veto it. + // Master accepted all six. + it.each([ + "The wake was probably a duplicate dispatch so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "This is presumably a retry wake so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "The retry was apparently spurious so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "The PR may have been updated since but already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "Possibly a duplicate wake and already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "The wake payload was probably stale so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + ])("BLO-31374: an assumption about the wake is still a skip (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ + status: "already_reviewed", + }); + }); + + // Eighth Ally pass, masking direction: the complementizer elided, the verb + // bound to the clause by the copula at the clause edge. + it.each([ + "I cannot say this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I cannot state this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not confirm this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I could not establish this head had been already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: rejects an elided complementizer bound by a copula (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + // An assumption in the PREVIOUS clause does not reach the review clause. it("BLO-31374: an assumption in an earlier clause does not veto the clause", () => { expect( @@ -1033,6 +1063,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`I did not find a newer head so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I did not find that this head was already reviewed at \`${sha}\`.`, false], [`Possibly already reviewed at \`${sha}\`, but I could not check.`, false], + [`The wake was probably a duplicate dispatch so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I cannot say this head was already reviewed at \`${sha}\`.`, false], [`Unable to establish that this head was already reviewed at \`${sha}\`.`, false], [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index da95938433ec..94d1c8ea18c4 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9465,9 +9465,10 @@ function prReviewOutputHasSelfReviewSkip( // `pr_review_output_missing` and flipped Ally to `error`. The cited sha is the // live head, which the wake context cannot know (the wake head is exactly what // went stale), so the clause is anchored to a 7–40 hex sha rather than to the -// wake head — a sha-less "already reviewed" claim is NOT accepted. Three vetoes +// wake head — a sha-less "already reviewed" claim is NOT accepted. Five cues // keep the widened shape from masking a run that did NOT post (per Ally reviews -// of #1613): a negated clause ("was **not** already reviewed at …" — the +// of #1613) — an adjacent negation, plus four tested against the clause scope +// (hedge, assumption, priorHead, negation/elided-complement): a negated clause ("was **not** already reviewed at …" — the // negation prefix tolerates the same markdown the clause does, so bold/italic // cannot slip past it), a hedge that governs the clause in the same sentence // ("could not confirm whether … already reviewed at …", "Unclear if already @@ -9524,7 +9525,19 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // head was already reviewed at …", "I doubt this head was …"). Seventh Ally // pass: nine hedges outside the four original stems reached `already_reviewed` // in the backtick shapes this function added. - const assumption = /\b(?:possibly|probably|presumably|assuming|apparently|doubt(?:s|ed|ful)?|may\s+have\s+been|might\s+have\s+been)\b/i; + // + // END-ANCHORED, like `negation`'s establishing verbs are marker-bound: the + // stem governs the clause only when it sits immediately before it or reaches + // it through a copula. Hedging about the WAKE — "The wake was probably a + // duplicate dispatch so already reviewed at …" — is the most natural thing + // this exit says, and a bare word list vetoed six such correct skips that + // master accepted (eighth Ally pass). + const assumeStem = + "\\b(?:possibly|probably|presumably|assuming|apparently|doubt(?:s|ed|ful)?|may\\s+have\\s+been|might\\s+have\\s+been)"; + // A copula at the clause edge is what binds a governing verb to the clause; + // a correct skip has `so`/`but`/`and`/`because` there instead. + const copulaEdge = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; + const assumption = new RegExp(`${assumeStem}(?:[\\s\`*_]*$|\\s+${copulaEdge})`, "i"); // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; // An epistemic negation governing the clause from further back in the same @@ -9542,9 +9555,9 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // is a correct skip whose negated object is the newer head, not the // review — the seventh Ally pass measured six such phrasings that the // verb-only list (sixth pass) turned into false `missing`. - // Known limit: an elided complementizer ("I cannot say this head was - // already reviewed at …") is indistinguishable from an unrelated object and - // is not vetoed. + // An elided complementizer ("I cannot say this head was already reviewed + // at …") is handled by `elidedComplement` below, on the same copula-edge + // discriminator `assumption` uses. // NOT a bare negation — "Exiting without posting since …", "No action taken // because …" are how a correct skip explains itself. `cannot`/`can't` are // listed because `\bnot\b` does not match inside `cannot`. @@ -9554,10 +9567,26 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { const establishingVerb = "(?:see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tell)\\s+(?:\\w+\\s+){0,3}(?:that|whether|if)"; const negation = new RegExp(`${negationPrefix}(?:${epistemicNoun}|${establishingVerb})\\b`, "i"); + // The complementizer elided: "cannot say this head WAS already reviewed at + // …". The verb is bound to the clause by the copula immediately preceding + // it, not by `that`/`whether`. A correct skip never has a copula in that + // position — it has so / but / and / because — so this closes the residual + // the seventh pass documented as a limit (eighth Ally pass). + const sayVerb = + "(?:say|says|said|state|states|assert|asserts|claim|claims|tell|tells|confirm|verify|establish|determine|see|find|prove)"; + const elidedComplement = new RegExp(`${negationPrefix}${sayVerb}\\s+${copulaEdge}`, "i"); for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; - if (hedge.test(before) || assumption.test(before) || priorHead.test(before) || negation.test(before)) continue; + if ( + hedge.test(before) || + assumption.test(before) || + priorHead.test(before) || + negation.test(before) || + elidedComplement.test(before) + ) { + continue; + } return true; } return false; From c0139776877f1869c65ec2a746da1161ba4683e7 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 3 Sep 2026 21:20:22 -0700 Subject: [PATCH 13/34] fix(heartbeat): unify already-reviewed veto cues on one copula bind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ally's ninth-pass review of #1613 found two cues still unbound, each leaking in a different direction: 1. `epistemicNoun` matched with no complement marker, so a negation about a NON-review object ("No evidence of a force-push, so already reviewed at ...") falsely vetoed a correct skip. That is a regression against master in the false-`missing` direction this PR exists to eliminate. 2. `hedge` required a listed stem before `if|whether`, so a hedge outside that vocabulary leaked through in the masking direction. Rather than bind one more cue, every cue now shares a single discriminator: a governing head vetoes the clause only when it reaches it through a copula at the clause edge (`copulaEdge` / `bind`). The three separate head lists (`epistemicNoun`, `establishingVerb`, `sayVerb`) collapse into one `epistemicHead`, `elidedComplement` is dropped, and `questioned` becomes stem-agnostic with the same two arms `assumption` already used — adjacent to the clause, or copula-bound. Mutation-tested, each mutant killed by a distinct row set: nouns exempt from bind -> 9 failures questioned drops copula arm -> 2 hedge unbound -> 2 questioned not consulted -> 10 hedge cue removed -> 3 265 tests pass across the four heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 58 +++++++++ server/src/services/heartbeat.ts | 115 ++++++++++-------- 2 files changed, 125 insertions(+), 48 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 0eca7ea2fbea..afed2f385fbb 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -794,6 +794,7 @@ describe("evaluatePrReviewCompletionEvidence", () => { { label: "could not confirm whether", summary: "I could not confirm whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`; the API call failed." }, { label: "unclear if", summary: "Unclear if already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`. Aborting before the post step." }, { label: "prior head, did not post", summary: "The prior head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`, but the branch moved and I did not post a review for the new head." }, + { label: "unclear that + copula", summary: "It is unclear that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`." }, ])("BLO-31374: rejects hedged or prior-head already-reviewed narration ($label)", ({ summary }) => { expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ status: "missing", @@ -1022,6 +1023,46 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // Ninth Ally pass, over-veto direction: an epistemic NOUN whose negated + // object is not the review claim. Master accepted all of these. + it.each([ + "No evidence of a force-push, so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "No indication the branch advanced, so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "There is no record of a newer wake, and already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I do not believe the payload was fresh, but already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "Nothing suggests a retry storm; already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + "I did not check whether a newer head exists so already reviewed at 2026-09-02T23:31:00Z for 8b237675b19fa5ae061821fd3b1d87cd8cd1836f", + ])("BLO-31374: a negated NON-review object is still a skip (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toEqual({ + status: "already_reviewed", + }); + }); + + // …and the same nouns bound to the clause still veto. + it.each([ + "No evidence that this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "No indication this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I do not believe this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: an epistemic noun bound to the clause still vetoes (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + + // Ninth pass, masking direction: a hedge outside the stem vocabulary that + // still questions THIS clause through `whether`. + it.each([ + "It remains an open question whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "The evidence is inconclusive as to whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + "I would have to guess whether this head was already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + ])("BLO-31374: any question whose complement is the clause vetoes (%#)", (summary) => { + expect(evaluatePrReviewCompletionEvidence(reviewerContext, { summary })).toMatchObject({ + status: "missing", + errorCode: "pr_review_output_missing", + }); + }); + // An assumption in the PREVIOUS clause does not reach the review clause. it("BLO-31374: an assumption in an earlier clause does not veto the clause", () => { expect( @@ -1065,8 +1106,13 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Possibly already reviewed at \`${sha}\`, but I could not check.`, false], [`The wake was probably a duplicate dispatch so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I cannot say this head was already reviewed at \`${sha}\`.`, false], + [`No evidence of a force-push, so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence that this head was already reviewed at \`${sha}\`.`, false], + [`It remains an open question whether this head was already reviewed at \`${sha}\`.`, false], [`Unable to establish that this head was already reviewed at \`${sha}\`.`, false], [`Unclear if already reviewed at \`${sha}\`. Aborting.`, false], + [`It is unclear that this head was already reviewed at \`${sha}\`.`, false], + [`Unclear that a rerun helps so already reviewed at \`${sha}\`.`, true], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], [`The prior head was already reviewed at \`${sha}\`, but the branch moved.`, false], [`alreadyreviewed at \`${sha}\`.`, false], @@ -1076,6 +1122,18 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // A hedge about something OTHER than the review does not mask the skip: the + // `that`-complement closes before the clause, so no copula reaches it. Drop + // the copula bind from the hedge cue and this row flips to `missing` — the + // false-`missing` regression this PR exists to eliminate. + it("BLO-31374: a hedge whose complement closes before the clause still skips", () => { + expect( + evaluatePrReviewCompletionEvidence(reviewerContext, { + summary: "Unclear that a rerun helps so already reviewed at `8b237675b19fa5ae061821fd3b1d87cd8cd1836f`.", + }), + ).toMatchObject({ status: "already_reviewed" }); + }); + // `notalready` is not a negation and not the clause: no word boundary before // `already`, so the shape does not match at all and the run stays `missing`. it("BLO-31374: a glued `notalready` is neither a negation nor the clause", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 36c1da3e85b4..8c2f0c172ce3 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9482,7 +9482,8 @@ function prReviewOutputHasSelfReviewSkip( // wake head — a sha-less "already reviewed" claim is NOT accepted. Five cues // keep the widened shape from masking a run that did NOT post (per Ally reviews // of #1613) — an adjacent negation, plus four tested against the clause scope -// (hedge, assumption, priorHead, negation/elided-complement): a negated clause ("was **not** already reviewed at …" — the +// (hedge, assumption, questioned, priorHead, negation), all sharing one +// clause-binding rule: a negated clause ("was **not** already reviewed at …" — the // negation prefix tolerates the same markdown the clause does, so bold/italic // cannot slip past it), a hedge that governs the clause in the same sentence // ("could not confirm whether … already reviewed at …", "Unclear if already @@ -9528,67 +9529,85 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { ); // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; - // A hedge that governs the clause ("unclear whether …", "not clear that …", - // "could not fully confirm whether …") — not a bare `if`/`whether` anywhere - // nearby. Hedge stems take `that` as well as `if`/`whether`; the negated-verb - // arm keeps `if`/`whether` only, since its `that` form belongs to `negation`. - const hedge = - /\b(?:(?:unclear|unsure|uncertain|unknown|unverified|unconfirmed|ambiguous|not\s+sure|not\s+clear|no\s+idea)\s+(?:if|whether|that)|(?:could\s+not|couldn['\u2019]?t|cannot|can['\u2019]?t|unable\s+to|not\s+able\s+to|failed\s+to|did\s+not|didn['\u2019]?t)\s+(?:\w+\s+){1,3}(?:if|whether))\b/i; + // THE discriminator, shared by every governing cue below: a head only vetoes + // this clause if it REACHES it, and the copula sitting at the clause edge is + // what proves that ("no evidence that this head WAS already reviewed at …", + // "I cannot say this head WAS already reviewed at …"). A correct skip has + // so / but / and / because in that position instead ("no evidence of a + // force-push, so already reviewed at …", "I did not find a newer head so + // already reviewed at …"). + // + // A complementizer alone is NOT enough and is deliberately not part of the + // rule: `that`/`whether` binds the head to SOME clause, not necessarily this + // one — "I did not check whether a newer head exists so already reviewed at + // …" is a correct skip whose whether-clause is about the head, not the + // review. An earlier draft of this pass accepted a bare complementizer and + // vetoed exactly that sentence. The copula is the anchor; a complementizer + // may precede it and is simply absorbed as one of the intervening words. + // + // Ally passes six through nine each found one more cue still matching + // unbound; rather than binding them one at a time, every head shares `bind`. + const copulaEdge = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; + const bind = `\\s+${copulaEdge}`; + // A hedge governing the clause through `that` — "It is not clear THAT this + // head was already reviewed at …". `that` also introduces assertions ("I + // confirmed that this head was already reviewed at …"), so unlike + // `whether`/`if` it needs a hedging stem in front of it; and like every + // other cue it needs `bind`, so a hedge about something else does not reach + // this clause. Every `whether`/`if` form — including "could not confirm + // whether …" — is handled stem-agnostically by `questioned` below, which is + // why this arm no longer enumerates negated verbs (ninth pass: that arm was + // unbound and vetoed "I did not check whether a newer head exists so already + // reviewed at …", a correct skip). + const hedgeStem = + "\\b(?:unclear|unsure|uncertain|unknown|unverified|unconfirmed|ambiguous|inconclusive|not\\s+sure|not\\s+clear|no\\s+idea)"; + const hedge = new RegExp(`${hedgeStem}\\s+(?:\\w+\\s+){0,3}that${bind}`, "i"); // An assumption governing the clause with neither a negation nor a // complementizer to hang on ("Possibly already reviewed at …", "Assuming this // head was already reviewed at …", "I doubt this head was …"). Seventh Ally // pass: nine hedges outside the four original stems reached `already_reviewed` // in the backtick shapes this function added. // - // END-ANCHORED, like `negation`'s establishing verbs are marker-bound: the - // stem governs the clause only when it sits immediately before it or reaches - // it through a copula. Hedging about the WAKE — "The wake was probably a - // duplicate dispatch so already reviewed at …" — is the most natural thing - // this exit says, and a bare word list vetoed six such correct skips that - // master accepted (eighth Ally pass). + // END-ANCHORED, like every other governing cue here: the stem vetoes only + // when it sits at the clause edge or reaches it through a copula. Hedging + // about the WAKE — "The wake was probably a duplicate dispatch so already + // reviewed at …" — is the most natural thing this exit says, and a bare word + // list vetoed six such correct skips that master accepted (eighth pass). const assumeStem = "\\b(?:possibly|probably|presumably|assuming|apparently|doubt(?:s|ed|ful)?|may\\s+have\\s+been|might\\s+have\\s+been)"; - // A copula at the clause edge is what binds a governing verb to the clause; - // a correct skip has `so`/`but`/`and`/`because` there instead. - const copulaEdge = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; const assumption = new RegExp(`${assumeStem}(?:[\\s\`*_]*$|\\s+${copulaEdge})`, "i"); + // `whether`/`if` whose complement IS this clause: the run is QUESTIONING the + // claim, not asserting it, whatever word introduced the question ("Unknown + // whether this head was …", "It remains an open question whether this head + // was …", "Unclear if already reviewed at …" with the subject elided). Two + // arms, the same two `assumption` uses: adjacent to the clause, or reaching + // it through the copula edge. So a question about something ELSE — "I did + // not check whether a newer head exists so already reviewed at …" — is + // untouched, since its complement is closed off before the clause. Ninth pass: `hedge` required a LISTED stem, so any hedge + // outside that vocabulary reached `already_reviewed` (the masking direction). + const questioned = new RegExp(`\\b(?:whether|if)(?:[\\s\`*_]*$|\\s+${copulaEdge})`, "i"); // The clause's subject is a superseded head, not the live one. const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; // An epistemic negation governing the clause from further back in the same - // clause ("no evidence … was already reviewed at", "cannot see that … was - // already reviewed at", "could not verify that … was already reviewed at"): - // a negation word followed within three words by an epistemic head. Two - // kinds of head, discriminated by what ties them to THIS clause: - // (a) evidence nouns and mental-state predicates are inherently about the - // claim, so no complement marker is required ("no evidence …"); - // (b) the establishing verbs (see / confirm / verify / establish / - // determine / find / check / locate …) are transitive over ARBITRARY - // objects, so they veto only with the complementizer (that / whether / - // if) that binds them to the already-reviewed clause. Without it, - // "I did not find a newer head so already reviewed at for " - // is a correct skip whose negated object is the newer head, not the - // review — the seventh Ally pass measured six such phrasings that the - // verb-only list (sixth pass) turned into false `missing`. - // An elided complementizer ("I cannot say this head was already reviewed - // at …") is handled by `elidedComplement` below, on the same copula-edge - // discriminator `assumption` uses. + // clause: a negation word, then within three words a head that could + // establish the claim, then `bind`. One list, one binding rule — evidence + // nouns are NOT exempt (ninth pass: indicates / believe / think / appears / + // suggests / aware are transitive over arbitrary objects too, so an unbound + // noun vetoed correct skips like "no indication the branch advanced, so + // already reviewed at …" — a regression against master in the false-`missing` + // direction this function exists to remove). + // // NOT a bare negation — "Exiting without posting since …", "No action taken // because …" are how a correct skip explains itself. `cannot`/`can't` are - // listed because `\bnot\b` does not match inside `cannot`. + // listed because a bare not-boundary does not match inside `cannot`. const negationPrefix = "\\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\\s+to|failed\\s+to)\\s+(?:\\w+\\s+){0,3}"; - const epistemicNoun = "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware)"; - const establishingVerb = - "(?:see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tell)\\s+(?:\\w+\\s+){0,3}(?:that|whether|if)"; - const negation = new RegExp(`${negationPrefix}(?:${epistemicNoun}|${establishingVerb})\\b`, "i"); - // The complementizer elided: "cannot say this head WAS already reviewed at - // …". The verb is bound to the clause by the copula immediately preceding - // it, not by `that`/`whether`. A correct skip never has a copula in that - // position — it has so / but / and / because — so this closes the residual - // the seventh pass documented as a limit (eighth Ally pass). - const sayVerb = - "(?:say|says|said|state|states|assert|asserts|claim|claims|tell|tells|confirm|verify|establish|determine|see|find|prove)"; - const elidedComplement = new RegExp(`${negationPrefix}${sayVerb}\\s+${copulaEdge}`, "i"); + // Evidence nouns and mental-state predicates, the establishing verbs, and the + // reporting verbs whose complementizer is routinely elided ("cannot say this + // head WAS already reviewed at …" — caught by `bind`'s copula arm). + const epistemicHead = + "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware|see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tells?|says?|said|states?|asserts?|claims?)"; + const negation = new RegExp(`${negationPrefix}${epistemicHead}${bind}`, "i"); for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; @@ -9596,8 +9615,8 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { hedge.test(before) || assumption.test(before) || priorHead.test(before) || - negation.test(before) || - elidedComplement.test(before) + questioned.test(before) || + negation.test(before) ) { continue; } From 46f243529940a88e09e9ad067a2cf8498cd8d1b7 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 3 Sep 2026 21:39:50 -0700 Subject: [PATCH 14/34] fix(heartbeat): make the clause bind structural, not per-cue Ally passes six through ten each found exactly one more already-reviewed veto cue matching unbound. Binding them one at a time kept producing the next one, so this closes the class instead of the instance. A cue is now a STEM only. `CLAUSE_REACH` is appended at the single place cues are compiled (`GOVERNING_CUE_STEMS` -> `GOVERNING_CUES`), so an unbound cue is not expressible in the table. The reach has two arms, the pair `assumption` already used: adjacent to the clause, or through the copula edge. Three findings from the tenth pass, all the same class: 1. `priorHead` was the last unbound cue, so a correct skip that merely MENTIONED a stale head in order to say it checked and found none ("Nothing indicates the branch moved so already reviewed at ...") was classified `missing` -- the false-`missing` direction that flips Ally to `error` and trips the BLO-3202 sweep after three. 2. `assumeStem` had no `guess`, so "I am guessing this head was already reviewed at ..." reached `already_reviewed` -- the masking direction, where no verdict is ever posted on that head. 3. `doubt` fired regardless of a negation in front of it, but a negated doubt is an affirmation: "there is no doubt this head was already reviewed at ..." is the strongest possible statement of a correct skip and was classified `missing`. Guarded with a bounded lookbehind rather than another cue. Also `at head` with no separator is no longer the clause, matching every other junction in the pattern. Mutation-tested, each mutant killed by a distinct row set: priorHead exempt from CLAUSE_REACH -> 4 failures guess stem removed -> 3 doubt lookbehind removed -> 5 head/commit separator back to `*` -> 1 CLAUSE_REACH emptied for every cue -> 25 CLAUSE_REACH loses adjacent arm -> 4 316 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 20 ++ server/src/services/heartbeat.ts | 192 ++++++++++-------- 2 files changed, 123 insertions(+), 89 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index afed2f385fbb..6da1682bde58 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1115,8 +1115,28 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Unclear that a rerun helps so already reviewed at \`${sha}\`.`, true], [`I could not fully confirm whether this head was already reviewed at \`${sha}\`.`, false], [`The prior head was already reviewed at \`${sha}\`, but the branch moved.`, false], + [`Nothing indicates the branch moved so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Checked that no stale head is involved so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`The old head and the current head are the same so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No force-push, so the branch moved nowhere and already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`The branch has moved so the earlier head was already reviewed at \`${sha}\`.`, false], + [`The stale head was already reviewed at \`${sha}\`.`, false], + [`The reviews API 500'd; I am guessing this head was already reviewed at \`${sha}\`.`, false], + [`I guess this head was already reviewed at \`${sha}\`.`, false], + [`My best guess is that this head was already reviewed at \`${sha}\`.`, false], + [`The wake was a guess at the head so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`My guess about the retry cause is irrelevant so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`There is no doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I have no doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Without doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Beyond doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It is not in doubt that this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I doubt this head was already reviewed at \`${sha}\`.`, false], + [`There is some doubt this head was already reviewed at \`${sha}\`.`, false], + [`It is doubtful this head was already reviewed at \`${sha}\`.`, false], [`alreadyreviewed at \`${sha}\`.`, false], [`Already reviewed at${sha}.`, false], + [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8c2f0c172ce3..8b17ad9aa638 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9510,6 +9510,107 @@ function prReviewOutputHasSelfReviewSkip( // comma-free run-on longer than that escapes the vetoes, which no real // reviewer sentence has approached (commas are themselves boundaries), and an // uncapped `[^…]*$` scan is quadratic on long outputs. +// THE discriminator, shared by every governing cue in GOVERNING_CUES below: a +// head only vetoes the already-reviewed clause if it REACHES it. Either it sits +// at the clause edge, or it gets there through the copula ("no evidence that +// this head WAS already reviewed at …", "I cannot say this head WAS already +// reviewed at …"). A correct skip has so / but / and / because in that position +// instead ("no evidence of a force-push, so already reviewed at …", "I did not +// find a newer head so already reviewed at …"). +// +// A complementizer alone is NOT enough and is deliberately not part of the +// rule: `that`/`whether` binds the head to SOME clause, not necessarily this +// one — "I did not check whether a newer head exists so already reviewed at …" +// is a correct skip whose whether-clause is about the head, not the review. An +// earlier draft accepted a bare complementizer and vetoed exactly that +// sentence. The copula is the anchor; a complementizer may precede it and is +// simply absorbed as one of the intervening words. +const COPULA_EDGE = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; +const CLAUSE_REACH = `(?:[\\s\`*_]*$|\\s+${COPULA_EDGE})`; + +// Ally passes six through ten each found exactly one more cue matching +// unbound — a veto reaching a clause it does not govern, which is the +// false-`missing` direction this function exists to remove. Binding them one +// at a time kept producing the next one, so the binding is structural: a cue +// is a STEM only, and CLAUSE_REACH is appended here, at the single place cues +// are compiled. There is no way to express an unbound cue in this table. +const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ + // A hedge governing the clause through `that` — "It is not clear THAT this + // head was already reviewed at …". `that` also introduces assertions ("I + // confirmed that this head was already reviewed at …"), so unlike + // `whether`/`if` it needs a hedging stem in front of it. Every `whether`/`if` + // form — including "could not confirm whether …" — is handled + // stem-agnostically by `questioned`, which is why this stem no longer + // enumerates negated verbs (ninth pass: that arm was unbound and vetoed "I + // did not check whether a newer head exists so already reviewed at …", a + // correct skip). + { + name: "hedge", + stem: + "\\b(?:unclear|unsure|uncertain|unknown|unverified|unconfirmed|ambiguous|inconclusive|not\\s+sure|not\\s+clear|no\\s+idea)" + + "\\s+(?:\\w+\\s+){0,3}that", + }, + // An assumption governing the clause with neither a negation nor a + // complementizer to hang on ("Possibly already reviewed at …", "Assuming this + // head was already reviewed at …", "I doubt this head was …"). Seventh pass: + // nine hedges outside the four original stems reached `already_reviewed`. + // Tenth pass added `guess`, which was reaching `already_reviewed` in the + // masking direction ("I am guessing this head was already reviewed at …"). + // + // `doubt` carries a negative lookbehind because a NEGATED doubt is an + // affirmation: "there is no doubt this head was already reviewed at …" is the + // strongest possible statement of a correct skip, and an unguarded stem + // classified it `missing` (tenth pass: 5 such phrasings regressed against + // master). `in` covers the "not in doubt that …" form. + { + name: "assumption", + stem: + "\\b(?:possibly|probably|presumably|assuming|apparently|guess(?:es|ing|ed)?" + + "|(? = GOVERNING_CUE_STEMS.map( + ({ name, stem }) => ({ name, re: new RegExp(`${stem}${CLAUSE_REACH}`, "i") }), +); + // Exported for direct table tests of the veto boundaries. export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. @@ -9523,103 +9624,16 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // `+` after `at` too: `at8b237675…` with no separator is not the clause. "\\balready[\\s`*_]+reviewed[\\s`*_]+at[\\s`*_]+" + // optional " for" (plain shape) and/or a "head"/"commit" noun - `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)${md})?` + + `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)[\\s\`*_]+)?` + "([0-9a-f]{7,40})(?![0-9a-f])", "gi", ); // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; - // THE discriminator, shared by every governing cue below: a head only vetoes - // this clause if it REACHES it, and the copula sitting at the clause edge is - // what proves that ("no evidence that this head WAS already reviewed at …", - // "I cannot say this head WAS already reviewed at …"). A correct skip has - // so / but / and / because in that position instead ("no evidence of a - // force-push, so already reviewed at …", "I did not find a newer head so - // already reviewed at …"). - // - // A complementizer alone is NOT enough and is deliberately not part of the - // rule: `that`/`whether` binds the head to SOME clause, not necessarily this - // one — "I did not check whether a newer head exists so already reviewed at - // …" is a correct skip whose whether-clause is about the head, not the - // review. An earlier draft of this pass accepted a bare complementizer and - // vetoed exactly that sentence. The copula is the anchor; a complementizer - // may precede it and is simply absorbed as one of the intervening words. - // - // Ally passes six through nine each found one more cue still matching - // unbound; rather than binding them one at a time, every head shares `bind`. - const copulaEdge = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; - const bind = `\\s+${copulaEdge}`; - // A hedge governing the clause through `that` — "It is not clear THAT this - // head was already reviewed at …". `that` also introduces assertions ("I - // confirmed that this head was already reviewed at …"), so unlike - // `whether`/`if` it needs a hedging stem in front of it; and like every - // other cue it needs `bind`, so a hedge about something else does not reach - // this clause. Every `whether`/`if` form — including "could not confirm - // whether …" — is handled stem-agnostically by `questioned` below, which is - // why this arm no longer enumerates negated verbs (ninth pass: that arm was - // unbound and vetoed "I did not check whether a newer head exists so already - // reviewed at …", a correct skip). - const hedgeStem = - "\\b(?:unclear|unsure|uncertain|unknown|unverified|unconfirmed|ambiguous|inconclusive|not\\s+sure|not\\s+clear|no\\s+idea)"; - const hedge = new RegExp(`${hedgeStem}\\s+(?:\\w+\\s+){0,3}that${bind}`, "i"); - // An assumption governing the clause with neither a negation nor a - // complementizer to hang on ("Possibly already reviewed at …", "Assuming this - // head was already reviewed at …", "I doubt this head was …"). Seventh Ally - // pass: nine hedges outside the four original stems reached `already_reviewed` - // in the backtick shapes this function added. - // - // END-ANCHORED, like every other governing cue here: the stem vetoes only - // when it sits at the clause edge or reaches it through a copula. Hedging - // about the WAKE — "The wake was probably a duplicate dispatch so already - // reviewed at …" — is the most natural thing this exit says, and a bare word - // list vetoed six such correct skips that master accepted (eighth pass). - const assumeStem = - "\\b(?:possibly|probably|presumably|assuming|apparently|doubt(?:s|ed|ful)?|may\\s+have\\s+been|might\\s+have\\s+been)"; - const assumption = new RegExp(`${assumeStem}(?:[\\s\`*_]*$|\\s+${copulaEdge})`, "i"); - // `whether`/`if` whose complement IS this clause: the run is QUESTIONING the - // claim, not asserting it, whatever word introduced the question ("Unknown - // whether this head was …", "It remains an open question whether this head - // was …", "Unclear if already reviewed at …" with the subject elided). Two - // arms, the same two `assumption` uses: adjacent to the clause, or reaching - // it through the copula edge. So a question about something ELSE — "I did - // not check whether a newer head exists so already reviewed at …" — is - // untouched, since its complement is closed off before the clause. Ninth pass: `hedge` required a LISTED stem, so any hedge - // outside that vocabulary reached `already_reviewed` (the masking direction). - const questioned = new RegExp(`\\b(?:whether|if)(?:[\\s\`*_]*$|\\s+${copulaEdge})`, "i"); - // The clause's subject is a superseded head, not the live one. - const priorHead = /\b(?:prior|previous|earlier|stale|old|superseded)\s+head\b|\bbranch\s+(?:has\s+)?moved\b|\bhead\s+(?:has\s+)?moved\b/i; - // An epistemic negation governing the clause from further back in the same - // clause: a negation word, then within three words a head that could - // establish the claim, then `bind`. One list, one binding rule — evidence - // nouns are NOT exempt (ninth pass: indicates / believe / think / appears / - // suggests / aware are transitive over arbitrary objects too, so an unbound - // noun vetoed correct skips like "no indication the branch advanced, so - // already reviewed at …" — a regression against master in the false-`missing` - // direction this function exists to remove). - // - // NOT a bare negation — "Exiting without posting since …", "No action taken - // because …" are how a correct skip explains itself. `cannot`/`can't` are - // listed because a bare not-boundary does not match inside `cannot`. - const negationPrefix = - "\\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\\s+to|failed\\s+to)\\s+(?:\\w+\\s+){0,3}"; - // Evidence nouns and mental-state predicates, the establishing verbs, and the - // reporting verbs whose complementizer is routinely elided ("cannot say this - // head WAS already reviewed at …" — caught by `bind`'s copula arm). - const epistemicHead = - "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware|see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tells?|says?|said|states?|asserts?|claims?)"; - const negation = new RegExp(`${negationPrefix}${epistemicHead}${bind}`, "i"); for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; - if ( - hedge.test(before) || - assumption.test(before) || - priorHead.test(before) || - questioned.test(before) || - negation.test(before) - ) { - continue; - } + if (GOVERNING_CUES.some(({ re }) => re.test(before))) continue; return true; } return false; From 382d96fee3705e2fea9fea66c1076db7be0e62cd Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 3 Sep 2026 23:50:01 -0700 Subject: [PATCH 15/34] fix(heartbeat): stop the copula filler crossing connectives; widen doubt guard Two Important findings from Ally's eleventh pass, both in the false-`missing` direction this PR exists to remove. 1. `COPULA_EDGE`'s filler was a bare `(?:\w+\s+){0,4}`, and `\w+` matches `so`/`but`/`and`/`because` -- the very words the design comment names as the discriminator for a correct skip. The filler swallowed them, so a negation governing a DIFFERENT noun phrase reached the clause across the connective: "No newer commits were found so this head was already reviewed at ..." was classified `missing` (5 of 5 measured rows regressed against master). The filler is now `COPULA_FILLER`, which excludes the connectives, so the code holds the invariant the comment already documented. 2. The `doubt` negative lookbehind was adjacency-only, inspecting exactly one token, so any intervening adjective defeated it -- and an adjective there is the ordinary phrasing, with "beyond reasonable doubt" a stock idiom. It now spans up to two intervening words (6 of 6 measured rows regressed). Both Suggestions taken: - The two counts in the design comment went stale when `priorHead` joined the table; they now read six cues total and five clause-scoped, matching GOVERNING_CUE_STEMS. - `GOVERNING_CUES[].name` was carried but never read. It now backs an exported `prReviewAlreadyReviewedVetoCue`, so attributing a veto is one call instead of the by-hand bisection every one of these eleven passes has needed. A 14-row attribution table pins one representative clause per cue plus five correct skips that no cue governs. Mutation-tested, each mutant killed by a distinct row set: doubt lookbehind back to adjacency-only -> 7 failures copula filler back to bare `\w+` -> 8 attribution always reports first cue -> 8 connective list loses `so` -> 6 343 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 39 +++++++++++++++++++ server/src/services/heartbeat.ts | 34 +++++++++++++--- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 6da1682bde58..65b131b82c87 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -4,6 +4,7 @@ import { derivePaperclipPrReview, evaluatePrReviewCompletionEvidence, mergeCoalescedContextSnapshot, + prReviewAlreadyReviewedVetoCue, prReviewOutputHasAlreadyReviewedSkip, summarizeHeartbeatRunContextSnapshot, summarizeHeartbeatRunListResultJson, @@ -1136,12 +1137,50 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`It is doubtful this head was already reviewed at \`${sha}\`.`, false], [`alreadyreviewed at \`${sha}\`.`, false], [`Already reviewed at${sha}.`, false], + [`There is no real doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`There is no serious doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Beyond reasonable doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Without any doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I have no genuine doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`There is not the slightest doubt this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer commits were found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer head was found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing new was seen so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No later commit was found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No other review was found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer head was found but this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer head was found because this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); }); + // Attribution: which cue vetoes. Eleven review passes have each needed a + // by-hand bisection to answer this; the table below pins one representative + // clause per cue so the next regression report can name the cue directly. + describe("BLO-31374: veto attribution names the governing cue", () => { + it.each([ + ["It is unclear that this head was ", "hedge"], + ["Possibly ", "assumption"], + ["I am guessing this head was ", "assumption"], + ["I doubt this head was ", "assumption"], + ["It remains an open question whether this head was ", "questioned"], + ["Unclear if ", "questioned"], + ["The prior head was ", "priorHead"], + ["No evidence that this head was ", "negation"], + ["I cannot say this head was ", "negation"], + // Correct skips: no cue governs the clause, so nothing vetoes. + ["No evidence of a force-push, so ", null], + ["I did not check whether a newer head exists so ", null], + ["No newer commits were found so this head was ", null], + ["Beyond reasonable doubt this head was ", null], + ["The wake was probably a duplicate dispatch so ", null], + ])("%s -> %s", (before, cue) => { + expect(prReviewAlreadyReviewedVetoCue(before)).toBe(cue); + }); + }); + // A hedge about something OTHER than the review does not mask the skip: the // `that`-complement closes before the clause, so no copula reaches it. Drop // the copula bind from the hedge cue and this row flips to `missing` — the diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8b17ad9aa638..799e929245d2 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9481,8 +9481,9 @@ function prReviewOutputHasSelfReviewSkip( // went stale), so the clause is anchored to a 7–40 hex sha rather than to the // wake head — a sha-less "already reviewed" claim is NOT accepted. Five cues // keep the widened shape from masking a run that did NOT post (per Ally reviews -// of #1613) — an adjacent negation, plus four tested against the clause scope -// (hedge, assumption, questioned, priorHead, negation), all sharing one +// of #1613) — an adjacent negation, plus the five GOVERNING_CUE_STEMS tested +// against the clause scope (hedge, assumption, questioned, priorHead, +// negation), so six in total, all sharing one // clause-binding rule: a negated clause ("was **not** already reviewed at …" — the // negation prefix tolerates the same markdown the clause does, so bold/italic // cannot slip past it), a hedge that governs the clause in the same sentence @@ -9494,8 +9495,8 @@ function prReviewOutputHasSelfReviewSkip( // Ally pass: five plausible phrasings of a correct skip fell to `missing`). // Also NOT vetoed: stale/prior-head narration AFTER the clause ("… — the wake // carried a stale head, superseded by this one") — that is how a correct skip -// explains the wake (third Ally pass), so both cues look only at the text -// BEFORE the clause, and that scope ends at . : ; , — – or a newline, so a +// explains the wake (third Ally pass), so every clause-scoped cue looks only at +// the text BEFORE the clause, and that scope ends at . : ; , — – or a newline, so a // clause joined by a colon or dash does not inherit a hedge from the previous // clause. The same before-scope also carries an EPISTEMIC negation cue for the // non-adjacent form ("no evidence this head was already reviewed at …", @@ -9525,7 +9526,14 @@ function prReviewOutputHasSelfReviewSkip( // earlier draft accepted a bare complementizer and vetoed exactly that // sentence. The copula is the anchor; a complementizer may precede it and is // simply absorbed as one of the intervening words. -const COPULA_EDGE = "(?:\\w+\\s+){0,4}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s`*_]*$"; +// The filler EXCLUDES the connectives, so the code holds the invariant the +// comment above documents. `\w+` matches `so`/`but`/`and`/`because`, so a bare +// filler swallowed the very words that mark a correct skip and let a negation +// governing a DIFFERENT noun phrase reach the clause across the connective — +// "No newer commits were found so this head was already reviewed at …" +// (eleventh pass: 5 of 5 regressed against master). +const COPULA_FILLER = "(?:(?!(?:so|but|and|because|since|therefore|thus|hence)\\b)\\w+\\s+){0,4}"; +const COPULA_EDGE = `${COPULA_FILLER}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s\`*_]*$`; const CLAUSE_REACH = `(?:[\\s\`*_]*$|\\s+${COPULA_EDGE})`; // Ally passes six through ten each found exactly one more cue matching @@ -9562,11 +9570,16 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // strongest possible statement of a correct skip, and an unguarded stem // classified it `missing` (tenth pass: 5 such phrasings regressed against // master). `in` covers the "not in doubt that …" form. + // + // The lookbehind spans up to two intervening words, because an adjective + // between the negation and the noun is the ORDINARY way these are written + // and "beyond reasonable doubt" is a stock idiom. An adjacency-only + // lookbehind restored none of them (eleventh pass: 6 of 6 regressed). { name: "assumption", stem: "\\b(?:possibly|probably|presumably|assuming|apparently|guess(?:es|ing|ed)?" + - "|(? = GOVERNING_CU ({ name, stem }) => ({ name, re: new RegExp(`${stem}${CLAUSE_REACH}`, "i") }), ); +// Which governing cue vetoes `before`, or null when none does. Every review +// pass so far has had to bisect the cues by hand to attribute a veto; this +// makes that attribution a single call, and keeps GOVERNING_CUES[].name +// load-bearing rather than decorative. +// Exported for attribution tests; not used by the classifier itself. +export function prReviewAlreadyReviewedVetoCue(before: string): string | null { + return GOVERNING_CUES.find(({ re }) => re.test(before))?.name ?? null; +} + // Exported for direct table tests of the veto boundaries. export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // Markdown that may sit between tokens: whitespace, backticks, bold/italic. From 0e8da91cd91226bdc928ccaf7a00ec4ef2b2963d Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 00:10:20 -0700 Subject: [PATCH 16/34] fix(heartbeat): veto bare belief and appearance hedges Ally's twelfth pass: the `negation` list already carries believe / think / appears / suggests, but only reachable AFTER a negation word, so "no indication" and "cannot see" were caught while the bare hedging form was not. `assumption` covered the adverbs but no belief predicate. Result: 12 phrasings of a run that never established the claim exited `already_reviewed` -- the masking failure this mechanism exists to prevent. This is a widening to close, not a veto regression. `master` has no vetoes at all and accepts these too on the plain shape; the exposure is specific to the shapes this PR newly admits, where master's mandatory-`for` anchor rejected the text incidentally. Added to `assumption` (inheriting CLAUSE_REACH, so clause-bound like every other stem): perhaps / maybe / likely / plausible / seem / appear / believe / think / thought / suspect / looks like, plus the `assumption` noun form, which was the one row Ally's own fix left open. Added to `negation`: confirmation / verification / determination / rule out. 15 new rows, including three over-veto controls where the hedge governs something other than the review and the skip must stand ("My assumption about the wake was wrong so ...", "It appears no new commits landed so ...", "This looks like a duplicate wake so ..."). Mutation-tested, each mutant killed by a distinct row set: belief/appearance predicates removed -> 9 failures epistemic nouns + rule-out removed -> 2 `assumption` noun removed -> 1 358 tests pass across the five heartbeat suites; typecheck clean. Ally's structural note -- that enumerating vetoes may not converge, because the veto side must anticipate an open set of English hedges while the accept side needs one phrasing to slip -- is correct and is tracked as BLO-31526 (require a positive evidence marker near the clause instead). It is not done here because it needs the reviewer agent to emit that marker; landing the accept-side change alone would fail every real skip closed. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../__tests__/heartbeat-context-summary.test.ts | 15 +++++++++++++++ server/src/services/heartbeat.ts | 16 ++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 65b131b82c87..be6347237c46 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1150,6 +1150,21 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No other review was found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer head was found but this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer head was found because this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It appears this head was already reviewed at \`${sha}\`.`, false], + [`It seems likely this head was already reviewed at \`${sha}\`.`, false], + [`I believe this head was already reviewed at \`${sha}\`.`, false], + [`I think this head was already reviewed at \`${sha}\`.`, false], + [`I suspect this head was already reviewed at \`${sha}\`.`, false], + [`Perhaps this head was already reviewed at \`${sha}\`.`, false], + [`Maybe this head was already reviewed at \`${sha}\`.`, false], + [`It looks like this head was already reviewed at \`${sha}\`.`, false], + [`It is plausible this head was already reviewed at \`${sha}\`.`, false], + [`There is no confirmation this head was already reviewed at \`${sha}\`.`, false], + [`I cannot rule out that this head was already reviewed at \`${sha}\`.`, false], + [`My assumption is that this head was already reviewed at \`${sha}\`.`, false], + [`My assumption about the wake was wrong so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It appears no new commits landed so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`This looks like a duplicate wake so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 799e929245d2..5c170e5045f1 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9565,6 +9565,16 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // Tenth pass added `guess`, which was reaching `already_reviewed` in the // masking direction ("I am guessing this head was already reviewed at …"). // + // Twelfth pass added the BELIEF and APPEARANCE predicates. `negation` below + // already lists believe / think / appears / suggests, but only reachable + // AFTER a negation word — so "no indication" and "cannot see" were caught + // while the bare hedging form ("I believe this head was …", "It appears this + // head was …") reached `already_reviewed` unvetoed: 12 phrasings of a run + // that never established the claim. That is the masking failure this whole + // mechanism exists to prevent, and it is specific to the shapes this + // function newly admits — master's mandatory-`for` pattern rejected the text + // incidentally, so there is no veto regression, only a widening to close. + // // `doubt` carries a negative lookbehind because a NEGATED doubt is an // affirmation: "there is no doubt this head was already reviewed at …" is the // strongest possible statement of a correct skip, and an unguarded stem @@ -9578,7 +9588,9 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ { name: "assumption", stem: - "\\b(?:possibly|probably|presumably|assuming|apparently|guess(?:es|ing|ed)?" + + "\\b(?:possibly|probably|presumably|assuming|assumption|apparently|guess(?:es|ing|ed)?" + + "|perhaps|maybe|likely|plausible|seem(?:s|ed)?|appears?|believ(?:e|es|ed)" + + "|think(?:s)?|thought|suspect(?:s|ed)?|looks?\\s+like" + "|(? = [ // Evidence nouns and mental-state predicates, the establishing verbs, and // the reporting verbs whose complementizer is routinely elided ("cannot // say this head WAS already reviewed at …" — caught by the copula arm). - "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware|see|seen|confirms?|confirmed|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tells?|says?|said|states?|asserts?|claims?)", + "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware|see|seen|confirms?|confirmed|confirmation|verification|determination|rule\\s+out|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tells?|says?|said|states?|asserts?|claims?)", }, ]; From 7be948502180ec2bfd6e2c55b45dabaf3bcaa250 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 00:30:31 -0700 Subject: [PATCH 17/34] chore(shards): correct the stale coverage count in the manifest comment Ally's thirteenth pass: $comment still read "coverage is 443/443" while durations holds 483 entries after this PR's top-ups. Cosmetic and self-correcting on the next weekly refresh-shard-manifest.yml run, but the number is load-bearing when a human reads the manifest to judge shard balance. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- scripts/general-server-shard-durations.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/general-server-shard-durations.json b/scripts/general-server-shard-durations.json index 6973118752a6..5da4b5f0fbdd 100644 --- a/scripts/general-server-shard-durations.json +++ b/scripts/general-server-shard-durations.json @@ -1,5 +1,5 @@ { - "$comment": "Per-suite Vitest durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from four successful ARC PR jobs in run 31248977534 on 2026-08-08, then topped up as master added suites, most recently on 2026-08-24 via scripts/measure-general-server-shard-durations.mjs; coverage is 443/443. This sentence is regenerated on every refresh; see \"$notes\" for guidance that is meant to persist.", + "$comment": "Per-suite Vitest durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from four successful ARC PR jobs in run 31248977534 on 2026-08-08, then topped up as master added suites, most recently on 2026-08-24 via scripts/measure-general-server-shard-durations.mjs; coverage is 483/483. This sentence is regenerated on every refresh; see \"$notes\" for guidance that is meant to persist.", "$notes": "DURABLE NOTES -- unlike \"$comment\" (regenerated on every refresh), this key is never rewritten by .github/workflows/refresh-shard-manifest.yml. Put guidance that must outlive a re-sample here. NOTE ON UNITS: these are per-file TEST-EXECUTION durations (the Vitest JSON reporter's testResults[].startTime->endTime), not full wall-clock -- they exclude each file's transform/setup/import cost, which is why a trivial suite can read as single-digit ms. That fixed per-file cost is roughly uniform and the LPT partition also lands near-equal file counts per shard, so it does not skew balance; do not 'fix' a small value here by hand. Suites absent from this manifest use the median fallback (defaultSuiteWeight in scripts/general-server-shard.mjs); the refresh workflow re-samples weekly so refresh does not depend on a human noticing drift.", "unit": "ms", "durations": { From 7fc5455bf6211321580eb9beda70e1cb93b1bc14 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 00:54:42 -0700 Subject: [PATCH 18/34] fix(heartbeat): make the sha anchor hold its documented guarantee Ally's fourteenth pass approved with no blockers and left three suggestions to judgement. Taking all three; the middle one is why. 1. **The comment lied.** `:9482` promises unconditionally that "a sha-less `already reviewed` claim is NOT accepted", but `([0-9a-f]{7,40})` matches any run of 7+ hex digits, so the compact ISO form `20260902T204153Z` satisfied the anchor on its `20260902` prefix, as did a bare `1234567`. That is the same defect class as the eleventh pass -- code contradicting an invariant its own comment claims -- and it is in the masking direction: a run that never reviewed exits `already_reviewed` and the PR is never reviewed. An all-decimal token is now a commit only when a `for`/`head`/`commit` marker says so, which still admits a genuinely all-decimal abbreviated sha (~3.7% of 7-char abbreviations, not negligible). Lowercase is re-checked because the pattern's `i` flag exists for `Already`, not for the sha. 2. `COPULA_FILLER`'s connective list held the eight highest-frequency connectives but not the adversative and consecutive ones, so a negation crossed all twelve and vetoed a clause it does not govern. Unlike the hedge vocabulary this set is CLOSED, so completing it converges -- that is the distinction from the thirteenth-pass suggestion, which was declined for being an open enumeration. 3. The `negated` group required the negation adjacent to `already`, so an inverted copula leaked in the masking direction ("This head never was already reviewed at ..."). `was|were|is|are` join the intervening set. Mutation-tested, each mutant killed by a distinct row set: adversative/consecutive connectives removed -> 12 failures all-decimal guard removed -> 2 all-decimal rejected unconditionally -> 3 lowercase-hex re-check removed -> 3 inverted copula removed -> 2 The lowercase re-check initially killed NOTHING: the all-decimal guard incidentally rejects uppercase too, so its own row proved nothing. Three rows that separate the two guards (uppercase WITH a marker, and mixed case) were added before claiming it. 385 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 27 +++++++++++++++++ server/src/services/heartbeat.ts | 29 ++++++++++++++++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index be6347237c46..bfe57ca10c35 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1165,6 +1165,33 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`My assumption about the wake was wrong so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`It appears no new commits landed so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`This looks like a duplicate wake so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer commits were found yet this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found however this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found though this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found although this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found still this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found nonetheless this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found nevertheless this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found whereas this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found while this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found then this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found consequently this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found accordingly this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found, yet this head was already reviewed at \`${sha}\`.`, true], + [`Already reviewed at 20260902T204153Z`, false], + [`Already reviewed at 1234567`, false], + [`Already reviewed at 2026-09-02T20:41:53Z`, false], + [`Already reviewed at 2026-09-02T23:31:00Z for 1234567`, true], + [`Already reviewed at head 1234567`, true], + [`Already reviewed at commit 1234567`, true], + [`Already reviewed at 8B237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], + [`Already reviewed at \`${sha}\`.`, true], + [`This head never was already reviewed at \`${sha}\`.`, false], + [`This head not yet was already reviewed at \`${sha}\`.`, false], + [`This head was never already reviewed at \`${sha}\`.`, false], + [`Already reviewed at head 8B237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], + [`Already reviewed at 2026-09-02T23:31:00Z for 8B237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], + [`Already reviewed at 8b237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 5c170e5045f1..8d6d5e636850 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9532,7 +9532,13 @@ function prReviewOutputHasSelfReviewSkip( // governing a DIFFERENT noun phrase reach the clause across the connective — // "No newer commits were found so this head was already reviewed at …" // (eleventh pass: 5 of 5 regressed against master). -const COPULA_FILLER = "(?:(?!(?:so|but|and|because|since|therefore|thus|hence)\\b)\\w+\\s+){0,4}"; +const CONNECTIVES = + "so|but|and|because|since|therefore|thus|hence" + + // Adversative and consecutive (fourteenth pass): a negation crossed all of + // these and vetoed a clause it does not govern. Unlike the hedge vocabulary, + // the connective set is CLOSED, so completing it converges. + "|yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then|consequently|accordingly"; +const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b)\\w+\\s+){0,4}`; const COPULA_EDGE = `${COPULA_FILLER}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s\`*_]*$`; const CLAUSE_REACH = `(?:[\\s\`*_]*$|\\s+${COPULA_EDGE})`; @@ -9654,18 +9660,33 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // by whitespace or markdown. The same `+` class joins already/reviewed/at, // so "**Already reviewed** at ``" (markdown closing before the sha) // matches while `alreadyreviewed` cannot. - "(?\\b(?:not|never|wasn['\u2019]?t|weren['\u2019]?t|isn['\u2019]?t|aren['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been)[\\s`*_]+){0,2})?" + + "(?\\b(?:not|never|wasn['\u2019]?t|weren['\u2019]?t|isn['\u2019]?t|aren['\u2019]?t|hasn['\u2019]?t)[\\s`*_]+(?:(?:yet|been|was|were|is|are)[\\s`*_]+){0,2})?" + // `+` after `at` too: `at8b237675…` with no separator is not the clause. "\\balready[\\s`*_]+reviewed[\\s`*_]+at[\\s`*_]+" + // optional " for" (plain shape) and/or a "head"/"commit" noun - `(?:[^\\s\`*_]{1,40}${md}for${md})?(?:(?:head|commit)[\\s\`*_]+)?` + - "([0-9a-f]{7,40})(?![0-9a-f])", + `(?[^\\s\`*_]{1,40}${md}for${md})?(?(?:head|commit)[\\s\`*_]+)?` + + "(?[0-9a-f]{7,40})(?![0-9a-f])", "gi", ); // Same-sentence scope: a clause boundary is any of . : ; , — – or a newline. const clauseBefore = /[^.\n:;,\u2014\u2013]*$/; for (const m of text.matchAll(pattern)) { if (m.groups?.negated) continue; + // A sha is not a timestamp, and the comment above promises a sha-less + // claim is refused — so the code has to hold that, not merely document it + // (fourteenth pass). The compact ISO form `20260902T204153Z` contains a + // 7+ hex run (`20260902`) whose next character is not hex, so the anchor + // alone accepted it, as it did a bare `1234567`. An all-decimal token is + // therefore a commit only when a `for`/`head`/`commit` marker says so; + // that still admits a genuinely all-decimal abbreviated sha, which is + // ~3.7% of 7-character abbreviations and not negligible. + // + // Lowercase is re-checked here because the pattern's `i` flag exists for + // `Already`, not for the sha, and the attestation contract mandates + // lowercase hex. + const sha = m.groups?.sha ?? ""; + if (!/^[0-9a-f]+$/.test(sha)) continue; + if (!/[a-f]/.test(sha) && !m.groups?.forMarker && !m.groups?.nounMarker) continue; const before = clauseBefore.exec(text.slice(Math.max(0, m.index - 120), m.index))?.[0] ?? ""; if (GOVERNING_CUES.some(({ re }) => re.test(before))) continue; return true; From 0a9f4a5ed3e9b4054adf1457850cbd126b547d5e Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 01:12:39 -0700 Subject: [PATCH 19/34] fix(heartbeat): condition the connective exclusion on role, not word Self-inflicted regression from the previous head. The twelve CONNECTIVES are DUAL-ROLE words, and excluding them from COPULA_FILLER unconditionally broke the negation's reach in their filler role: connective role: "...were found yet this head was already reviewed at X" -> the filler must NOT cross it filler role: "no confirmation yet THAT this head was already reviewed" -> ordinary English for "I could not establish this"; the negation MUST still reach the clause Ally measured the previous head as an exact 1:1 trade -- 12 filler-sense vetoes lost for 12 connective-sense acceptances -- and the losing side is the one a reviewer actually writes. 9 of 12 realistic masking phrasings were newly accepted, i.e. a run that never established the claim exited already_reviewed. What the unconditional exclusion bought was also narrower than it looked: all twenty comma-forms already classified correctly at both heads, since the comma is itself a clause boundary, so the only phrasings gained were comma-less run-ons. The discriminator is the complementizer, so the exclusion is now conditioned on the role via a nested lookahead rather than applied to the word. I took the unconditional version last pass arguing that the connective set is closed and therefore converges. That was true about convergence and irrelevant to whether excluding those words breaks their other sense, which I did not check. Mutation-tested against BOTH predecessors, which is what establishes that this is strictly better rather than a trade-back: revert to the unconditional exclusion (prev head) -> 11 failures remove the exclusion entirely (head before that) -> 26 failures lookahead accepts any word, not that/whether -> 26 failures 17 new rows assert both senses simultaneously: 11 filler-sense that must veto, 6 connective-sense that must classify. 402 tests pass across the five heartbeat suites; typecheck clean. ReDoS bound re-measured independently: 98KB of filler, 629KB of alternating connectives, and 3000 repeated clauses all complete well inside a 500ms assertion. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../__tests__/heartbeat-context-summary.test.ts | 17 +++++++++++++++++ server/src/services/heartbeat.ts | 12 +++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index bfe57ca10c35..015fbb2fdf7a 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1192,6 +1192,23 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Already reviewed at head 8B237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], [`Already reviewed at 2026-09-02T23:31:00Z for 8B237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], [`Already reviewed at 8b237675B19FA5AE061821FD3B1D87CD8CD1836F`, false], + [`No confirmation yet that this head was already reviewed at \`${sha}\`.`, false], + [`No evidence yet that this head was already reviewed at \`${sha}\`.`, false], + [`There is no record yet that this head was already reviewed at \`${sha}\`.`, false], + [`No indication yet that this head was already reviewed at \`${sha}\`.`, false], + [`I have no proof yet that this head was already reviewed at \`${sha}\`.`, false], + [`No sign yet that this head was already reviewed at \`${sha}\`.`, false], + [`Cannot confirm yet that this head was already reviewed at \`${sha}\`.`, false], + [`No verification yet that this head was already reviewed at \`${sha}\`.`, false], + [`No confirmation still that this head was already reviewed at \`${sha}\`.`, false], + [`No evidence however that this head was already reviewed at \`${sha}\`.`, false], + [`No record though that this head was already reviewed at \`${sha}\`.`, false], + [`No newer commits were found yet this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found however this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found though this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found still this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found nonetheless this head was already reviewed at \`${sha}\`.`, true], + [`No newer commits were found whereas this head was already reviewed at \`${sha}\`.`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8d6d5e636850..9ec0ab0b4de6 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9538,7 +9538,17 @@ const CONNECTIVES = // these and vetoed a clause it does not govern. Unlike the hedge vocabulary, // the connective set is CLOSED, so completing it converges. "|yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then|consequently|accordingly"; -const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b)\\w+\\s+){0,4}`; +// The connectives are DUAL-ROLE words, so the exclusion is conditioned on the +// role rather than applied to the word. In the CONNECTIVE role the word is +// followed by the subject ("...were found yet this head was already reviewed"), +// and the filler must not cross it. In the FILLER role it is followed by a +// complementizer ("no confirmation yet THAT this head was already reviewed"), +// where it is ordinary English for "I could not establish this" and the +// negation must still reach the clause. Excluding them unconditionally traded +// 12 filler-sense vetoes for 12 connective-sense acceptances 1:1 and lost the +// side a reviewer actually writes (fifteenth pass); the nested lookahead is +// strictly better than either. +const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:that|whether)\\b))\\w+\\s+){0,4}`; const COPULA_EDGE = `${COPULA_FILLER}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s\`*_]*$`; const CLAUSE_REACH = `(?:[\\s\`*_]*$|\\s+${COPULA_EDGE})`; From fb5378b1b306bd79f4a31005925fe4835ad283d4 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 01:24:45 -0700 Subject: [PATCH 20/34] fix(heartbeat): land the three deferred follow-ups All three items previously deferred to BLO-31526, now done here. **1. A rejected sha token no longer consumes the clause** (Ally pass 15, suggestion 1). `matchAll` yields non-overlapping matches, so `continue` after the decimal guard swallowed the only `already reviewed at` prefix and never reached a real sha later in the SAME clause: "already reviewed at 1234567 (sha `<40-hex>`)" classified as not-a-skip. The remainder of the clause is now re-scanned, but the later token must be marker-qualified (`sha`/`commit`/ `head`) -- mirroring the rule the rejected token just failed. Without that constraint any incidental hex word in the tail ("...and the deadbeef branch") would be read as the cited commit, which is the masking direction. **2. The filler bound is derived, and deliberately non-binding** (pass 15, suggestion 2). The literal `{0,4}` was a second, undocumented cap on cue reach that truncated it before the connective logic ran, so "no evidence while checking that this head was already reviewed at ..." was accepted. There is now exactly ONE operative limit -- CLAUSE_SCOPE_CHARS -- with the word bound sized at CLAUSE_SCOPE_CHARS / 5 purely to keep the quantifier bounded for ReDoS. Measuring this corrected my own comment: a filler that long exhausts the character window and crowds out the cue word itself, so the character cap always binds first. "No evidence" + 23 filler words + "was" does NOT veto. Two rows pin that boundary, and raising CLAUSE_SCOPE_CHARS to 400 fails one of them, which is what proves the two caps are not redundant. Fixing 2 exposed a second cause behind the same row: `while` is followed by a participle, not a complementizer, so the role test read it as a connective and blocked the negation anyway. The complementizer may now sit up to two words out. **3. Pass-13 inflections, predicative adjectives and epistemic nouns.** `appear(?:s|ed)?`, `think(?:s|ing)?`, `look(?:s|ed)?\s+like`, plus probable/possible/conceivable/presumable/apparent and presumption/belief/impression/understanding. The adverb-but-not-adjective and `assumption`-but-not-`belief` asymmetries were accidents of how the list grew. Four over-veto controls assert the stance still has to govern the review. Mutation-tested, each mutant killed by a distinct row set: filler bound back to the literal 4 -> 3 failures role test back to an adjacent complementizer -> 1 pass-13 adjectives + nouns removed -> 9 pass-13 inflections reverted -> 3 rescue accepts an unmarked tail token -> 2 rescue not attempted at all -> 2 rescue may cross a clause boundary -> 1 clause cap raised to 400 -> 1 427 tests pass across the five heartbeat suites; typecheck clean. ReDoS re-measured with the raised bound, worst case 6.8ms: 98KB plain filler 6.8ms, 147KB alternating connectives 3.7ms, 182KB of 3000 clauses 0.1ms, 210KB of decimal-rescue tails 2.9ms, 117KB connective-only filler 1.2ms. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 27 +++++++++ server/src/services/heartbeat.ts | 56 ++++++++++++++++--- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 015fbb2fdf7a..dec538ea27c7 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1209,6 +1209,33 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No newer commits were found still this head was already reviewed at \`${sha}\`.`, true], [`No newer commits were found nonetheless this head was already reviewed at \`${sha}\`.`, true], [`No newer commits were found whereas this head was already reviewed at \`${sha}\`.`, true], + [`No evidence while checking that this head was already reviewed at \`${sha}\`.`, false], + [`No confirmation from the reviews API yet that this head was already reviewed at \`${sha}\`.`, false], + [`I could not find any record in the last hour that this head was already reviewed at \`${sha}\`.`, false], + [`No newer commits were found in the last hour so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It appeared this head was already reviewed at \`${sha}\`.`, false], + [`It looked like this head was already reviewed at \`${sha}\`.`, false], + [`I am thinking this head was already reviewed at \`${sha}\`.`, false], + [`It is probable this head was already reviewed at \`${sha}\`.`, false], + [`It is possible this head was already reviewed at \`${sha}\`.`, false], + [`It is conceivable this head was already reviewed at \`${sha}\`.`, false], + [`It is presumable this head was already reviewed at \`${sha}\`.`, false], + [`It is apparent this head was already reviewed at \`${sha}\`.`, false], + [`My presumption is that this head was already reviewed at \`${sha}\`.`, false], + [`My belief is that this head was already reviewed at \`${sha}\`.`, false], + [`My impression is that this head was already reviewed at \`${sha}\`.`, false], + [`My understanding is that this head was already reviewed at \`${sha}\`.`, false], + [`It appeared no new commits landed so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`My understanding of the wake was wrong so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`My impression about the retry cause is irrelevant so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It is possible to re-run the gate so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Already reviewed at 1234567 (sha \`${sha}\`)`, true], + [`Already reviewed at 20260902T204153Z commit ${sha}`, true], + [`Already reviewed at 1234567 and the deadbeef branch`, false], + [`Already reviewed at 1234567 (see deadbeef)`, false], + [`Already reviewed at 1234567. The commit ${sha} is unrelated.`, false], + [`No evidence ${"word ".repeat(23)}was already reviewed at \`${sha}\`.`, true], + [`No evidence ${"word ".repeat(6)}was already reviewed at \`${sha}\`.`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 9ec0ab0b4de6..fbd39a9aa8bc 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9544,11 +9544,30 @@ const CONNECTIVES = // and the filler must not cross it. In the FILLER role it is followed by a // complementizer ("no confirmation yet THAT this head was already reviewed"), // where it is ordinary English for "I could not establish this" and the -// negation must still reach the clause. Excluding them unconditionally traded +// negation must still reach the clause. The complementizer need not be +// adjacent — "no evidence while CHECKING that this head was …" puts a +// participle in between — so the role test allows it up to two words out. Excluding them unconditionally traded // 12 filler-sense vetoes for 12 connective-sense acceptances 1:1 and lost the // side a reviewer actually writes (fifteenth pass); the nested lookahead is // strictly better than either. -const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:that|whether)\\b))\\w+\\s+){0,4}`; +// The filler bound is DERIVED from the clause cap, and is deliberately +// NON-BINDING. There is exactly one operative limit on how far a cue reaches — +// CLAUSE_SCOPE_CHARS — and this bound exists only to keep the quantifier +// bounded for ReDoS. Sizing it at CLAUSE_SCOPE_CHARS / 5 (a conservative 5 +// characters per word including its space) makes it wider than the character +// window can ever fill: a filler of that many words consumes the whole window +// and crowds out the cue word itself, so the character cap always binds first. +// Measured: "No evidence" + 23 filler words + "was" does NOT veto, because the +// negation falls outside the 120-character slice. +// +// The previous literal 4 was a SECOND, tighter cap with no derivation, and it +// silently truncated reach before the connective logic ran: "no evidence while +// checking that this head was already reviewed at …" is five filler words, so +// the negation never reached the clause and the claim was accepted (fifteenth +// pass, pre-existing at every prior head). +const CLAUSE_SCOPE_CHARS = 120; +const COPULA_FILLER_WORDS = Math.floor(CLAUSE_SCOPE_CHARS / 5); +const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:\\w+\\s+){0,2}(?:that|whether)\\b))\\w+\\s+){0,${COPULA_FILLER_WORDS}}`; const COPULA_EDGE = `${COPULA_FILLER}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s\`*_]*$`; const CLAUSE_REACH = `(?:[\\s\`*_]*$|\\s+${COPULA_EDGE})`; @@ -9605,8 +9624,14 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ name: "assumption", stem: "\\b(?:possibly|probably|presumably|assuming|assumption|apparently|guess(?:es|ing|ed)?" + - "|perhaps|maybe|likely|plausible|seem(?:s|ed)?|appears?|believ(?:e|es|ed)" + - "|think(?:s)?|thought|suspect(?:s|ed)?|looks?\\s+like" + + "|perhaps|maybe|likely|plausible|seem(?:s|ed)?|appear(?:s|ed)?|believ(?:e|es|ed)" + + "|think(?:s|ing)?|thought|suspect(?:s|ed)?|look(?:s|ed)?\\s+like" + + // Thirteenth pass: predicative adjectives whose ADVERB was already + // listed, and the epistemic NOUNS beside the listed `assumption`. Same + // stance, different part of speech; the adverb/noun asymmetry was an + // accident of how the list grew, not a distinction worth keeping. + "|probable|possible|conceivable|presumable|apparent" + + "|presumption|belief|impression|understanding" + "|(?`)" (fifteenth + // pass). Re-scan the remainder of this clause, but require the later + // token to be marker-qualified (`sha`/`commit`/`head`), mirroring the + // rule the rejected token just failed: without that, any incidental hex + // word in the tail ("…and the deadbeef branch") would be read as the + // cited commit, which is the masking direction. + const tail = text.slice(m.index + m[0].length); + const sameClause = /^[^.\n:;,—–]*/.exec(tail)?.[0] ?? ""; + const rescued = /\b(?:sha|commit|head)[\s`*_:=]+([0-9a-f]{7,40})(?![0-9a-f])/i.exec(sameClause); + const rescuedSha = rescued?.[1] ?? ""; + if (!/^[0-9a-f]+$/.test(rescuedSha) || !/[a-f]/.test(rescuedSha)) continue; + } + const before = clauseBefore.exec(text.slice(Math.max(0, m.index - CLAUSE_SCOPE_CHARS), m.index))?.[0] ?? ""; if (GOVERNING_CUES.some(({ re }) => re.test(before))) continue; return true; } From 4b1b962862e1cb1bed662664ef092c9226f9ac66 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 01:59:30 -0700 Subject: [PATCH 21/34] fix(heartbeat): close the probe-found gaps; bound the rescue slice Two sources landed together here, both measured rather than reasoned. **A. Ally pass 16, both Important findings and the Suggestion.** 1. QUADRATIC, self-inflicted two commits ago. The decimal-guard rescue sliced the ENTIRE remaining text per rejected match, and `matchAll` yields one per `already reviewed at `. Measured on repeated timestamp lines: 68KB 51ms, 137KB 201ms, 273KB 769ms, 547KB 3115ms -- clean 4x per doubling. Bounded by CLAUSE_SCOPE_CHARS, which `sameClause` could never see past anyway: 547KB -> 33ms. My earlier ReDoS check used one rescue per 400-byte clause and never exercised the dense case, which is why it passed. 2. MASKING: an elided complementizer after a filler-role connective. The role test keys on a FOLLOWING `that`, so "No evidence yet this head was ..." read the connective as connective-role and the negation never reached the clause -- 35/35 such phrasings accepted, at every head since the connectives were excluded. The head noun cannot disambiguate (`found` is itself a head); the discriminator is the GAP between negation and head. New cue `negatedHeadConnective`, which inherits CLAUSE_REACH like every other. 3. A demonstrative `that ` after a connective is not a complementizer and must not flip the connective into filler role: "found yet THAT HEAD was" false-vetoed 20/20 connectives. **B. Adversarial probe workflow (14 agents, 7 families, 98 of 124 claims refuted by independent verifiers).** Of 26 confirmed gaps, the 5 that replace enumeration with a derivation are landed; the rest go to BLO-31526. 4. Anchor noun tolerates a determiner, a `sha` apposition, and punctuation ("at head: ", "at head sha " were refused). A bare `sha` noun is deliberately NOT admitted -- it would qualify decimals. 5. Contracted and inflected negation auxiliaries (didn't/hasn't/haven't/...). The SAME FILE's prReviewOutputHasPostedReviewNegation already lists them. 6. Epistemic heads are morphological STEMS, not surface forms: the list had `indicates` and `indicating` but not `indicated`, `record` but not `records`. Enumerating inflections does not converge; enumerating lemmas does. Also adds show/mention/report, the reporting verbs the probe found. 7. `unaware`-class FUSED negations get their own cue: the morpheme is inside the head word, so the `negation` prefix can never anchor on it. Measured: "not aware that" vetoed, "unaware that" did not. 8. A relative pronoun abutting the copula is not a complementizer -- a complementizer is always followed by a SUBJECT -- so "for the head THAT WAS already reviewed" was false-vetoed. Lookbehind on the copula. 9. The filler token is anything that is NOT a clause boundary, derived from clauseBefore's own set, so a backtick or possessive no longer blocks the cue from its own citation ("I cannot confirm `` was ..."). The negation cue's two halves are now named constants (NEGATION_PREFIX_WORD, EPISTEMIC_HEAD) so cue 2 reuses them without a copy that drifts. Mutation-tested, each killed by a distinct row set: negatedHeadConnective unmatchable -> see run demonstrative guard removed -> 3 anchor noun back to bare head|commit -> 7 contracted auxiliaries removed -> 5 indicat stem back to exact forms -> 1 plural nouns back to singular -> 2 relative-pronoun lookbehind removed -> 4 filler token back to \w+ -> 5 show/mention/report removed -> 2 fusedNegation removed -> 1 rescue slice unbounded -> 547KB 3115ms vs 33ms (timing, above) One mutant was WRONG the first time: renaming a cue's `name` does not disable it (name is attribution-only), so the first M73 killed 1 row and looked like weak coverage. Attribution showed all 18 elided-that rows fire the new cue; the corrected mutant makes the stem unmatchable. 507 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 78 +++++++++++++ server/src/services/heartbeat.ts | 106 ++++++++++++++++-- 2 files changed, 174 insertions(+), 10 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index dec538ea27c7..8e72778cc3b2 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1236,6 +1236,80 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Already reviewed at 1234567. The commit ${sha} is unrelated.`, false], [`No evidence ${"word ".repeat(23)}was already reviewed at \`${sha}\`.`, true], [`No evidence ${"word ".repeat(6)}was already reviewed at \`${sha}\`.`, false], + [`Already reviewed at head: ${sha}`, true], + [`Already reviewed at the head ${sha}`, true], + [`Already reviewed at current head ${sha}`, true], + [`Already reviewed at head sha ${sha}`, true], + [`Already reviewed at commit sha ${sha}`, true], + [`Already reviewed at head=${sha}`, true], + [`Already reviewed at sha 1234567`, false], + [`Already reviewed at head sha 1234567`, true], + [`I haven't confirmed that this head was already reviewed at \`${sha}\`.`, false], + [`I hadn't verified that this head was already reviewed at \`${sha}\`.`, false], + [`The run didn't establish that this head was already reviewed at \`${sha}\`.`, false], + [`The API hasn't confirmed that this head was already reviewed at \`${sha}\`.`, false], + [`The checks weren't showing that this head was already reviewed at \`${sha}\`.`, false], + [`I am failing to confirm that this head was already reviewed at \`${sha}\`.`, false], + [`There was a failure to confirm that this head was already reviewed at \`${sha}\`.`, false], + [`Nothing indicated this head was already reviewed at \`${sha}\`.`, false], + [`There is no suggestion that this head was already reviewed at \`${sha}\`.`, false], + [`There are no records that this head was already reviewed at \`${sha}\`.`, false], + [`There are no traces that this head was already reviewed at \`${sha}\`.`, false], + [`I never saw that this head was already reviewed at \`${sha}\`.`, false], + [`I am unaware that this head was already reviewed at \`${sha}\`.`, false], + [`No statement that this head was already reviewed at \`${sha}\`.`, false], + [`Nothing was demonstrating that this head was already reviewed at \`${sha}\`.`, false], + [`The run never told me that this head was already reviewed at \`${sha}\`.`, false], + [`I could not rule out that this head was already reviewed at \`${sha}\`.`, false], + [`Nothing indicated a newer commit so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No records of a force-push so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I never saw a newer head so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I am unaware of any newer commits so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer commit was found for the head that was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing found since the commit that was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I could not find any commit newer than the one that was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`The head moved to a commit which was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence that this head was already reviewed at \`${sha}\`.`, false], + [`No evidence while checking that this head was already reviewed at \`${sha}\`.`, false], + [`I cannot confirm \`8b23767\` was already reviewed at \`${sha}\`.`, false], + [`I could not verify this PR's head was already reviewed at \`${sha}\`.`, false], + [`I could not verify (after two retries) this head was already reviewed at \`${sha}\`.`, false], + [`There is no evidence - none - this head was already reviewed at \`${sha}\`.`, false], + [`I cannot confirm "this" head was already reviewed at \`${sha}\`.`, false], + [`I cannot confirm the wake. This head was already reviewed at \`${sha}\`.`, true], + [`No evidence of a force-push; this head was already reviewed at \`${sha}\`.`, true], + [`I remain unconvinced that this head was already reviewed at \`${sha}\`.`, false], + [`Unaware whether this head was already reviewed at \`${sha}\`.`, false], + [`I am unaware of any newer commits so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Unconvinced the wake was legitimate so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing showed that this head was already reviewed at \`${sha}\`.`, false], + [`Nothing showed a newer commit so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No evidence still this head was already reviewed at \`${sha}\`.`, false], + [`No evidence however this head was already reviewed at \`${sha}\`.`, false], + [`No indication yet this head was already reviewed at \`${sha}\`.`, false], + [`No indication still this head was already reviewed at \`${sha}\`.`, false], + [`No indication however this head was already reviewed at \`${sha}\`.`, false], + [`No confirmation yet this head was already reviewed at \`${sha}\`.`, false], + [`No confirmation still this head was already reviewed at \`${sha}\`.`, false], + [`No confirmation however this head was already reviewed at \`${sha}\`.`, false], + [`I cannot say yet this head was already reviewed at \`${sha}\`.`, false], + [`I cannot say still this head was already reviewed at \`${sha}\`.`, false], + [`I cannot say however this head was already reviewed at \`${sha}\`.`, false], + [`There is no record yet this head was already reviewed at \`${sha}\`.`, false], + [`There is no record still this head was already reviewed at \`${sha}\`.`, false], + [`There is no record however this head was already reviewed at \`${sha}\`.`, false], + [`Nothing suggests yet this head was already reviewed at \`${sha}\`.`, false], + [`Nothing suggests still this head was already reviewed at \`${sha}\`.`, false], + [`Nothing suggests however this head was already reviewed at \`${sha}\`.`, false], + [`No newer commits were found yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing new was seen still this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence of a force-push however this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer commits were found yet that head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer commits were found so that commit was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing new was seen still that head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence yet that this head was already reviewed at \`${sha}\`.`, false], + [`No evidence still that this head was already reviewed at \`${sha}\`.`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1256,6 +1330,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["The prior head was ", "priorHead"], ["No evidence that this head was ", "negation"], ["I cannot say this head was ", "negation"], + ["I am unaware that this head was ", "fusedNegation"], + ["No evidence yet this head was ", "negatedHeadConnective"], + ["No evidence that this head was ", "negation"], + ["I am not aware that this head was ", "negation"], // Correct skips: no cue governs the clause, so nothing vetoes. ["No evidence of a force-push, so ", null], ["I did not check whether a newer head exists so ", null], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index fbd39a9aa8bc..f1be9b82d76e 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9567,10 +9567,59 @@ const CONNECTIVES = // pass, pre-existing at every prior head). const CLAUSE_SCOPE_CHARS = 120; const COPULA_FILLER_WORDS = Math.floor(CLAUSE_SCOPE_CHARS / 5); -const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:\\w+\\s+){0,2}(?:that|whether)\\b))\\w+\\s+){0,${COPULA_FILLER_WORDS}}`; -const COPULA_EDGE = `${COPULA_FILLER}(?:was|were|is|are|has\\s+been|had\\s+been)[\\s\`*_]*$`; +// The filler token is anything that is NOT a clause boundary, rather than a +// word character. The excluded set is EXACTLY clauseBefore's boundary set plus +// whitespace, so the filler can cross a backtick, a possessive, a parenthesis +// or a hyphen but can never cross a boundary — one definition of "clause", +// used in both places. With `\\w+` the cue was blocked by its own citation: +// "I cannot confirm `` was already reviewed at …" leaked (masking). +// …and a complementizer immediately followed by an anchor NOUN is a +// demonstrative, not a complementizer ("yet THAT HEAD was" vs "yet that THIS +// head was"), so it must not flip the connective into its filler role and let +// the negation cross (Ally pass 16 suggestion: 20/20 connectives false-vetoed). +const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:[^\\s.\\n:;,\u2014\u2013]+\\s+){0,2}(?:that|whether)\\b(?!\\s+(?:head|commit|sha|branch|pr)\\b)))[^\\s.\\n:;,\u2014\u2013]+\\s+){0,${COPULA_FILLER_WORDS}}`; +// A relative pronoun abutting the copula is NOT a complementizer, and the +// distinction is syntactic rather than lexical: a complementizer is always +// followed by a SUBJECT before the copula ("confirm that this head was"), so +// `that` sitting directly against the copula can only head a relative clause +// modifying the preceding noun phrase. Without the lookbehind the cue reached +// across it and refused correct skips like "no newer commit was found for the +// head that was already reviewed at …" (false-missing direction). +const COPULA_EDGE = `${COPULA_FILLER}(? = [ "\\b(?:(?:prior|previous|earlier|stale|old|superseded)\\s+head" + "|branch\\s+(?:has\\s+)?moved|head\\s+(?:has\\s+)?moved)", }, + // A negated head IMMEDIATELY followed by a connective, with the + // complementizer elided: "No evidence yet this head was …", "I cannot say + // yet this head was …". The role test above keys on a FOLLOWING + // complementizer, so when `that` is dropped the filler-role connective reads + // as connective-role and the negation never reaches the clause (Ally pass + // 16: 35/35 such phrasings accepted, at every head since the connectives + // were excluded). The discriminator is the GAP between the negation and its + // head: zero intervening words is the filler role ("no evidence yet"), one + // or more is the connective role ("no newer commits were found yet") — the + // head noun alone cannot tell them apart, since `found` is itself a head. + // Dropping the exclusion instead trades back 20/20 connective-sense skips. + { + name: "negatedHeadConnective", + stem: `${NEGATION_PREFIX_WORD}\\s+${EPISTEMIC_HEAD}\\s+(?:${CONNECTIVES})\\b`, + }, // An epistemic negation governing the clause from further back: a negation // word, then within three words a head that could establish the claim. One // list, one binding rule — evidence nouns are NOT exempt (ninth pass: @@ -9662,14 +9726,22 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // NOT a bare negation — "Exiting without posting since …", "No action taken // because …" are how a correct skip explains itself. `cannot`/`can't` are // listed because a bare not-boundary does not match inside `cannot`. + // A FUSED negation: the negative morpheme is part of the head word itself + // ("I am UNaware that this head was …", "UNconfirmed whether this head was + // …"), so there is no separate negation word for the `negation` prefix to + // anchor on, and listing these as heads left them unreachable — measured: + // "not aware that" vetoed while "unaware that" did not. They get their own + // stem, which inherits CLAUSE_REACH like every other, so the fused form binds + // exactly as the spelled-out one does. `hedge` already covers unclear/unsure/ + // uncertain/unconfirmed/unverified; this entry adds the mental-state + // participles that `hedge` does not carry. + { + name: "fusedNegation", + stem: "\\b(?:unaware|unconvinced|unpersuaded|unsatisfied)\\s+(?:\\w+\\s+){0,3}(?:that|whether|if)", + }, { name: "negation", - stem: - "\\b(?:no|not|never|nothing|neither|nor|cannot|can['\u2019]t|couldn['\u2019]t|doesn['\u2019]t|don['\u2019]t|isn['\u2019]t|wasn['\u2019]t|unable\\s+to|failed\\s+to)\\s+(?:\\w+\\s+){0,3}" + - // Evidence nouns and mental-state predicates, the establishing verbs, and - // the reporting verbs whose complementizer is routinely elided ("cannot - // say this head WAS already reviewed at …" — caught by the copula arm). - "(?:evidence|indications?|indicates?|indicating|signs?|record|proof|trace|believe|think|appears?|suggests?|aware|see|seen|confirms?|confirmed|confirmation|verification|determination|rule\\s+out|verif(?:y|ies|ied)|establish(?:es|ed)?|determin(?:e|es|ed)|find|finds|found|check(?:s|ed)?|locat(?:e|es|ed)|ascertain(?:ed)?|validat(?:e|es|ed)|prov(?:e|es|en)|demonstrat(?:e|es|ed)|tells?|says?|said|states?|asserts?|claims?)", + stem: `${NEGATION_PREFIX_WORD}\\s+(?:\\w+\\s+){0,3}${EPISTEMIC_HEAD}`, }, ]; @@ -9699,7 +9771,15 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // `+` after `at` too: `at8b237675…` with no separator is not the clause. "\\balready[\\s`*_]+reviewed[\\s`*_]+at[\\s`*_]+" + // optional " for" (plain shape) and/or a "head"/"commit" noun - `(?[^\\s\`*_]{1,40}${md}for${md})?(?(?:head|commit)[\\s\`*_]+)?` + + `(?[^\\s\`*_]{1,40}${md}for${md})?` + + // The anchor noun tolerates a determiner, an optional `sha` apposition, + // and punctuation separators: "at head: ", "at the head " and + // "at head sha " were all refused (false-missing direction, found by + // a parallel probe rather than by review). The separator quantifier stays + // `+`, so `head` with no separator is still not the clause. A bare + // `sha` noun is deliberately NOT admitted: it would widen the all-decimal + // surface, since "at sha 1234567" would then count as marker-qualified. + "(?(?:(?:the|this|current|latest)[\\s`*_]+)?(?:head|commit)(?:[\\s`*_]+sha)?[:=,\\s`*_]+)?" + "(?[0-9a-f]{7,40})(?![0-9a-f])", "gi", ); @@ -9733,7 +9813,13 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { // rule the rejected token just failed: without that, any incidental hex // word in the tail ("…and the deadbeef branch") would be read as the // cited commit, which is the masking direction. - const tail = text.slice(m.index + m[0].length); + // Bounded by the clause cap: an unbounded slice copies O(n) per rejected + // token and `matchAll` yields one per `already reviewed at `, so + // the classifier went quadratic on run output — measured 12.8s at 547KB + // against 11ms before (Ally pass 16). `sameClause` stops at the first + // boundary anyway, so nothing past CLAUSE_SCOPE_CHARS was ever reachable. + const start = m.index + m[0].length; + const tail = text.slice(start, start + CLAUSE_SCOPE_CHARS); const sameClause = /^[^.\n:;,—–]*/.exec(tail)?.[0] ?? ""; const rescued = /\b(?:sha|commit|head)[\s`*_:=]+([0-9a-f]{7,40})(?![0-9a-f])/i.exec(sameClause); const rescuedSha = rescued?.[1] ?? ""; From 04ecfa0d05c151b0c1d8762cdc9f3fdffba0d8d6 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 02:05:12 -0700 Subject: [PATCH 22/34] fix(heartbeat): the two probe gaps that contradicted my own deferral rule I deferred sixteen probe findings to BLO-31526 on the rule "land what replaces enumeration with a derivation; park open-set enumeration". Two of the sixteen were neither, and I said so in the issue rather than fixing them. Fixing them. 1. `I assume this head was ...` -- `assuming` and `assumption` were both listed, the finite verb was not. Pure list accident, now the lemma `assum(?:e|es|ed|ing|ption|ptions)` plus `presum(...)`, so "I assumed", "the run assumes", "my assumptions are" all veto. 2. `Possibly, this head was ...` -- a sentence-initial hedge adverb's OWN trailing comma was read as a clause boundary, severing the adverb from the clause it governs, so a stem the classifier already carried never reached it. The comma is rejoined only in that one shape: anchored to a sentence start, so a comma genuinely separating two clauses ("the wake was possibly a duplicate, so already reviewed at ...") stays a boundary and that correct skip still classifies. Mutation-tested: assum stem back to assuming|assumption -> 5 failures leading-adverb comma no longer rejoined -> 3 rejoin loses its sentence-start anchor -> see run 518 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 11 +++++++++++ server/src/services/heartbeat.ts | 19 +++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 8e72778cc3b2..c87d1a38522e 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1310,6 +1310,17 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Nothing new was seen still that head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No evidence yet that this head was already reviewed at \`${sha}\`.`, false], [`No evidence still that this head was already reviewed at \`${sha}\`.`, false], + [`I assume this head was already reviewed at \`${sha}\`.`, false], + [`I assumed this head was already reviewed at \`${sha}\`.`, false], + [`The run assumes this head was already reviewed at \`${sha}\`.`, false], + [`My assumptions are that this head was already reviewed at \`${sha}\`.`, false], + [`I presume this head was already reviewed at \`${sha}\`.`, false], + [`Possibly, this head was already reviewed at \`${sha}\`.`, false], + [`Probably, this head was already reviewed at \`${sha}\`.`, false], + [`Perhaps, this head was already reviewed at \`${sha}\`.`, false], + [`I checked the wake. Possibly, a retry so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`The wake was possibly a duplicate, so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Not possibly, this head was already reviewed at \`${sha}\`.`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index f1be9b82d76e..69a7e5073dd0 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9620,6 +9620,13 @@ const EPISTEMIC_HEAD = "|stat(?:e|es|ed|ing)|assert(?:s|ed|ing)?|claim(?:s|ed|ing)?" + "|rul(?:e|es|ed|ing)\\s+out)"; +// See the use site in prReviewOutputHasAlreadyReviewedSkip: a sentence-initial +// hedge adverb's own trailing comma is rejoined to its clause. Anchored to a +// sentence start (beginning of window, or after . : ; newline) so it cannot +// touch a comma that is genuinely separating two clauses. +const LEADING_ADVERB_COMMA = + /(^|[.:;\n]\s*)((?:possibly|probably|presumably|apparently|perhaps|maybe|likely|arguably|seemingly|supposedly|allegedly|reportedly)),\s*/gi; + // Ally passes six through ten each found exactly one more cue matching // unbound — a veto reaching a clause it does not govern, which is the // false-`missing` direction this function exists to remove. Binding them one @@ -9672,7 +9679,7 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ { name: "assumption", stem: - "\\b(?:possibly|probably|presumably|assuming|assumption|apparently|guess(?:es|ing|ed)?" + + "\\b(?:possibly|probably|presumably|assum(?:e|es|ed|ing|ption|ptions)|presum(?:e|es|ed|ption|ptions)|apparently|guess(?:es|ing|ed)?" + "|perhaps|maybe|likely|plausible|seem(?:s|ed)?|appear(?:s|ed)?|believ(?:e|es|ed)" + "|think(?:s|ing)?|thought|suspect(?:s|ed)?|look(?:s|ed)?\\s+like" + // Thirteenth pass: predicative adjectives whose ADVERB was already @@ -9825,7 +9832,15 @@ export function prReviewOutputHasAlreadyReviewedSkip(text: string): boolean { const rescuedSha = rescued?.[1] ?? ""; if (!/^[0-9a-f]+$/.test(rescuedSha) || !/[a-f]/.test(rescuedSha)) continue; } - const before = clauseBefore.exec(text.slice(Math.max(0, m.index - CLAUSE_SCOPE_CHARS), m.index))?.[0] ?? ""; + // A sentence-initial hedge adverb followed by its own comma ("Possibly, + // this head was …") is one clause, not two: the comma belongs to the + // adverb, and treating it as a boundary severs the adverb from the clause + // it governs — measured as a masking gap by the parallel probe. Only that + // one shape is rejoined; a comma anywhere else remains a boundary. + const window = text + .slice(Math.max(0, m.index - CLAUSE_SCOPE_CHARS), m.index) + .replace(LEADING_ADVERB_COMMA, "$1$2 "); + const before = clauseBefore.exec(window)?.[0] ?? ""; if (GOVERNING_CUES.some(({ re }) => re.test(before))) continue; return true; } From fe8ed114f5584332c9883481c4143519c9cbfc45 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 02:24:18 -0700 Subject: [PATCH 23/34] fix(heartbeat): let an adjective sit between a negation and its head Ally pass 17: `negatedHeadConnective` required the epistemic head IMMEDIATELY after the negation, so one adjective -- the ordinary way these are written -- reopened the masking gap the cue was added to close: "No CLEAR evidence yet this head was already reviewed at ..." accepted, 6/6. Same shape pass 11 settled for `doubt`. The stem now allows up to two intervening words, EXCLUDING auxiliaries, since "no newer commits WERE found yet ..." keeps its verb between negation and head and that verb is what marks a real clause. Applying Ally's recommended stem as-is regressed one seventh-pass row that its 8 controls did not cover: "nothing else found SO already reviewed at ..." -- a correct skip -- began to veto. Measuring every connective against every head shape showed the cue was conflating two roles. A CONSEQUENCE connective (so/because/since/therefore/thus/hence) introduces a new clause whose subject is what follows, so a negated head before it governs the PREVIOUS clause and can never carry an elided complement. An ADVERSATIVE (yet/still/however/...) can sit inside one clause with `that` dropped. The measurement also found the new stem had silently broken six correct skips of the form "no evidence so this head was ...", none of which were in the corpus. CONNECTIVES is now the union of two named halves; the copula filler still uses the union, the elided-complement cue uses the adversative half only. Mutation-tested: cue back to all connectives -> 7 failures slack back to adjacency-only -> 6 auxiliary exclusion removed -> 2 534 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 16 +++++++++++ server/src/services/heartbeat.ts | 27 ++++++++++++------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index c87d1a38522e..b14e0854e44a 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1321,6 +1321,22 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`I checked the wake. Possibly, a retry so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`The wake was possibly a duplicate, so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Not possibly, this head was already reviewed at \`${sha}\`.`, true], + [`No clear evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No direct evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No strong indication yet this head was already reviewed at \`${sha}\`.`, false], + [`No independent confirmation yet this head was already reviewed at \`${sha}\`.`, false], + [`There is no real evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No further indication still this head was already reviewed at \`${sha}\`.`, false], + [`No newer commits were found yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer revisions were seen yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing new has been found still this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer head is known yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No clear evidence because this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing else found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing new seen therefore this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No confirmation hence this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No indication thus this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 69a7e5073dd0..e7312ebcd68a 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9532,12 +9532,17 @@ function prReviewOutputHasSelfReviewSkip( // governing a DIFFERENT noun phrase reach the clause across the connective — // "No newer commits were found so this head was already reviewed at …" // (eleventh pass: 5 of 5 regressed against master). -const CONNECTIVES = - "so|but|and|because|since|therefore|thus|hence" + - // Adversative and consecutive (fourteenth pass): a negation crossed all of - // these and vetoed a clause it does not govern. Unlike the hedge vocabulary, - // the connective set is CLOSED, so completing it converges. - "|yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then|consequently|accordingly"; +// Two kinds of connective, which the copula filler treats alike but the +// elided-complement cue must NOT: a CONSEQUENCE connective introduces a new +// clause whose subject is what follows ("nothing else found SO this head was"), +// so a negated head before it governs the PREVIOUS clause and never this one; +// an ADVERSATIVE can sit inside a single clause with the complementizer +// dropped ("no clear evidence YET this head was"). Measured (pass 17): treating +// them alike vetoed 6 correct skips of the form "no evidence so this head was". +const CONSEQUENCE_CONNECTIVES = "so|because|since|therefore|thus|hence|and|but"; +const ADVERSATIVE_CONNECTIVES = + "yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then|consequently|accordingly"; +const CONNECTIVES = `${CONSEQUENCE_CONNECTIVES}|${ADVERSATIVE_CONNECTIVES}`; // The connectives are DUAL-ROLE words, so the exclusion is conditioned on the // role rather than applied to the word. In the CONNECTIVE role the word is // followed by the subject ("...were found yet this head was already reviewed"), @@ -9715,13 +9720,17 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // as connective-role and the negation never reaches the clause (Ally pass // 16: 35/35 such phrasings accepted, at every head since the connectives // were excluded). The discriminator is the GAP between the negation and its - // head: zero intervening words is the filler role ("no evidence yet"), one - // or more is the connective role ("no newer commits were found yet") — the + // head. An ADJECTIVE does not widen that gap — "no CLEAR evidence yet this + // head was …" is the filler role written the ordinary way (Ally pass 17: + // adjacency-only accepted 6/6 such phrasings; the same shape pass 11 settled + // for `doubt`). So the stem allows up to two intervening words, EXCLUDING + // auxiliaries: "no newer commits WERE found yet …" keeps its verb between + // negation and head, and that verb is what marks the connective role. // head noun alone cannot tell them apart, since `found` is itself a head. // Dropping the exclusion instead trades back 20/20 connective-sense skips. { name: "negatedHeadConnective", - stem: `${NEGATION_PREFIX_WORD}\\s+${EPISTEMIC_HEAD}\\s+(?:${CONNECTIVES})\\b`, + stem: `${NEGATION_PREFIX_WORD}\\s+(?:(?!(?:was|were|is|are|has|have|had|been|be|did|does|do)\\b)\\w+\\s+){0,2}${EPISTEMIC_HEAD}\\s+(?:${ADVERSATIVE_CONNECTIVES})\\b`, }, // An epistemic negation governing the clause from further back: a negation // word, then within three words a head that could establish the claim. One From a60804a176e15ebd7c19e89acf68aa70fc5563ee Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Fri, 4 Sep 2026 05:03:40 -0700 Subject: [PATCH 24/34] fix(heartbeat): split connectives by sense; slack only noun heads Ally pass 18, two Important findings, both in my pass-17 fix. 1. The consequence/adversative split was decided by WORD, not by sense. `consequently` and `accordingly` have no adversative reading -- they are exact synonyms of therefore/hence, which the same commit moved to the consequence set -- so an identical frame diverged purely on which synonym the run wrote: "nothing else found THEREFORE this head was" accepted, "...CONSEQUENTLY..." refused. Moved. `then` stays adversative on purpose: it has a real filler reading ("no evidence THEN that" = at that time), so its veto is defensible. 2. The `{0,2}` slack absorbed a SUBJECT noun phrase. The auxiliary exclusion marks the connective role only for verbs that TAKE an auxiliary; a present-tense verb has none, so "no newer commits APPEAR yet this head was" had its subject swallowed by the filler and `appear` consumed as the negation's head -- a false veto, regressed from the parent. The real discriminator is part of speech: an adjective may precede a NOUN head ("no clear evidence yet"), but for a VERB head the intervening words are necessarily the subject. Slack now applies to the noun subset only (EPISTEMIC_NOUN_HEAD); verb heads keep adjacency. appear/indicate/ suggest/show/mention/report/claim -- the whole present-tense half of the list -- were exposed. Suggestion taken: the shard manifest's "most recently on" date was stale against its own coverage figure, and heartbeat-context-summary.test.ts carried a weight of 54ms while this PR grew it to ~7.5s; re-measured. Mutation-tested: consequently/accordingly back to adversative -> 1 failure slack applied to all heads again -> 3 verb arm removed entirely -> 6 543 tests pass across the five heartbeat suites; typecheck clean. Refs BLO-31374 Co-Authored-By: Claude Fable 5.1 --- scripts/general-server-shard-durations.json | 4 ++-- .../heartbeat-context-summary.test.ts | 9 ++++++++ server/src/services/heartbeat.ts | 22 ++++++++++++++++--- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/scripts/general-server-shard-durations.json b/scripts/general-server-shard-durations.json index 5da4b5f0fbdd..4aecdb41d78d 100644 --- a/scripts/general-server-shard-durations.json +++ b/scripts/general-server-shard-durations.json @@ -1,5 +1,5 @@ { - "$comment": "Per-suite Vitest durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from four successful ARC PR jobs in run 31248977534 on 2026-08-08, then topped up as master added suites, most recently on 2026-08-24 via scripts/measure-general-server-shard-durations.mjs; coverage is 483/483. This sentence is regenerated on every refresh; see \"$notes\" for guidance that is meant to persist.", + "$comment": "Per-suite Vitest durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from four successful ARC PR jobs in run 31248977534 on 2026-08-08, then topped up as master added suites, most recently on 2026-09-04 via scripts/measure-general-server-shard-durations.mjs; coverage is 483/483. This sentence is regenerated on every refresh; see \"$notes\" for guidance that is meant to persist.", "$notes": "DURABLE NOTES -- unlike \"$comment\" (regenerated on every refresh), this key is never rewritten by .github/workflows/refresh-shard-manifest.yml. Put guidance that must outlive a re-sample here. NOTE ON UNITS: these are per-file TEST-EXECUTION durations (the Vitest JSON reporter's testResults[].startTime->endTime), not full wall-clock -- they exclude each file's transform/setup/import cost, which is why a trivial suite can read as single-digit ms. That fixed per-file cost is roughly uniform and the LPT partition also lands near-equal file counts per shard, so it does not skew balance; do not 'fix' a small value here by hand. Suites absent from this manifest use the median fallback (defaultSuiteWeight in scripts/general-server-shard.mjs); the refresh workflow re-samples weekly so refresh does not depend on a human noticing drift.", "unit": "ms", "durations": { @@ -179,7 +179,7 @@ "server/src/__tests__/heartbeat-auto-checkout.test.ts": 7, "server/src/__tests__/heartbeat-ccrotate-capacity-retry.test.ts": 13324, "server/src/__tests__/heartbeat-comment-wake-batching.test.ts": 22996, - "server/src/__tests__/heartbeat-context-summary.test.ts": 54, + "server/src/__tests__/heartbeat-context-summary.test.ts": 7485, "server/src/__tests__/heartbeat-cooldown.test.ts": 12, "server/src/__tests__/heartbeat-cost-accounting.test.ts": 9, "server/src/__tests__/heartbeat-dependabot-stale-wake-backfill.test.ts": 12121, diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index b14e0854e44a..04c78ec78179 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1337,6 +1337,15 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Nothing new seen therefore this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No confirmation hence this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No indication thus this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing else found consequently this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No confirmation accordingly this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Nothing else found therefore this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No evidence then this head was already reviewed at \`${sha}\`.`, false], + [`No newer commits appear yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No newer revisions indicate yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No other branches suggest yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No clear evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No strong indication still this head was already reviewed at \`${sha}\`.`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index e7312ebcd68a..943f0fcdc90e 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9539,9 +9539,14 @@ function prReviewOutputHasSelfReviewSkip( // an ADVERSATIVE can sit inside a single clause with the complementizer // dropped ("no clear evidence YET this head was"). Measured (pass 17): treating // them alike vetoed 6 correct skips of the form "no evidence so this head was". -const CONSEQUENCE_CONNECTIVES = "so|because|since|therefore|thus|hence|and|but"; +const CONSEQUENCE_CONNECTIVES = "so|because|since|therefore|thus|hence|consequently|accordingly|and|but"; const ADVERSATIVE_CONNECTIVES = - "yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then|consequently|accordingly"; + "yet|however|though|although|still|nonetheless|nevertheless|whereas|while|then"; +// `consequently`/`accordingly` are exact synonyms of therefore/hence and have no +// adversative reading, so they belong with the consequences (pass 18: an +// identical frame diverged purely on which synonym the run wrote). `then` stays +// here on purpose: it has a real filler reading — "no evidence THEN that …" = +// "no evidence at that time" — so its veto is defensible. const CONNECTIVES = `${CONSEQUENCE_CONNECTIVES}|${ADVERSATIVE_CONNECTIVES}`; // The connectives are DUAL-ROLE words, so the exclusion is conditioned on the // role rather than applied to the word. In the CONNECTIVE role the word is @@ -9612,6 +9617,15 @@ const NEGATION_PREFIX_WORD = // `records`. A parallel probe found seven such gaps at once. Enumerating // inflections does not converge; enumerating LEMMAS does, so each entry // now tolerates its own paradigm. +const EPISTEMIC_NOUN_HEAD = + "(?:evidence|indications?|signs?|records?|proofs?|traces?|suggestions?|statements?|assertions?|awareness|findings?|validations?|confirmation|verification|determination)"; +// The NOUN subset of EPISTEMIC_HEAD. An adjective may sit between a negation and +// a noun head ("no CLEAR evidence yet"), but for a VERB head the intervening +// words are necessarily the SUBJECT ("no COMMITS appear yet"), so slack there +// swallows the subject and misreads the main verb as the negation's head. +// Present-tense verbs take no auxiliary, so the auxiliary exclusion alone did +// not mark them (pass 18: appear/indicate/suggest all regressed). The +// discriminator is part of speech, not the auxiliary. const EPISTEMIC_HEAD = "(?:evidence|indications?|indicat(?:e|es|ed|ing)|signs?|records?|proofs?|traces?" + "|suggestions?|statements?|assertions?|awareness|findings?|validations?" + @@ -9730,7 +9744,9 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // Dropping the exclusion instead trades back 20/20 connective-sense skips. { name: "negatedHeadConnective", - stem: `${NEGATION_PREFIX_WORD}\\s+(?:(?!(?:was|were|is|are|has|have|had|been|be|did|does|do)\\b)\\w+\\s+){0,2}${EPISTEMIC_HEAD}\\s+(?:${ADVERSATIVE_CONNECTIVES})\\b`, + stem: + `(?:${NEGATION_PREFIX_WORD}\\s+(?:(?!(?:was|were|is|are|has|have|had|been|be|did|does|do)\\b)\\w+\\s+){0,2}${EPISTEMIC_NOUN_HEAD}` + + `|${NEGATION_PREFIX_WORD}\\s+${EPISTEMIC_HEAD})\\s+(?:${ADVERSATIVE_CONNECTIVES})\\b`, }, // An epistemic negation governing the clause from further back: a negation // word, then within three words a head that could establish the claim. One From 1ddcad775ff237a2eb227056debeb7f0aea2dadf Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 12:44:17 -0700 Subject: [PATCH 25/34] fix(heartbeat): an adverb may sit between a negation and a verb head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 18 split the negatedHeadConnective cue by part of speech: an adjective may sit between a negation and a NOUN head ("no clear evidence yet"), but a VERB head takes no slack, because the intervening words would be its subject ("no COMMITS appear yet" — the commits, not the negation, govern the verb). That split treats every intervening word as a subject, and a pre-verbal ADVERB is a third category: "I cannot FULLY confirm yet this head was already reviewed at " hedges without introducing a subject. Measured 12/12 such phrasings accepted at the parent head — the masking direction, and a regression the parent introduced, since strict adjacency is what dropped them. The slack is a CLOSED set of adverbs, not `\w+ly`. The suffix variant scored strictly worse: English nouns end in -ly too, so it false-vetoed 6/6 correct skips of the form "No anomaly/reply/supply indicates yet …". A closed set converges; a suffix pattern does not. Ten rows pin the gap: six adverb hedges that must veto, three -ly nouns that must still skip, and one attribution row naming the cue. Mutation-checked — removing the slack fails 7, widening it to `\w+ly` fails 4. Co-Authored-By: Claude Fable 5.1 --- .../__tests__/heartbeat-context-summary.test.ts | 10 ++++++++++ server/src/services/heartbeat.ts | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 04c78ec78179..dd380131ebca 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1346,6 +1346,15 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No other branches suggest yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No clear evidence yet this head was already reviewed at \`${sha}\`.`, false], [`No strong indication still this head was already reviewed at \`${sha}\`.`, false], + [`I cannot fully confirm yet this head was already reviewed at \`${sha}\`.`, false], + [`I am not fully aware yet this head was already reviewed at \`${sha}\`.`, false], + [`I cannot definitively say yet this head was already reviewed at \`${sha}\`.`, false], + [`We do not currently believe yet this head was already reviewed at \`${sha}\`.`, false], + [`I have not conclusively verified yet this head was already reviewed at \`${sha}\`.`, false], + [`No longer aware yet this head was already reviewed at \`${sha}\`.`, false], + [`No anomaly indicates yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No reply suggests yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No supply shows yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1368,6 +1377,7 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["I cannot say this head was ", "negation"], ["I am unaware that this head was ", "fusedNegation"], ["No evidence yet this head was ", "negatedHeadConnective"], + ["I cannot fully confirm yet this head was ", "negatedHeadConnective"], ["No evidence that this head was ", "negation"], ["I am not aware that this head was ", "negation"], // Correct skips: no cue governs the clause, so nothing vetoes. diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 943f0fcdc90e..c9e51fe9b3be 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9621,7 +9621,8 @@ const EPISTEMIC_NOUN_HEAD = "(?:evidence|indications?|signs?|records?|proofs?|traces?|suggestions?|statements?|assertions?|awareness|findings?|validations?|confirmation|verification|determination)"; // The NOUN subset of EPISTEMIC_HEAD. An adjective may sit between a negation and // a noun head ("no CLEAR evidence yet"), but for a VERB head the intervening -// words are necessarily the SUBJECT ("no COMMITS appear yet"), so slack there +// words are the SUBJECT or a pre-verbal ADVERB ("no COMMITS appear yet"), so +// unrestricted slack there // swallows the subject and misreads the main verb as the negation's head. // Present-tense verbs take no auxiliary, so the auxiliary exclusion alone did // not mark them (pass 18: appear/indicate/suggest all regressed). The @@ -9638,6 +9639,15 @@ const EPISTEMIC_HEAD = "|demonstrat(?:e|es|ed|ing)|show(?:s|ed|ing|n)?|mention(?:s|ed|ing)?|report(?:s|ed|ing)?|tell(?:s|ing)?|told|say(?:s|ing)?|said" + "|stat(?:e|es|ed|ing)|assert(?:s|ed|ing)?|claim(?:s|ed|ing)?" + "|rul(?:e|es|ed|ing)\\s+out)"; +// A CLOSED set of pre-verbal adverbs that may sit between a negation and a +// verb/adjective head without being its subject ("cannot FULLY confirm yet", +// "not CURRENTLY aware yet"). Pass 19: the strict-adjacent verb arm from pass +// 18 dropped exactly this third category and accepted 12/12 such hedges — the +// masking direction. NOT `\w+ly`: English nouns end in -ly too (anomaly, reply, +// supply, family), and that variant false-vetoed 6/6 correct skips, scoring +// worse than no slack at all. A closed set converges; a suffix pattern does not. +const EPISTEMIC_ADVERB = + "(?:yet|longer|ever|still|now|quite|fully|really|actually|truly|entirely|completely|definitively|conclusively|reliably|independently|positively|currently|firmly|confidently|necessarily|clearly|directly|certainly|categorically|absolutely)"; // See the use site in prReviewOutputHasAlreadyReviewedSkip: a sentence-initial // hedge adverb's own trailing comma is rejoined to its clause. Anchored to a @@ -9746,7 +9756,7 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ name: "negatedHeadConnective", stem: `(?:${NEGATION_PREFIX_WORD}\\s+(?:(?!(?:was|were|is|are|has|have|had|been|be|did|does|do)\\b)\\w+\\s+){0,2}${EPISTEMIC_NOUN_HEAD}` + - `|${NEGATION_PREFIX_WORD}\\s+${EPISTEMIC_HEAD})\\s+(?:${ADVERSATIVE_CONNECTIVES})\\b`, + `|${NEGATION_PREFIX_WORD}\\s+(?:${EPISTEMIC_ADVERB}\\s+){0,2}${EPISTEMIC_HEAD})\\s+(?:${ADVERSATIVE_CONNECTIVES})\\b`, }, // An epistemic negation governing the clause from further back: a negation // word, then within three words a head that could establish the claim. One From 6770aa7b6f1364fe027467a6acefc851fe1390db Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 13:42:31 -0700 Subject: [PATCH 26/34] fix(heartbeat): the epistemic adjective family belongs in EPISTEMIC_HEAD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EPISTEMIC_HEAD carried exactly one epistemic adjective, `aware`. Its whole predicative family was absent, so a negated adjectival hedge had no cue that reached the clause: "I am not certain yet this head was already reviewed at " was accepted as a genuine already-reviewed exit. The masking direction. The file already knew these words, in the wrong places. `fusedNegation` lists unconvinced/unpersuaded/unsatisfied, so the FUSED form vetoed while the spelled-out form did not — the exact mirror of the defect that cue's own comment records solving in the other direction. `hedge` hand-adds `not sure|not clear`, but its stem hardcodes a following `that`, so the elided-complementizer form this PR added negatedHeadConnective for leaked. Putting the family in EPISTEMIC_HEAD rather than a fourth list makes it reachable by `negation`, `negatedHeadConnective` and the copula arm at once, and it inherits CLAUSE_REACH like every other head. Measured 2/14 -> 13/14 on the family, with 0/18 new false-vetoes on the same adjectives in non-hedge roles, because COPULA_FILLER already refuses to cross a consequence connective. `positive` is included, against the review's recommendation to drop it, because mutation-testing that recommendation refuted its premise: "no positive result" measures identically either way — `result` is not an EPISTEMIC_NOUN_HEAD, so the noun arm never matches it — while excluding the word left three masking phrasings open. With it: those 3 veto and 7/7 adversarial non-epistemic controls stay correct skips. "I cannot BE certain yet …" remains open and is recorded as a known residual rather than pinned as correct. Mutation-checked: dropping the family fails 8 rows, dropping `positive` alone fails 2. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 15 ++++++ server/src/services/heartbeat.ts | 51 +++++++++++++++---- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index dd380131ebca..0761daf07ab5 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1355,6 +1355,19 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No anomaly indicates yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No reply suggests yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No supply shows yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I am not certain yet this head was already reviewed at \`${sha}\`.`, false], + [`I am not convinced that this head was already reviewed at \`${sha}\`.`, false], + [`It is not clear yet this head was already reviewed at \`${sha}\`.`, false], + [`It is not apparent yet this head was already reviewed at \`${sha}\`.`, false], + [`I am not positive yet this head was already reviewed at \`${sha}\`.`, false], + [`I am not positive that this head was already reviewed at \`${sha}\`.`, false], + [`Everything is clear so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I am satisfied with the checks so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`The evidence is conclusive so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No blocker is apparent so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No positive drift detected, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No result was positive, but this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No coverage delta was positive, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1378,6 +1391,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["I am unaware that this head was ", "fusedNegation"], ["No evidence yet this head was ", "negatedHeadConnective"], ["I cannot fully confirm yet this head was ", "negatedHeadConnective"], + ["I am not certain yet this head was ", "negatedHeadConnective"], + ["I am not convinced that this head was ", "negation"], ["No evidence that this head was ", "negation"], ["I am not aware that this head was ", "negation"], // Correct skips: no cue governs the clause, so nothing vetoes. diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index c9e51fe9b3be..a0428adfd80a 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9619,19 +9619,52 @@ const NEGATION_PREFIX_WORD = // now tolerates its own paradigm. const EPISTEMIC_NOUN_HEAD = "(?:evidence|indications?|signs?|records?|proofs?|traces?|suggestions?|statements?|assertions?|awareness|findings?|validations?|confirmation|verification|determination)"; -// The NOUN subset of EPISTEMIC_HEAD. An adjective may sit between a negation and -// a noun head ("no CLEAR evidence yet"), but for a VERB head the intervening -// words are the SUBJECT or a pre-verbal ADVERB ("no COMMITS appear yet"), so -// unrestricted slack there -// swallows the subject and misreads the main verb as the negation's head. -// Present-tense verbs take no auxiliary, so the auxiliary exclusion alone did -// not mark them (pass 18: appear/indicate/suggest all regressed). The -// discriminator is part of speech, not the auxiliary. +// The NOUN subset of EPISTEMIC_HEAD. An adjective may sit between a negation +// and a noun head ("no CLEAR evidence yet"), but for a VERB head the +// intervening words are the SUBJECT or a pre-verbal ADVERB ("no COMMITS appear +// yet"), so unrestricted slack there swallows the subject and misreads the main +// verb as the negation's head. Present-tense verbs take no auxiliary, so the +// auxiliary exclusion alone did not mark them (pass 18: appear/indicate/suggest +// all regressed). The discriminator is part of speech, not the auxiliary. +// +// EPISTEMIC_HEAD also carries the epistemic ADJECTIVES. Pass 20: it held only +// `aware`, so "I am not certain yet …" and "It is not clear yet …" masked while +// the FUSED `unconvinced` vetoed — the mirror of the defect fusedNegation's own +// comment records solving in the other direction. `hedge` hand-adds +// `not sure|not clear`, but its stem hardcodes a following `that`, so the +// elided-complementizer form leaked. The family belongs HERE rather than in a +// fourth list: one insertion makes it reachable by `negation`, +// `negatedHeadConnective` and the copula arm at once, and it inherits +// CLAUSE_REACH like every other head. Measured 2/14 -> 13/14 on the family with +// 0/18 new false-vetoes on the same adjectives in NON-hedge roles ("Everything +// is clear so …", "The evidence is conclusive so …") — COPULA_FILLER already +// refuses to cross a consequence connective, so an adjective governing the +// PREVIOUS clause still cannot reach this one. +// +// `positive` is INCLUDED, against the pass-20 review's recommendation to drop +// it, because mutation-testing that recommendation refuted its premise. The +// phrasing it was justified by — "no positive result" — measures identically +// with and without the word, since `result` is not an EPISTEMIC_NOUN_HEAD and +// so the noun arm never matches it. Excluding it therefore bought nothing and +// left "I am not positive yet/that this head was …" masking (3/3). Included: +// those 3 veto, and 7/7 NON-epistemic controls built to break it stay correct +// skips ("No result was positive, but …", "No metric was positive however …", +// "No coverage delta was positive, so …") — the connective-role exclusion in +// COPULA_FILLER holds them. Strictly dominant, so the ambiguity of the word is +// a reason to pin it with controls, not to omit it. +// +// Known residual, deliberately NOT pinned as correct: "I cannot BE certain yet +// this head was …" still masks. `be` is neither an EPISTEMIC_ADVERB nor +// admissible in the noun arm's non-auxiliary slack, so the +// copula-between-negation-and-adjective shape has no cue that reaches the +// clause. Narrower than the family above; recorded so the next pass does not +// re-derive it. const EPISTEMIC_HEAD = "(?:evidence|indications?|indicat(?:e|es|ed|ing)|signs?|records?|proofs?|traces?" + "|suggestions?|statements?|assertions?|awareness|findings?|validations?" + "|believ(?:e|es|ed)|think(?:s|ing)?|thought|appear(?:s|ed|ing)?" + - "|suggest(?:s|ed|ing)?|aware|see|sees|seen|saw|seeing" + + "|suggest(?:s|ed|ing)?|aware|certain|convinced|confident|persuaded|satisfied" + + "|sure|clear|evident|apparent|conclusive|definitive|positive|see|sees|seen|saw|seeing" + "|confirm(?:s|ed|ing)?|confirmation|verif(?:y|ies|ied|ying)|verification" + "|establish(?:es|ed|ing)?|determin(?:e|es|ed|ing)|determination" + "|find|finds|found|finding|check(?:s|ed|ing)?|locat(?:e|es|ed|ing)" + From 3e08e52f8546f9f4a706d4fa58943e9bce20bcef Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 15:03:42 -0700 Subject: [PATCH 27/34] fix(heartbeat): make the fused hedges bare, as the spelled-out ones now are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 20 fixed the spelled-out side of an asymmetry and left its mirror open. `hedge`'s stem ends `\s+(?:\w+\s+){0,3}that` and `fusedNegation`'s ended `(?:that|whether|if)`, so with the complementizer elided — the very shape this PR added `negatedHeadConnective` for — all 12 fused hedges masked (measured 0/12, cue null) while their spelled-out counterparts vetoed 7/7. Fixing one side without the other did not close an old asymmetry so much as create a new one. The fix is the same structural move, applied to the other list: make the stem bare and let CLAUSE_REACH bind it, rather than enumerating a complementizer-free variant. 12/12 now veto, 0 rows change in the committed corpus, and 12/12 adversarial non-hedge uses stay correct skips. Two comments claimed things that were false and are corrected here. The pass-20 comment said the fused `unconvinced` vetoed — it did so only with an explicit complementizer. The fusedNegation comment said the fused form "binds exactly as the spelled-out one does" and that `hedge` "already covers" the eight adjectives — both true only of the complementizer-bearing case. `obvious` joins the adjective family as an exact synonym of `apparent` and `evident`, which were already in it. The four other candidates that leak the same frame — settled, decided, documented, visible — are recorded as known residuals rather than added: they describe the state of the world, not the speaker's confidence. The leading `\b` on the fused stem is now pinned by a row that actually discriminates it. My first attempt used a connective-bearing control, and the mutation did not bite — the clause-boundary rules block the veto either way, so the row has to be the bare frame ("It is unambiguous this head was …", a correct skip that false-vetoes without the boundary). Mutation-checked: restoring the complementizer requirement fails 4 rows, dropping `obvious` fails 2, dropping the `\b` fails 1. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 11 ++++ server/src/services/heartbeat.ts | 55 +++++++++++++++---- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 0761daf07ab5..465f5fa71062 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1361,6 +1361,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`It is not apparent yet this head was already reviewed at \`${sha}\`.`, false], [`I am not positive yet this head was already reviewed at \`${sha}\`.`, false], [`I am not positive that this head was already reviewed at \`${sha}\`.`, false], + [`It is unclear this head was already reviewed at \`${sha}\`.`, false], + [`I am unaware this head was already reviewed at \`${sha}\`.`, false], + [`It is inconclusive this head was already reviewed at \`${sha}\`.`, false], + [`It is not obvious yet this head was already reviewed at \`${sha}\`.`, false], [`Everything is clear so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I am satisfied with the checks so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`The evidence is conclusive so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1368,6 +1372,11 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No positive drift detected, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No result was positive, but this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No coverage delta was positive, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`Results are unambiguous so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It is unambiguous this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I resolved every unclear row, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`It is obvious no newer head exists so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No unknown commits were found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1393,6 +1402,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["I cannot fully confirm yet this head was ", "negatedHeadConnective"], ["I am not certain yet this head was ", "negatedHeadConnective"], ["I am not convinced that this head was ", "negation"], + ["It is unclear this head was ", "fusedNegation"], + ["It is not obvious yet this head was ", "negatedHeadConnective"], ["No evidence that this head was ", "negation"], ["I am not aware that this head was ", "negation"], // Correct skips: no cue governs the clause, so nothing vetoes. diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index a0428adfd80a..13143ae6d275 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9628,11 +9628,14 @@ const EPISTEMIC_NOUN_HEAD = // all regressed). The discriminator is part of speech, not the auxiliary. // // EPISTEMIC_HEAD also carries the epistemic ADJECTIVES. Pass 20: it held only -// `aware`, so "I am not certain yet …" and "It is not clear yet …" masked while -// the FUSED `unconvinced` vetoed — the mirror of the defect fusedNegation's own -// comment records solving in the other direction. `hedge` hand-adds -// `not sure|not clear`, but its stem hardcodes a following `that`, so the -// elided-complementizer form leaked. The family belongs HERE rather than in a +// `aware`, so "I am not certain yet …" and "It is not clear yet …" masked. Pass +// 21 corrects how that pass described the asymmetry: it said the FUSED +// `unconvinced` vetoed, and it does so ONLY with an explicit complementizer. +// With the complementizer elided both sides masked, so fixing the spelled-out +// side here briefly created a NEW asymmetry (spelled-out 7/7 vetoing, fused +// 0/12) rather than closing an old one. `fusedNegation` is bare as of pass 21 +// and both sides now bind through CLAUSE_REACH. The family belongs HERE rather +// than in a // fourth list: one insertion makes it reachable by `negation`, // `negatedHeadConnective` and the copula arm at once, and it inherits // CLAUSE_REACH like every other head. Measured 2/14 -> 13/14 on the family with @@ -9641,6 +9644,14 @@ const EPISTEMIC_NOUN_HEAD = // refuses to cross a consequence connective, so an adjective governing the // PREVIOUS clause still cannot reach this one. // +// `obvious` joined the family in pass 21 as an exact synonym of `apparent` and +// `evident`, which were already in it. Probing the same frame across 24 +// candidate adjectives left four other leaks — `settled`, `decided`, +// `documented`, `visible` — which are deliberately NOT added: they describe the +// state of the world rather than the speaker's confidence, so they are likelier +// to appear in a correct skip than in a hedge. They remain known residuals, not +// asserted-correct behaviour, on the same reasoning the `\w+ly` rejection gives. +// // `positive` is INCLUDED, against the pass-20 review's recommendation to drop // it, because mutation-testing that recommendation refuted its premise. The // phrasing it was justified by — "no positive result" — measures identically @@ -9664,7 +9675,7 @@ const EPISTEMIC_HEAD = "|suggestions?|statements?|assertions?|awareness|findings?|validations?" + "|believ(?:e|es|ed)|think(?:s|ing)?|thought|appear(?:s|ed|ing)?" + "|suggest(?:s|ed|ing)?|aware|certain|convinced|confident|persuaded|satisfied" + - "|sure|clear|evident|apparent|conclusive|definitive|positive|see|sees|seen|saw|seeing" + + "|sure|clear|evident|apparent|obvious|conclusive|definitive|positive|see|sees|seen|saw|seeing" + "|confirm(?:s|ed|ing)?|confirmation|verif(?:y|ies|ied|ying)|verification" + "|establish(?:es|ed|ing)?|determin(?:e|es|ed|ing)|determination" + "|find|finds|found|finding|check(?:s|ed|ing)?|locat(?:e|es|ed|ing)" + @@ -9806,13 +9817,35 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // …"), so there is no separate negation word for the `negation` prefix to // anchor on, and listing these as heads left them unreachable — measured: // "not aware that" vetoed while "unaware that" did not. They get their own - // stem, which inherits CLAUSE_REACH like every other, so the fused form binds - // exactly as the spelled-out one does. `hedge` already covers unclear/unsure/ - // uncertain/unconfirmed/unverified; this entry adds the mental-state - // participles that `hedge` does not carry. + // stem, which inherits CLAUSE_REACH like every other. + // + // Pass 21: that stem was NOT bare — it required `(?:that|whether|if)`, and + // `hedge` likewise hardcodes a following `that`. So the claim that "the fused + // form binds exactly as the spelled-out one does" held only for the + // complementizer-bearing case: with it elided, all 12 of these words masked + // (measured 0/12, cue `null`) while their spelled-out counterparts vetoed + // 7/7 after pass 20. The stem is bare now, which is the same structural move + // pass 20 made for the spelled-out side — let CLAUSE_REACH bind it instead of + // enumerating a complementizer-free variant. Measured 12/12 vetoing with 0 + // rows changed in the committed corpus and 0/12 new false-vetoes on + // adversarial non-hedge uses ("No unknown commits were found so …", "I + // resolved every unclear row, so …"). The leading `\b` is load-bearing and + // separately pinned: "It is unambiguous this head was …" is a correct skip + // that false-vetoes without it, because `ambiguous` matches inside + // `unambiguous`. A connective- or comma-bearing control does NOT pin it — the + // clause-boundary rules block the veto either way, so the row has to be the + // bare frame. Measured: with `\b` true, without `\b` false. + // + // The eight `hedge` words are deliberately listed in BOTH places: `hedge`'s + // `that`-form stays the cheaper match, and this bare stem is what makes them + // reachable when the complementizer is dropped. `ambiguous` is the one member + // that is not morphologically a fused negation; it is kept here because it + // hedges identically and needs the same bare binding. { name: "fusedNegation", - stem: "\\b(?:unaware|unconvinced|unpersuaded|unsatisfied)\\s+(?:\\w+\\s+){0,3}(?:that|whether|if)", + stem: + "\\b(?:unaware|unconvinced|unpersuaded|unsatisfied|unclear|unsure|uncertain" + + "|unknown|unverified|unconfirmed|ambiguous|inconclusive)", }, { name: "negation", From 6d0db522f435abc6205833a331a05d49b242523e Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 15:50:20 -0700 Subject: [PATCH 28/34] fix(heartbeat): scope two overclaimed control sets, and pin the residual MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both said "N new false-vetoes" without qualifying the shape, and in both every control carried a connective — so they exercised only what COPULA_FILLER had already made immune. The pass-21 claim is the one the review caught; the pass-20 claim beside it has the identical defect and is scoped here too, since fixing only the instance that was reported is the pattern this file keeps repeating. Measured, parent -> head: a preposition-led adjunct with no connective, no comma and no sentence boundary false-vetoes 11/11 for the bare fused stem ("Despite the unclear wake payload this head was …"), and 2 more for the adjective family ("Given no certain match …" from pass 20, "Aside from no obvious drift …" from pass 21). It is a residual of CLAUSE_REACH rather than of any one cue: the same frame built on the untouched `negation` cue false-vetoes at every head in this PR's history. So these passes widened an accepted trade, they did not open a hole — and it fails toward `missing`, a re-review rather than a masked non-review. 6/6 house-style skips containing the same words are unaffected, because real output puts a connective, a comma or a sentence boundary in that position. The residual is now a characterization table, not only prose. Nine rows assert what the classifier currently does, labelled as known-wrong, with instructions to flip them to `true` if a later pass fixes CLAUSE_REACH rather than re-narrowing a cue. Twenty-one passes have read these comments to decide what was already settled; an unqualified claim is exactly what stops the next one re-deriving a residual. Also closes the last stem that hardcoded a trailing token. An audit of all seven cues found `hedge` still requiring `that`, and `no idea` was the one word unique to it — "I have no idea this head was already reviewed at " was accepted. `ideas?` joins the noun heads, so 5/5 of those frames veto and 6/6 non-hedge controls stay skips. Mutation-checked: dropping `ideas?` fails 3 rows, re-binding fusedNegation to a complementizer fails 6. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 50 +++++++++++++++++++ server/src/services/heartbeat.ts | 36 ++++++++++--- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 465f5fa71062..2c9a1b239bb1 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1365,6 +1365,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`I am unaware this head was already reviewed at \`${sha}\`.`, false], [`It is inconclusive this head was already reviewed at \`${sha}\`.`, false], [`It is not obvious yet this head was already reviewed at \`${sha}\`.`, false], + [`I have no idea this head was already reviewed at \`${sha}\`.`, false], + [`No idea yet this head was already reviewed at \`${sha}\`.`, false], [`Everything is clear so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I am satisfied with the checks so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`The evidence is conclusive so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1374,6 +1376,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No coverage delta was positive, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Results are unambiguous so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`It is unambiguous this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`No new idea landed so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + [`I dropped the idea of a rebase, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I resolved every unclear row, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`It is obvious no newer head exists so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No unknown commits were found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1404,6 +1408,7 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["I am not convinced that this head was ", "negation"], ["It is unclear this head was ", "fusedNegation"], ["It is not obvious yet this head was ", "negatedHeadConnective"], + ["I have no idea this head was ", "negation"], ["No evidence that this head was ", "negation"], ["I am not aware that this head was ", "negation"], // Correct skips: no cue governs the clause, so nothing vetoes. @@ -1417,6 +1422,51 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); + // CHARACTERIZATION, NOT CORRECTNESS. Every row below is a KNOWN FALSE-VETO: + // the verdict asserted is the one the classifier currently produces, and it + // is wrong. They are pinned so that a future change which widens or narrows + // the residual is visible in the diff rather than discovered by the twenty- + // third review pass. + // + // The shape is a preposition-led adjunct with no connective, no comma and no + // sentence boundary, so nothing in CLAUSE_REACH separates the adjunct from + // the review clause. It has never been survivable for any epistemic cue — + // the `negation` row here false-vetoes at every head in this PR's history — + // and it fails toward `missing`, a re-review rather than a masked + // non-review. Passes 20-22 each widened it to more words; none opened it. + // + // If a later pass fixes CLAUSE_REACH so an adjunct no longer swallows the + // clause, these expectations flip to `true` and SHOULD be updated to `true`. + // Do not "fix" a failure here by re-narrowing a cue. + describe("BLO-31374: known CLAUSE_REACH adjunct residual (characterization)", () => { + const sha = "8b237675b19fa5ae061821fd3b1d87cd8cd1836f"; + it.each([ + // pre-existing at every head — the control that proves the class is old + [`Despite no evidence of a force-push this head was already reviewed at \`${sha}\`.`, false], + // widened by the pass-21 bare fusedNegation stem + [`Despite the unclear wake payload this head was already reviewed at \`${sha}\`.`, false], + [`Following the inconclusive CI run this head was already reviewed at \`${sha}\`.`, false], + // widened by the pass-20 adjective family and the pass-21 `obvious` + [`Given no certain match this head was already reviewed at \`${sha}\`.`, false], + [`Aside from no obvious drift this head was already reviewed at \`${sha}\`.`, false], + ])("known residual: %s -> %s", (text, want) => { + expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); + }); + + // The same words in the shapes real output actually uses. These are the + // rows that must never change: a connective, a comma or a sentence + // boundary separates the adjunct, and the skip survives. + it.each([ + `Skipping: the merge state is unknown but this head was already reviewed at \`${sha}\`.`, + `The wake head was unknown, so this head was already reviewed at \`${sha}\`.`, + `Merge state unknown; nothing to do. This head was already reviewed at \`${sha}\`.`, + `Nothing ambiguous remained, so this head was already reviewed at \`${sha}\`.`, + ])("house-style skip containing a residual word still classifies (%#)", (text) => { + expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(true); + }); + }); + + // A hedge about something OTHER than the review does not mask the skip: the // `that`-complement closes before the clause, so no copula reaches it. Drop // the copula bind from the hedge cue and this row flips to `missing` — the diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 13143ae6d275..ba9a19f4c8fc 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9635,14 +9635,23 @@ const EPISTEMIC_NOUN_HEAD = // side here briefly created a NEW asymmetry (spelled-out 7/7 vetoing, fused // 0/12) rather than closing an old one. `fusedNegation` is bare as of pass 21 // and both sides now bind through CLAUSE_REACH. The family belongs HERE rather -// than in a -// fourth list: one insertion makes it reachable by `negation`, +// than in a fourth list: one insertion makes it reachable by `negation`, // `negatedHeadConnective` and the copula arm at once, and it inherits // CLAUSE_REACH like every other head. Measured 2/14 -> 13/14 on the family with -// 0/18 new false-vetoes on the same adjectives in NON-hedge roles ("Everything -// is clear so …", "The evidence is conclusive so …") — COPULA_FILLER already +// 0/18 new false-vetoes on CONNECTIVE- OR COMMA-BEARING non-hedge uses +// ("Everything is clear so …", "The evidence is conclusive so …") — COPULA_FILLER // refuses to cross a consequence connective, so an adjective governing the -// PREVIOUS clause still cannot reach this one. +// PREVIOUS clause cannot reach this one. +// +// Pass 22 scopes that claim, which was unqualified and had the same defect the +// pass-21 one did: every control carried a connective. The family alone in a +// bare adjunct is inert ("Despite the obvious drift this head was …" stays a +// skip, since these words bind only after a negation), but a bare NEGATED +// adjunct does false-veto, and two of those are new here: "Given no certain +// match this head was …" (pass 20) and "Aside from no obvious drift this head +// was …" (pass 21) both flip true -> false, while "Despite no evidence of a +// force-push this head was …" false-vetoes at every head. Same CLAUSE_REACH +// residual as the fused stem's; see the characterization rows. // // `obvious` joined the family in pass 21 as an exact synonym of `apparent` and // `evident`, which were already in it. Probing the same frame across 24 @@ -9672,7 +9681,7 @@ const EPISTEMIC_NOUN_HEAD = // re-derive it. const EPISTEMIC_HEAD = "(?:evidence|indications?|indicat(?:e|es|ed|ing)|signs?|records?|proofs?|traces?" + - "|suggestions?|statements?|assertions?|awareness|findings?|validations?" + + "|suggestions?|statements?|assertions?|awareness|findings?|validations?|ideas?" + "|believ(?:e|es|ed)|think(?:s|ing)?|thought|appear(?:s|ed|ing)?" + "|suggest(?:s|ed|ing)?|aware|certain|convinced|confident|persuaded|satisfied" + "|sure|clear|evident|apparent|obvious|conclusive|definitive|positive|see|sees|seen|saw|seeing" + @@ -9828,8 +9837,19 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ // pass 20 made for the spelled-out side — let CLAUSE_REACH bind it instead of // enumerating a complementizer-free variant. Measured 12/12 vetoing with 0 // rows changed in the committed corpus and 0/12 new false-vetoes on - // adversarial non-hedge uses ("No unknown commits were found so …", "I - // resolved every unclear row, so …"). The leading `\b` is load-bearing and + // CONNECTIVE- OR COMMA-BEARING non-hedge uses ("No unknown commits were found + // so …", "I resolved every unclear row, so …"). That qualifier is the whole + // claim: both controls carry a connective, so they exercise only the shape + // COPULA_FILLER already made immune. A connective-free, comma-free + // preposition-led adjunct DOES false-veto — "Despite the unclear wake payload + // this head was …" measures 11/11 flipping true -> false at this commit. It + // is a residual of CLAUSE_REACH, not of this stem: the same frame built on + // the untouched `negation` cue ("Despite no evidence of a force-push this + // head was …") false-vetoes at BOTH heads, so a bare adjunct has never been + // survivable for any epistemic cue. This commit widens an accepted trade to + // 12 more words rather than opening a new hole, and it fails toward + // `missing` — a re-review, not a masked non-review. Pinned as a + // CHARACTERIZATION row, not as correct behaviour. The leading `\b` is load-bearing and // separately pinned: "It is unambiguous this head was …" is a correct skip // that false-vetoes without it, because `ambiguous` matches inside // `unambiguous`. A connective- or comma-bearing control does NOT pin it — the From 466b28acf6c981495f7a6212ec1bd11d784ad81a Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 17:35:21 -0700 Subject: [PATCH 29/34] fix(heartbeat): give the negation cue the connective-role exclusion it never had MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `negation` stem's pre-head filler was a bare `(?:\w+\s+){0,3}`, so a negation governing the PREVIOUS clause reached across a consequence connective and false-vetoed correct skips: "Nothing changed so I see this head was already reviewed at " dropped the run to missing. Four review passes deferred this on the grounds that fixing it "trades against the filler-role balance" pass 15 measured. It does not: that balance lives in `negatedHeadConnective` and `questioned`, and measured 7/7 filler-role connectives and 3/3 demonstrative guards are byte-identical before and after. What "conservative" meant in practice was word-counting. "No force-push so I confirm …" survived and "No drift so we confirm …" did not, with the same grammar, because only the first exhausted the `{0,3}` bound. COPULA_FILLER already encoded the rule — a connective in connective role may not be crossed, one in filler role may — so the fix is a refactor: the per-word unit is extracted as NON_CONNECTIVE_WORD and both bounds are built from it. 10/10 previous-clause negations now skip (was 3/10), 10/10 governing negations still veto, and 0 rows change in the committed corpus. Also closes pass 23: the characterization table pinned the residual for pass 20's and pass 21's words but not for pass 22's own `ideas?`, whose two controls were connective-bearing — the exact validation gap the same commit corrected one screen away. One row and one pass label, plus the rationale comment `ideas?` was the only added stem to lack. "new here" is reworded to name the passes it refers to, and the double blank line is rejoined. Mutation-checked: a bare negation filler fails 5 rows; dropping `ideas?` fails 4. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 18 +++++++++++-- server/src/services/heartbeat.ts | 25 ++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 2c9a1b239bb1..55de8f5ecea6 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1381,6 +1381,16 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`I resolved every unclear row, so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`It is obvious no newer head exists so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No unknown commits were found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + // pass 23: a negation governing the PREVIOUS clause may not cross a + // consequence connective into this one (was false-vetoed 7/10). + [`Nothing changed so I see this head was already reviewed at \`${sha}\`.`, true], + [`Not stale since I verified this head was already reviewed at \`${sha}\`.`, true], + [`No drift so we confirm this head was already reviewed at \`${sha}\`.`, true], + [`Nothing moved therefore I see this head was already reviewed at \`${sha}\`.`, true], + // …but a connective in FILLER role (complementizer follows) still lets + // the negation govern this clause — the pass-15 balance, unchanged. + [`No sign but that this head was already reviewed at \`${sha}\`.`, false], + [`No evidence so far that this head was already reviewed at \`${sha}\`.`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1415,6 +1425,7 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["No evidence of a force-push, so ", null], ["I did not check whether a newer head exists so ", null], ["No newer commits were found so this head was ", null], + ["Nothing changed so I see this head was ", null], ["Beyond reasonable doubt this head was ", null], ["The wake was probably a duplicate dispatch so ", null], ])("%s -> %s", (before, cue) => { @@ -1433,7 +1444,9 @@ describe("evaluatePrReviewCompletionEvidence", () => { // the review clause. It has never been survivable for any epistemic cue — // the `negation` row here false-vetoes at every head in this PR's history — // and it fails toward `missing`, a re-review rather than a masked - // non-review. Passes 20-22 each widened it to more words; none opened it. + // non-review. Passes 20-22 each widened it to more words; none opened it. The + // pass-23 connective-role exclusion on `negation`'s filler does not touch + // it either: these frames carry no connective for the exclusion to act on. // // If a later pass fixes CLAUSE_REACH so an adjunct no longer swallows the // clause, these expectations flip to `true` and SHOULD be updated to `true`. @@ -1449,6 +1462,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { // widened by the pass-20 adjective family and the pass-21 `obvious` [`Given no certain match this head was already reviewed at \`${sha}\`.`, false], [`Aside from no obvious drift this head was already reviewed at \`${sha}\`.`, false], + // widened by the pass-22 `ideas?` lemma + [`No new idea landed for this head was already reviewed at \`${sha}\`.`, false], ])("known residual: %s -> %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); @@ -1466,7 +1481,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { }); }); - // A hedge about something OTHER than the review does not mask the skip: the // `that`-complement closes before the clause, so no copula reaches it. Drop // the copula bind from the hedge cue and this row flips to `missing` — the diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index ba9a19f4c8fc..8dfe9fb1e2ea 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9587,7 +9587,16 @@ const COPULA_FILLER_WORDS = Math.floor(CLAUSE_SCOPE_CHARS / 5); // demonstrative, not a complementizer ("yet THAT HEAD was" vs "yet that THIS // head was"), so it must not flip the connective into its filler role and let // the negation cross (Ally pass 16 suggestion: 20/20 connectives false-vetoed). -const COPULA_FILLER = `(?:(?!(?:${CONNECTIVES})\\b(?!\\s+(?:[^\\s.\\n:;,\u2014\u2013]+\\s+){0,2}(?:that|whether)\\b(?!\\s+(?:head|commit|sha|branch|pr)\\b)))[^\\s.\\n:;,\u2014\u2013]+\\s+){0,${COPULA_FILLER_WORDS}}`; +// One filler WORD that may not be a connective in its connective role. The +// negative lookahead lets a connective through only when a complementizer +// follows within two words (filler role: "No evidence YET THAT this head was"), +// and not when that complementizer is a demonstrative before an anchor noun. +const NON_CONNECTIVE_WORD = `(?!(?:${CONNECTIVES})\\b(?!\\s+(?:[^\\s.\\n:;,\u2014\u2013]+\\s+){0,2}(?:that|whether)\\b(?!\\s+(?:head|commit|sha|branch|pr)\\b)))[^\\s.\\n:;,\u2014\u2013]+\\s+`; +const COPULA_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,${COPULA_FILLER_WORDS}}`; +// The same unit at the negation cue's own bound. Pass 23: the `negation` stem +// used a bare `(?:\\w+\\s+){0,3}`, so a negation governing the PREVIOUS clause +// crossed a consequence connective and false-vetoed correct skips. +const NEGATION_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,3}`; // A relative pronoun abutting the copula is NOT a complementizer, and the // distinction is syntactic rather than lexical: a complementizer is always // followed by a SUBJECT before the copula ("confirm that this head was"), so @@ -9647,7 +9656,7 @@ const EPISTEMIC_NOUN_HEAD = // pass-21 one did: every control carried a connective. The family alone in a // bare adjunct is inert ("Despite the obvious drift this head was …" stays a // skip, since these words bind only after a negation), but a bare NEGATED -// adjunct does false-veto, and two of those are new here: "Given no certain +// adjunct does false-veto, and two were opened by earlier passes of this PR: "Given no certain // match this head was …" (pass 20) and "Aside from no obvious drift this head // was …" (pass 21) both flip true -> false, while "Despite no evidence of a // force-push this head was …" false-vetoes at every head. Same CLAUSE_REACH @@ -9661,6 +9670,16 @@ const EPISTEMIC_NOUN_HEAD = // to appear in a correct skip than in a hedge. They remain known residuals, not // asserted-correct behaviour, on the same reasoning the `\w+ly` rejection gives. // +// `ideas?` (pass 22) replaces `hedge`'s complementizer-bound `no\s+idea`. An +// audit of all seven cues found `hedge` the last stem hardcoding a trailing +// token, and `no idea` the one word unique to it: "I have no idea this head was +// already reviewed at " was accepted. As a noun head under `negation` it +// binds through CLAUSE_REACH like the rest — 5/5 hedge frames veto, 6/6 +// connective- or comma-bearing controls stay skips. It is inert without a +// preceding negation, and it widens the bare-adjunct residual by one more word +// ("No new idea landed for this head was …"), pinned in the characterization +// table with its pass label like the pass-20 and pass-21 widenings. +// // `positive` is INCLUDED, against the pass-20 review's recommendation to drop // it, because mutation-testing that recommendation refuted its premise. The // phrasing it was justified by — "no positive result" — measures identically @@ -9869,7 +9888,7 @@ const GOVERNING_CUE_STEMS: ReadonlyArray<{ name: string; stem: string }> = [ }, { name: "negation", - stem: `${NEGATION_PREFIX_WORD}\\s+(?:\\w+\\s+){0,3}${EPISTEMIC_HEAD}`, + stem: `${NEGATION_PREFIX_WORD}\\s+${NEGATION_FILLER}${EPISTEMIC_HEAD}`, }, ]; From 4205e15b9fa67ccfc075eb9ddacd487f339e3f27 Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 18:20:08 -0700 Subject: [PATCH 30/34] docs(heartbeat): NEGATION_FILLER changed two things; record the second MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sharing NON_CONNECTIVE_WORD with the `negation` stem gave it the connective-role exclusion, which pass 23 documented — and the non-boundary token class in place of `\w+`, which it did not. That half moves the classifier in both directions and neither was written down: - It closes on `negation` the citation-masking hole the token-class note records closing on COPULA_FILLER. "I have no `direct` evidence this head was …", "No (verified) proof that …", "No force-push evidence that …" were all ACCEPTED at the parent because `\w+` could not cross a backtick, parenthesis or hyphen, so the cue never fired. 4/4 now veto. A real fix, pinned as four boundary rows and an attribution row so a revert of the token class fails. - It widens the CLAUSE_REACH bare-adjunct residual by one more shape — punctuated filler tokens with no connective — and the characterization block's prose said pass 23 "does not touch it". True of the connective half, false of the token-class half: "No force-push evidence this head was …" flips true -> false at this head. The claim is scoped to the half it is true of and the widening gets its own row and pass label, the same repair this block received for pass 22 one commit ago. Also recorded while the bound is freshly named: `{0,3}` is a known second cap, inherited verbatim and not derived from CLAUSE_SCOPE_CHARS — five filler words escape the veto where four do not. Left as-is; raising it interacts with the filler-role balance and deserves its own measurement. And stated explicitly that only consequence connectives are affected: the adversative family measures 0 frames changed, which is the pass-17/18 line and the right one to hold. Mutation-checked: reverting only the token class (exclusion kept) fails 6 rows. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 16 ++++++++-- server/src/services/heartbeat.ts | 30 +++++++++++++++++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 55de8f5ecea6..ad9a6fe0cd2e 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1391,6 +1391,12 @@ describe("evaluatePrReviewCompletionEvidence", () => { // the negation govern this clause — the pass-15 balance, unchanged. [`No sign but that this head was already reviewed at \`${sha}\`.`, false], [`No evidence so far that this head was already reviewed at \`${sha}\`.`, false], + // pass 23/24: the negation filler's token class can cross a citation or + // punctuation; with `\\w+` these four were accepted (masking). + [`I have no \`direct\` evidence this head was already reviewed at \`${sha}\`.`, false], + [`No (verified) proof that this head was already reviewed at \`${sha}\`.`, false], + [`No force-push evidence that this head was already reviewed at \`${sha}\`.`, false], + [`There is no up-to-date confirmation that this head was already reviewed at \`${sha}\`.`, false], [`Already reviewed at head${sha}.`, false], ])("%s → %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); @@ -1425,6 +1431,7 @@ describe("evaluatePrReviewCompletionEvidence", () => { ["No evidence of a force-push, so ", null], ["I did not check whether a newer head exists so ", null], ["No newer commits were found so this head was ", null], + ["No force-push evidence that this head was ", "negation"], ["Nothing changed so I see this head was ", null], ["Beyond reasonable doubt this head was ", null], ["The wake was probably a duplicate dispatch so ", null], @@ -1444,9 +1451,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { // the review clause. It has never been survivable for any epistemic cue — // the `negation` row here false-vetoes at every head in this PR's history — // and it fails toward `missing`, a re-review rather than a masked - // non-review. Passes 20-22 each widened it to more words; none opened it. The - // pass-23 connective-role exclusion on `negation`'s filler does not touch - // it either: these frames carry no connective for the exclusion to act on. + // non-review. Passes 20-23 each widened it to more words or shapes; none + // opened it. Pass 23's connective-role exclusion does not touch these frames + // (they carry no connective for it to act on), but the token class it + // shares with COPULA_FILLER does, by one shape: punctuated filler tokens. // // If a later pass fixes CLAUSE_REACH so an adjunct no longer swallows the // clause, these expectations flip to `true` and SHOULD be updated to `true`. @@ -1464,6 +1472,8 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Aside from no obvious drift this head was already reviewed at \`${sha}\`.`, false], // widened by the pass-22 `ideas?` lemma [`No new idea landed for this head was already reviewed at \`${sha}\`.`, false], + // widened by the pass-23 NON_CONNECTIVE_WORD token class + [`No force-push evidence this head was already reviewed at \`${sha}\`.`, false], ])("known residual: %s -> %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8dfe9fb1e2ea..6d6f7873a51b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9596,6 +9596,31 @@ const COPULA_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,${COPULA_FILLER_WORDS}}`; // The same unit at the negation cue's own bound. Pass 23: the `negation` stem // used a bare `(?:\\w+\\s+){0,3}`, so a negation governing the PREVIOUS clause // crossed a consequence connective and false-vetoed correct skips. +// +// Sharing the unit changes TWO things, and pass 24 caught that only one was +// written down. Besides the connective-role exclusion, NEGATION_FILLER now +// inherits NON_CONNECTIVE_WORD's non-boundary token class in place of `\\w+`, +// which cuts both ways: +// - It closes on `negation` the citation-masking hole that the token-class +// note above records closing on COPULA_FILLER: "I have no `direct` evidence +// this head was …", "No (verified) proof that …", "No force-push evidence +// that …" were ACCEPTED at the parent because `\\w+` could not cross the +// backtick, parenthesis or hyphen, so the cue never fired. 4/4 now veto. +// - It widens the CLAUSE_REACH bare-adjunct residual by one more shape — +// punctuated filler tokens with no connective ("No force-push evidence this +// head was …", 4/4 true -> false) — pinned in the characterization table +// under its own pass label. Fails toward `missing`, like the rest of it. +// Only CONSEQUENCE connectives are affected by the exclusion; the adversative +// family (`yet`/`however`/`still`/…) is deliberately untouched — "No evidence +// yet this head was …" is a hedge, not a skip — and measured 0 frames changed. +// +// `{0,3}` is a KNOWN SECOND CAP, inherited verbatim from the old stem and not +// derived from CLAUSE_SCOPE_CHARS the way COPULA_FILLER_WORDS is. It binds: +// "I found no really very quite fully clear evidence that this head was …" +// (five filler words) escapes the veto — masking — while four filler words +// veto. Left as-is because raising it interacts with the filler-role balance +// and deserves its own measurement; recorded so it is not mistaken for a +// derived bound. const NEGATION_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,3}`; // A relative pronoun abutting the copula is NOT a complementizer, and the // distinction is syntactic rather than lexical: a complementizer is always @@ -9656,8 +9681,9 @@ const EPISTEMIC_NOUN_HEAD = // pass-21 one did: every control carried a connective. The family alone in a // bare adjunct is inert ("Despite the obvious drift this head was …" stays a // skip, since these words bind only after a negation), but a bare NEGATED -// adjunct does false-veto, and two were opened by earlier passes of this PR: "Given no certain -// match this head was …" (pass 20) and "Aside from no obvious drift this head +// adjunct does false-veto, and two were opened by earlier passes of this PR: +// "Given no certain match this head was …" (pass 20) and "Aside from no +// obvious drift this head // was …" (pass 21) both flip true -> false, while "Despite no evidence of a // force-push this head was …" false-vetoes at every head. Same CLAUSE_REACH // residual as the fused stem's; see the characterization rows. From f8c25e0b357c3ccfb2f8515d8bbc2bd97375aa34 Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 21:25:01 -0700 Subject: [PATCH 31/34] docs(heartbeat): the connective exclusion is family-agnostic, not consequence-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Only CONSEQUENCE connectives are affected by the exclusion; the adversative family … is deliberately untouched" described a property nobody chose. The exclusion applies to CONNECTIVES, which is both families; there is no adversative-specific carve-out, and CONNECTIVES sits one screen up saying so. Measured: putting a connective in the NEGATION_FILLER slot flips 120/168 consequence frames and 120/168 adversative frames, identically, because the lookahead cannot tell them apart. What actually holds "No evidence yet this head was …" as a hedge is position, not family: a grammatical adversative lands after the epistemic head, inside COPULA_FILLER's span, where the pass-17/18 balance governs it — not in NEGATION_FILLER's slot, where the exclusion lives. 0/7 grammatical adversative frames change, which is true and reproduces, but the mechanistic description invites a future pass to go looking for a guard that isn't there, find nothing, and add one. Rewritten to state the positional cause, with an adversative control row pinned in the boundary table now that the claim is load-bearing. Also rejoins an orphaned line from the pass-24 reword and re-wraps two bullets that ran past the region's ~80-column margin. No code change; the exclusion is correct as written. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 4 ++ server/src/services/heartbeat.ts | 37 ++++++++++++------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index ad9a6fe0cd2e..253d7bb08c3f 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1474,6 +1474,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No new idea landed for this head was already reviewed at \`${sha}\`.`, false], // widened by the pass-23 NON_CONNECTIVE_WORD token class [`No force-push evidence this head was already reviewed at \`${sha}\`.`, false], + // adversative control: the exclusion is family-agnostic, but a + // grammatical adversative sits in COPULA_FILLER's span, not + // NEGATION_FILLER's, so it stays a hedge rather than a skip. + [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], ])("known residual: %s -> %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 6d6f7873a51b..b33c8be9ab66 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9602,17 +9602,26 @@ const COPULA_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,${COPULA_FILLER_WORDS}}`; // inherits NON_CONNECTIVE_WORD's non-boundary token class in place of `\\w+`, // which cuts both ways: // - It closes on `negation` the citation-masking hole that the token-class -// note above records closing on COPULA_FILLER: "I have no `direct` evidence -// this head was …", "No (verified) proof that …", "No force-push evidence -// that …" were ACCEPTED at the parent because `\\w+` could not cross the -// backtick, parenthesis or hyphen, so the cue never fired. 4/4 now veto. +// note above records closing on COPULA_FILLER: "I have no `direct` +// evidence this head was …", "No (verified) proof that …", "No +// force-push evidence that …" were ACCEPTED at the parent because `\\w+` +// could not cross the backtick, parenthesis or hyphen, so the cue never +// fired. 4/4 now veto. // - It widens the CLAUSE_REACH bare-adjunct residual by one more shape — -// punctuated filler tokens with no connective ("No force-push evidence this -// head was …", 4/4 true -> false) — pinned in the characterization table -// under its own pass label. Fails toward `missing`, like the rest of it. -// Only CONSEQUENCE connectives are affected by the exclusion; the adversative -// family (`yet`/`however`/`still`/…) is deliberately untouched — "No evidence -// yet this head was …" is a hedge, not a skip — and measured 0 frames changed. +// punctuated filler tokens with no connective ("No force-push evidence +// this head was …", 4/4 true -> false) — pinned in the characterization +// table under its own pass label. Fails toward `missing`, like the rest +// of it. +// The exclusion covers BOTH connective families (CONNECTIVES, above) — it is +// family-agnostic, not consequence-only. Only consequence connectives are +// affected IN PRACTICE: a grammatical adversative sits after the epistemic +// head, inside COPULA_FILLER's span rather than NEGATION_FILLER's, so the +// exclusion never reaches it — "No evidence yet this head was …" stays a +// hedge. Measured 0 grammatical frames changed; the flips are confined to +// ungrammatical strings with the adversative IN the filler slot ("No yet +// evidence …"), which is why the measurement looks clean without being a +// property anyone chose. Do not add a same-family guard here — there is +// nothing to guard against, and the effect is positional. // // `{0,3}` is a KNOWN SECOND CAP, inherited verbatim from the old stem and not // derived from CLAUSE_SCOPE_CHARS the way COPULA_FILLER_WORDS is. It binds: @@ -9683,10 +9692,10 @@ const EPISTEMIC_NOUN_HEAD = // skip, since these words bind only after a negation), but a bare NEGATED // adjunct does false-veto, and two were opened by earlier passes of this PR: // "Given no certain match this head was …" (pass 20) and "Aside from no -// obvious drift this head -// was …" (pass 21) both flip true -> false, while "Despite no evidence of a -// force-push this head was …" false-vetoes at every head. Same CLAUSE_REACH -// residual as the fused stem's; see the characterization rows. +// obvious drift this head was …" (pass 21) both flip true -> false, while +// "Despite no evidence of a force-push this head was …" false-vetoes at every +// head. Same CLAUSE_REACH residual as the fused stem's; see the +// characterization rows. // // `obvious` joined the family in pass 21 as an exact synonym of `apparent` and // `evident`, which were already in it. Probing the same frame across 24 From c023863aa5704034a4084e9d8a4b87a70be344ae Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 21:50:29 -0700 Subject: [PATCH 32/34] docs(heartbeat): name the cue, not the position, for the adversative asymmetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 25's "positional" explanation was wrong, and disprovable in this same file: put a consequence and an adversative connective in the IDENTICAL slot (after the epistemic head, before the copula) and they verdict oppositely — "No evidence yet this head was …" is a hedge, "No evidence so this head was …" is a skip. Position cannot be the cause of an outcome that differs at the same position. The actual cause is `negatedHeadConnective`, whose stem names ADVERSATIVE_CONNECTIVES explicitly — a deliberate, hand-written adversative- only guard, load-bearing since pass 16 for 35/35 elided-complementizer phrasings. Two ablations confirm it: deleting the entry drops grammatical adversatives vetoed from 6/6 to 0/6; widening its stem to CONNECTIVES raises grammatical consequence frames vetoed from 0/6 to 6/6. The prior comment's closing line — "do not add a same-family guard, there is nothing to guard against" — had it backwards: the guard exists, and generalizing it is exactly what would reopen pass 16's 35 cases in the false-skip direction. The test row asserting this frame was also filed in the wrong block. It sat in "known CLAUSE_REACH adjunct residual (characterization)", whose header instructs a future CLAUSE_REACH fix to flip its rows to `true`. This row is not a residual — `false` is the correct, designed verdict — so that instruction would have silently converted a hedge into an asserted skip. Moved to the boundary table beside the existing filler-role rows, paired with a consequence- side control so the asymmetry is pinned from both directions. Mutation-checked: deleting `negatedHeadConnective` fails 44 rows; widening its stem to CONNECTIVES fails 6. Co-Authored-By: Claude Fable 5.1 --- .../heartbeat-context-summary.test.ts | 8 +++--- server/src/services/heartbeat.ts | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 253d7bb08c3f..7e696244a773 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1391,6 +1391,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { // the negation govern this clause — the pass-15 balance, unchanged. [`No sign but that this head was already reviewed at \`${sha}\`.`, false], [`No evidence so far that this head was already reviewed at \`${sha}\`.`, false], + // negatedHeadConnective's adversative-only guard, not position: same + // slot, opposite verdict for the two families (pass 26). + [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], + [`No evidence so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], // pass 23/24: the negation filler's token class can cross a citation or // punctuation; with `\\w+` these four were accepted (masking). [`I have no \`direct\` evidence this head was already reviewed at \`${sha}\`.`, false], @@ -1474,10 +1478,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No new idea landed for this head was already reviewed at \`${sha}\`.`, false], // widened by the pass-23 NON_CONNECTIVE_WORD token class [`No force-push evidence this head was already reviewed at \`${sha}\`.`, false], - // adversative control: the exclusion is family-agnostic, but a - // grammatical adversative sits in COPULA_FILLER's span, not - // NEGATION_FILLER's, so it stays a hedge rather than a skip. - [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], ])("known residual: %s -> %s", (text, want) => { expect(prReviewOutputHasAlreadyReviewedSkip(text)).toBe(want); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index b33c8be9ab66..1882b67ada21 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9614,14 +9614,23 @@ const COPULA_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,${COPULA_FILLER_WORDS}}`; // of it. // The exclusion covers BOTH connective families (CONNECTIVES, above) — it is // family-agnostic, not consequence-only. Only consequence connectives are -// affected IN PRACTICE: a grammatical adversative sits after the epistemic -// head, inside COPULA_FILLER's span rather than NEGATION_FILLER's, so the -// exclusion never reaches it — "No evidence yet this head was …" stays a -// hedge. Measured 0 grammatical frames changed; the flips are confined to -// ungrammatical strings with the adversative IN the filler slot ("No yet -// evidence …"), which is why the measurement looks clean without being a -// property anyone chose. Do not add a same-family guard here — there is -// nothing to guard against, and the effect is positional. +// affected IN PRACTICE, but NOT for a positional reason — pass 25 tried that +// explanation here and it is wrong, disprovable in this same file: consequence +// and adversative connectives in the IDENTICAL slot (after the epistemic head, +// before the copula) verdict oppositely ("No evidence yet this head was …" is +// a hedge, "No evidence so this head was …" is a skip), so position cannot be +// the cause. The actual cause is `negatedHeadConnective` (below), whose stem +// names ADVERSATIVE_CONNECTIVES explicitly — a deliberate, hand-written +// adversative-only guard, load-bearing since pass 16 (35/35 elided- +// complementizer phrasings). Ablated against this head: deleting that entry +// drops grammatical adversatives vetoed from 6/6 to 0/6; widening its stem to +// CONNECTIVES raises grammatical consequence frames vetoed from 0/6 to 6/6. +// See the attribution pin at heartbeat-context-summary.test.ts:1420, which +// already names this cue for this exact frame. Measured 0 grammatical frames +// changed BY THIS EXCLUSION; the flips are confined to ungrammatical strings +// with the adversative IN the filler slot ("No yet evidence …"). Do NOT +// generalise `negatedHeadConnective`'s ADVERSATIVE_CONNECTIVES to +// CONNECTIVES — it is load-bearing for pass 16's 35/35, not redundant here. // // `{0,3}` is a KNOWN SECOND CAP, inherited verbatim from the old stem and not // derived from CLAUSE_SCOPE_CHARS the way COPULA_FILLER_WORDS is. It binds: From 32621a7ed11561f661ff332cfe8ae8c99586a2e7 Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 22:14:04 -0700 Subject: [PATCH 33/34] test(heartbeat): delete duplicate rows; cite the attribution pin by frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 26's pin added no coverage. Both rows it added were byte-identical, including the expectation, to rows already present in the same `it.each` array — the family asymmetry had been pinned from both sides since before that commit. My prior remedy asked for a MOVE; the correct action was a DELETE, since the boundaries block already had the coverage. Removed the duplicate pair and put the rationale comment where the coverage actually lives, with a pointer to the consequence-side control instead of restating it. Also fixed the cross-reference this same rationale relies on: it cited an absolute line number that was already off by one before this commit and drifted further as the file grew. Rewritten to cite by frame text, which does not drift. Added the suggested minimal pair for the consequence-side control: the existing row differs from its adversative counterpart in two variables at once (connective family AND the sha-citation shape), so a regression in the `forMarker` arm would misread as a connective-family regression. A second row isolates the connective as the only variable. Mutation-checked against the relocated rows: deleting `negatedHeadConnective` still fails 43; widening its stem to CONNECTIVES still fails 6. Co-Authored-By: Claude Fable 5.1 --- .../src/__tests__/heartbeat-context-summary.test.ts | 11 +++++++---- server/src/services/heartbeat.ts | 6 ++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 7e696244a773..8809e5f4d480 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1284,6 +1284,10 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Unconvinced the wake was legitimate so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing showed that this head was already reviewed at \`${sha}\`.`, false], [`Nothing showed a newer commit so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + // negatedHeadConnective's adversative-only guard, not position: same slot, + // opposite verdict for the two families. Consequence-side control: + // `No evidence so this head was …` (below, and a minimal-pair backtick-sha + // form beside the consequence-connective rows further down). [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], [`No evidence still this head was already reviewed at \`${sha}\`.`, false], [`No evidence however this head was already reviewed at \`${sha}\`.`, false], @@ -1332,6 +1336,9 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Nothing new has been found still this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer head is known yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No evidence so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], + // minimal pair with the adversative row above: same backtick-sha shape, + // connective family is the only variable that changes. + [`No evidence so this head was already reviewed at \`${sha}\`.`, true], [`No clear evidence because this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing else found so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing new seen therefore this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1391,10 +1398,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { // the negation govern this clause — the pass-15 balance, unchanged. [`No sign but that this head was already reviewed at \`${sha}\`.`, false], [`No evidence so far that this head was already reviewed at \`${sha}\`.`, false], - // negatedHeadConnective's adversative-only guard, not position: same - // slot, opposite verdict for the two families (pass 26). - [`No evidence yet this head was already reviewed at \`${sha}\`.`, false], - [`No evidence so this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], // pass 23/24: the negation filler's token class can cross a citation or // punctuation; with `\\w+` these four were accepted (masking). [`I have no \`direct\` evidence this head was already reviewed at \`${sha}\`.`, false], diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 1882b67ada21..3094ea79bb2f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -9625,8 +9625,10 @@ const COPULA_FILLER = `(?:${NON_CONNECTIVE_WORD}){0,${COPULA_FILLER_WORDS}}`; // complementizer phrasings). Ablated against this head: deleting that entry // drops grammatical adversatives vetoed from 6/6 to 0/6; widening its stem to // CONNECTIVES raises grammatical consequence frames vetoed from 0/6 to 6/6. -// See the attribution pin at heartbeat-context-summary.test.ts:1420, which -// already names this cue for this exact frame. Measured 0 grammatical frames +// See the attribution pin for "No evidence yet this head was " in +// heartbeat-context-summary.test.ts, which already names this cue for this +// exact frame — cited by frame, not line number, since this file grows by +// several lines per pass. Measured 0 grammatical frames // changed BY THIS EXCLUSION; the flips are confined to ungrammatical strings // with the adversative IN the filler slot ("No yet evidence …"). Do NOT // generalise `negatedHeadConnective`'s ADVERSATIVE_CONNECTIVES to From 1aac7755248c31ed21739d591c28ce96e87221b7 Mon Sep 17 00:00:00 2001 From: CTO Date: Fri, 4 Sep 2026 23:12:16 -0700 Subject: [PATCH 34/34] test(heartbeat): delete the 12 pre-existing duplicate boundary rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass 27 flagged these as out of scope for that finding — the pattern predated the pass-26 pair it was reporting on — but noted them by example (:1206==:1168, :1272==:1111, :1347==:1324). Swept the boundaries block programmatically (same text, same expectation, same it.each array) rather than fixing only the three named examples, and found exactly 12, matching the count Ally reported. Verified each pair's exact text against the live file before deleting the later occurrence, and confirmed no duplicate carried a standalone comment that would be lost. Re-swept after deleting: 0 duplicates remain in the block, and a whole-file sweep (reset per it.each array, so intentional cross-describe repeats are not flagged) finds 0 elsewhere. Coverage is unaffected — every deleted row's text+expectation pair was already asserted by the row it duplicated. 544 -> 532 rows in this file; full suite count drops by exactly 12. Co-Authored-By: Claude Fable 5.1 --- .../src/__tests__/heartbeat-context-summary.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/server/src/__tests__/heartbeat-context-summary.test.ts b/server/src/__tests__/heartbeat-context-summary.test.ts index 8809e5f4d480..a390e7f18efb 100644 --- a/server/src/__tests__/heartbeat-context-summary.test.ts +++ b/server/src/__tests__/heartbeat-context-summary.test.ts @@ -1203,12 +1203,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No confirmation still that this head was already reviewed at \`${sha}\`.`, false], [`No evidence however that this head was already reviewed at \`${sha}\`.`, false], [`No record though that this head was already reviewed at \`${sha}\`.`, false], - [`No newer commits were found yet this head was already reviewed at \`${sha}\`.`, true], - [`No newer commits were found however this head was already reviewed at \`${sha}\`.`, true], - [`No newer commits were found though this head was already reviewed at \`${sha}\`.`, true], - [`No newer commits were found still this head was already reviewed at \`${sha}\`.`, true], - [`No newer commits were found nonetheless this head was already reviewed at \`${sha}\`.`, true], - [`No newer commits were found whereas this head was already reviewed at \`${sha}\`.`, true], [`No evidence while checking that this head was already reviewed at \`${sha}\`.`, false], [`No confirmation from the reviews API yet that this head was already reviewed at \`${sha}\`.`, false], [`I could not find any record in the last hour that this head was already reviewed at \`${sha}\`.`, false], @@ -1269,8 +1263,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`Nothing found since the commit that was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`I could not find any commit newer than the one that was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`The head moved to a commit which was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], - [`No evidence that this head was already reviewed at \`${sha}\`.`, false], - [`No evidence while checking that this head was already reviewed at \`${sha}\`.`, false], [`I cannot confirm \`8b23767\` was already reviewed at \`${sha}\`.`, false], [`I could not verify this PR's head was already reviewed at \`${sha}\`.`, false], [`I could not verify (after two retries) this head was already reviewed at \`${sha}\`.`, false], @@ -1280,7 +1272,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No evidence of a force-push; this head was already reviewed at \`${sha}\`.`, true], [`I remain unconvinced that this head was already reviewed at \`${sha}\`.`, false], [`Unaware whether this head was already reviewed at \`${sha}\`.`, false], - [`I am unaware of any newer commits so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Unconvinced the wake was legitimate so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing showed that this head was already reviewed at \`${sha}\`.`, false], [`Nothing showed a newer commit so already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1312,7 +1303,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No newer commits were found yet that head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer commits were found so that commit was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing new was seen still that head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], - [`No evidence yet that this head was already reviewed at \`${sha}\`.`, false], [`No evidence still that this head was already reviewed at \`${sha}\`.`, false], [`I assume this head was already reviewed at \`${sha}\`.`, false], [`I assumed this head was already reviewed at \`${sha}\`.`, false], @@ -1331,7 +1321,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No independent confirmation yet this head was already reviewed at \`${sha}\`.`, false], [`There is no real evidence yet this head was already reviewed at \`${sha}\`.`, false], [`No further indication still this head was already reviewed at \`${sha}\`.`, false], - [`No newer commits were found yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer revisions were seen yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`Nothing new has been found still this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer head is known yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], @@ -1351,7 +1340,6 @@ describe("evaluatePrReviewCompletionEvidence", () => { [`No newer commits appear yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No newer revisions indicate yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], [`No other branches suggest yet this head was already reviewed at 2026-09-02T23:31:00Z for ${sha}`, true], - [`No clear evidence yet this head was already reviewed at \`${sha}\`.`, false], [`No strong indication still this head was already reviewed at \`${sha}\`.`, false], [`I cannot fully confirm yet this head was already reviewed at \`${sha}\`.`, false], [`I am not fully aware yet this head was already reviewed at \`${sha}\`.`, false],