feat: Box QA hears its last verdict #210
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/box-qa-hears-its-verdict"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The second of three slices on the Box creation ↔ Box QA loop, after #209. Until now a review that sent its page back heard only "You checked this page before and sent work back." After a restart or a retry from the run monitor it heard not even that. So on a hard page, a later verdict could ask for the opposite of the one before, because nothing of the earlier opinion reached it.
What's in
The row keeps the verdict (ADR 0029, 0018 amended)
needs_workverdict: thefeedback, and the named regions with their problems (Execution.VerdictFeedback,VerdictRegions,LastVerdict).Execution.RecordVerdictwrites them on every delivered verdict. Aneeds_workkeeps them. Anok, a send-back and a success drop them. A start, a failed attempt, a requeue and the boot's reset keep them.needs_workwith neither half:report_qa's rule, held again on the write path.BoxQaExecutorrecords the verdict above its branches, so the last attempt's verdict is kept too. That is the one #209 now leaves for you.AddExecutionVerdict:VerdictFeedbackTEXT NULL,VerdictRegionsTEXT NOT NULL DEFAULT'[]'. That default and not EF's"", the same trap asFeedbackRegions. No backfill: no verdict was kept before.QA hears it
StageContext.LastVerdict).RetryVoiceappends it after anyone else's words, each half under the name of the parameter it was given in:The brief — one group just before your words:
The first rule is against anchoring: QA still judges the page fresh, and the verdict only tells it what it asked for. The second makes a reversal explicit, in the half that reaches the agent that acts on it.
Tests
5377 passed, 3 skipped, against 5355 on main at
35923865. That is +22: Domain +4, UseCases +16, Integration +2. Both runs are fresh, each project under the 60 s cap.Flipped: the re-review test used to assert round 1's complaint is absent from round 2. It now pins the exact verdict block after "That work is done.", and that the
okdropped it.Engine:
okdrops it, even when that attempt then fails on the page's fact.Domain:
needs_workkeeps both halves, trimmed, and anokdrops them, regions included;needs_worknames something;Store and migration: the EF round trip through a failed attempt and a requeue, by value (the problem holds a
+). The upgrade test covers a row written before the columns.Texts: exact pins on five renders (after a requeue, reworked, broke off, after your words, after your words and regions), each under the plain-words checks. Every brief sentence is pinned, and
LastVerdictLineis pinned too.Mutation probes: 36 single edits; 31 failed a test at first. Three of the five survivors were real gaps and are now pinned, and a re-run fails all three:
okkeeping the regions half;oknot dropping the verdict;The other two change nothing on SQLite:
HasMaxLengthon the feedback, and theIgnoreof a computed property.Notes
feedbackalone, and the approval stays hidden from every agent (ADR 0031).answer_feedback), shown to QA after this verdict block. It is ready and opens once this merges.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.6%
n
ration
n
one
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 95.2%
Orihon.UseCases - 94.6%
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__BalloonWords_7
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Capital_0
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__CoordinateWords_1
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__JustBefore_4
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__KindWords_5
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Label_9
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__OutlineWords_6
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__PositionalProse_2
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__SideOfTheBox_3
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Spaces_8
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A verdict that remembers itself~ The row keeps its own last opinion and whispers it back after a restart, a retry, even after its own attempt broke off — and nobody has to guess what the review used to want. State on the row, not a kept conversation. That is exactly the right shape for this, and I read every line of it twice, because I care~ ♡
Verdict: ✅ Looks good to me~
No blocking issues. I hunted for them, truly — the smile stayed, the knife came out, and it found nothing to cut~
🔪 What I checked, and why the knife went back in the sheath
AnnotationExecutors.cs) — the last attempt's verdict is kept becauseRecordVerdictAsyncruns before theLastAttemptreturn, theok-drops path, andMarkPageBoxesDone. Moving it is precisely what your two cap tests pin, and I did NOT take your word for it (see probes below). fufu~RecordVerdictwritesVerdictFeedbackandVerdictRegionsas one,Succeed/SendBackdrop them as one. Same doctrine the row already preaches forFeedback/FeedbackRegions("a region list outliving its prose would silently narrow the next re-run"). The new state obeys the old sermon~RecordVerdict's guards —RequireRunning, theBoxQa-only stage guard, and theneeds_work-names-somethingArgumentExceptionmirrorreport_qa's refusal (ReportQaTool.Unrouted), held again on the write path. The write-side re-validation matches howSendBackbehaves. Consistent~DEFAULT '[]'— I checkedJsonColumnMapper.AsJsonList:Deserialize<List<T>>(json)!on""would throw on the first read of an older row, so the hand-written default is not pedantry, it is the actual trap avoided — andExecutionVerdictMigrationTestsplants a pre-column row through raw SQL and reads it back. The read is the assertion. I love it when a test cannot lie~RecordVerdictAsyncinIRunStore/EfRunStore/TestDoublesmirrorsRecordReviewNoteAsyncline for line, and the EF configuration mirrorsFeedbackRegions/ReviewNote(AsJsonList,HasMaxLength(4000),Ignoreon the computedLastVerdict). Snapshot matches the configuration. No missing index, no wrong column type — nothing for me to sharpen my claws on.VerdictVoiceappends after everyone else's words, each half under its parameter's name, exactly as pinned. The brief's reversal rule ("put the reason in the same half of your verdict as the new point") is a lovely touch — it routes the explanation to whichever agent actually acts on it.StageContext.LastVerdictis passed for every stage, but only a Box QA row can ever hold one (the domain guard enforces it), so Transcription QA stays untouched, exactly as ADR 0029's amendment says.✅ What I liked~
ok-drops-it test that fails the page's fact after the ok — that is the nastiest little ordering edge in this PR, and it is pinned by name.+" in the integration round trip — a one-character guard against a whole class of encoding laziness. fufu~Automated review by Jibril · 2026-10-17
CI: green for head SHA
564e7675· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)