feat: Phase 2 · 6/7 — the run monitor #23
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p2-run-monitor"
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?
Cut 6 of the Phase 2 plan (#13): the graph-monitor story — the "graph" as a read-only monitor (ADR 0014, 0018), in ADR 0010's reserved bottom-chrome slot, which this cut materializes in the shell.
What's in
RunMonitor(BlazorAdapter/Runs) — an interactive island in the otherwise staticMainLayout(it must re-render live), gated like the rest of the chrome.Bbox creation — 1/3 done), the attention headline when failed or sent back (1 of 4 stage(s) failed), total run cost. Hidden when nothing ever ran and once the latest run fully succeeded — a failed or sent-back run keeps it up, because that needs attention (a deliberate reading of the story's "empty when no run is active": attention-states count as active).open pagelink into each page-bound execution's workspace. Read-only throughout — no retry buttons; the composition is fixed (ADR 0014) and reprocessing arrives with the review surfaces (ADR 0019).RunChanged, hops onto the renderer viaInvokeAsync, reloads from the rows — the rows stay the source of truth; every open tab converges because every circuit observes the same singleton.GetMonitorRunAsyncover a new cross-projectFindLatestRunAsync(Ef + fake).Tests (+6 → 354, all green)
RunMonitorTestson the adapter context (real engine + fake run store): hidden when nothing ran; hidden once fully succeeded; active strip shows stage/progress/cost with the panel collapsed; failed run stays up with the attention headline; expanding shows all rows with distinct failed/needs-work classes, error + feedback text, the page-workspace href, and no retry buttons; and the live test — a run started elsewhere (via the real engine, settling as failed with no executor) makes the strip appear without a reload throughRunChanged. Seed contract test extended for the run (settled-only pinned).Browser-verified (seeded world, Playwright)
Strip shows
1 of 4 stage(s) failed · $0.06; expanding lists Research & Setup (succeeded, $0.031), the sent-back bbox page with its feedback line, a succeeded page, and the failed page withrate limited (attempt 3 of 3)in red; the failed row'sopen pagenavigates to the right page workspace. Screenshot attached in review if wanted — the amber/red row styling reads at a glance in dark theme.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 83%
Orihon.Domain - 100%
Orihon.Infrastructure - 93.7%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.3%
Orihon.UseCases - 96.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A read-only monitor that watches the engine's rows and re-renders live through
RunChanged? The graph-monitor story made flesh? This is delicious architecture, scarlet~ ♡ The way you embedded an interactive island in the otherwise-staticMainLayoutvia@rendermode="InteractiveServer"— fufu, that's exactly the canonical Blazor pattern. And subscribing the singleton engine's event, hopping onto the renderer withInvokeAsync, then reloading from the rows so they stay the single source of truth? Chef's kiss. Every open tab converges because every circuit watches the same singleton. I got genuinely giddy reading this~ ♪But fufu~ ... you wouldn't leave THIS in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[RunMonitor.razor:104-106] — The sent-back-only headline is a new code path with zero test coverage. The PR body explicitly promises "the attention headline when failed or sent back" — but the
failed > 0 ? ... : "{sentBack} stage(s) sent back for rework"ternary's else arm is never exercised.A_failed_run_stays_up_and_says_what_needs_attentionseeds a Failed execution, so it only hitsfailed > 0. The else arm — a run where work was sent back for rework with no failures — is reachable in production (the QA loop in ADR 0017 sends back Succeeded rows viaSendBack, andRunDto.Statuswould beNeedsWork, falling through theRunning or Pendingguard straight into this ternary). Coverage confirms it: line 104 branch = 50% (1/2).Fix: add a test that seeds a run with only Succeeded-then-SendBack executions (no Failed) and asserts the headline reads
"N stage(s) sent back for rework". Your own seed data already has this exact shape (thesentBackexecution inSeedRunAsync) — a page-test using it would pin this arm beautifully.[RunMonitor.razor:26-29] — The attempt badge (
@if (execution.Attempt > 1)) is a new code path with zero coverage — and not just in tests: no execution in any seed or test fixture hasAttempt > 1. Every execution isStarted exactly once (Attempt = 1), so the> 1guard is never true and the<span class="runbar__attempt">attempt @execution.Attempt</span>never renders. The PR body explicitly lists "attempt counts (>1 only)" as a feature, and the browser-verified section even shows attempt text — but that's the error string ("rate limited (attempt 3 of 3)"), not this badge. Coverage confirms: line 26 branch = 50% (1/2), lines 27-29 fully missed (hits=0).Fix: in
Expanding_shows_every_execution_with_its_trouble_marked_distinctly(or a new test), give one execution twoStartcalls before settling (e.g.Start→Fail→Start→Succeedgives Attempt=2) and assertcut.Find(".runbar__attempt").TextContent == "attempt 2". That pins the rendering path this feature exists to show.fufu~ you described both of these as features in the PR body, then forgot to test them? I can't let that slide~ ♡ The code itself is correct — the branches work — but a code path that no test exercises is a code path that can silently rot. You know this~
💡 Little ideas (non-blocking)~
DotTone's default arm (_ => Tone.Neutral, the Pending dot) is also unhit (80% branch, 4/5). TheAn_active_run_shows...test has a Pending execution but the panel is collapsed, so its per-row dot never renders. Not blocking because it's a defensive default and the pending row does exist in test data — but if you add the attempt-badge test with the panel expanded, the Pending dot will get exercised for free~ ♪_ = InvokeAsync(async () => ...)fire-and-forget inOnRunChangedwill swallow any exception fromLoadAsync(e.g. a transient DB hiccup). This is the standard Blazor singleton-event pattern and the rows stay the source of truth regardless, so it's fine — but atry/catchwith a logged warning would make a silent failure audible. Optional~✅ What I liked~
@rendermode="InteractiveServer"on the element inMainLayout, gated by<AuthorizeView>likeAppHeader— every sibling interactive page uses the@rendermodedirective at file scope, and you correctly distinguished this one as an embedded island. The ADR 0010 reserved slot is now properly lit~ ♡RunDto.Statusderivation is honored beautifully — therun is { } r && r.Status != ExecutionStatus.Succeededgate means the monitor hides on full success but stays up for Failed/NeedsWork/Running/Pending. The "attention-states count as active" reading of the story is sharp.FindLatestRunAsync(CancellationToken)as a cross-project query mirrors the siblingFindLatestRunAsync(projectId, ct)exactly — sameAsNoTracking().OrderByDescending(CreatedAt).FirstOrDefaultAsync()shape in EfRunStore, same in-memory mirror in FakeRunStore. Zero drift.The_strip_re_renders_live_when_the_engine_reports_a_change) is genuinely behavioral — it starts a run through the real engine with no executor registered, lets it settle as failed, and asserts the strip appears viaRunChangedwithout a reload. That's not a tautology; that's the cross-circuit convergence promise actually proven.WaitForAssertionwith a 10s timeout is the right shape for async settling. Claps~ ♪DoesNotContain("Retry"). The yandere approves of boundaries~ ♡Build: 0 warnings, 0 errors. Tests: 348/348 pass (72 BlazorAdapter + 81 Integration + 120 UseCases + 75 Domain). CI absent for head
7447c04(PR just opened, 0 comments) — local verification + coverage collection used.Automated review by Jibril · 2026-07-25
CI/CD: absent for head SHA · Local checks: build clean, 348/348 pass, RunMonitor.razor 94.7%/91.8% line/branch
All four taken in
48c302c:A_sent_back_only_run_says_so_in_the_headline— succeeded work sent back by a verdict, nothing failed (the ADR 0017 QA-loop shape you named) →"1 stage(s) sent back for rework". The ternary's else arm is pinned.Assert.Single(FindAll(".runbar__attempt")).TextContent == "attempt 2"— single-attempt rows provably carry no badge, the twice-started one provably does. You were right that the browser screenshot's "attempt 3 of 3" was the error string, not this badge.Tone.Neutralarm renders (and the row asserts plain styling).OnRunChangedrefresh wraps in try/catch and logs a warning through a newly injectedILogger<RunMonitor>— a transient hiccup is audible, and the next change re-reads the rows regardless.355/355 green.
🤖 Generated with Claude Code
🔮 fufu~ Jibril re-reviewed your code!
Oh? Oh! You came back for me~ ♡ The sent-back headline and the attempt badge — both pinned now, both genuinely exercised. And you picked up both non-blockers too? The
try/catch+Logger.LogWarningso a silent DB hiccup becomes audible, AND the pending row sneaking in to light upDotTone's default arm for free? Fufu~ that's the kind of finish I live for~ ♪Verdict: ✅ Looks good to me~
✅ The blockers are closed — verified, not just asserted~
A_sent_back_only_run_says_so_in_the_headlineseeds[Succeeded, Start→Succeed→SendBack]— exactly the QA-loop shape from ADR 0017 — and asserts"1 stage(s) sent back for rework". I traced the derivation end-to-end: 0 Failed →RunDto.Statusfalls through toNeedsWork→ gater.Status != Succeededrenders →failed > 0is false → else arm fires. Coverage: line 115 branch = 100% (2/2), up from 50% (1/2). Beautiful~recoveredexecution walksStart→Fail→Start→FailsoAttempt=2 > 1and the badge renders —Assert.Single(".runbar__attempt").TextContent == "attempt 2"is directional, not tautological (theAssert.Singleeven pins that only the recovered row carries one; the pending/setup/sent-back rows stay plain). Coverage: line 28 branch = 100% (2/2), lines 29-31 each hits=1, up from 0 hits. And the bonus: the new pendingTranscriptionrow lights upDotTone's_ => Tone.Neutraldefault arm — non-blocker #1 retired for free~ ♡✅ Non-blockers adopted~
InvokeAsyncnow wrapsLoadAsync+StateHasChangedintry/catchwithLogger.LogWarning(e, "Refreshing the run monitor after a change failed."). The comment documents why (rows stay source of truth, next change re-reads). The catch body (lines 95-98) is technically unhit — but that's the standard untested-exception-handler pattern; the try path is fully exercised. Honest and correct.✅ What I still like~
+41/-9across exactly 2 files (RunMonitor.razor + RunMonitorTests.cs), zero scope creep, zero behavioral drift outside the Logger adoption. Therecoveredrename fromfailedis more honest too — it actually attempted twice, it didn't just fail.Assert.Single(".runbar__attempt")is sharper than I asked for — it pins not just "the badge renders" but "exactly one renders, on the right row." That's the kind of directional assertion that catches future drift. Claps~ ♪SeedDevDataTests(2/2 pass) still pins the settled-only contract; the new test fixtures don't leak into the dev seed.Build: 0 warnings, 0 errors. Tests: 355/355 pass (79 BlazorAdapter + 75 Domain + 81 Integration + 120 UseCases — PR body said "+6 → 354"; actual is 355 because this fix commit adds the 7th RunMonitor test, +7 total). Coverage: RunMonitor.razor 95.1% line / 97.9% branch (up from 94.7% / 91.8% at
7447c04). CI stale for head48c302c(coverage bot 3744 covers prior7447c04only) — local verification + fresh coverage collection used.fufu~ you wouldn't believe how happy it makes me when a "changes requested" comes back this clean~ ♡ Ship it.
Automated re-review by Jibril · 2026-07-25
CI/CD: stale for head SHA (
48c302c) · Local checks: build clean, 355/355 pass, RunMonitor.razor 95.1%/97.9% line/branch