fix: a review's last verdict waits for the human, and a named box loses its approval #209
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/box-qa-loop-small-fixes"
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 first of three slices on the Box creation ↔ Box QA loop. It follows a study of the loop, which found:
You chose three fixes: these small ones, Box QA hearing its last verdict, and Box creation answering Box QA. The other two follow as their own PRs once this one merges, stacked on it.
What's in
The last attempt sends nothing back (ADR 0018, 0029 amended)
needs_workstill sends the chain back. Creation and refinement then run a fourth time, and nothing reviews that redo. Transcription QA does the same with its regions.needs_workfails it for good, sends nothing back, and leaves its verdict for you asneeds work, left for you: ….QaReportSink.Summaryjoins them with " | ";Complaintused to drop the regions half of a mixed verdict.StageContext.LastAttempt), from the same comparison that fails the row. The review itself is not told, so it judges the same way every time.needs_workthat does send the page back now also shows both halves on Box QA's row while the redo runs.A named box loses its approval (ADR 0031 corrected)
ApproveBoxesAsynconly ever approved. A named box the refiner left alone kept an earlier verdict's approval, and itsqa approvedbadge.Region.ClearBoxApproval, which leaves the version alone), in the EF store and the fake.needs_work, a label written infeedbacknames that box too (RegionWrites.LabelsIn, ASCII lookarounds sop1r2のstill reads). Box QA writes there the labels a page-level problem concerns, so a box it criticised there is not one it accepted.ok's feedback goes nowhere, so its labels are not scanned.report_qarefuse feedback labels missing fromregions. Merges, splits and reading order name labels by design, and forcing them intoregionswould send Box refinement problems it cannot fix. ADR 0031 records this under Alternatives.Every review reply is paid for
needs_work, one that never calledreport_qa, and anokwhose page fact failed to save all dropped their cost, so QA loops looked cheaper on the monitor than they were. They now spend it into the ledger.A transcript keeps the whole task message
Tests
5355 passed, 3 skipped, against 5335 on main at
1eae837a. That is +20: Domain +2, UseCases +15, Integration +3. Both runs are fresh, each project under the 60 s cap.Flipped:
capcreation runs, notcap + 1, plus the kept verdict's exact error and its cost;capreads and theleft for youerror;p1r1now refits p1r1, because the verdict did not approve it.New tests:
An_attempt_knows_whether_a_failure_ends_its_streak(false, false, true, then a fresh streak after a hand retry)The_reviews_last_verdict_leaves_both_halves_for_the_human_and_writes_its_approvals. Its p1r2 is approved only by the last verdict, and it also pins the error while the redo runs.A_box_the_verdict_names_loses_the_approval_an_earlier_verdict_gave_itA_box_named_only_in_the_feedback_is_not_approved_and_is_fitted_againAn_ok_whose_feedback_names_a_box_still_approves_itApproveBoxesTests, including the label reader's theory (p1r12vsp1r1, uppercase, Japanese neighbours)ClearBoxApproval, and an EF store test against SQLiteMutation probes: 34 single edits, of which 26 failed a test the first time. Of the 8 that survived, 5 were missing pins:
ok;Those pins are added. A sixth gap from the review, the order of the cost and the early return, is pinned too, and a re-run kills all six. The other three change no output for any input today: the old
ComplaintequalsFeedbackunder its guard,LeftForYou's prefix is never read past attempt 1, and the kickoff'sTrim.Notes
feedback("the mark left of p1r2") costs that box one more fit. Missing a criticised box would cost more.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.5%
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.5%
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 little loop-surgery PR — three fixes, each one cutting exactly where the study said the loop was deaf~ I read every line of the diff, chased every sibling, and ran my little knives through the three riskiest new branches. My heart is quite full ♡
Verdict: ✅ Looks good to me~
✅ What I liked~
LeftForYoucannot lie to the detector.SentItsPageBackmatchesPreviousError?.StartsWith("needs work: ")— and"needs work, left for you: "does not start with it (comma vs colon,.Ordinal). A capped verdict that sent nothing back correctly reads as not a send-back. That boundary is exactly where this kind of fix usually bleeds~Summarycan never be empty on the path that prints it.report_qarefuses aneeds_workwith neitherregionsnorfeedback, so no blank-verdict error string can ever reach the row. I checked, fufu~ApproveBoxessemantics are honest in both stores. The EF store andFakeRegionStoreimplement the same three rules (case-insensitive labels, named→clear, rough→never approved), all callers of the oldexceptLabelsshape are updated, and the rename fromComplaint→FeedbackinSendChainBackAsyncis a true no-op under its own guard.\bsop1r2のstill reads; the theory pinsp1r12vsp1r1, uppercase, and the null/empty arms. I stabbed it — more below ♪report_qa-never (0.03), and both failed page-fact arms (0.01). No arm spends in silence anymore.ClearBoxApprovalmirrorsApproveBoxexactly — verdict-side write, version untouched, no-op when there was nothing to clear — with Domain tests plus an EF/SQLite round-trip. Sibling symmetry makes me so happy~💡 Little ideas (non-blocking)~
SaveChangesAsyncunconditionally, where the old code skipped it when nothing was to approve. EF treats it as a no-op save, so this is purely a tidiness trade — the removal of thetoApprove.Countbranch is worth it. Nothing to do, just know I saw it ♡🔪 Mutation probes (3 run, 0 survived~)
LastAttemptearly-return →if (false && …):The_reviews_last_verdict_leaves_both_halves…andA_review_that_never_passes…both FAIL. Killed.nullinstead ofsink.Ok is false ? sink.Feedback : null:A_box_named_only_in_the_feedback…and the flippedQA_needs_work_sends_the_chain_back…both FAIL. Killed.LabelsIntrailing lookahead removed: thexp1r1 p1r1x p1 r1theory case FAILS. Killed. (Thep1r12case specifically guards against digit-starving mutants — the boundary mutant that does fire there was caught by its sibling case, so the net holds.)Your own 34-probe ledger in the description matches what I felt poking around in here. The three survivors you left (guard-shadowed
Complaint, unreadLeftForYouprefix past attempt 1, kickoffTrim) all change no observable output today — I agree they're not worth pinning. The boot-recovery rerun not hearing its last verdict is cleanly documented as the next slice's business, and with the new prefix it at least no longer misreads as a send-back~Merge it, merge it! The loop finally hears itself~ ♡
Automated review by Jibril · 2026-10-17
CI: green for head SHA
39a6dc1b· Mutation probes: 3 run, 0 survived / 0 skipped · Local suite: skipped (CI green)