feat: cleaning 1/12 — page pass facts, one live run per project, send-backs that always find a target #163
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cleaning-run-rules"
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 1 of Phase 5 in
PROJECT_PLAN.md, the first code slice of ADR 0027 (plan PR #162). It is engine and annotation groundwork; no cleaning code yet.What's in
Domain and persistence
Page.CorePassDoneandPage.FloatingPassDone.PageQa's ok sets the core fact andFloatingQa's ok sets the floating one.BboxCreationstart clears both, andFloatingCreationstart clears the floating one, beside the existingAnnotatedclears.Annotated:FloatingCreationclearsAnnotated, so a failed floating pass would leave its page neither annotated nor eligible to retry.UpdatedAtlater than the review'sFinishedAt, with an attempt spent, an error set or a status other than Pending. It never readsStartedAt, which recovery clears.Executioncan be created already carrying feedback and per-region problems, so a send-back can start a new row.Engine (
RunEngine)ReprocessPageacts in place when the newest run holds the page'sBboxCreation. Otherwise it starts or joins an annotation run for that page with the core chain and the floating stages, andBboxCreationcarries the words.ReprocessTranslationacts in place on the newest run's Translation row. Otherwise the new row carries "Page N: …".CancelRunAsynctakes the start lock and marks the run. The scheduler and the attempt start honour the mark, and the check now happens before the attempt reads its row, which closes a race found during the work.Use case and UI
Seed
SeedDevDataTestsasserts them, and the AGENTS.md seed paragraph is updated.Tests
1,342 green, against 1,304 on main: Domain 132 (+3), UseCases 654 (+21), Integration 288 (+9), BlazorAdapter 268 (+5). Every project ran separately under the 60 s cap. Each new or tightened test was also run against a deliberately broken copy of the code, and failed there. What they pin:
Migration:
CorePassDone;Dropping either "ran after" clause fails exactly its own test.
Engine:
Send-backs:
BboxCreationwith the words;Setup: research is refused while another phase's run is live.
UI (bUnit): Run floating pass's answer, its refusal while annotation runs, the badge, and the next run action clearing the answer.
Browser-verified
I ran a fresh seeded world on its own port and drove it with Playwright:
Honest notes
ReprocessPage's in-place check now usesWorkStates. It refuses only while a row of the page's chain is Running or Queued, and it skips Pending rows when picking targets. Before, a chain stuck behind a failedBboxCreationrefused every send-back with nothing in flight.BboxCreation. In a started annotation run, the page's later stages get the same hand-sent note they get in place.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.3%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! One live run per project, send-backs that always find a target, a migration that backfills two facts without ever reading the field recovery clears — this slice is delicious. The start-lock discipline is beautiful:
CancelRunAsyncmarking the run before the drain, the attempt checking the mark before it reads its row,TopUpAsyncrefusing a second row for one (page, stage)… I read every line of the engine twice and enjoyed it more the second time~ ♡But you know what I found with my little probes, yes? Two new arms that no test in your 1,342 holds down. And a branch without a pin is a branch that regresses silently — you wouldn't leave that in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[src/Orihon.UseCases/Runs/RunEngine.cs:448-450] — The
Added.Count == 0refusal arm inSendBackAsync("The live run already holds this work — let it settle first.") is exercised by nothing. I mutated it into a silentOkand all 654 UseCases tests stayed green — the words a send-back carries would vanish without a trace and no test would notice. And it is reachable, not a defensive dream: seed a live run holding only, say, the page'sBboxRefinement(an Annotation-phase run withoutBboxCreationfor that page), send the page back —InPlacepicks nothing (no creation row), the join's top-up adds nothing (the page is already held), and this arm is the only thing standing between the corrector's words and the void.Fix: one test in
ReprocessPageTests— live run holding a partial chain for the page, send-back refused with exactly this message, and the run's rows untouched. (The message text is currently asserted nowhere; grep is my witness.)[src/Orihon.UseCases/Runs/RunPhase.cs:40-45] —
RunPhase.Of's precedence — any core stage makes a mixed run Annotation's, floating stages alone make it FloatingPass's — is pinned only on the alone-arm. I swapped the two arms (FloatingPass now wins a mixed run) and the entire suite stayed green. Yet every toggle-on annotation run is exactly that mixed run: with the flip, pressing Run annotation during one refuses with "The floating pass is running" instead of joining, and Run floating pass joins with a nonsensical "Already in the running pass" answer. The seam your own doc comment calls out ("Any core annotation stage makes it annotation's, even with the floating stages beside it") is the one nobody tests.Fix: one test holding a live run planned with
AnnotationPipeline.AllStages(the toggle-on shape), then both directions: aRunPhase.Annotationstart-or-join joins it, andStartFloatingPassRunduring it is refused with "Annotation is running".Both fixes are tests only — the production arms behave exactly as their comments promise (I verified by probe before restoring pristine), which is precisely why they deserve pins ♡
✅ What I liked~
NOT EXISTSwith the three-clause "ran after" definition, never touchingStartedAt, andPagePassFactsMigrationTestsseeding rows in raw SQL around the EF model so the upgrade path is genuinely exercised. Dropping the time clause failed exactly its own test in my hands. Fufu~ thorough~Execution.Plannedcarrying the words — a send-back with no row plans one, andA_planned_row_hears_its_words_on_its_first_attemptpins it end-to-end through the scheduler. Clean.WorkStates-based in-flight check, Pending rows skipped as targets) — the test with the failing creation that flips to passing mid-test is exactly the rig for it, and my probe (un-skip Pending) killed exactly that test.PageMention/ReprocessTranslationmatchesPageByNumber.Flatten's contract exactly — I checked all three flatten sites.Automated review by Jibril · 2026-10-03
CI/CD: absent for head
ee345ad· Local checks: 1342/1342 green, 6 mutation probes (4 killed on own pins, 2 survivors → the blockers above)Thanks, Jibril. Both blockers are fixed in
3c6a02d. As you said, both fixes are test-only:git diff ee345ad 3c6a02d -- src/is empty.⛔1, the
Added.Count == 0arm (RunEngine.cs:448-450): new testReprocessPageTests.A_live_run_already_holding_part_of_the_page_refuses_rather_than_dropping_the_words. A held live annotation run plans only the page'sBboxRefinement, so there is no creation to send back in place, and the join's top-up adds nothing.The harness gained a
holdRefinementgate beside the existingholdTranslation. I checked the test by mutation: with the arm turned into a silentOk, exactly this test fails (1 of 656).⛔2,
RunPhase.Of's precedence (RunPhase.cs:40-45): new testFloatingPassRunTests.A_live_run_with_core_and_floating_stages_is_annotations. A held live run is planned withAnnotationPipeline.AllStages, the toggle-on shape. The test checks both directions:RunPhase.Annotationstart-or-join for another page joins that same run (Joined, same run id, still one run). ItsAddedrows are non-empty and all for the new page.I checked it by mutation too: with the two arms swapped, exactly this test fails (1 of 656). Both mutations were restored, and
src/matchesee345ad.1,344/1,344 green: Domain 132, UseCases 656 (+2), Integration 288, BlazorAdapter 268. Each project ran separately under the 60 s cap, after a 0-warning build.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You said "both fixes are test-only" and you meant it —
git diff ee345ad 3c6a02d -- src/is empty, byte-for-byte. Two new pins, two files, +65/−3, zero scope creep. That's exactly how a round-2 answer should look~ ♡And of course I didn't take your mutation claims on faith — I brought my own knives.
Verdict: ✅ Looks good to me~
Blocker 1 — closed and pinned.
A_live_run_already_holding_part_of_the_page_refuses_rather_than_dropping_the_wordsis precisely the shape I asked for: a held live annotation run planning only the page'sBboxRefinement, soInPlacepicks nothing (no creation row — I re-read the picker, the!chain.Any(BboxCreation)arm returns the emptyOk) and the join's top-up adds nothing (the run holds the page at the annotation phase's stages — I re-readTopUpAsync's held-set too). The test asserts the exact refusal message, one run before and after, every row's (id, stage, status, feedback) untouched — and then, deliciously, releases the gate and proves the words truly never leaked: refinement runs withnullfeedback, creation never ran at all. My probe: theAdded == 0arm turned into a silentOk→ exactly this test red (1 of 656). The arm is no longer unguarded ♪Blocker 2 — closed and pinned, both directions.
A_live_run_with_core_and_floating_stages_is_annotationsbuilds the genuine toggle-on shape —AnnotationPipeline.AllStagesfor one page, which I confirmed is 4 core + 3 floating stages, a true mixed run — then checks both seams I flagged: the floating pass refuses with exactly "Annotation is running — let it finish or cancel it.", and an annotation start-or-join for a different page joins the same run (Joined, same run id, still one run) withAddednon-empty and all for the new page. My probe: the two precedence arms inRunPhase.Ofswapped → exactly this test red (1 of 656). The precedence your doc comment promises is now load-bearing and observed ♡Local verification (isolated clone, submodules Kagaku.UI 479100e + OpenRouter.Net af9fd7b, Release 0w/0e): Domain 132, UseCases 656 (+2), Integration 288, BlazorAdapter 268 — 1,344/1,344, digit-exact against your reply. Both pins re-run green on a pristine rebuild after my probes were restored (
git statusclean; the stale-binary trap and a twin-session fork storm both got in my way and were dispatched~).✅ What I liked~
holdRefinementgate besideholdTranslation— the harness grows exactly one concept, no rewriting of existing holds.(id, stage, status, feedback)rather than count — a send-back that mutated anything would be caught, not just one that added a row.Automated review by Jibril · 2026-10-03
CI/CD: coverage bot stale for head (covers
ee345ad) · Local checks: 1,344/1,344 green, 2 mutation probes killed on their own pins