diff --git a/trios/agent-server/apps/server/src/api/services/queen-tick.ts b/trios/agent-server/apps/server/src/api/services/queen-tick.ts index 3cc0b37810..8d7ac6b5c1 100644 --- a/trios/agent-server/apps/server/src/api/services/queen-tick.ts +++ b/trios/agent-server/apps/server/src/api/services/queen-tick.ts @@ -1495,6 +1495,45 @@ async function boundaryStrays( * line's text and not the other way round. The containment guard exists * because a one-word line would otherwise be contained by everything and mark * every criterion judged; an exact match always counts, however short. + * + * THE TEMPLATE'S OWN NUMBER IS NOT PART OF THE CRITERION, and leaving it in + * disabled one of those two directions completely. + * + * The brief hands the bee numbered slots and asks for `- 1. : met`, + * so `parseVerdictBlock` returns a criterion that BEGINS "1. ". The promised + * text carries no such number. `want.includes(line)` therefore could not match + * once - not for a long criterion, not for a short one, not ever - because the + * line always held a prefix the promise did not. Only `line.includes(want)` + * survived, and that direction demands the bee reproduce the criterion whole: + * shorten it by one trailing sentence and the criterion reads as never + * answered. + * + * Measured 2026-09-06 over the 153 send-backs that had spent no retry budget: + * EVERY ONE had a complete, parseable VERDICT block, and every one was sent + * back for criteria it had answered. Stripping this prefix clears 148 of the + * 153; the remaining five are unjudged for reasons of their own. None of the + * 153 had a single criterion tested and FAILED - the entire group is work that + * was finished and returned over the Queen's own numbering. + * + * Stripped on BOTH sides, inside `normalize`, so this stays one rule rather + * than a special case at one call site, and the filter returns the ORIGINAL + * strings, so nothing a person reads loses its number. + * + * DO NOT "SIMPLIFY" THIS INTO SLOT MATCHING. `missingVerdictSlots` reports + * these same 153 as complete, which makes it look like the answer this should + * have been using all along. It is not. It counts a slot covered only when a + * parsed line BEGINS with its number, and measured across the whole board on + * 2026-09-06: of 358 blocks with criteria, 156 carry no numbering at all - and + * on every one of those 156, slot matching finds NOTHING covered. Wiring it as + * the review's matcher would have marked every criterion in 44% of the board + * unanswered, which is the present defect with a wider blast radius. + * + * The two functions answer the same question and disagree on 347 of 358 rows, + * in BOTH directions, while the comment on `missingVerdictSlots` asserts they + * cannot disagree. Neither is right alone: text matching was blind to the + * number, slot matching is blind to its absence. Stripping the number and then + * matching by text is the only rule that reads both the numbered briefs and + * the unnumbered ones, which is why the fix is here and not a swap. */ export function unjudgedCriteria( promised: string[], @@ -1502,6 +1541,7 @@ export function unjudgedCriteria( ): string[] { const normalize = (text: string): string => text + .replace(/^\s*\d{1,3}\.\s+/, '') .toLowerCase() .replace(/[^a-z0-9]+/g, ' ') .replace(/\s+/g, ' ') @@ -1811,7 +1851,11 @@ export function parseVerdictBlock( // Trying each and keeping the longest parse is stable under either // convention, so a worker running an older brief is not punished for it. const starts: number[] = [] - for (let i = text.indexOf('## VERDICT'); i >= 0; i = text.indexOf('## VERDICT', i + 1)) { + for ( + let i = text.indexOf('## VERDICT'); + i >= 0; + i = text.indexOf('## VERDICT', i + 1) + ) { starts.push(i) } if (!starts.length) return [] @@ -1884,10 +1928,20 @@ function parseVerdictFrom( * * The second half of the same defect: the brief now hands the bee a template * with numbered slots, and this is the check that names the numbers a given - * report did not fill. It reads the SAME block `parseVerdictBlock` reads - - * one parser, so a line this counts as answered is exactly a line the review - * counts as judged - and calls a slot covered only when a parsed verdict - * line carries its number. + * report did not fill. It reads the same block `parseVerdictBlock` reads, and + * calls a slot covered only when a parsed verdict line carries its number. + * + * IT DOES NOT AGREE WITH THE REVIEW, and the sentence that used to sit here + * said it did: "one parser, so a line this counts as answered is exactly a + * line the review counts as judged". Sharing a parser is not sharing a rule. + * The review matches by TEXT and this matches by NUMBER, and measured across + * the whole board on 2026-09-06 they disagree on 347 of 358 rows, in both + * directions - 156 blocks carry no numbering at all, and this reports every + * criterion in every one of them unanswered. + * + * A comment claiming two functions agree is a test that has not been written. + * This one was false for as long as it stood, and it is what made the numbered + * prefix in `unjudgedCriteria` look like someone else's already-solved problem. * * A NUMBER, NOT A POSITION. The old contract was order-based: the third * verdict line was the third criterion whether or not it said so, which is diff --git a/trios/agent-server/apps/server/tests/api/queen-review-unjudged.test.ts b/trios/agent-server/apps/server/tests/api/queen-review-unjudged.test.ts index 8a6cd28f65..c720607631 100644 --- a/trios/agent-server/apps/server/tests/api/queen-review-unjudged.test.ts +++ b/trios/agent-server/apps/server/tests/api/queen-review-unjudged.test.ts @@ -65,7 +65,10 @@ const FIVE = [ /** A verdict block answering the first N criteria, met or unmet as given. */ const block = (answers: Array<{ of: string; met: boolean }>): string => - ['## VERDICT', ...answers.map((a) => `- ${a.of}: ${a.met ? 'met' : 'unmet'}`)].join('\n') + [ + '## VERDICT', + ...answers.map((a) => `- ${a.of}: ${a.met ? 'met' : 'unmet'}`), + ].join('\n') describe('unjudgedCriteria, the difference itself', () => { /** @@ -87,7 +90,10 @@ describe('unjudgedCriteria, the difference itself', () => { it('returns nothing when the block answered every criterion', () => { expect( - unjudgedCriteria(FIVE, FIVE.map((c) => ({ criterion: c, met: true }))), + unjudgedCriteria( + FIVE, + FIVE.map((c) => ({ criterion: c, met: true })), + ), ).toEqual([]) }) @@ -130,9 +136,10 @@ describe('unjudgedCriteria, the difference itself', () => { */ it('does not let a short line judge everything that mentions it', () => { expect( - unjudgedCriteria(['The warm-up waits for the gate'], [ - { criterion: 'warm-up', met: true }, - ]), + unjudgedCriteria( + ['The warm-up waits for the gate'], + [{ criterion: 'warm-up', met: true }], + ), ).toEqual(['The warm-up waits for the gate']) }) @@ -141,6 +148,119 @@ describe('unjudgedCriteria, the difference itself', () => { unjudgedCriteria(['warm-up'], [{ criterion: 'warm-up', met: true }]), ).toEqual([]) }) + + /** + * THE CASE THE SUITE ABOVE NEVER SENT, and the only shape that fails. + * + * Every criterion in the tests above is unnumbered, and production has not + * sent an unnumbered one since the brief began handing out numbered slots. + * The parsed criterion begins "1. " and the promise does not, so + * `want.includes(line)` can never match - the prefix is always in the line + * and never in the promise. Only `line.includes(want)` survives, and that + * demands the bee reproduce the criterion WHOLE. + * + * So a numbered line that quotes its criterion in full still matches on the + * broken code; it is a bee that SHORTENS one that is lost. That is why this + * survived a suite with a case for each direction: the numbered cases were + * never written, and the shortening cases were never numbered. + * + * Measured 2026-09-06 over the 153 send-backs that had spent no retry + * budget: every one carried a complete VERDICT block, not one had a + * criterion tested and FAILED, and stripping the prefix clears 148. + */ + it('matches a numbered line that shortened a long criterion', () => { + const words = Array.from({ length: 50 }, (_, i) => `word${i}`) + const long = words.join(' ') + const quoted = `7. ${words.slice(0, 30).join(' ')}` + expect(long.length).toBeGreaterThan(300) + expect( + unjudgedCriteria([long], [{ criterion: quoted, met: true }]), + ).toEqual([]) + }) + + /** + * The production shape, copied from #1527: the issue states the criterion + * and then adds a sentence constraining it; the bee answers the criterion + * and stops at the sentence boundary. Before the strip this reads as a + * criterion nobody answered, and the work is returned for it. + */ + it('matches a numbered line that stopped at the sentence boundary', () => { + const promised = [ + '`LC_ALL=C grep -cP "[^\\x00-\\x7F]" src/LivenessDot.tsx` prints 0, and the ' + + 'raw output is quoted in the report. The command MUST NOT name or enumerate ' + + 'the specific items it is counting.', + ] + const judged = [ + { + criterion: + '1. `LC_ALL=C grep -cP "[^\\x00-\\x7F]" src/LivenessDot.tsx` prints 0, and ' + + 'the raw output is quoted in the report', + met: true, + }, + ] + expect(unjudgedCriteria(promised, judged)).toEqual([]) + }) + + /** + * GUARDS, not bug-catchers. Each of these passes before the strip as well as + * after, and they are here so the strip cannot buy the cases above at their + * expense - a fix that also made every short line judge everything would + * pass the two tests above and be worse than the defect. + */ + it('guard: a numbered line quoting its criterion in full still matches', () => { + expect( + unjudgedCriteria( + ['`make check` exits 0, and the raw output is quoted.'], + [ + { + criterion: '1. `make check` exits 0, and the raw output is quoted.', + met: true, + }, + ], + ), + ).toEqual([]) + }) + + it('guard: the number is stripped on both sides, so one rule serves both', () => { + expect( + unjudgedCriteria( + ['2. the gate is red before it is green'], + [{ criterion: '2. the gate is red before it is green', met: true }], + ), + ).toEqual([]) + }) + + it('guard: a short numbered line still does not judge a long criterion', () => { + expect( + unjudgedCriteria( + ['The warm-up waits for the gate'], + [{ criterion: '1. warm-up', met: true }], + ), + ).toEqual(['The warm-up waits for the gate']) + }) + + it('guard: a criterion that opens with a decimal keeps its meaning', () => { + expect( + unjudgedCriteria( + ['3.5 seconds is the ceiling for a warm start'], + [ + { + criterion: '3.5 seconds is the ceiling for a warm start', + met: true, + }, + ], + ), + ).toEqual([]) + }) + + it('guard: a criterion nobody answered is still reported', () => { + expect( + unjudgedCriteria( + ['the first thing', 'the second thing entirely'], + [{ criterion: '1. the first thing', met: true }], + ), + ).toEqual(['the second thing entirely']) + }) }) interface FinishedRow { @@ -246,9 +366,7 @@ describe('the review, against the real policy', () => { expect(reviewed.acted).toEqual([`#${ISSUE}:sendBack`]) // FR-001: judged and unjudged recorded as separate numbers. - expect(reviewed.tally).toEqual([ - { issue: ISSUE, judged: 2, unjudged: 3 }, - ]) + expect(reviewed.tally).toEqual([{ issue: ISSUE, judged: 2, unjudged: 3 }]) const update = reviewUpdate(queries) expect(update?.params[1]).toBe('sendBack') @@ -283,9 +401,7 @@ describe('the review, against the real policy', () => { const reviewed = await reviewFinishedDispatches(pool) - expect(reviewed.tally).toEqual([ - { issue: ISSUE, judged: 5, unjudged: 0 }, - ]) + expect(reviewed.tally).toEqual([{ issue: ISSUE, judged: 5, unjudged: 0 }]) const update = reviewUpdate(queries) expect(update?.params[1]).toBe('sendBack') @@ -318,9 +434,7 @@ describe('the review, against the real policy', () => { const reviewed = await reviewFinishedDispatches(pool) - expect(reviewed.tally).toEqual([ - { issue: ISSUE, judged: 2, unjudged: 3 }, - ]) + expect(reviewed.tally).toEqual([{ issue: ISSUE, judged: 2, unjudged: 3 }]) const update = reviewUpdate(queries) expect(update?.params[1]).toBe('sendBack') @@ -356,22 +470,17 @@ describe('the review, against the real policy', () => { * the policy is asked about zero verdicts and answers wait, and nothing is * spent. Told apart, too: judged 0 is a different fact from judged 2. */ - it.if(present)( - 'still reads a wholly absent block as a wait', - async () => { - const { pool, queries } = reviewPool([ - finishedRow({ said: 'I finished. It all looks fine to me.' }), - ]) + it.if(present)('still reads a wholly absent block as a wait', async () => { + const { pool, queries } = reviewPool([ + finishedRow({ said: 'I finished. It all looks fine to me.' }), + ]) - const reviewed = await reviewFinishedDispatches(pool) + const reviewed = await reviewFinishedDispatches(pool) - expect(reviewed.tally).toEqual([ - { issue: ISSUE, judged: 0, unjudged: 5 }, - ]) + expect(reviewed.tally).toEqual([{ issue: ISSUE, judged: 0, unjudged: 5 }]) - const update = reviewUpdate(queries) - expect(update?.params[1]).toBe('wait') - expect(update?.params[4]).toBe(false) - }, - ) + const update = reviewUpdate(queries) + expect(update?.params[1]).toBe('wait') + expect(update?.params[4]).toBe(false) + }) })