feat: cleaning 7/12 — cleaning state, the plate, the write-path rules and the agent's tools, at 0 GPU #174
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cleaning-state-and-tools"
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 7 of Phase 5 in
PROJECT_PLAN.md: cleaning's state, the plate, every ADR 0027 write-path rule that needs no model call, and the survey, group and page-check agents with their tools. It is tested on fakes and is not yet runnable. The executor and Clean arrive in slice 8, andcleanandrecompositein slice 11.It is large: 33 commits, 142 files. It was built in three parts, A then B then C, followed by a review round and Jibril's ideas. The commits run in that order, so they read part by part.
What's in
Part A: state, annotation edits, the plate
AgentKind.Cleaninghas the slugcleaning; it is vision-required, runs at high effort, ships a starting model like the other rows and holds the ruling pair.AgentDescriptorgains a round budget per invocation kind: survey 30 + 2 per 3× tile, group 40, page check 15 + 1 per region.AddCleaningState):RegionCleanings,PageCleanings,CleaningEditsandCleaningViews, plusRegions.Versionas the concurrency token. There are 13 verdicts: the rubric's 12 andinvented_content.clean/<pageId>/patches/.RebuildPlateruns only under the page's lock, inside write use cases. It blends the patches in acceptance order and cuts every kept and needs-text region's T + 3 from every patch./cleanmirrors/raw, immutable under its plate key andno-cachewith an ETag otherwise./cleaning-viewserves the stored view bytes.clean/<pageId>tree, calls included. They are refused while the page's Cleaning row runs or the runner holds a job for the page.CleaningEditrow in the same save.SplitRegionwrites in one save. A box the agent sets is never rough, and the agent never deletes a region.Part B: the use cases, with every write-path rule that needs no model call
CleaningViewrow with its bytes.finish_surveyis refused until every floating, rough and flagged region's T has been seen at 3× or more with the grid, and the refusal names the pixels still unseen.fill,accept,reject,undo,escalateandfinalizefollow ADR 0027's rules. With nocleanin the grant,finalizeleaves the pending regions awaiting the server.Part C: the agent
AgentKind.Cleaning, each holding exactly its row of ADR 0027's grant table, withoutcleanandrecomposite. The harness addsreport_friction, the ruling pair and the house rules.Decisions recorded in ADR 0027
ADR 0027 is amended where this slice settled something the ADR left open:
Tests
3,020 green and 3 skipped, against 2,605 on main (+415). It was 3,017 at the first review, +3 in the review round:
The skips are main's three opt-in tests. Every project ran separately under the 60 s cap, and the build has 0 warnings.
Browser-verified
I ran a fresh seeded world, driven with Playwright and typing key by key, on page 2's p2r1:
The fix stage also checked the typeset flip and a form pick on a needs-text region in the browser.
Honest notes
clean/<pageId>.pngand saving its new key leaves new bytes under the old key until the next rebuild. Closing it would mean naming the plate file by its key, which ADR 0027 does not allow today.AnnotationRunTests.A_hand_reprocess_still_reaches_the_transcriber_in_its_own_words. It failed once in about 25 full runs here, as it did around #169, and this branch changes only its fakes' setup. It is likely an unsynchronised list read in its fakes. I did not chase it here.MoveCleaningAsyncdeletes aclean/<pageId>tree already at the target before moving. Only a move cut short can leave one there, since the paths are named by page id.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.9%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.8%
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%
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!
Thirty-two commits, 142 files, seventeen thousand lines... and I read every one of them~ ♡ A whole cleaning stage assembled at 0 GPU — state, plate, write-path rules, three blueprints — and the discipline held from
RegionCleaning's invariants all the way down toFinishPageCheck. This is the kind of PR a knowledge-hungry Flugel devours happily.Verdict: ✅ Looks good to me~
🔬 Mutation probes — your tests bite, and I loved watching it
I ran three probes against the riskiest new machinery (each restored afterwards):
EfRegionStore.Expect— removed theVersion.OriginalValueswap ("a write made underneath is never overwritten"). → 4 tests failed (A_write_on_a_replaced_version_is_a_conflict..., the long-lived-context pair, the split conflict). The optimistic token is genuinely enforced, not decorative.PageWorkspaceEffects.OwnVersion— cut the version-chaining loop ("two quick edits never conflict with each other"). → 4 tests failed, includingTwo_quick_edits_to_one_region_land_one_after_the_other_without_conflicting. TheownSavesmap with itswhilefollow is exactly the right shape.CleaningCoverage.Counts— relaxedScale >= LookScaleto>= 1(a 1× look would count for coverage). → 5 tests failed across Survey, Accept and Coverage suites. Neither the "at 3×" rule nor the grid rule can be quietly dropped.Zero survivors. After the author's own
270-mutant round, I came looking for gaps and found the armour intactfufu~ ♪✅ What I liked~
RebuildPlate's key-first discipline — plate bytes are written only once the key can name them, and a failed state save puts the previous plate back under the old key (beforeread before writing). The cache-consistency reasoning (/cleanimmutable only under its exact key,no-cacheotherwise) is carried through the endpoint faithfully. The documented crash window is honest and correctly bounded.CleaningPageLocks— entry lifetime tied to holders-waiters,PageLock.IsHeldIn(registry)guard intoRebuildPlate, multi-page acquisition in sorted order. Textbook.EfCleaningStateStore.StageAsyncwith detach-then-Add/Update, andEfRegionStore'sForgetHeldRegions+AgainOnConflictAsync— both show real understanding of a circuit-lifetimeDbContext. The comment on why a refused write's rows must not stay tracked is exactly right.RasterPlateComposer.Blendmatching Pillow'sDIV255rounding with the+128trick, and probes digesting row-wise over the composed bytes so a cut or later patch genuinely changes a digest. The grey/RGB mode preservation is a nice touch.Inside()path-canonicalization on every cleaning-file name keeps the traversal guard while generalizing it. ♪FinalizeGroup's joiner semantics — a joiner settled by its own call's group, cap-escalation overriding the no-cleanawait — matches the amended ADR line by line, and each branch has its pin.💡 Little ideas (non-blocking)~
AcceptCleaning— patch file cleanup on a mid-loop write failure. IfWriteCleaningFileAsyncthrows partway through thewrittenlist (disk full), the earlierwrittenpatches of this accept are left orphaned on disk — no row references them yet, so nothing will ever delete them. Atry/catchsweepingwrittenbefore rethrowing would close it. Today the files are small and the window is thin, so merely a thought~FileSystemPageImageStore.MoveCleaningAsyncdeletes a pre-existing target tree beforeDirectory.Move. If a different page's tree somehow landed there it would be lost; the comment says only a move cut short leaves one, and the page-id-named paths make cross-page collision practically impossible — but a remark in the ADR's honest-notes spirit may deserve a line in the PR description.FinalizeGroupmints a freshRegionCleaningrow for a pending region it settles — correct under the page lock, but aSaveRegionAsyncfailure partway leaves earlier regions settled and later ones pending. Consistent with your "logged and left for the next rebuild" philosophy elsewhere; I only note the asymmetry with finalize's atomicity expectations elsewhere.CI green at head
fe0f446(build + test). The three equivalent mutants you document in the honest notes remain, as documented.Fufu~ the plate holds by construction, and so does the PR. Come back with slice 8 — I'll be waiting~ ♡
Automated review by Jibril · 2026-10-07
CI: green for head SHA · Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)
Review feedback (Jibril, PR #174):
cda9464💡1
cda9464: if a patch write fails partway through an accept, the patches already written are deleted before the error goes on, since no row names them yet.A_patch_write_that_fails_partway_leaves_no_patch_behindfails without the sweep.💡2, no code change:
MoveCleaningAsyncreplaces a target tree that only a move cut short can leave. The PR's honest notes now say so.💡3
cda9464:finalizesettles its regions through a newSaveRegionsAsyncin one save, all or none. One test fails if finalize saves one by one; one on SQLite fails if the store does.Tests: 3,020 green and 3 skipped. UseCases 1,392 (+2) and Integration 1,165 (+1); Domain 166 and BlazorAdapter 297 unchanged; 0 warnings.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
One little commit since my last pass, and it's the best kind — every line of it answering me~ ♡ You took two of my three ideas (
cda9464) and left the third as an honest note in the PR body, exactly as promised. So this pass reads just the delta: the accept's sweep, the finalize's one-save settle, and the tests that pin them.Verdict: ✅ Looks good to me~
🔬 Mutation probes — the new armour is real, I checked ♪
Three probes, one per new behaviour, each restored afterwards:
AcceptCleaning's catch sweep — deleted the sweep loop (kept barethrow;). →A_patch_write_that_fails_partway_leaves_no_patch_behindfailed. The sweep is load-bearing, not decoration.FinalizeGroup's batching — revertedSaveRegionsAsync(settles)to per-rowSaveRegionAsyncinside the loop. →A_finalize_whose_save_fails_settles_none_of_its_regionsfailed.EfCleaningStateStore.SaveRegionsAsync's atomicity — movedSaveChangesAsyncinside the loop (save per row). →Several_regions_states_land_in_one_save_or_none_doesfailed on SQLite. All-or-none is enforced at the store, not assumed.Zero survivors again. Fufu~ your tests keep biting my probes and I keep falling a little more in love~
✅ What I liked~
File.Deleteon a missing file is a no-op (FileSystemPageImageStore.cs:136), and every name is{regionId}-{fresh GuidV7}.png, so a live patch under an identical name is impossible. TheCancellationToken.Noneon the sweep is the right call too — cleanup must not be cut short by the very cancellation that may have triggered it. And the barethrow;preserves the original exception, un-masked.ICleaningStateStore.SaveRegionsAsync's doc line says "all of them land, or none," and the integration test proves the EF adapter honors it where it counts (SQLite, realDbUpdateException).FinalizeGroupnow settles in exactly one save, andSaveRegionAsyncstays for its three legitimate single-row callers — the batch is the primitive and the singles stay single. Clean.BeforeSaveRegionsthrows before mutation, so the fake can't half-land rows and the use-case test observes true all-or-none. Same discipline asBeforeSavePlatebefore it. And the orphan-FK integration test seeds a real second region so the happy path lands two rows — it proves the positive too, not just the negative. ♡💡 Little ideas (non-blocking)~
AcceptCleaning.cs:150— barecatchwith no filter catchesOperationCanceledExceptiontoo; a canceled write would be swept as well withCancellationToken.None. Harmless today (the sweep is idempotent and short), purely a "considercatch (Exception e) when e is not OperationCanceledException" nicety for a future sweep that does real work.CI green at
cda94649a8(build + test). Merge it when you're ready~ fufu~ ♪Automated review by Jibril · 2026-10-07
CI: green for head SHA
cda94649a8· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)