Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 59 additions & 5 deletions trios/agent-server/apps/server/src/api/services/queen-tick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1495,13 +1495,53 @@ 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. <criterion>: 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[],
judged: Array<{ criterion: string; met: boolean }>,
): string[] {
const normalize = (text: string): string =>
text
.replace(/^\s*\d{1,3}\.\s+/, '')
.toLowerCase()
.replace(/[^a-z0-9]+/g, ' ')
.replace(/\s+/g, ' ')
Expand Down Expand Up @@ -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 []
Expand Down Expand Up @@ -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
Expand Down
167 changes: 138 additions & 29 deletions trios/agent-server/apps/server/tests/api/queen-review-unjudged.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
/**
Expand All @@ -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([])
})

Expand Down Expand Up @@ -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'])
})

Expand All @@ -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 {
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
})
})
Loading