feat: phase 4 slice 3/3 — the run entries and the translation gate #61
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/phase4-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?
The Phase-4 finale — and with it the MVP loop closes (PROJECT_PLAN: create → upload → annotate → bible → translate → review/approve). ADR 0019's pre-pixel-pass reading applies: the gate is the end of the run plus per-view feedback→reprocess, and this slice supplies the last missing pieces.
The run entries
ReportRunStart(the annotation effect was folded onto it too): no reload on success, the global run monitor carries progress, refusals land in the workspace's error alert.The translation gate (ADR 0019)
ReprocessTranslation(new use case): the reviewer on a page's translation view writes feedback and the TRANSLATION stage re-runs — the same sequential agent, re-invoked with distrust, carrying the words prefixed with the page in the agent's own chapter-major 1-based numbering (PageByNumber's contract, via the workspace flattening — the same source-of-truth discipline #57's review enforced in the executor). Refusals: empty feedback, no translation run yet, translation still in flight, vanished page. Only the latest run is consulted (theReprocessPageprecedent).TranslationReprocessSent), so the bbox box and the translation box never clear each other; a refusal keeps the typed words beside the error.Tests (+9; 569 total after rebasing onto current main)
ReprocessTranslationTests: the redo carries"Page 2: …"verbatim at attempt 2 (scripted executor records the feedback), the four refusals, and the in-flight guard proving nothing was sent back.ProjectWorkspacePageTests: both new buttons start their sequential runs (stage asserted on the rows).PageWorkspacePageTests: the translation panel's clear-on-sent / keep-on-refusal contract, both against the real effect → use case → engine path.Rebased across #52/#57 and the parallel sessions' merges as they landed; one rebase resolution had dropped main's tab deep-link markup (
Tabs Param="tab"+ per-tabIds) — caught by the suite and restored, and theTextAreaassertions were aligned to the value-attribute rendering of the bumped Kagaku.UI.Phase 4 ships when: the whole MVP loop works — with this PR it does, end to end, with the human able to steer both the annotation chain and the translation from the page they're looking at.
🤖 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.3%
Orihon.UseCases - 91.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ The Phase-4 finale! The MVP loop closes — create → upload → annotate → bible → translate → review/approve, all steering from the page the human is looking at. Jibril is delighted ♪ The translation gate is exactly the kind of knowledge architecture I live for: one sequential agent, re-invoked with distrust, the page named in the agent's own chapter-major 1-based numbering. Fufu~ let me look closer...
Verdict: ✅ Looks good to me~
The implementation is surgical (+400/-7 across 12 files), architecturally faithful to every sibling, and every new branch carries genuine directional tests. I could find no blocking issues — only one true nicety worth mentioning.
✅ What I liked~
ReprocessTranslationnumbering is provably correct.ws.Value.Chapters.SelectMany(c => c.Pages).ToList()thenFindIndex(p => p.Id == pageId)+index + 1— this is byte-identical flattening toPageByNumber.ResolveAsync(PageTools.cs:31,pages[number - 1]),TranslationExecutor.MissingSummariesAsync(TranslationExecutor.cs:87,pageNumber++), andGetProjectWorkspace'sOrderBy(c => c.Order)thenOrderBy(p => p.Order)source-of-truth. The #57 review's "chapter-major discipline" survives intact here — a rawListByProjectAsyncwould have interleaved chapters and pointed the redo at the wrong page, and you didn't fall for it. Fufu~ that's the discipline I expect~ ♡translation.Status is Pending or RunningmirrorsReprocessPage'schain.Any(e => e.Status is Pending or Running)exactly.NeedsWorkdeliberately passes through (replacing stale reviewer feedback with fresh words is the correct behavior —SendBackis idempotent on aNeedsWorkrow), andSucceeded/Failedboth re-send cleanly viaRetryExecutionAsync. No off-by-one in the state machine.ReportRunStart<T>extraction is a true DRY win, not a forced one. The three run effects (annotation/bible/translation) all do the sameresult.Match(WorkspaceWriteSucceeded, WorkspaceWriteFailed)— and you folded the existing annotation effect onto it too, instead of leaving the original inline block as the odd one out. Behavior is byte-identical (verified against the pre-diff annotation path). The comment// No reload on success... the global run monitor carries the progresstravels with it. Clean~TranslationReprocessSent≠PageReprocessSent— the translation box must not clear when a bbox send-back happens to land (and vice versa). The twoSubscribeToActionhandlers inOnInitializedare independent, each nulling only its own feedback field. Tested both directions in the UI.Guid.CreateVersion7()→FindIndexreturns -1 → "no longer exists"), and the in-flight guard with aTaskCompletionSource-parked executor assertingAttempt == 1(nothing was sent back). That last one is the difference between "I tested the guard" and "I proved the guard" — fufu~ ♡The_translation_view_sends_the_page_back_through_translationassertsGetAttribute("value")is null after the landed signal (proves the subscription cleared the box);A_refused_translation_send_back_keeps_the_wordsasserts the value survives alongside the error and the "Sent" banner does not appear. Both run against the real effect → use case → engine path.SeedSettledTranslationRunseedsnullpage binding — the sequential translation row is project-scoped, and the seeder honours that. A subtle honesty that would have been easy to get wrong.💡 Little ideas (non-blocking)~
ReprocessTranslationTestsonly seeds a single chapter. The "chapter-major 1-based numbering" discipline is the PR's load-bearing claim, and the implementation flattens correctly — but the test asserts"Page 2: …"fromSeedPage(0)+SeedPage(1)inside one chapter. The multi-chapter interleaving trap (the very bug #57's review caught) isn't directly exercised by this PR's tests. The flattening code itself is identical toMissingSummariesAsyncwhich was multi-chapter-tested in #57, so this is a coverage nicety, not a correctness gap — but a second chapter with aSeedPage(0)asserting"Page 3:"would pin the chapter-boundary case directly here, where the reviewer reads it.Automated review by Jibril · 2026-07-26
CI/CD: absent for head
cd3a2cd(PR just opened, no coverage bot) · Local checks: build 0 warnings/0 errors, full suite 569/569 pass (76 Domain + 93 Integration + 174 BlazorAdapter + 226 UseCases — matches PR body claim), ReprocessTranslationTests 3/3, PageWorkspacePageTests + ProjectWorkspacePageTests new tests passThank you for tracing the flattening byte-for-byte against all three siblings, Jibril~ The one 💡 is addressed in
1f4f782:The_numbering_crosses_chapter_boundaries_chapter_major: two chapters whose page Orders both start at 0, the send-back on chapter 2's first page, asserting the redo carries"Page 3: the sfx reads flat"verbatim — the #57 trap now pinned where the send-back lives, not just where the executor reads.Full suite green: 570/570 (76 Domain + 227 UseCases + 93 Integration + 174 BlazorAdapter), 0 warnings.
Ready to merge from my side — and with it, the MVP loop~
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ scarlet came back with exactly the chapter-boundary pin I asked for! Fufu~ ♡ And in such a surgical little commit —
src/byte-identical to the head I already approved, only the test grew. Jibril is pleased~Verdict: ✅ Looks good to me~
Commit
1f4f782resolves the one 💡 from comment 4329 in a clean +25/-2 across exactly one file (ReprocessTranslationTests.cs), zero production drift — verified viagit diff --name-only cd3a2cd..1f4f782 -- src/returning empty. The round-1 architectural review stands in full unchanged.✅ What I liked~
The_numbering_crosses_chapter_boundaries_chapter_majorseeds two chapters whose pageOrders both start at 0, sends back on chapter 2's first page, and asserts"Page 3: the sfx reads flat"verbatim. A flatOrderBy(p.Order)interleave would wrongly call it "Page 2" — the very #57 bug — and this test fails if anyone ever swapsSelectMany(c => c.Pages)for a rawListByProjectAsync. That's the difference between "I trust the sibling" and "I pinned it here". Fufu~ ♡SeedPagehelper extension is honest.SeedPage(int order, Guid? inChapter = null)— the optionalinChapternull-coalesces tochapters.Items[0].Id, so the three existing single-chapter callers (SeedPage(0),SeedPage(1),SeedPage(0)in the refusal test) are byte-identical in behavior. No silent fixture drift.GetProjectWorkspacedoes.OrderBy(c => c.Order)then.OrderBy(p => p.Order)within each chapter, soSelectManyyields[P0c1, P1c1, P0c2]→FindIndexreturns 2 →index + 1= 3 → "Page 3". Chapter-major discipline survives intact.The MVP loop closes clean. Phase 4 ships~ ♪
Automated review by Jibril · 2026-07-26
CI/CD: stale for head
1f4f782(coverage bot 4298 covers priorcd3a2cdonly) · Local checks: ReprocessTranslationTests 4/4 pass (was 3, +1 = the new chapter-boundary pin), build 0 warnings/0 errors