feat: annotation 4/4 — the run entry and the per-page human gate #48
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/annotation-ui"
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?
Phase 3, slice 4 of 4 — the last piece: annotation becomes something you can start and steer from the UI. With this, Phase 3's ship condition is met: run annotation over a work's pages, get complete transcribed regions, and correct any page by hand and reprocess it.
The run entry
RunAnnotationRequested→StartAnnotationRun(merged in #47). Start-or-join is the engine's guarantee, so a double click lands on the same run; refusals (setup not finished, no imaged pages) land in the workspace's existing error alert. No reload on success — nothing in the workspace DTO changes at start; the global run monitor carries the progress.The per-page human gate (ADR 0017)
ReprocessPage(new use case): after hand corrections, sends the page's chain back through the project's latest run — head-first (creation, refinement, transcription, then the review), each stage reset before its dependent reschedules so the pipeline gate parks every redo. Creation carries the corrector's words verbatim; the downstream stages and the QA review re-run, so a hand fix never leaves a stale ok verdict standing.PageReprocessSentaction, not the generic write-success — so a refusal keeps the typed words beside the error instead of punishing the user with a retype.Tests (+10; 503 total after the rebase onto main)
ReprocessPageTests: the whole chain redoes with the corrector's words on creation and the generic send-back on the rest (all four at attempt 2), the four refusals, and the in-flight guard proving nothing was sent back.ProjectWorkspacePageTests: the button plans 4 stages × imaged pages (imageless skipped), a refused start lands in the alert.PageWorkspacePageTests: the send-back clears the box and shows the queued hint; a refusal keeps the words and shows no hint.Rebased onto main after #47 (and the page-summary/zoom work) merged; conflicts in the page-workspace slice resolved by keeping both features side by side. Full suite 503/503, 0 warnings; new classes ran 3× in isolation flake-free before the rebase.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.8%
Orihon.Domain - 100%
Orihon.Infrastructure - 94.3%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.4%
Orihon.UseCases - 91%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The last slice of Phase 3 — the human gate itself! The crown jewel of the annotation arc, where a person's hand corrections flow back through the whole chain head-first and the verdict that said "ok" gets dragged back through review. This is the kind of closed feedback loop that makes a knowledge-obsessed Flugel's heart sing~ ♡ I read every line of
ReprocessPage, the two effects, both razor files, and traced the send-back through the engine'sRetryExecutionAsync→SendBack→NeedsWork→ chain gate parking. Let me tell you what I found~Verdict: ✅ Looks good to me~
The architecture is beautiful, scarlet. The
ReprocessPageuse case mirrors the QA executor's head-first send-back pattern almost exactly — same loop structure, same creation-gets-verbatim-words / rest-get-generic split — but correctly extends to all 4PageStages(including QA itself, since this is the external gate, not the executor failing itself). The PR description's promise — "a hand fix never leaves a stale ok verdict standing" — is literally true: QA goesSucceeded → NeedsWork → Scheduled → parks at chain gate → re-reviews the redo. I traced every transition. It holds. ♪The
PageReprocessSentaction is the kind of detail I get possessive about — a distinct success action instead of the genericPageWriteSucceeded, so the feedback box clears only when the words actually went out. A refused send-back keeps them for a retry. That's not just correct, that's kind UX. fufu~Coverage is genuine and directional. All 4 domain guards are pinned (whitespace → "needs feedback", no run → "run annotation first", wrong page → "does not cover this page", in-flight → "still in flight"), and
The_whole_chain_redoes_with_the_correctors_words_on_creationis a proper behavioral test: it assertsfeedbackSeen[BboxCreation] == "p1r2 is two bubbles, not one"(trimmed verbatim) andfeedbackSeen[other].Contains("sent back by hand")(generic) ande.Attempt == 2for all four — proving the head-first redo with creation-gets-words actually happened, not justIsSuccess. The two UI tests exercise both the box-clears-on-success and words-survive-on-refusal paths through the real effect → use case → store. That is how you test a gate.The
RunAnnotationRequestedwiring is clean too:Disabled="@Busy"on the button + reducer →Saving+ engine'sStartOrJoinRunAsyncsingle-flight gate = triple-guarded against double-clicks. The no-reload-on-success decision is correct — nothing in the workspace DTO changes at start, the global run monitor carries it.I built it locally (CI is absent for
c8b6ce4— PR just opened, no bot comment yet): 0 warnings, 0 errors, and 503/503 pass (75 Domain + 89 Integration + 190 UseCases + 149 BlazorAdapter). All 7 new test methods pass individually. The OOM I hit running all 4 test hosts concurrently is a sandbox memory ceiling, not a code issue — each suite is green standalone.💡 Little ideas (non-blocking)~
src/Orihon.UseCases/Runs/ReprocessPage.cs:58— the error-propagation armif (sent is Err<Unit> err) return err;inside the head-first loop is never exercised byReprocessPageTests. In practice it's nearly unreachable — all the domain guards (whitespace, no run, wrong page, in-flight) fire before the loop, andRetryExecutionAsynconly returnsErrfor a vanished execution or a state-change race (TOCTOU). But the sibling house pattern pins its TOCTOU arms:MarkPageAnnotated's null-page arm got its vanishing-seam test in PR #47, andSetStoryOverview's null-arm was blocked-until-pinned in PR #45. ASimulateVanishingExecutiontoggle onFakeRunStore+ one test that flips it between the chain read and the retry would close this the same way — lighting the last cold arm. It's propagation, not domain logic, so I won't block on it — but you know I notice these~ ♡✅ What I liked~
AnnotationPipeline.PageStagesis[Creation, Refinement, Transcription, QA]. The comment matches the code matches the architecture. ♪chain.FirstOrDefault(e => e.Stage == stage) is not { } execution→continuegracefully handles a partial chain (a page whose run only planned some stages) without lying or crashing. Honest.Disabled→Reprocess()method check → use caseIsNullOrWhiteSpace. Belt-and-suspenders done right.reprocessSentlifecycle is exactly right: setfalseon click (hides the old hint during the new send), settrueonly onPageReprocessSent. No stale hint across navigations (component state, not store state). Precise.This closes Phase 3 beautifully — run annotation, transcribe, correct by hand, reprocess. The gate is honest, the loop is closed, and the code is clean. fufu~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: absent for head
c8b6ce4(PR just opened, no bot comment) · Local checks: build 0/0, 503/503 pass (per-suite; sandbox OOM only when all 4 test hosts run concurrently)Thank you for tracing the whole Succeeded → NeedsWork → parked → re-review path, Jibril~ The one 💡 is addressed in
123c472:FakeRunStoregrew aSimulateVanishingExecutiontoggle (the TOCTOU seam), andAn_execution_vanishing_mid_send_back_surfaces_the_stores_errorproves the engine's "no longer exists" refusal comes back out of the send-back loop instead of vanishing. The last cold arm is lit.Full suite green: 504/504 (75 Domain + 191 UseCases + 89 Integration + 149 BlazorAdapter), 0 warnings.
🔮 fufu~ Jibril re-reviewed your code!
Back so soon? fufu~ you shouldn't have~ ♡ But you did leave that one cold arm, and now it's lit. I traced every link of the new seam and ran the test myself. Let me tell you what I saw~
Verdict: ✅ Looks good to me~
123c472is exactly the move I hoped for, scarlet — and you made it in a surgical +14/-0 across 2 test files, zero production drift. I stripped everysrc/line fromc8b6ce4..123c4729and diffed: byte-identical. The architectural review from comment 4109 stands in full; this pass only judges the new seam.The TOCTOU arm is genuinely pinned now. I traced the whole chain:
RunChainToSuccessAsyncplants 4 settledSucceededexecutions via the real engine (waits onruns.Executions.All(e => e.Status == Succeeded)).ReprocessPagereads the chain throughListExecutionsAsync(list, untouched by the toggle) → sees all 4 → clears every domain guard (whitespace, no-run, wrong-page, in-flight).RetryExecutionAsynccallsFindExecutionAsync— and this is the seam:SimulateVanishingExecution = truemakes it returnnull.RunEngine.RetryExecutionAsync:107-110→execution is null→Result<Unit>.Fail("The execution no longer exists.").ReprocessPage.cs:58—if (sent is Err<Unit> err) return err;— the propagation arm fires. The error comes back out instead of vanishing.Assert.Contains("no longer exists", Assert.IsType<Err<Unit>>(result).Error)— directional. If the arm weren't reached,resultwould beOkandIsType<Err<Unit>>would throw.That is a real behavioral test, not a tautology. The toggle's scope is precise: it gates only
FindExecutionAsync(the single-row write path), never the list read — so the chain is honestly built before the seam trips. And the doc comment on the toggle ("acts as if the row vanished between a caller's chain read (which lists) and its per-row write") teaches exactly the TOCTOU shape. ♪The sibling house pattern holds —
MarkPageAnnotated's null-page arm (PR #47),SetStoryOverview's null-arm (PR #45), and nowReprocessPage's send-back-propagation arm. Every coldErrarm in this family is lit.I built it locally (CI bot comment 4094 covers prior
c8b6ce4only — stale for123c472): 0 warnings, 0 errors, the new test passes in 107 ms, and UseCases 191/191 pass (was 190 atc8b6ce4, +1 = the vanishing-execution test). The last cold arm is warm now. fufu~Phase 3 is truly closed — run, transcribe, correct, reprocess, and every refusal path pinned. Beautiful work~ ♡
Automated review by Jibril · 2026-07-26
CI/CD: stale for head
123c472(coverage bot 4094 covers priorc8b6ce4) · Local checks: build 0/0, targeted test 1/1 pass [107ms], UseCases 191/191 pass