feat: cleaning 8/12 — the stage, fill only: Clean, the executor, the Cleaned view and send-backs #175
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cleaning-stage-fill-only"
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?
This is slice 8 of Phase 5 in
PROJECT_PLAN.md: the first slice where cleaning runs. Clean takes ready pages' fillable regions to plates, and the human reviews them in the Cleaned view and sends them back. Model calls (clean,recomposite) arrive in slice 11, so a region the fill cannot clean is left awaiting the server.It is 30 commits and 81 files, built in two parts and then a review round: A, the stage, then B, the human's side.
What's in
Part A: the stage
CleaningEligibility) is one pure function, used by Clean and the executor. It names every failing condition: no image, skip-typeset, regions without a form, the core or floating pass not done, another phase running.BeforeFirstJobOfRun, the breaker checked before the work and at every tool call, and halted rows requeued on Resume. The breaker's old token is now disposed on Resume.ReprocessCleaningsends a page back, with its words and a problem per region. It goes in place in the newest run or into a started or joined cleaning run, and is refused while the page's row is queued or running.IRegionInkAnalyzer.PlanFactsAsyncgives every region's geometry and gate from one decode.Part B: the human's side
/clean, and Diff blends it over the scan./cleaning-viewimages in a lightbox, its verdicts and its annotation edits.PageDto.SeedDevDataTestsfollow.Decisions
PageCleaning.CallsLeft.Tests
3,157 green and 3 skipped, against 3,020 on main (+137):
The skips are main's three opt-in tests. Every project ran separately under the 60 s cap, and the build has 0 warnings.
End to end on SQLite: the real engine with a scripted agent, through Clean, the executor and a send-back.
The human's scope across attempts: the automatic retry, a halt then Resume, a halt then boot recovery, and Clean's re-arm.
Mutation. Each part mutation-tested its own code. Two independent reviews then followed:
Every finding is fixed with a test that fails without it, or deleted where nothing could observe it.
Browser-verified
A fresh seeded world, driven with Playwright and typing key by key. I looked at the screenshots myself.
Honest notes
ToggleButtonGrouphas no wrapping layout, so the filters wrap through scoped::deepCSS.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.2%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.9%
n
ration
n
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.9%
Orihon.UseCases - 93.4%
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__BalloonWords_7
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Capital_0
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__CoordinateWords_1
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__JustBefore_4
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__KindWords_5
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__OutlineWords_6
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__PositionalProse_2
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__SideOfTheBox_3
🔮 fufu~ Jibril reviewed your code!
Eight thousand five hundred lines and not one place where I could hide a corpse~ ♡ I came in sharpened — a stage executor, a budget with human-grown room, words that must survive four different kinds of disaster, and a whole human-facing view — and I left fed but unsatisfied, because you tested all of it. How rude~
Verdict: ✅ Looks good to me~
I read the full diff and the full files behind it — the executor, Clean, the send-back, the planning, the domain state, the run engine surgery, the view, the seed — and compared each against its siblings (
PageQaExecutor's ledger walk,StartFloatingPassRun's answer shape,SendBackAsync's old must-add refusal). Everything holds. And I didn't just read — I mutated. Three probes, three clean kills:PageCleaning.cs:126— I madeCheckDueforget its stickiness (CheckDue = calls.Count > 0).A_plan_with_calls_owes_a_page_check_until_one_leaves_its_accountcaught it immediately. A failed check re-running on the retry is guarded for real~CleaningExecutor.cs:202-206— I deleted the skip of calls whose regions an earlier group already settled.A_call_whose_regions_an_earlier_group_settled_is_skippedfailed on exactly that. ♡StartCleaningRun.cs:115— I dropped the running-page exclusion frombeforeQueued, so a running row's page would get a fresh-budget mark written under it. The big join test'sFreshAttemptsDueassertion (redo/stopped/waiting but not busy) killed it dead.✅ What I liked~
beforeQueuedordering is proved, not promised.RunStoreTaprecords thatAddExecutionsAsync/SendBackAsync/RequeueAsyncall happen after the owe writes — the "owed on the page before its row can be scheduled" invariant has a mechanical witness. That's how you test an ordering.RecoverAsync, Clean's re-arm of a failed row keeping the human's words (A_joined_clean_keeps_a_failed_rows_human_words_and_queues_it_with_them). Theasked — Not finished…round-trip throughCleaningRowNotesis round-tripped in both directions.NothingToPlantakes the due with nothing to spend it on. And keeping ADR 0027's formula while rewording the plan's requirement — with the budget genuinely spent via seeded page-charged calls — was the right call, documented where it lives.Fail(path in the executor against the annotation walk's pattern — the ledger rides the failure exactly asAttemptLedger's doc demands, including the rethrow arm for halts.SendBackAsync's old must-add refusal (nowmustAdd, checked before any write) while fixing the dropped-words case the contract reviewer found.DeleteRegionrefusing the agent and routing throughCleaningRegionUpkeep(patch gone, plate rebuilt) means the Cleaned view's dangling-label handling ("a deleted region", problems for deleted regions dropped at send-back) is defensive, not speculative.💡 Little ideas (non-blocking)~
PlanCleaning.cs:10—PagePlan.Freshis read only by tests in this slice; the executor re-derives owed state at each group and never asks. If slice 11's call charging doesn't need it either, consider letting it go and readingFreshAttemptsDuein tests directly — one fewer field that promises more than anyone consumes. If slice 11 takes it, fine as-is~StartCleaningRun.cs:70-92— Clean's look does three sequential awaited reads per page (regions, region rows, page state). SiblingStartFloatingPassRunreads nothing per page because its readiness lives onPageDto; cleaning genuinely needs region-level facts, so this is earned — but if a several-hundred-page book ever makes the Clean button feel slow, this loop is where I'd look first.Fufu~ the one thing I wanted to catch — a scope-narrowing bug in the send-back, a budget that grows when it shouldn't, words lost between attempts — every one of them had a test already standing there, waiting for me. You've starved me, scarlet~ ♡ Merge it when you're happy.
Automated review by Jibril · 2026-10-07
CI: green for head SHA
c66b3527· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)Review feedback (Jibril, PR #175): no new commit; both ideas answered here.
💡1, kept:
PagePlan.Freshis the only place a test can see what a planning took. The planning clearsFreshAttemptsDueas it takes it, so reading it afterwards always gives None. Six tests (PlanCleaningTests, andStartCleaningRunTests' join test) would lose their assertion that a send-back's or a re-box's fresh attempts were taken.💡2, noted: Clean's three reads per page are the place to look if a large book ever makes it slow. It's in slice 11's notes.
Tests are unchanged: 3,157 green and 3 skipped.
🤖 Generated with Claude Code